GNOME Bugzilla – Bug 781583
gtk_image_new_from_resource does not work
Last modified: 2017-07-12 13:56:29 UTC
In our resources.xml file, we have an image: <file preprocess='to-pixdata'>right-arrow-icon.png</file> When we call gtk_image_new_from_resource, we get only the picture of a broken image. However, when we call gdk_pixbuf_new_from_resource and gtk_image_new_from_pixbuf, then the expected image is displayed fine. We also use gtk_css_provider_load_from_resource with the same path to resources and it works fine, so I think that our resources are set up correctly.
This happens because gtk_image_new_from_resource uses GdkPixbufLoader and there is no pixdata loader module since gdk-pixbuf-2.36.1 and then the test in gdk-pixbuf-io.c:892 always fails. The same issue occurs in gnumeric (see #781988).
This might work: diff --git a/gtk/gtkimage.c b/gtk/gtkimage.c index d788270abd..d270ba30af 100644 --- a/gtk/gtkimage.c +++ b/gtk/gtkimage.c @@ -889,7 +889,7 @@ gtk_image_set_from_resource (GtkImage *image, return; } - animation = load_scalable_with_loader (image, NULL, resource_path, &scale_factor); + animation = gdk_pixbuf_animation_new_from_resource (image, NULL); if (animation == NULL) { It'd be helpful if you can make a minimal test for this though.
image has type GtkImage; gdk_pixbuf_animation_new_from_resource wants a string, probably resource_path.
With this patch, I can get the broken icons from EasyTAG to work again (see bug 782307). I needed scale_factor set properly to get it to work, so I expect this would need a replacement. --- a/gtk/gtkimage.c +++ b/gtk/gtkimage.c @@ -1027,7 +1028,8 @@ gtk_image_set_from_resource (GtkImage return; } - animation = load_scalable_with_loader (image, NULL, resource_path, &scale_factor); + animation = gdk_pixbuf_animation_new_from_resource (resource_path, NULL); + scale_factor = 1; if (animation == NULL) {
*** Bug 782307 has been marked as a duplicate of this bug. ***
Created attachment 354964 [details] [review] image: Fix loading of pixdata GResources Pixdata is deprecated but some software already use GtkImage widgets with image data loaded from GResource-backed pixdata. As the security-problem ridden pixdata loader was removed, we need to manually check whether the GResource data is pixdata, and load it manually.
The git-bz finished before I could cancel it, so the above isn't even compile tested. I'd recommend this for the stable release, and something which throws a warning instead for master, so gtk+ is weaned off its pixdata usage.
Created attachment 354971 [details] [review] image: Fix loading of pixdata GResources Pixdata is deprecated but some software already use GtkImage widgets with image data loaded from GResource-backed pixdata. As the security-problem ridden pixdata loader was removed, we need to manually check whether the GResource data is pixdata, and load it manually.
It's unclear to me why gtk+ needs to be weaned off its pixdata usage, at least as far as resources are concerned. Resource files have a documented option preprocess='to-pixdata'.
(In reply to Morten Welinder from comment #9) > It's unclear to me why gtk+ needs to be weaned off its pixdata usage, at > least as far as resources are concerned. Resource files have > a documented option preprocess='to-pixdata'. Because GdkPixdata is deprecated in gdk-pixbuf, and the option is offered in glib's gio. Those are not on the same development cycle as GTK+, so unless we wanted to make a gdk-pixbuf 3.0 and a glib 3.0, warning at run-time is the only option. "to-pixdata" has no clear benefits compared to bundling PNG or jpeg files directly.
Review of attachment 354971 [details] [review]: I tested the patch on top of GTK 3 git, and all the icons are now shown in EasyTAG.
> Because GdkPixdata is deprecated in gdk-pixbuf, and the option is offered in > glib's gio. Presumably it was deprecated because someone thought "who in the world would use that format nowadays?" forgetting that resources was an active user. It is not a complicated format to read, so undeprecation would be nice. > "to-pixdata" has no clear benefits compared to bundling PNG or jpeg files directly. Maybe, but that fails to consider existing binaries. Having one's program break repeatedly due to tool chain churn is not helpful.
(In reply to Morten Welinder from comment #13) > > Because GdkPixdata is deprecated in gdk-pixbuf, and the option is offered in > > glib's gio. > > Presumably it was deprecated because someone thought "who in the world would > use that format nowadays?" forgetting that resources was an active user. > It is not a complicated format to read, so undeprecation would be nice. The GTK+ code was relying on an external loader which made it possible to have non-embedded files on disk, for a completely proprietary image format. This is now fixed. GdkPixdata predates GResource by at least 10 years. I don't know why "to-pixdata" was considered a good idea, but it really isn't given the security concerns of the pixdata format. Obsolescence is definitely called for. > Maybe, but that fails to consider existing binaries. Having one's program > break repeatedly due to tool chain churn is not helpful. Yeah, bugs never happen...
Created attachment 355429 [details] [review] image: Warn when attempting to load pixdata GResources GdkPixdata is deprecated. Warn when the application tries to load pixdata embedded resources. The application developer will have to remove the "to-pixdata" keyword from the GResource definition file.
So the documentation for glib-compile-resources really needs to be fixed. It should probably not allow such resources to be compiled in the first place, if the code on the receiving end is going to suddenly start rejecting them.
Both look good to me, thanks.
(In reply to Daniel Boles from comment #16) > So the documentation for glib-compile-resources really needs to be fixed. > > It should probably not allow such resources to be compiled in the first > place, if the code on the receiving end is going to suddenly start rejecting > them. GTK+ 3.x will continue accepting this construct for as long as it's being maintained.
Comment on attachment 354971 [details] [review] image: Fix loading of pixdata GResources Fix for GTK+ 3.x: Attachment 354971 [details] pushed as a6dcb80 - image: Fix loading of pixdata GResources
Comment on attachment 355429 [details] [review] image: Warn when attempting to load pixdata GResources Committed for GTK+ 4.x. I've also pushed the GTK+ 3.x patch to 3.20 and 3.18.
(In reply to Bastien Nocera from comment #18) > (In reply to Daniel Boles from comment #16) > > So the documentation for glib-compile-resources really needs to be fixed. > > > > It should probably not allow such resources to be compiled in the first > > place, if the code on the receiving end is going to suddenly start rejecting > > them. > > GTK+ 3.x will continue accepting this construct for as long as it's being > maintained. Right, yeah. It would be nice to mention/emphasise the deprecation in the current stable docs, though. I'll prepare a patch.