After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 753510 - iconhelper: allow to fall back from pixbuf, to gicon, to named icon
iconhelper: allow to fall back from pixbuf, to gicon, to named icon
Status: RESOLVED WONTFIX
Product: gtk+
Classification: Platform
Component: .General
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on: 753506
Blocks: 753512
 
 
Reported: 2015-08-11 13:07 UTC by Paolo Borelli
Modified: 2016-01-23 06:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (4.69 KB, patch)
2015-08-11 13:08 UTC, Paolo Borelli
none Details | Review
reftest (4.46 KB, patch)
2015-08-11 15:27 UTC, Paolo Borelli
none Details | Review

Description Paolo Borelli 2015-08-11 13:07:04 UTC
Iconhelper is to eager to clear the area when setting a null pixbuf or gicon etc.
This prevents fallback from pixbuf to named-icon or similar cases.

This is in turn needed to allow a CellRendererPixbuf to use a model that has colums for pixbuf, gicons, and icon-names
Comment 1 Paolo Borelli 2015-08-11 13:08:14 UTC
I am marking this as dependent on #753506, even if they are not strictly dependendent, but both patches are needed to make the CellRenderer use case work
Comment 2 Paolo Borelli 2015-08-11 13:08:43 UTC
Created attachment 309070 [details] [review]
patch
Comment 3 Paolo Borelli 2015-08-11 15:27:51 UTC
Created attachment 309078 [details] [review]
reftest

This reftest fails before the patches and passes with the patches applied
Comment 4 Matthias Clasen 2015-08-12 03:19:49 UTC
Review of attachment 309070 [details] [review]:

Where are the precedence rules documented ?
Comment 5 Paolo Borelli 2015-08-12 05:58:42 UTC
In the gtkcellrenderepixbuf properties: for instance

"The name of the themed icon to display. This property only has an effect if not overridden by "stock_id" or "pixbuf" properties."
Comment 6 Matthias Clasen 2015-08-14 13:21:47 UTC
Review of attachment 309070 [details] [review]:

To be honest, I don't really understand why you would call set_pixuf(helper, pixbuf) if pixbuf is NULL and expect the icon helper to not be cleared afterwards. Why not just do

if (pixbuf)
  set_pixbuf (helper, pixbuf)
else
  set_icon_name (helper, "named-icon")

?
Comment 7 Matthias Clasen 2015-08-14 13:22:13 UTC
Review of attachment 309070 [details] [review]:

To be honest, I don't really understand why you would call set_pixuf(helper, pixbuf) if pixbuf is NULL and expect the icon helper to not be cleared afterwards. Why not just do

if (pixbuf)
  set_pixbuf (helper, pixbuf)
else
  set_icon_name (helper, "named-icon")

?
Comment 8 Paolo Borelli 2015-08-14 14:47:03 UTC
The example was clearly not realistic. I need this because I have a tree model with three columns (pixbuf, icon name and gicon) and each row can set one of the columns. Without this patch only rows that set the pixbuf show an icon sbecause for the other rows the null pixbuf wins over the icon name.

The final use case is to support symbolic icons in the completion popular of builder. See the patch in the dependent gtksourceview bug.
Comment 9 Paolo Borelli 2015-08-14 14:52:07 UTC
Popular -> popup 

Yay phone automatic spelling
Comment 10 Matthias Clasen 2015-08-16 03:06:23 UTC
I'm not convinced that it is right to hardcode such fallback in the icon helper
Comment 11 Paolo Borelli 2015-08-16 07:16:16 UTC
I can understand that... not however that with the patch, widgets are still able to obtain the current behavior by calling helper_clear() themselves, while without the patch the problem in gtkcellrendererpixbuf cannot be fixed.


(For the GtkSourceView patch in the mean time I changed approach so this bug is not blocking it anymore)