GNOME Bugzilla – Bug 69398
improve handling of failure in loading stock icons
Last modified: 2013-08-14 00:22:41 UTC
If a GTK+ stock icon is overriden thru a gtkrc file and the image can not be found in the pixmap path, the stock icon shouldn't be overiden with an empty icon_set. At the moment this is the case which results in the broken image icon being displayed instead of falling back to the builtin stock icon. I'll attach a simple patch that addresses this problem.
Created attachment 6478 [details] [review] a simple patch that solves the problem
Havoc, can you check this over? (Since it's your code) The patch, at a quick glance, looks reasonable, though I don't understand the comment addition: /* We continue parsing even if we didn't find the pixmap so that rest of the - * file is read, even if the syntax is bad + * file is read, even if the syntax is bad. However we don't validate the + * icon_set so the caller can choose not to install it. If it's trying to say, "it's not an error if an icon source references an icon which is not there", that's certainly not true... what we are talking about is what to do recovering from such an error.
The icon_source is not referring to an icon which is not found since the icon_source is not added to the icon_set in case the icon coulnd't be found (see around line 3594). What happens is that with the old code an empty icon_set was added to the icon_factory (around line 3665). Since there is no API to check if an icon_set contains any icon_sources I've choosen to let gtk_rc_parse_icon_source() set the boolean value icon_set_valid when it adds an icon_source to the icon_set. Please feel free to change the comment I've added or just ignore that part of the patch.
I don't think the patch is really solving the core problem, which is that in gtk_icon_set_render_icon() if the gdk_pixbuf_new_from_file() fails we should fall back. Of course, I have no good suggestions for how to fix the core problem. All I can think of is that empty icon sets would be magically removed from all icon factories they're inside, which sounds like a terrible idea, or to dispense with lazy loading, which sounds bad too. So this patch is probably the best thing for now.
I have applied the patch to CVS but I'm not sure if you want me to close this bug-report since Havoc seems to think the patch only works around the real problem.
I don't think it's fixable in the current API; we'd need a gtk_icon_set_render_icon() variant that could return failure and a way for gtk_widget_render_icon() to get the "default" icon set for an icon.
not keeping this open any longer; gtkstock apis have been deprecated