Last Comment Bug 709259 - Appearance of "spinning" cursor regressed sometime between Thunderbird 8 and 9
: Appearance of "spinning" cursor regressed sometime between Thunderbird 8 and 9
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Widget: Gtk (show other bugs)
: Trunk
: All Linux
: -- minor with 1 vote (vote)
: mozilla11
Assigned To: Chris Coulson
:
Mentors:
: 693517 (view as bug list)
Depends on:
Blocks: gtk3
  Show dependency treegraph
 
Reported: 2011-12-09 12:56 PST by Chris Coulson
Modified: 2014-07-03 07:00 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
+
affected


Attachments
Video of the "spinning" cursor in Thunderbird 8 (34.63 KB, video/ogg)
2011-12-09 12:56 PST, Chris Coulson
no flags Details
Video of the "spinning" cursor in Thunderbird 9 (broken) (37.96 KB, video/ogg)
2011-12-09 12:57 PST, Chris Coulson
no flags Details
Try creating a named cursor before a bitmap cursor (4.89 KB, patch)
2011-12-12 10:58 PST, Chris Coulson
no flags Details | Diff | Review
Try creating a named cursor before a bitmap cursor (4.86 KB, patch)
2011-12-12 11:02 PST, Chris Coulson
karlt: review-
Details | Diff | Review
Try creating a named cursor before a bitmap cursor (v2) (4.88 KB, patch)
2011-12-13 00:51 PST, Chris Coulson
karlt: review+
lukasblakk+bugs: approval‑mozilla‑esr10-
Details | Diff | Review

Description Chris Coulson 2011-12-09 12:56:51 PST
Created attachment 580512 [details]
Video of the "spinning" cursor in Thunderbird 8

I've noticed whilst running Thunderbird 9 beta that when my mailboxes refresh, my mouse pointer changes to a really ugly busy cursor. In Thunderbird 8, the cursor seemed to not look too bad.

I just rebuilt the latest comm-beta after reverting http://hg.mozilla.org/releases/mozilla-beta/rev/875cb4f20eac, and it seems to fix it. I've not fully investigated why this patch would break it just yet.

The less attractive spinning cursor also appears occasionally in Firefox too, although it appears a lot more in Thunderbird.

