| dfaure | |
| dvratil | |
| ervin |
| Frameworks |
Use mimeapps.list "Default Applications" to prepend the service to the offers for this mimetype.
So that KMimeTypeTrader::preferredService will respect the setting.
Use KMimeTypeTrader::preferredService to replace usage of KEmailSettings / emaildefaults.
Specs:
https://specifications.freedesktop.org/shared-mime-info-spec/shared-mime-info-spec-latest.html
https://specifications.freedesktop.org/mime-apps-spec/mime-apps-spec-latest.html
CCBUG: 403499
FIXED-IN: 5.67
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
I mean here to build consensus then implement about the need for this, then on how to implement it.
I think KService should handle building this when parsing mimeapps.list files.
And then expose this as KService::findDefaultSchemeHandler(scheme)
s/Schema/Scheme/ everywhere (method name, commit log)
I want to deprecate KMimeTypeTrader, this should rather go into the upcoming KApplicationTrader D25698, if new API is needed.
I'm pretty sure you can use KMimeTypeTrader::query(mimeHandler) instead of parsing mimeapps.list. And then I wonder if you really need new API :)
Let me be clearer about my intend.
Basically it is about fixing the TODO in kmimeassociation.cpp added in 3116911fa0d02dc4c5f05923cc6d7bf490a580e5
// TODO "Default Applications" is a separate query and a separate algorithm, says the spec. // For now this is better than nothing though. parseAddedAssociations(KConfigGroup(&profile, "Default Applications"), file, basePreference);
For instance say, I have in ~/.config/mimeapps.list :
[Added Associations] x-scheme-handler/mailto=org.kde.kate.org;thunderbird.desktop [Default Applications] x-scheme-handler/mailto=thunderbird.desktop;
Thunderbird will think it is the default email client (according to the spec) but KMimeTypeTrader::preferredService("x-scheme-handler/mailto") KApplicationTrader::preferredService("x-scheme-handler/mailto") will return "org.kde.kate.org".
And in general we have no alternative way to find default association to mimetype according to the xdg specs. It may work as long as we don't have any "Added Associations", or when they match "Default Applications" but non-kde apps are not expected to deviate from the spec for our limitations.
Perhaps all we need is to improve parsing of "Default Applications" so that it is returned by KMimeTypeTrader::preferredService, that is prepend default apps to the list of apps associated with mimetypes.
I will update this PR soonish to implement this idea.
Thanks for looking into this, I'm glad that finally someone does dig into this code.
I'm a bit surprised by the solution though. The spec simply says
This doesn't need any reverse iteration or prepending, it's just about reading Default Applications before Added Associations rather than the other way around, isn't it?
I think my comment 5 years ago also meant that the xdg spec allows for the default app (left-click in file manager) to be different from the preferred app (RMB / Open With).
But looking at the spec now (Application ordering) it really treats "Default Applications" as higher-priority Added Associations, which proves that having separated the two is just complete nonsense, they serve the same purpose. Bleh. At least it's easier to implement this way :-)
| src/sycoca/kmimeassociations.cpp | ||
|---|---|---|
| 112 | typo: precedence | |
| 113 | Iterating with rbegin/rend (instead of the range-for on line 119) would be faster. Or .... factorize the rest of the loop with the other method, since that's all duplicated otherwise? Then that's a good reason to keep the same range-for. | |
| 125 | Replace the last argument with true, clearly this service is allowed as default :) | |
| 176 | I must be tired, but I don't see any difference between the code of this method and the one in addServiceOffer? (and depending on what the difference should be, I think this should be a single method with an enum argument, to reduce duplication) | |
I'm quite confused by all this. Wouldn't it be enough to do http://www.davidfaure.fr/2020/kmimeassociations.cpp.diff ?
The modified unittest passes :-)
No because, Default Applications containst lists, means a same mime type in this group may have several
Well I think it does the trick.
Simply by adding 25 to the basePreferrence is way more simple.
I will integrate this to this patch as well as the unit test.
Thanks
| src/sycoca/kmimeassociations.cpp | ||
|---|---|---|
| 104 | Shouldn't the comment also include why the value 25 (and not 1, or 42 or NaN ;-)) for mere mortals like me? | |
| src/sycoca/kmimeassociations.cpp | ||
|---|---|---|
| 104 | No. What about my job security? ;-) It's half of the +50 from line 86. The range 1000-1025 is used by Added Associations, the range 1025-1050 is used by Default Applications, and so on for the next file. | |
| src/sycoca/kmimeassociations.cpp | ||
|---|---|---|
| 104 | The code kinda assumes we don't have more than 25 entries by configgroup. | |
| src/sycoca/kmimeassociations.cpp | ||
|---|---|---|
| 104 | More precisely, it assumes that a single mimetype entry doesn't have more than 25 desktop files associated with it. And even then, what would happen is that the ordering is wrong after the first 25. I'd say after 25 you start to not really care about the order.... | |
| src/sycoca/kmimeassociations.cpp | ||
|---|---|---|
| 104 | The first 25 with the highest preferences cannot be affected but then they can have the same base reference as the next most category loaded. @ervin Do you still feel it needs more comment ? | |
| src/sycoca/kmimeassociations.cpp | ||
|---|---|---|
| 104 | The first one was fine, but what I meant was a comment in the code for when people bump into those magic numbers. ;-) | |