Review Request: Do not fallback to Hicolor when Oxygen provides a "-x-generic" version of an icon

[Michael Pyne]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4403/#review6198
-----------------------------------------------------------


I'll not that KIconLoader is not "incorrect" in that the icon-theme-spec does specify that hicolor should be searched if the desired icon does not exist in a theme. shared-mime-info kind of overrides that by saying that if no specific icon is located, to search for a generic fallback icon, but that should only be mime icons.

One question is what do we do if hicolor happens to have an exact match of the required size, do we accept hicolor or try to fallback to generic? I would say accept hicolor's more specific video-mp4 in preference to video-x-generic, but that could look ugly in several themes.


trunk/KDE/kdelibs/kdeui/icons/kiconloader.cpp
<http://reviewboard.kde.org/r/4403/#comment5794>

    Be careful with this, I'm pretty sure this code path can be run for any non-User icon, not just mime icons, so doing this replace would break icons under subdirectories that are currently found.



trunk/KDE/kdelibs/kdeui/icons/kiconloader.cpp
<http://reviewboard.kde.org/r/4403/#comment5795>

    Is there a reason to move generic fallback support into findMatchingIcon where it must be used for all icons? KIconLoader is plenty slow as it currently stands, I'm not sure this would help matters.


- Michael


On 2010-06-20 09:18:37, Aurélien Gâteau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4403/
> -----------------------------------------------------------
> 
> (Updated 2010-06-20 09:18:37)
> 
> 
> Review request for kdelibs and Rafael Fernández López.
> 
> 
> Summary
> -------
> 
> When loading an icon for video-mp4, KIconLoader incorrectly returns "video.png" from Hicolor, instead of returning "video-x-generic.png" from Oxygen. Since Hicolor only has a 16x16 version, this results in blurry icons in Dolphin or Gwenview when icons are bigger than 16x16.
> 
> This is because the current code fallbacks to Hicolor before looking for a "-x-generic" version.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kdeui/icons/kiconloader.cpp 1140302 
>   trunk/KDE/kdelibs/kdeui/tests/kiconloader_unittest.cpp 1140302 
> 
> Diff: http://reviewboard.kde.org/r/4403/diff
> 
> 
> Testing
> -------
> 
> - Added a unit-test entry to check we get video-x-generic.
> - Icons are no longer blurry after the patch.
> 
> 
> Thanks,
> 
> Aurélien
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20100620/f0b595b3/attachment.htm>