Let KUrlCombo operate on QIcon instead of QPixmap
Changes PlannedPublic

Authored by broulik on Sep 12 2018, 2:10 PM.

Details

Reviewers
dfaure
Group Reviewers
Frameworks
VDG
Summary

Lets the view load the correctly scaled icon on demand.

Test Plan

Before


After

KIO::pixmapForUrl and KIconLoader::loadMimeTypeIcon says something about "loading additional icon themes" but they're hardly used in KIO anymore so it's likely the file list itself would have a hard time finding the icons anyway, so we can just use QIcon::fromTheme here

Diff Detail

Repository
R241 KIO
Lint
Lint Skipped
Unit
Unit Tests Skipped
broulik created this revision.Sep 12 2018, 2:10 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 12 2018, 2:10 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
broulik requested review of this revision.Sep 12 2018, 2:10 PM
dfaure requested changes to this revision.Sep 18 2018, 7:14 AM

loadMimeTypeIcon also has the fallback to application-octet-stream if the mimetype icon isn't found, you could pass that as 2nd argument to fromTheme().

As to the extra desktop themes, I admit that I never really understood what that was about. KIconLoaderPrivate::addExtraDesktopThemes() seems to add *all* icon themes available on the system and named default.* (other than default.kde)... Which use case does this cover? And anyway if any kiconloader does this then fromTheme() will end up there too (just like the code for mimetype-generic-icons, which is hopefully also triggered when going via fromTheme....)

This revision now requires changes to proceed.Sep 18 2018, 7:14 AM

@broulik, do you want to rebase and update this merge request, to add the missing fallback to "application-octet-stream" fallback to QIcon::fromTheme() calls?

This change request is very much in line with other work being done in T11637.

broulik planned changes to this revision.Fri, Oct 18, 11:11 AM

Yes.

However, setDummyHistoryEntry has some logic for icon being null and "reuse previous pixmap if null" which would be moot if I change it to always fall back to application-octet-stream?