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 760141 - New gdk_cursor_new_from_name() fallback breaks behavior expected by Firefox
New gdk_cursor_new_from_name() fallback breaks behavior expected by Firefox
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
3.18.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-01-05 03:40 UTC by Evangelos Foutras
Modified: 2016-01-09 18:23 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Evangelos Foutras 2016-01-05 03:40:10 UTC
(Relaying a bug originally tracked in Mozilla's Bugzilla. [1])

After switching Firefox to GTK3, zoom in/out icons no longer appear (e.g. when viewing an image); instead the default pointer cursor is shown.

I tracked this down to the following commit:

    commit 17cd3c321838dc3d67c3588956d1e4d68ca70df3
    Author: Matthias Clasen <mclasen@redhat.com>
    Date:   Fri May 8 09:37:39 2015 -0400

        X11: Make css cursor names work

        Map css cursor names to traditional X cursor names to increase
        our chance of finding a good cursor in the cursor theme.

Firefox depends on gdk_cursor_new_from_name() returning NULL for nonexistent icon names so that it uses its own bitmaps for the zoom in/out cursors.

I believe name_fallback() should not return "left_ptr" when it does not find a cursor name corresponding to the CSS name. Additionally, _gdk_x11_display_get_cursor_for_name() should not load "left_ptr" as a last resort when the cursor returned by name_fallback() fails to load.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1233868#c9
Comment 1 Evangelos Foutras 2016-01-05 03:46:07 UTC
Please note that any fix will need to be applied to the Wayland side too:

    commit 5434609b2167c5a4e33f494af0c0a82cd3f2639e
    Author: Matthias Clasen <mclasen@redhat.com>
    Date:   Fri May 8 09:38:56 2015 -0400

        Wayland: Make css cursor names work
    
        Map css cursor names to traditional X cursor names to increase
        our chance of finding a good cursor in the cursor theme.
Comment 2 Matthias Clasen 2016-01-05 18:53:01 UTC
I'm not sure what firefox does to map css cursor names to traditional names, but that seems to be getting in the way here. GTK+ will now give you a zoom cursor if you simply use the css names zoom-in and zoom-out. I would suggest to just do that.
Comment 3 Evangelos Foutras 2016-01-05 19:31:16 UTC
For zoom-in it will select a MOZ_CURSOR_ZOOM_IN cursor. This is part of some custom defined cursors in [1]. That header contains bitmap data for a few cursors as well as their hash (based on XcursorImageHash() I think).

Looking at its cursor loading code [2], it will do the following:

1) Call gdk_cursor_new_from_name() with the hash value of the cursor bitmap data as defined in [1]; in the case of a zoom-in cursor the hash would be "f41c0e382c94c0958e07017e42b00462".

2) If the above fails, it will create a 32x32 pixbuf and fill it with its own zoom-in cursor bitmap data. Then, it will pass it to gdk_cursor_new_from_pixbuf().

In GTK+ 3.17.2 and later, the gdk_cursor_new_from_name() call in step (1) will return a "left_ptr" cursor instead of NULL and that's where the issue originates from.

This change in gdk_cursor_new_from_name() goes beyond simply mapping CSS names to traditional names and it presents an unneeded break in long-existing behavior.

Please consider removing the "left_ptr" fallback and perhaps backporting the fix to GTK 3.18.x as well.

[1] http://hg.mozilla.org/mozilla-central/file/29258f59e545/widget/gtk/nsGtkCursors.h#l396
[1] http://hg.mozilla.org/mozilla-central/file/29258f59e545/widget/gtk/nsWindow.cpp#l5303
Comment 4 Matthias Clasen 2016-01-06 12:47:59 UTC
(In reply to Evangelos Foutras from comment #3)
> For zoom-in it will select a MOZ_CURSOR_ZOOM_IN cursor. This is part of some
> custom defined cursors in [1]. That header contains bitmap data for a few
> cursors as well as their hash (based on XcursorImageHash() I think).
> 
> Looking at its cursor loading code [2], it will do the following:
> 
> 1) Call gdk_cursor_new_from_name() with the hash value of the cursor bitmap
> data as defined in [1]; in the case of a zoom-in cursor the hash would be
> "f41c0e382c94c0958e07017e42b00462".
> 
> 2) If the above fails, it will create a 32x32 pixbuf and fill it with its
> own zoom-in cursor bitmap data. Then, it will pass it to
> gdk_cursor_new_from_pixbuf().
> 
> In GTK+ 3.17.2 and later, the gdk_cursor_new_from_name() call in step (1)
> will return a "left_ptr" cursor instead of NULL and that's where the issue
> originates from.
> 
> This change in gdk_cursor_new_from_name() goes beyond simply mapping CSS
> names to traditional names and it presents an unneeded break in
> long-existing behavior.
> 
> Please consider removing the "left_ptr" fallback and perhaps backporting the
> fix to GTK 3.18.x as well.

I think that it is more useful to have the fallback and push for improved cursor theme coverage of CSS names than to enable application-side workarounds.

I would consider a patch that only does the fallback for CSS names, not for random other names, or hashes.
Comment 5 Evangelos Foutras 2016-01-06 20:36:01 UTC
Yeah, that sounds better than my suggestion to always return NULL; thanks for implementing it! (I can confirm that d9befb9086ba [x11: Only do cursor name fallback for standard names] fixes the Firefox zoom in/out cursor issue.)

Are the two commits going to be applied to the gtk-3-18 branch as well? I suppose downstream distributions can apply the patches to their GTK3 package (and we will probably do this in Arch) but it would be nice to have it in the next 3.18.x release too.
Comment 6 Michael Catanzaro 2016-01-09 18:23:16 UTC
(In reply to Evangelos Foutras from comment #5)
> Are the two commits going to be applied to the gtk-3-18 branch as well? I
> suppose downstream distributions can apply the patches to their GTK3 package
> (and we will probably do this in Arch) but it would be nice to have it in
> the next 3.18.x release too.

Yes, they'll be in 3.18.7.