Last Comment Bug 346690 - Implement CSS3 CR cursor:none
: Implement CSS3 CR cursor:none
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.9beta3
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
:
Mentors:
http://www.w3.org/TR/css3-ui/#cursor
Depends on: 381744
Blocks: acid3
  Show dependency treegraph
 
Reported: 2006-07-31 10:01 PDT by Jonathan Watt [:jwatt] (back in October - email directly if necessary)
Modified: 2010-09-15 06:54 PDT (History)
24 users (show)
mtschrep: blocking1.9-
jwalden+bmo: in‑testsuite+
jwalden+bmo: in‑litmus?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (24.41 KB, patch)
2008-01-15 15:44 PST, Jeff Walden [:Waldo] (remove +bmo to email)
dbaron: review+
Details | Diff | Splinter Review
With more .rc file madness (32.31 KB, patch)
2008-01-18 08:07 PST, Jeff Walden [:Waldo] (remove +bmo to email)
jwalden+bmo: review+
mats: superreview+
mtschrep: approval1.9+
Details | Diff | Splinter Review
patch for BeOS against current trunk (1.02 KB, patch)
2008-01-18 11:12 PST, Sergei Dolgov
thesuckiestemail: review+
Details | Diff | Splinter Review
Testcase (4.12 KB, text/html)
2008-01-18 14:02 PST, Mats Palmgren (:mats)
no flags Details
OS/2 fix (checked in) (8.57 KB, patch)
2008-01-18 14:57 PST, Peter Weilbacher
mozilla: review+
Details | Diff | Splinter Review

Description Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2006-07-31 10:01:01 PDT
The CSS3 Basic User Interface Module Candidate Recommendation adds the value |none| to the values allowed for the |cursor| property. The description given by the CR for this value is: "No cursor is rendered for the element." This behavior can be simulated by using a completely transparent png as a custom cursor, but it would be more convenient to have support for the value |none| built in.
Comment 1 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2006-07-31 10:01:45 PDT
Pointers on where to start with this would be gratefully received.
Comment 2 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2006-07-31 10:19:18 PDT
See whether something that causes no cursor to show up is in the lists of OS cursor constants?  (Look at the nsWindow::SetCursor or nsWidget::SetCursor in each platform widget implementation.)
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2006-08-07 21:06:00 PDT
Aaron, are there accessibility issues to consider here?
Comment 4 Aaron Leventhal 2006-08-08 06:56:50 PDT
(In reply to comment #3)
> Aaron, are there accessibility issues to consider here?

Thank you for asking. It will be disorienting if it's for a large area. 

Yes, users with cognitive or learning challenges will be especially affected, but so will elderly users etc. In fact, only users who are technically savvy have a chance at not being confused by it. 

It may not be an accessibility issue as much as a potential usability annoyance and security issue. Can you still click with the invisible cursor? What is it useful for? Will it make the web better or worse?

I could see it being useful over videos and images where the pointer is annoying. However, in that case the cursor should still be visible while it's moving and only become invisible when it is still for a moment. That's what I see in video player software.

Comment 5 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2006-08-08 07:15:42 PDT
David: Thanks for the tip.

Aaron: Yup, videos is the only real use I can think of for this. Whether the user should be able to click is an interesting question. I guess for my use I'd want to make the cursor visible on the first click while it's hidden and not take any other actions that may otherwise have occurred. As to whether implementing this would make the Web a better place, I guess it depends on whether people will abuse it. There will be moz people who can speak to that from previous issues where abuse has been a concern, but I'd just be speculating.

Maybe a -moz-hide-cursor:<number-of-milliseconds> would be more useful for me instead (the cursor is hidden after being stationary for <number-of-milliseconds>). Again, I don't see how you'd prevent it from being abused.
Comment 6 Aaron Leventhal 2006-08-08 07:29:01 PDT
> I guess it depends on whether people will abuse it
Hehe. Why would that ever happen on the web? :)

Comment 7 Aaron Leventhal 2006-08-08 07:44:00 PDT
Actually, if this is for videos then I think it's the plugin itself which should implement mouse cursor hiding. The plugin isn't looking at the CSS anyway, I don't believe. Since the plugin knows its a video it can hide the mouse cursor when the mouse is still and unclicked, especially in fullscreen mode.

