Closed Bug 411273 Opened 12 years ago Closed 6 years ago

Improve appearance of busy/busyButClickable cursor

Categories

(Core :: Widget: Cocoa, enhancement)

All
macOS
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: louise6380, Assigned: jsbruner)

References

Details

Attachments

(3 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2008010801 Minefield/3.0b3pre
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2008010801 Minefield/3.0b3pre

Firefox changes mouse cursor to old black and white wheel when busy.
It should use the rainbow windmill for better OS integration.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Works fine for me on Tiger: I get the colorful beachball when I do e.g.
  javascript:while(1);
to hang Firefox.
I actually tried your example and in my Leopard (10.5.1) it also showed the rainbow windmill.
However, I still see the black and white wheel.  I can't clarify exactly when but I see them more often than the colorful ones.
Version: unspecified → Trunk
Do you mean a little spinning wheel with an arrow (I'll attach a test case next) ? You can see that sometimes on slow loading sites. That is the correct CSS cursor for 'progress' (showing page activity).

The colour beachball indicates a hang/freeze
Attached file test
Is that the icon you mean ?
(In reply to comment #4)
> Created an attachment (id=296085) [details]
> test
> 
> Is that the icon you mean ?
> 

yes, I was talking about this black and white wheel.  Was this intentional?

Interesting, Firefox uses the black-and-white wheel for cursor:progress but Safari uses a blue stationary beachball.
(In reply to comment #5)

> ...  Was this intentional?

Yes, that is intentional. That cursor icon has different semantics than the spinning beachball (rainbow windmill). See bug 362623.

Shall I dupe this bug to 362623 ? Or morph this bug as a RFE to improve the appearance of this particular cursor icon ?
(I'm not sold on the look of Safari's implementation, btw).

(In reply to comment #7)
> Shall I dupe this bug to 362623 ? Or morph this bug as a RFE to improve the
> appearance of this particular cursor icon ?
> (I'm not sold on the look of Safari's implementation, btw).
> 

Thanks, I would like to have this bug changed for improving the look of the progress cursor ( the black and white wheel).  
I read bug 362623 and while I personally think the behavior of Firefox in OS X should be close to that of Safari, this seems to take time to get fixed so we should have better icon that match the entire OS in the mean time.
changing the title.
Firefox should use something that match to the theme of OS X better for the progress cursor.
Summary: Firefox should use rainbow windmill for busy icon → Improve appearance of progress cursor on Mac OS X from black &white wheel
Component: General → Widget: Cocoa
Product: Firefox → Core
QA Contact: general → cocoa
Hardware: PowerPC → All
We should be using the blue circular icon that Safari uses here.
Status: UNCONFIRMED → NEW
Ever confirmed: true
That cursor is available either through the Carbon API or by linking to the JavaVM Framework. It's not part of the general NSCursor API, only via additions in the JavaVM Framework.

I did file a radar asking for it to be promoted to the regular NSCursor API.
That cursor is available either through the Carbon API or by linking to the JavaVM Framework. It's not part of the general NSCursor API, only via additions in the JavaVM Framework.

I did file a radar asking for it to be promoted to the regular NSCursor API.
I'll go ahead and address this when I have a little extra time (since this is definitely not high-priority)

(In reply to Andrew Thompson from comment #12)
> That cursor is available either through the Carbon API or by linking to the
> JavaVM Framework. It's not part of the general NSCursor API, only via
> additions in the JavaVM Framework.
> 
> I did file a radar asking for it to be promoted to the regular NSCursor API.

Correct, however, we do not use the NSCursor API at all here. Instead, we have custom cursors and create them ourselves. It is possible (though I haven't checked), that we use initWithImage:hotSpot: to create NSCursors with our images. This would make the most sense.
Assignee: nobody → josiah
The code is here:
http://dxr.mozilla.org/mozilla-central/widget/cocoa/nsCursorManager.mm

But then I don't need to look very hard, because I wrote most of it ;) I see someone has added Retina support, which is neat.

I am hoping Apple exposes this cursor in the NSCursor API in the next release of Mac OS X. I just checked and RADAR #12939978 is still OPEN. You could file a similar RADAR because multiple votes for the same issue apparently count for something inside Apple, so their engineers say. You can read what I posed in RADAR here: http://openradar.appspot.com/12939978
(In reply to Andrew Thompson from comment #14)
> The code is here:
> http://dxr.mozilla.org/mozilla-central/widget/cocoa/nsCursorManager.mm
> 
> But then I don't need to look very hard, because I wrote most of it ;) I see
> someone has added Retina support, which is neat.
> 
> I am hoping Apple exposes this cursor in the NSCursor API in the next
> release of Mac OS X. I just checked and RADAR #12939978 is still OPEN. You
> could file a similar RADAR because multiple votes for the same issue
> apparently count for something inside Apple, so their engineers say. You can
> read what I posed in RADAR here: http://openradar.appspot.com/12939978

Cool thanks! Just took a look at the code there, and apparently using native NSCursor APIs are not frowned upon (To my great pleasure). Unfortunately, even if the cursor was made available in the next version of the OSX SDK, we wouldn't be able to use it. Our API calls must be available since 10.6 I believe.

What I will do though is create the images and make a new cursor for earlier versions, and we can #IFDEF for 10.9 (If it is made available), and use native NSCursors instead.
Filed another bug to Apple about the issue. Also, if it is possible to access this cursor via a private call, that could be considered as a possible use, especially if we receive the cursor in the 10.9 SDK, in which case any altercations would go unnoticed.

Not saying we necessarily should (Creating another custom cursor may be the best way to go), but it is definitely a possibility.
Here's the image supplied via the Webkit.framework. However, it only supplies this single image. The animations looks like it still needs to be created...
Summary: Improve appearance of progress cursor on Mac OS X from black &white wheel → Improve appearance of busy/busyButClickable cursor
Attached patch Patch. (obsolete) — Splinter Review
So this ended up being substantially easier than I thought.

This patch switches from using our custom spinX cursors and instead uses the undocumented busyButClickableCursor method instead. Though undocumented, Markus Stange went through Appkit dumps from 10.9 to 10.4 and found they all contain the same method, so I conclude that it is safe to use.

Steven, could you review this?
Attachment #8359413 - Flags: review?(smichaud)
Status: NEW → ASSIGNED
> Steven, could you review this?

Sure.  I'll also test it on 10.6 and up.

I should be able to get to it later today or tomorrow.
Comment on attachment 8359413 [details] [diff] [review]
Patch.

One thing I can say right now, even before testing:

Since +[NSCursor busyButClickableCursor] is undocumented, there won't be a declaration for it in any of the standard AppKit header files.  So using it will (or should) trigger a warning.

To get around this you need to add something like the following to one of the Cocoa widgets header files (like nsCursorManager.h):

@interface NSCursor (Undocumented)
+[NSCursor busyButClickableCursor];
@end

Technically this is a category that will be added to the NSCursor class (which gets rid of the warnings).  Move your comment about busyButClickableCuror from where it is now to just above its declaration in this category.
> +[NSCursor busyButClickableCursor];

+ (NSCursor*)NSCursor busyButClickableCursor];
> + (NSCursor*)NSCursor busyButClickableCursor];

+ (NSCursor*)[NSCursor busyButClickableCursor];

Sigh :-(
You mean "+ (NSCursor*)busyButClickableCursor;"? 

I don't think "(NSCursor*)[NSCursor busyButClickableCursor];" will compile. Maybe I'm wrong though.
> You mean "+ (NSCursor*)busyButClickableCursor;"?

Yes.  I guess I was more tired than I realized :-(
Attached patch Patch. (obsolete) — Splinter Review
Added the interface.
Attachment #8359413 - Attachment is obsolete: true
Attachment #8359413 - Flags: review?(smichaud)
Attachment #8359794 - Flags: review?(smichaud)
Hmm. Maybe you meant move the comment inside the interface. I'll do that after the review though if that's what you wanted.
Comment on attachment 8359794 [details] [diff] [review]
Patch.

This looks fine to me.  And I've now tested it on all the major versions of OS X that we support (10.6 and up).  It worked fine, and looks much better than what we had before.

Yes, I did mean for the comment to be just above the declaration of busyButClickableCursor.
Attachment #8359794 - Flags: review?(smichaud) → review+
Attached patch Final Patch.Splinter Review
Moved comment. Carrying r+ flag...
Attachment #8359794 - Attachment is obsolete: true
Attachment #8360037 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/12ec0bd6b782
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Duplicate of this bug: 800233
You need to log in before you can comment on or make changes to this bug.