This can be easily reproduced in Firefox or Thunderbird by using a JS shell and doing window.setCursor("spinning"). I've attached 2 videos of me doing this before and after reverting the mentioned patch
Comment 1 Chris Coulson 2011-12-09 12:57:27 PST
Created attachment 580514 [details]
Video of the "spinning" cursor in Thunderbird 9 (broken)
Comment 2 Chris Coulson 2011-12-10 08:11:12 PST
Ok, so this is a side-effect of no longer using gdk_cursor_new_from_pixmap (as it's not available in GTK3).

gdk_cursor_new_from_pixmap uses XCreatePixmapCursor to create a cursor, and this does some magic via libxcursor to match a hash of the cursor bitmap to a themed pointer (left_ptr_watch), which gave us the nice looking pointer. There doesn't seem to be any support in xlib or gdk for creating a pointer of this type with any other mechanism though :(
Comment 3 Karl Tomlinson (ni?:karlt) 2011-12-11 17:23:48 PST
Is gdk_cursor_new_from_name() an option?
Comment 4 Benjamin Otte 2011-12-11 17:49:08 PST
FWIW, I removed that code, because we got rid of pixmaps in GTK3. I certainly did not intend to remove any functionality previously available, nor do I think I did that. Of course, it made some things different and possibly even harder.

From looking at the patch, I can't understand why it would break either though, because gdk_cursor_new_from_pixmap() cannot be used to create animated cursors afaics.

I guess what I'm saying is: If there's functionality missing in GTK, poke us so we can add it back.
Comment 5 Benjamin Otte 2011-12-11 17:53:34 PST
Oh, I'm just now seeing the explanation in comment 2. Do you have a cgit link to the X code for this handy? Because this sounds like something really really ugly inside X (wow, what a surprise ;))
Comment 6 Karl Tomlinson (ni?:karlt) 2011-12-11 18:25:43 PST
Looks like this is the Xcursor code:
http://cgit.freedesktop.org/xorg/lib/libXcursor/tree/src/xlib.c?id=e086eb1bf49f2a8c270eaebd5beb595c1dc2973e#n415

gdk_cursor_new_from_name() also uses XcursorLibraryLoadCursor(), so it should be possible to get the same behavior.

If the cursor can also be found under the "left_ptr_watch" name then that would be preferable for the lookup.  As a last resort, the name from the hash can be used.
Comment 7 Karl Tomlinson (ni?:karlt) 2011-12-11 18:51:22 PST
Looks like a left_ptr_watch lookup should be the same as a 08e8e1c95fe2fc01f976f1e063a24ccd.

http://cgit.freedesktop.org/xorg/data/cursors/tree/handhelds/Makefile.cfg

I guess we can't be certain the lookup will succeed, so we'd still have to fallback to the pixbuf.
Comment 8 Benjamin Otte 2011-12-11 18:58:53 PST
Yeah, I think the solution is to try a new_from_name() first for all cursors and if that returns NULL fall back to the image (or the default cursor if you're reasonably sure everybody supports the cursor you care about).
Comment 9 Chris Coulson 2011-12-12 10:58:41 PST
Created attachment 580970 [details] [diff] [review]
Try creating a named cursor before a bitmap cursor
Comment 10 Chris Coulson 2011-12-12 11:02:37 PST
Created attachment 580971 [details] [diff] [review]
Try creating a named cursor before a bitmap cursor

Oops, that wasn't the most recent version
Comment 11 Karl Tomlinson (ni?:karlt) 2011-12-12 16:10:34 PST
Comment on attachment 580971 [details] [diff] [review]
Try creating a named cursor before a bitmap cursor

Thanks very much.
Comment 12 Karl Tomlinson (ni?:karlt) 2011-12-12 17:07:13 PST
Comment on attachment 580971 [details] [diff] [review]
Try creating a named cursor before a bitmap cursor

>+  { moz_none_bits,           moz_none_mask_bits,           0,  0,  NULL }

>         newType = MOZ_CURSOR_NONE;
>         break;

>+    // If by now we don't have a xcursor, this means we have to make a custom
>+    // one. First, we try creating a named cursor based on the hash of our
>+    // custom bitmap, as libXcursor has some magic to convert bitmapped cursors
>+    // to themed cursors
>+    if (newType != 0xFF) {
>+        gdkcursor = gdk_cursor_new_from_name(gdk_display_get_default(),
>+                                             GtkCursors[newType].hash);

Doesn't this need to check that hash is non-null?
Comment 13 Chris Coulson 2011-12-13 00:51:05 PST
Created attachment 581198 [details] [diff] [review]
Try creating a named cursor before a bitmap cursor (v2)

Good catch, thanks
Comment 14 Dão Gottwald [:dao] 2011-12-14 02:42:23 PST
https://hg.mozilla.org/mozilla-central/rev/596e3eca4196
Comment 15 Philip Chee 2011-12-26 02:05:08 PST
*** Bug 693517 has been marked as a duplicate of this bug. ***
Comment 16 Alex Keybl [:akeybl] 2011-12-27 08:56:46 PST
Please nominate for Beta 10 approval if this fix is considered low risk.
Comment 17 Mike Hommey [:glandium] 2012-05-11 09:45:22 PDT
Comment on attachment 581198 [details] [diff] [review]
Try creating a named cursor before a bitmap cursor (v2)

[Approval Request Comment]
User impact if declined: Regression in aspect of various mouse cursors not following the system theme on Linux systems in Firefox with ui.use_activity_cursor set to true, and in Thunderbird.
Fix Landed on Version: 11
Risk to taking this patch (and alternatives if risky): 
String changes made by this patch: None
Comment 18 Mike Hommey [:glandium] 2012-05-11 09:46:49 PDT
(In reply to Mike Hommey [:glandium] from comment #17)
> Risk to taking this patch (and alternatives if risky): 

(forgot to fill here) very low risk, and has been tested widely for a while now (since it was released in 11). It would be nice if it was applied on ESR, though.
Comment 19 Lukas Blakk [:lsblakk] use ?needinfo 2012-05-14 12:57:39 PDT
Comment on attachment 581198 [details] [diff] [review]
Try creating a named cursor before a bitmap cursor (v2)

this didn't make it into beta 10 and it doesn't meet the criteria for ESR.

Note You need to log in before you can comment on or make changes to this bug.