Closed Bug 346690 Opened 18 years ago Closed 17 years ago

Implement CSS3 CR cursor:none

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9beta3

People

(Reporter: jwatt, Assigned: Waldo)

References

(Depends on 1 open bug, )

Details

Attachments

(4 files, 1 obsolete file)

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.
Pointers on where to start with this would be gratefully received.
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.)
Aaron, are there accessibility issues to consider here?
(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.
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.
> I guess it depends on whether people will abuse it Hehe. Why would that ever happen on the web? :)
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
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.
Given that we support url() cursors, I don't see the harm in cursor:none.
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?
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.
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?
Assignee: dbaron → nobody
QA Contact: ian → style-system
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?
Depends on: 381744
See bug 381744 for some code that might be useful when implementing this.
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).
Blocks: acid3
Assignee: nobody → jwalden+bmo
Attached patch Patch (obsolete) (deleted) — Splinter Review
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.
Attachment #297268 - Flags: review?(dbaron)
Requesting blocking because it's acid3 and the fix is mostly menial updates to existing functionality without any real complexity...
Flags: blocking1.9?
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.
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.
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.
Status: NEW → ASSIGNED
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.
Attachment #297268 - Flags: review?(dbaron) → review+
(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.
Attachment #297268 - Flags: superreview?(mats.palmgren)
ok, sounds good
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.
Attached patch With more .rc file madness (deleted) — Splinter Review
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. :-)
Attachment #297268 - Attachment is obsolete: true
Attachment #297775 - Flags: superreview?(mats.palmgren)
Attachment #297775 - Flags: review+
Attachment #297268 - Flags: superreview?(mats.palmgren)
Comment on attachment 297775 [details] [diff] [review] With more .rc file madness sr=mats
Attachment #297775 - Flags: superreview?(mats.palmgren) → superreview+
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)...
Attachment #297775 - Flags: approval1.9?
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! :-)
Attachment #297775 - Flags: approval1.9? → approval1.9+
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. :-)
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
that is.
Yep - not a blocker but we'll take the patch in hand.
Flags: blocking1.9? → blocking1.9-
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>
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite+
Flags: in-litmus?
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M11
Attached file Testcase (deleted) —
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...
Attached patch OS/2 fix (checked in) (deleted) — Splinter Review
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. ;-)
Attachment #297878 - Flags: review?(mozilla)
Comment on attachment 297821 [details] [diff] [review] patch for BeOS against current trunk hope logic there is quite clear
Attachment #297821 - Flags: review?(thesuckiestemail)
Attachment #297878 - Flags: review?(mozilla) → review+
Attachment #297878 - Attachment description: OS/2 fix → OS/2 fix (checked in)
Comment on attachment 297821 [details] [diff] [review] patch for BeOS against current trunk r=thesuckiestemail@yahoo.se
Attachment #297821 - Flags: review?(thesuckiestemail) → review+
Do I need approval to land BeOS patch?
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.
(In reply to comment #38) > Do I need approval to land BeOS patch? > Nope. NPOB so you are good to go!
(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
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]
> 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...
(In Preferences) Enable cursor shape manipulation [yes] [no] {should default to yes}
(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.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: