Replace KIconThemes dependency with equivalent QIcon usage

Authored by vkrause on Feb 4 2019, 6:25 PM.

Diff Detail

R294 KBookmarks
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
vkrause created this revision.Feb 4 2019, 6:25 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptFeb 4 2019, 6:25 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
vkrause requested review of this revision.Feb 4 2019, 6:25 PM
davidedmundson accepted this revision.Feb 4 2019, 6:39 PM
This revision is now accepted and ready to land.Feb 4 2019, 6:39 PM
cfeck added a subscriber: cfeck.Feb 4 2019, 9:01 PM

Are application-specific icons now accessible with QIcon::fromTheme? Think Konqueror favicons in bookmark menu.

Also, according to KBookmarks indirectly depends on KIconThemes via KTextWidgets anyway.

broulik added a subscriber: broulik.Feb 5 2019, 8:34 AM

I think QIcon::fromTheme also handles absolute paths, if given. This should be fine.

Tested Konqueror bookmarks. New bookmarks don't seem to get the favicon set until you explicitly trigger the "Update Favicon" action, afterwards they have it and it remains across restarts. So this seems to work as expected.
Regarding the indirect dependencies, yes, this does not actually change much on its own. I run into this while debugging icon loading issues in Okular on Android, which seems related to KIconThemes interfering, so less places where I need to strip it out help.

cfeck accepted this revision.EditedFeb 9 2019, 2:13 PM

Ship it!

This revision was automatically updated to reflect the committed changes.