Reduce reliance on the mime database #21382
Conversation
| if (mimeType.isNull()) | ||
| mimeType = QMimeDatabase().mimeTypeForFileNameAndData(path.data(), data).name(); | ||
|
|
||
| const bool isTranslatable = !m_isAltUIUsed && mimeType.startsWith(u"text/"_s); |
There was a problem hiding this comment.
I doubt it is acceptable change. mimeType.inherits(u"text/plain"_s) has a much larger reach.
As I said earlier I won't approve fighting with mime database usage as a goal. I would keep using QMimeType instead of type names.
There was a problem hiding this comment.
Well, there is no other way around it.
If you disapprove, close #21363 as wontfix.
There was a problem hiding this comment.
Well, there is no other way around it.
Just
keep using
QMimeTypeinstead of type names.
It shouldn't prevent you from hardcode some fileextension-to-mimetype mapping.
There was a problem hiding this comment.
I don't see how this could be possible. As far as I understand,
keep using
QMimeTypeinstead of type names.
is mutually exclusive with
hardcode some fileextension-to-mimetype mapping
because QMimeType requires the database to work.
The class can't even be constructed from arbitrary data. The only way to create it is to query QMimeDatabase.
There was a problem hiding this comment.
I don't see how this could be possible. As far as I understand,
keep using
QMimeTypeinstead of type names.is mutually exclusive with
hardcode some fileextension-to-mimetype mapping
because
QMimeTyperequires the database to work.
As I mentioned, I don't consider abandoning QMimeDatabase as a goal. It is enough to prevent the use of incorrect file type associations. To do this, we need to get associations for important files (such as html, js, css) not from the system mime database (where they can be corrupted), but hard-code them instead. We need QMimeDatabase::mimeTypeForName() to achieve it which I believe isn't affected by the issue we want to avoid.
There was a problem hiding this comment.
We need
QMimeDatabase::mimeTypeForName()to achieve it which I believe isn't affected by the issue we want to avoid.
It is affected. Everything in QMimeDatabase is affected. Because all types can be overridden in a user local database.
There was a problem hiding this comment.
And the whole thing seems hardcoded. I don't see a way to forbid it loading external databases. That could have been a perfect simple solution in our case.
There was a problem hiding this comment.
So, there is a dead end here? Should I close the PR?
There was a problem hiding this comment.
I expressed my opinion. We can wait for someone else.
There was a problem hiding this comment.
I guess nobody else wants to participate.
|
I continue to hold the opinion that the referenced Issues are not qBittorrent bugs, but problems on the user's side. I don't mind using some kind of workarounds to reduce the chances of their occurrence, but only without worsen the current functionality of qBittorrent. Using mime database is not a disadvantage in itself, IMO. |
Could you explain how exactly it worsen the functionality? I don't see any changes in behavior at all.
It is, IMO. I use a headless Qt build on a server, which doesn't depend on But I understand that this is an uncommon scenario. |
|
Hmm, indeed. It works without I had the described WebUI problem in the past though. And installing |
This is a two part PR. See a discussion from #21363 (comment).
Move Path mime extension retrieval into a separate method
Reasons:
Path::extension()callers do not need that.WebUI: Store common mime types inside the app
Makes WebUI usable if the mime database is absent or broken.
Related:
Content-Typesent by Web UI #15218