My 2 cents: don't implement cursor: none
Comment 8 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2006-08-09 04:49:39 PDT
I don't know what the reasons for adding cursor:none to CSS3 were. Video was my own use case, but for an XR app, not for video embedded in a web page. Perhaps cursor:none is not the way to do this though, especially if it's going to be controversial.
Comment 9 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2006-08-09 08:52:19 PDT
Given that we support url() cursors, I don't see the harm in cursor:none.
Comment 10 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2006-08-10 03:45:08 PDT
Indeed.

For a patch to be accepted for this bug, which platforms would it need to cater to? Looking at widget/src I see gtk, gtk2, mac, os2, photon, qt, windows and xlib. For the platforms that the patch doesn't have to fix, what's the procedure? Do you look for people who are interested in those platforms and CC them on the bug? Or do you put in stub code?
Comment 11 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2006-08-10 10:20:58 PDT
I suspect most platforms have a constant for this, so it shouldn't be hard to look it up in appropriate documentation and write code for all platforms, even if you can't test it.
Comment 12 Aaron Leventhal 2006-08-10 10:34:18 PDT
David, should the cursor still respond to clicks?

What if the author puts up a fake mouse cursor based on an absolutely positioned element that is offset from the real position, designed to try to get you to click on something other than what you intended?
Comment 13 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2007-05-23 10:08:28 PDT
I think mouse events should be dispatched so that a script that hides/shows the mouse cursor can show it when it gets a click/mousemove event, but maybe preventDefault should be set on click events if the cursor is hidden?
Comment 14 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2007-06-05 14:33:10 PDT
See bug 381744 for some code that might be useful when implementing this.
Comment 15 Jeff Walden [:Waldo] (remove +bmo to email) 2008-01-15 06:17:20 PST
The cursor 'none' is tested in acid3 test 47, although to be clear it's only testing for the presence of such a cursor type (so if you had some of the concerns mentioned here you could just make 'none' display as 'auto' and still pass, although that arguably goes against the spirit of the test).
Comment 16 Jeff Walden [:Waldo] (remove +bmo to email) 2008-01-15 15:44:07 PST
Created attachment 297268 [details] [diff] [review]
Patch

This works on Windows, Linux, and OS X and has stubs for other widget implementations; we pass an extra acid3 test with this patch.

A test URL for ensuring the cursor actually isn't displayed (and that style works correctly) is:

data:text/html,%20<div%20style="width:%20200px;%20height:%20200px;%20background:%20cyan;%20cursor:%20auto;">auto</div>%20%20<div%20style="width:%20200px;%20height:%20200px;%20background:%20gray;%20cursor:%20none;">none</div>%20%20<script>%20onload%20=%20function%20()%20alert(document.getElementsByTagName("div")[1].style.cursor);%20</script>

This passes the layout/style Mochitests.
Comment 17 Jeff Walden [:Waldo] (remove +bmo to email) 2008-01-15 15:56:46 PST
Requesting blocking because it's acid3 and the fix is mostly menial updates to
existing functionality without any real complexity...
Comment 18 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2008-01-15 16:01:58 PST
The normal way to handle 'none' values for CSS properties is to pass VARIANT_NONE as one of the flags to ParseVariant (in CSSParserImpl::ParseCursor) instead of adding none to the keyword list in nsCSSProps.cpp.  However, in this case, what you did may actually be preferable; it's probably better to keep it within eCSSUnit_Enumerated rather than having to add an eCSSUnit_None case.
Comment 19 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2008-01-15 16:13:14 PST
I'm a little suspicious of the cocoa code -- are you sure it's safe to call set/unset with the non-cursor?

And are the hot_x/hot_y 0-based or 1-based?  If the latter, then you may want 1,1 instead of 0,0.  Then again, they're probably 0-based...

Other than that this looks fine.
Comment 20 Jeff Walden [:Waldo] (remove +bmo to email) 2008-01-15 16:43:16 PST
You can't see it in the patch, but setCursor first looks in a cache for the cursor; if it's not there yet, it calls [nsCursorManager createCursor: aCursor].
The createCursor method, if passed a cursor it doesn't recognize, hits this default and returns an arrow cursor:

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/widget/src/cocoa/nsCursorManager.mm&rev=1.8&mark=179-180#160

So the cocoa part's safe.

