--===============6317621028953915695== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On 2010-06-20 18:39:13, Michael Pyne wrote: > > I'll not that KIconLoader is not "incorrect" in that the icon-theme-spe= c 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 i= f no specific icon is located, to search for a generic fallback icon, but t= hat 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. As an artist, I'd say ship it. If the name spec says something different, I= think we should change it. I've not reviewed this patch code-wise though. For the second problem of yours, but still as an artistic advice, I'd say i= t's better to load the generic mime icon over accepting hicolor's specific = version. - Riccardo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/4403/#review6198 ----------------------------------------------------------- On 2010-06-20 09:18:37, Aur=C3=A9lien G=C3=A2teau 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=C3=A1ndez L=C3=B3pez. > = > = > Summary > ------- > = > When loading an icon for video-mp4, KIconLoader incorrectly returns "vide= o.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 D= olphin 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=C3=A9lien > = > --===============6317621028953915695== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde= .org/r/4403/

On June 20th, 2010, 6:39 p.m., Michael Pyne= wrote:

I'll not that KIconLoader is not "incorrect" in that t=
he icon-theme-spec does specify that hicolor should be searched if the desi=
red 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 fal=
lback 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 wo=
uld say accept hicolor's more specific video-mp4 in preference to video=
-x-generic, but that could look ugly in several themes.
As an artist, I'd say ship it. If the name spec says something dif=
ferent, I think we should change it.
I've not reviewed this patch code-wise though.

For the second problem of yours, but still as an artistic advice, I'd s=
ay it's better to load the generic mime icon over accepting hicolor'=
;s specific version.

- Riccardo


On June 20th, 2010, 9:18 a.m., Aur=C3=A9lien G=C3=A2teau wrote:

Review request for kdelibs and Rafael Fern=C3=A1ndez L=C3=B3pez.
By Aur=C3=A9lien G=C3=A2teau.

Updated 2010-06-20 09:18:37

Descripti= on 

When loading an icon for video-mp4=
, KIconLoader incorrectly returns "video.png" from Hicolor, inste=
ad of returning "video-x-generic.png" from Oxygen. Since Hicolor =
only has a 16x16 version, this results in blurry icons in Dolphin or Gwenvi=
ew when icons are bigger than 16x16.

This is because the current code fallbacks to Hicolor before looking for a =
"-x-generic" version.

Testing <= /h1> 
- Added a unit-test entry to check=
 we get video-x-generic.
- Icons are no longer blurry after the patch.

Diffs=

  • trunk/KDE/kdelibs/kdeui/icons/kiconloader.cpp (1140302)
  • trunk/KDE/kdelibs/kdeui/tests/kiconloader_unittest.cpp (1140302)

View Diff
--===============6317621028953915695==--