Indicate that Gwenview can open Targa images
ClosedPublic

Authored by ngraham on Nov 10 2017, 6:20 PM.

Details

Summary

BUG: 332485

Test Plan

After updating the .desktop files and running sudo update-desktop-database, Gwenview was correctly registered as a handler for .tga files, and they opened and displayed exactly as they should.

Diff Detail

Repository
R260 Gwenview
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Nov 10 2017, 6:20 PM
ngraham edited the test plan for this revision. (Show Details)Nov 10 2017, 6:28 PM
rkflx requested changes to this revision.Nov 12 2017, 10:00 AM

TGA used to work in KDE3 times. Thanks for fixing it again :)

Checking xdg-mime query filetype on an example file each, mimetypes seem to be correct. TGA files now open in Gwenview by default instead of Okular, which makes sense. However, Krita files are still a bit off:

  • Krita files (which are primarily used for editing and not for viewing) now open with Gwenview by default instead of Krita. That's probably not something existing users will be too happy about. If this will be changed, we'd need at least a thumbs up from the Krita folks, but I'd favour lowering the priority of Gwenview below Krita in the file associations, to be honest.
  • Thumbnail generation for Krita files is broken (reuse if generated from Dolphin works, though). To reproduce, rm -r ~/.cache/thumbnails/ and observe https://bugs.kde.org/show_bug.cgi?id=355590 in action. (Not sure whether this brokenness is related to mimetypes or material for a separate patch.)
This revision now requires changes to proceed.Nov 12 2017, 10:00 AM

Great point about opening in Gwenview before Krita. We definitely don't want that; if users have both programs installed, Krita should always take priority. Where does that setting live? Here in Gwenview, or somewhere else?

IMHO https://bugs.kde.org/show_bug.cgi?id=355590 should be fixed with a separate patch. It's on my to-do list.

rkflx added a comment.Nov 12 2017, 2:45 PM

Great point about opening in Gwenview before Krita. We definitely don't want that; if users have both programs installed, Krita should always take priority. Where does that setting live? Here in Gwenview, or somewhere else?

No idea :) As a user, I'd just edit the filetype in Dolphin or the KCM and change the Application Preference Order. Maybe you can work it out from there? Or asked someone knowledgable.

IMHO https://bugs.kde.org/show_bug.cgi?id=355590 should be fixed with a separate patch. It's on my to-do list.

Of course. My comment was more about why Dolphin triggers creation of Krita thumbnails, but Gwenview does not. Essentially users can use Gwenview for Krita files on a opt-in basis. If we make this support "official" by adding the mimetype, thumbnails should work too. Or just file another bug for that?

(Got your "Gwenview" mail, BTW. Let me think about it…)

rkflx added a subscriber: cfeck.Nov 16 2017, 7:21 AM

@cfeck Just realized you weren't in CC of https://bugs.kde.org/show_bug.cgi?id=337272#c3. Just wanted to ask whether you know how to change the mimetype's relative order (probably trivial, but still takes time to research if you've never done this before…).

rkflx removed a subscriber: cfeck.Nov 24 2017, 10:40 PM

For reference / possible path forward regarding x-krita: https://www.mail-archive.com/kde-devel@kde.org/msg09796.html

Might also be worth splitting the patch, i.e. do x-tga here.

ngraham updated this revision to Diff 22941.Nov 26 2017, 3:28 AM

Only do .tga fix for now while I'm still trying to figure out how to add .kra support without accidentally making Gwenview open them by default even when Krita is installed

ngraham retitled this revision from Indicate that Gwenview can open Targa and Krita images to Indicate that Gwenview can open Targa images.Nov 26 2017, 3:29 AM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
rkflx accepted this revision.EditedNov 26 2017, 7:36 AM

Thanks, please commit to the stable branch. I'll merge to master Tuesday latest.

This revision is now accepted and ready to land.Nov 26 2017, 7:36 AM

@rkflx Which branch do you want this on? 17.12?

rkflx added a comment.Nov 27 2017, 9:19 PM

Yep, Applications/17.12 is the current stable branch.

This revision was automatically updated to reflect the committed changes.