D15451: Let KUrlCombo operate on QIcon instead of QPixmap

[Kai Uwe Broulik]

broulik added a comment.


  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`?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15451

To: broulik, #frameworks, dfaure, #vdg
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D15451: Let KUrlCombo operate on QIcon instead of QPixmap

[Kai Uwe Broulik]

broulik planned changes to this revision.
broulik added a comment.


  Yes.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15451

To: broulik, #frameworks, dfaure, #vdg
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D15451: Let KUrlCombo operate on QIcon instead of QPixmap

[David Faure]

dfaure added a comment.


  @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 
.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15451

To: broulik, #frameworks, dfaure, #vdg
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D15451: Let KUrlCombo operate on QIcon instead of QPixmap

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  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)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15451

To: broulik, #frameworks, dfaure, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15451: Let KUrlCombo operate on QIcon instead of QPixmap

[Kai Uwe Broulik]

broulik created this revision.
broulik added reviewers: Frameworks, dfaure, VDG.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  Lets the view load the correctly scaled icon on demand.

TEST PLAN
  Before
  F6256160: Screenshot_20180912_155136.png 

  After
  F6256162: Screenshot_20180912_160656.png 

  
  `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

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15451

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp

To: broulik, #frameworks, dfaure, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns