D13718: Support choosing .ico files in custom icon file chooser

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: Frameworks, cfeck.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  .ico files work fine, so we should allow them to be chosen in the custom icon 
file chooser dialog/
  
  BUG: 233201
  FIXED-IN: 5.48

TEST PLAN
  - Find a .ico file lying around, or turn a .png file into a .ico file using 
https://convertico.com/
  - Typ to apply it to a folder in Dolphin
  - You can select the .ico file in the dialog, and when applied, it appears 
correctly as a custom icon for the folder

REPOSITORY
  R302 KIconThemes

BRANCH
  ico-support-in-icon-file-chooser-dialog (branched from master)

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

AFFECTED FILES
  src/kicondialog.cpp

To: ngraham, #frameworks, cfeck
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13718: Support choosing .ico files in custom icon file chooser

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R302 KIconThemes

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

To: ngraham, #frameworks, cfeck
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13718: Support choosing .ico files in custom icon file chooser

[Christoph Feck]

cfeck added a comment.


  But does it actually work? Our icon loader removes the extension, and tries 
to find the icon with a set of known extensions (grep -i xpm in 
kiconloader.cpp).

REPOSITORY
  R302 KIconThemes

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

To: ngraham, #frameworks, cfeck
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13718: Support choosing .ico files in custom icon file chooser

[Nathaniel Graham]

ngraham added a comment.


  It does actually work; I tried it. The bug reporter also reported that it 
worked for him too if he manually entered the path to a .ico file.

REPOSITORY
  R302 KIconThemes

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

To: ngraham, #frameworks, cfeck
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13718: Support choosing .ico files in custom icon file chooser

[Kai Uwe Broulik]

broulik added a comment.


  I think if you pass a completely custom icon to it an absolute path will be 
looked up and as a result this (unintentionally) works

REPOSITORY
  R302 KIconThemes

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

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

D13718: Support choosing .ico files in custom icon file chooser

[Nathaniel Graham]

ngraham added a comment.


  So is this patch okay, or is more needed here?

REPOSITORY
  R302 KIconThemes

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

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

D13718: Support choosing .ico files in custom icon file chooser

[Christoph Feck]

cfeck accepted this revision.
cfeck added a comment.
This revision is now accepted and ready to land.


  I was thinking if we need general support for .ico files on Windows, but fear 
a slowdown while lookup of theme icons. But if this works for the special case 
of full-path icons, we can revisit the issue once/if we get a separate ticket.

REPOSITORY
  R302 KIconThemes

BRANCH
  ico-support-in-icon-file-chooser-dialog (branched from master)

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

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

D13718: Support choosing .ico files in custom icon file chooser

[Nathaniel Graham]

ngraham added a comment.


  In D13718#284112 , @cfeck wrote:
  
  > But if this works for the special case of full-path icons
  
  
  Right, and that's exactly what this patch enables.
  
  Thanks!

REPOSITORY
  R302 KIconThemes

BRANCH
  ico-support-in-icon-file-chooser-dialog (branched from master)

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

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

D13718: Support choosing .ico files in custom icon file chooser

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R302:0131f8afe845: Support choosing .ico files in custom icon 
file chooser (authored by ngraham).

REPOSITORY
  R302 KIconThemes

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13718?vs=36636&id=36799

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

AFFECTED FILES
  src/kicondialog.cpp

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