| aacid |
BUG: 386600
Applications using Okular as a KPart might set a file's mime type in the OpenUrlArguments.
Okular currently clears the arguments while opening a document. This revision fixes this, allowing
applications to actually pass a file's mime type to Okular.
Opened documents with mime type "text/markdown" and "application/rtf" in kate, using the new
preview plugin. The embedded Okular KPart displays these files correctly.
| No Linters Available |
| No Unit Test Coverage |
interesting, could you try to add an autotest for this? Seems like something that is a bit fragile and may break in the future depending on how we rework stuff in part.cpp (there's a big-ish rework coming for saveas)
Not sure that's the test we really want, i mean what we really want to check is that the proper backend was opened, no?
I mean maybe what makes sense is to set the mimetype to txt and then giving it a pdf file and checking that the txt backend was used instead of the pdf one?
| part.cpp | ||
|---|---|---|
| 1400 | I changed the order here. | |
| part.cpp | ||
|---|---|---|
| 1400 | I don't think this is a good idea, do you have an actual case in which this helps or is it just a guess? Also have you tried that this doesn't break the fixes that fba90677fcc9ccf0e6f5efe75e7446703d669d36 speaks of? | |
| part.cpp | ||
|---|---|---|
| 1400 | Sorry for the delay... From my understanding of that commit, my changes won't break it. Questionable is, what behaviour we should expect when Okular is used as a KPart and the external application sets the mime type. Docs say: QString KParts::OpenUrlArguments::mimeType ( ) const The mimetype to use when opening the url, when known by the calling application. Definition at line 93 of file openurlarguments.cpp. "The mimetype to use" sounds to me like we should trust the calling application and prefer the mimeType given to us, instead of guessing it from the file name. We still fall back to pathMime if opening it with argMime fails. An actual usecase would be creating a new rtf/markdown/whatever file in Kate. The new file would not have a file name from which we could derive the mime type from. Without my changes, Okular would guess that it's plain text, as a file name without an ending leads to pathMime "text/plain". It would therefore display the file as plain text, even if Kate knows that it's rtf/markdown/whatever (e.g. from the syntax highlighting mode). The reordering is also needed for the test you demanded.
This will not work without the reordering. A pdf file with ending ".pdf" has pathMime "application/pdf" and thus is displayed as pdf, not as plain text, even if argMime is set to "text/plain" | |
| part.cpp | ||
|---|---|---|
| 1400 | Just noticed that I'm wrong for the case of text files, as there are further checks by file content in this case. | |
@aacid Can we agree to revert to https://phabricator.kde.org/D8690?id=22028 and land this to get the bug fixed?
I'll set the test case you requested on my todo list.
We will need to define the prefered behaviour first and would need to do some changes to Okular to make such a test work. Those changes are however not required to fix the original issue.
https://phabricator.kde.org/D8690?id=22028 just links to this review globally for me, what do you want to revert?