GNOME Bugzilla – Bug 677567
icontheme clips bigger symbolic icons to 16x16
Last modified: 2012-07-13 13:02:44 UTC
At the time of creating a symbolic icon for a given colorset, GtkIconTheme creates an embedder piece of SVG to override the colors, also forcing a size of 16x16 on it. This results in icons being clipped (rather than resized) if the theme being used offers symbolic icons at bigger default sizes. I'm attaching a patch to have that code use the original icon size when overriding colors.
Created attachment 215758 [details] [review] patch
Review of attachment 215758 [details] [review]: ::: gtk/gtkicontheme.c @@ +3220,3 @@ " xmlns:xi=\"http://www.w3.org/2001/XInclude\"\n" + " width=\"", width, "\"\n" + " height=\"", height, "\">\n" What happens if you remove the width and height parameters from the CSS file completely instead? Would that pick up the size of the original image without requiring us to force it this way? Which themes use symbolic icons whose original size is not 16x16?
(In reply to comment #2) > Review of attachment 215758 [details] [review]: > What happens if you remove the width and height parameters from the CSS file > completely instead? > Would that pick up the size of the original image without requiring us to force > it this way? I think the width/height are expected in the root <svg> tag, removing those just shows the "image missing" icon with no further error though > > Which themes use symbolic icons whose original size is not 16x16? I first thought I stumbled with this bug on the Sugar theme, but it turned out to be bug #678087. Still, this theme has odd-sized icons that could hit this if symbolic icons are used more extensively, and the symbolic icon spec doesn't mention an icon size limit.
Review of attachment 215758 [details] [review]: Sounds fine to me then, except for the following comment ::: gtk/gtkicontheme.c @@ +3205,3 @@ + if (!pixbuf) + return FALSE; + This should return NULL
(In reply to comment #4) > ::: gtk/gtkicontheme.c > @@ +3205,3 @@ > + if (!pixbuf) > + return FALSE; > + > > This should return NULL I changed that and pushed, thanks!