hot_x and hot_y are relative to the cursor's origin, so 0's right.
Comment 21 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2008-01-15 22:54:14 PST
Comment on attachment 297268 [details] [diff] [review]
Patch

Well, it might be better not to go through the don't-know-what-to-do codepath (i.e., just hide the cursor straight away), but r=dbaron.
Comment 22 Jeff Walden [:Waldo] (remove +bmo to email) 2008-01-15 23:22:50 PST
(In reply to comment #21)
> (From update of attachment 297268 [details] [diff] [review])
> Well, it might be better not to go through the don't-know-what-to-do codepath
> (i.e., just hide the cursor straight away), but r=dbaron.

The problem with that is set has internal side effects with animated timers (at least I gather that's the reason), and I don't believe set is idempotent -- going from memory, I think we'd leak a timer if we called set twice.  That could be changed, but this seems easier and more self-contained.
Comment 23 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2008-01-15 23:49:05 PST
ok, sounds good
Comment 24 Mats Palmgren (:mats) 2008-01-18 06:34:39 PST
Comment on attachment 297268 [details] [diff] [review]
Patch

For many .rc files it seems we have a special version for OS/2 -
do we need to update those too?  ie. the following:
browser/app/splashos2.rc
calendar/sunbird/app/splashos2.rc
mail/app/splashos2.rc
suite/app/splashos2.rc
xulrunner/app/splashos2.rc
toolkit/library/xulrunos2.rc


Please add a XXX placeholder to Photon also, see "eCursor_zoom_in":
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/widget/src/photon/nsWidget.cpp&rev=1.133&root=/cvsroot&mark=578#534


>Index: widget/src/os2/nsWindow.cpp
>+    case eCursor_none:
>+      newPointer = gPtrArray[IDC_NONE-IDC_BASE];

You need to make room in the array.  Add IDC_NONE and update
IDC_COUNT here:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/widget/src/os2/wdgtos2rc.h&rev=1.5&root=/cvsroot


>Index: widget/src/os2/widget.rc
> POINTER IDC_HELP           res\help.ptr
>+POINTER IDC_NONE           res\help.ptr

Ok, but please file a new bug on making a "none.ptr" and updating
the line above, CC mozilla@kaply.com I think he can make one from
the Windows "none.cur" file.
Comment 25 Jeff Walden [:Waldo] (remove +bmo to email) 2008-01-18 08:07:25 PST
Created attachment 297775 [details] [diff] [review]
With more .rc file madness

This better?

I was planning on CCing people associated with the various ports after committing the patch, at which point they could do whatever needed to be done to deal with my hacks.  I doubt I'll file separate bugs for them as I'm not super-interested in getting that bugmail; whatever the ports need to do to handle this should be straightforward and basically uninteresting to me. :-)
Comment 26 Mats Palmgren (:mats) 2008-01-18 08:54:00 PST
Comment on attachment 297775 [details] [diff] [review]
With more .rc file madness

sr=mats
Comment 27 Jeff Walden [:Waldo] (remove +bmo to email) 2008-01-18 09:15:32 PST
Comment on attachment 297775 [details] [diff] [review]
With more .rc file madness

A fairly simple, if tedious, set of changes to increase our numeric acid3 score by 1 (not to mention support more of the CSS3 UI spec)...
Comment 28 Mike Schroepfer 2008-01-18 10:05:16 PST
Comment on attachment 297775 [details] [diff] [review]
With more .rc file madness

I'm approving this over my better judgement.  Jeff we really need to fix that stuff already landed - not add new code! :-)
Comment 29 Jeff Walden [:Waldo] (remove +bmo to email) 2008-01-18 10:52:06 PST
CCing a bunch of people who work on various ports...

I stubbed out code for a transparent cursor in a bunch of widget implementations, but I've only tested Windows, OS X, and gtk2.  It shouldn't be too much effort to fully implement it on new platforms, but I'd prefer if someone who can actually test the code did it.  Note that OS/2 should, if I've done things correctly, display the "help" cursor when |cursor: none| is specified, but it should be fairly obvious how to fix that.  :-)
Comment 30 Sergei Dolgov 2008-01-18 11:04:45 PST
for BeOS real implementation is simple, but requires code restructuring a bit.
case "none" should look like:
be_app->HideCursor(); - and that's all.

but we also should add statement somewhere:
be_app->ShowCursor() for all remaining cases.
Will try to create patch ASAP
Comment 31 Sergei Dolgov 2008-01-18 11:12:12 PST
Created attachment 297821 [details] [diff] [review]
patch for BeOS against current trunk

that is.
Comment 32 Mike Schroepfer 2008-01-18 12:32:13 PST
Yep - not a blocker but we'll take the patch in hand.  
Comment 33 Jeff Walden [:Waldo] (remove +bmo to email) 2008-01-18 13:42:06 PST
Patch is in; tinderboxen seem okay for all the various apps on all the various platforms (excluding what looks like a spurious and unrelated Windows error on a xulrunner tinderbox), so I think we're good here.

I'm marking in-testsuite+ on the basis of the property_database.js change.  Since we don't have a way to programmatically test that the cursor doesn't show or displays as nothing, however, we probably should have a manual test to ensure this.  Simply loading the following URL and mousing over the box to verify the cursor's hidden is a sufficient test:

data:text/html,<div%20style="min-width:100px;height:100px;background:#0f0;cursor:none">Shouldn't%20see%20cursor%20if%20you%20hover%20over%20this%20box</div>
Comment 34 Mats Palmgren (:mats) 2008-01-18 14:02:33 PST
Created attachment 297858 [details]
Testcase

FWIW, here's a testcase I originally did to test bug 163174.
It allows you to see how the cursors look against different backgrounds.
I've added cursor:none to it so it now has all the values we implement.
Maybe it can be used as a base for a litmus test...
Comment 35 Peter Weilbacher 2008-01-18 14:57:57 PST
Created attachment 297878 [details] [diff] [review]
OS/2 fix (checked in)

Thanks for CCing OS/2 people on this bug!

This is the patch with a new fully transparent OS/2 pointer and related changes to the splashos2.rc files to get it to display -- or actually not display. ;-)
Comment 36 Sergei Dolgov 2008-01-18 15:05:12 PST
Comment on attachment 297821 [details] [diff] [review]
patch for BeOS against current trunk

hope logic there is quite clear
Comment 37 tqh 2008-01-19 01:55:59 PST
Comment on attachment 297821 [details] [diff] [review]
patch for BeOS against current trunk

r=thesuckiestemail@yahoo.se
Comment 38 Sergei Dolgov 2008-01-19 02:56:54 PST
Do I need approval to land BeOS patch?
Comment 39 Peter Weilbacher 2008-01-19 03:11:13 PST
I guess for files in beos directories NPOTB (not part of the build) applies. If you want to be really sure you should post in mozilla.dev.planning and get an answer from the release managers.
Comment 40 Mike Schroepfer 2008-01-19 11:57:32 PST
(In reply to comment #38)
> Do I need approval to land BeOS patch?
> 

Nope. NPOB so you are good to go!
Comment 41 Sergei Dolgov 2008-01-19 14:22:43 PST
(In reply to comment #40)
> Nope. NPOB so you are good to go!
> 
Thanks, landed:
/cvsroot/mozilla/widget/src/beos/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 1.142; previous revision: 1.141
done
Comment 42 kguru 2008-01-19 22:12:09 PST
I want there to be an easily accessible and clearly labeled preference to enable or disable this functionality as I could see this being a very dangerous feature.  Perhaps leave it off by default, and if the browser hits cursor:none it should popup asking what you want it to do.

Something like this:

(In Preferences)
Enable cursor invisibility [yes] [no] [ask] {should default to ask}

(Upon hitting cursor:none)
If Preference is still ask:

"This site can cause your mouse cursor to become invisible.  Do you want this to occur." [yes [would then change the preference]] [no]
Comment 43 Boris Zbarsky [:bz] (still a bit busy) 2008-01-19 22:47:57 PST
> I want there to be an easily accessible and clearly labeled preference

What's the point?  See comment 9.  A transparent png cursor is just as invisible...
Comment 44 uamjet602 2008-01-24 03:00:15 PST
(In Preferences)
Enable cursor shape manipulation [yes] [no] {should default to yes}
Comment 45 Vincent Lefevre 2010-09-15 06:54:56 PDT
(In reply to comment #44)
> (In Preferences)
> Enable cursor shape manipulation [yes] [no] {should default to yes}

This would be useful. I've submitted a RFE in bug 596581.

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