Bug 68468

Summary: WIP: Implement Mouse Lock API
Product: WebKit Reporter: Vincent Scheib <scheib>
Component: PlatformAssignee: Vincent Scheib <scheib>
Status: RESOLVED INVALID    
Severity: Normal CC: abarth, ap, donggwan.kim, fishd, gregsimon, laszlo.gombos, ojan, scottmg, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch fishd: review-

Vincent Scheib
Reported 2011-09-20 13:53:28 PDT
The Mouse Lock specification http://goo.gl/9G8pd defines an API that provides scripted access to raw mouse movement data while locking the target of mouse events to a single element and removing the cursor from view. This is an essential input mode for certain classes of applications, especially first person perspective 3D applications and 3D modelling software.
Attachments
Patch (61.00 KB, patch)
2011-09-22 13:53 PDT, Vincent Scheib
fishd: review-
Alexey Proskuryakov
Comment 1 2011-09-20 17:11:59 PDT
Please e-mail webkit-dev when you are ready to discuss whether this API should be implemented in WebKit, as well as some level of detail for the implementation.
Vincent Scheib
Comment 2 2011-09-20 20:45:01 PDT
(In reply to comment #1) > Please e-mail webkit-dev when you are ready to discuss whether this API should be implemented in WebKit, as well as some level of detail for the implementation. https://lists.webkit.org/pipermail/webkit-dev/2011-September/017945.html
Vincent Scheib
Comment 3 2011-09-22 13:53:53 PDT
Vincent Scheib
Comment 4 2011-09-22 13:56:25 PDT
Some initial changes to .idl, adding compile/runtime switch, etc. Accessing .movementX/Y works in a prototype that forces mouse lock mode. Plumbing of methods (lockMouse, mouseLocked, ...) still TBD.
Alexey Proskuryakov
Comment 5 2011-09-22 13:58:49 PDT
Comment on attachment 108394 [details] Patch I'm still unsure if it makes sense to add code for non-fullscreen mode at this point.
Darin Fisher (:fishd, Google)
Comment 6 2011-10-13 21:46:31 PDT
(In reply to comment #5) > (From update of attachment 108394 [details]) > I'm still unsure if it makes sense to add code for non-fullscreen mode at this point. Alexey, I thought we had already went over this issue with you. Why do we need to repeat this discussion? To summarize past discussions: The API is designed so as to allow the embedder control over the policy for when this API may be used. It becomes a UA policy decision. We have discussed a UA policy that would only permit mouse lock when in fullscreen mode, but the API is not limited to fullscreen mode.
Alexey Proskuryakov
Comment 7 2011-10-13 23:18:53 PDT
Darin, please note that my latest comment on the bug was several weeks ago, right when we were having this discussion on webkit-dev. However, the discussion was not constructive. You have indicated that you are not confident in having a decent solution for non-fullscreen case, and then asserted that you want to expose general case in IDL regardless. > To summarize past discussions: The API is designed so as to allow the embedder control over the policy for when this API may be used. So, you are proposing yet another fragmentation point for WebKit based browsers, and for the Web in general. Locking the mouse in non-fullscreen mode will work in some browsers, but not in others. It should not come as a surprise that this is frowned upon. Considering UI conventions of different platforms is a very important step in creating specifications for the open Web.
Darin Fisher (:fishd, Google)
Comment 8 2011-10-13 23:48:45 PDT
(In reply to comment #7) > Darin, please note that my latest comment on the bug was several weeks ago, right when we were having this discussion on webkit-dev. Ah, I did indeed miss that. Thanks for pointing that out. > However, the discussion was not constructive. You have indicated that you are not confident in having a decent solution for non-fullscreen case, and then asserted that you want to expose general case in IDL regardless. > > > To summarize past discussions: The API is designed so as to allow the embedder control over the policy for when this API may be used. > > So, you are proposing yet another fragmentation point for WebKit based browsers, and for the Web in general. Locking the mouse in non-fullscreen mode will work in some browsers, but not in others. It should not come as a surprise that this is frowned upon. We are obviously very invested in the open web platform. It would be ideal to arrive at a solution that becomes a web standard. > Considering UI conventions of different platforms is a very important step in creating specifications for the open Web. WebKit does not implement any browser policy UI as far as I can tell. Perhaps it would be helpful if you could explain what you would change about the API if indeed it only ever worked in fullscreen mode? As far as I can tell, it just so happens that the API we like would support both fullscreen and non-fullscreen modes. We like that this leaves open the possibility of exploring a non-fullscreen mode in the future. We are moving conservatively by only adding fullscreen mode support for now.
Alexey Proskuryakov
Comment 9 2011-10-14 15:15:09 PDT
>Perhaps it would be helpful if you could explain what you would change about the API if indeed it only ever worked in fullscreen mode? As far as I can tell, it just so happens that the API we like would support both fullscreen and non-fullscreen modes. One thing that stands out to me is MouseLockable interface. Wouldn't we be better off with navigator.lockMouse(), which could potentially be extended to navigator.lockMouse(in Element target) later? I see that providing an element helps dispatch events. Yet the spec is not consistent anyway - some notifications use DOM events, while others use callback functions. On an unrelated theme, the definition of movementX/movementY seems too platform dependent to me. The spec says "movementX/Y must continue to provide the change in position of the mouse as when the mouse is unlocked". That's likely not what will work for games given dynamic acceleration on some platforms. Such acceleration would yield unacceptable results if applied to FPS shooter movement control. I intend to review the spec more closely soon.
Alexey Proskuryakov
Comment 10 2011-10-18 14:26:39 PDT
The above comment has most of what I had to say upon closer review. Smaller rough edges can be polished during the course of cross-platform and cross-browser implementation, and are not worth discussing here. My bigger new comments are below. I'm surprised by "jog movement over spinner controls" use case. Doesn't that work already? E.g. I can drag the mouse outside of browser window, while <input type=range> will continue to respond. Getting use cases right seems fairly important. Solution for "Synthetic cursor interaction with HTML DOM UI" use case appears to directly contradict HTML5. Synthetic mouse events should not behave like normal ones, it's a bug in WebKit when they do. I see that the spec used to say something about touch events, and no longer does. Regardless of what it used to say, it seems critically important to explain what happens with mouse events that are synthesized from touch events on mobile devices. Should touch capable devices such as phones and tablets just avoid supporting MouseLock?
Vincent Scheib
Comment 11 2011-10-19 22:20:15 PDT
Thanks Alexey, Your comments all seem spec related so I've addressed them on the spec's working group list, public-webevents http://lists.w3.org/Archives/Public/public-webevents/2011OctDec/0066.html Do you have any reason that we can not start landing work behind a flag?
WebKit Review Bot
Comment 12 2011-10-19 22:24:20 PDT
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Alexey Proskuryakov
Comment 13 2011-10-19 22:54:56 PDT
Is your ultimate goal to land attachment 108394 [details] without addressing any feedback? Otherwise, I'm not sure why you marked it for review.
Darin Fisher (:fishd, Google)
Comment 14 2011-10-19 23:28:30 PDT
Comment on attachment 108394 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108394&action=review Chromium WebKit API changes all look kosher to me! > Source/WebCore/dom/Document.idl:249 > + FIXME: Need to sort out how to make this compile: It might be better to separate the Document* changes out into a separate patch since there isn't much here anyways and it has this signature issue to still resolve. > Source/WebCore/dom/MouseEvent.idl:29 > + readonly attribute long movementX; I'd probably want to vendor prefix these attributes until they are finalized. > Source/WebCore/dom/MouseEvent.idl:49 > + in [Optional=CallWithDefaultValue] long movementX, ditto > Source/WebCore/dom/WheelEvent.h:54 > +#if ENABLE(MOUSE_LOCK_API) It might be nicer to avoid these #ifdefs and just unconditionally give these internal data structures the extra fields. However, that would imply expanding these structures for ports that haven't enabled this feature yet. I don't know how much that matters. I sort of prefer the cleanliness of less conditionally compiled code :)
Darin Fisher (:fishd, Google)
Comment 15 2011-10-19 23:32:14 PDT
(In reply to comment #13) > Is your ultimate goal to land attachment 108394 [details] without addressing any feedback? Otherwise, I'm not sure why you marked it for review. Did you notice the link Vincent included in comment #11? He produced a very detailed and thoughtful reply to your concerns. Perhaps you could reply to that in return? I believe Vincent posted the patch to get further feedback so that he could make improvements.
Vincent Scheib
Comment 16 2011-10-20 00:41:44 PDT
(In reply to comment #13) > Is your ultimate goal to land attachment 108394 [details] without addressing any feedback? Otherwise, I'm not sure why you marked it for review. Hey (In reply to comment #15) > (In reply to comment #13) > > Is your ultimate goal to land attachment 108394 [details] [details] without addressing any feedback? Otherwise, I'm not sure why you marked it for review. > > Did you notice the link Vincent included in comment #11? He produced a very detailed and thoughtful reply to your concerns. Perhaps you could reply to that in return? > > I believe Vincent posted the patch to get further feedback so that he could make improvements. Yes, as I requested on webkit-dev where I introduced this feature, I'm looking to make progress on the technical details of implementation, and was seeking review of the code for that purpose. Alexey, I haven't heard you raise a concern not already discussed in the public-webapps thread and codified in the draft spec and FAQs. If you have any issues please raise them explicitly. You also have not proposed why we can not proceed with development behind a flag.
Alexey Proskuryakov
Comment 17 2011-10-20 13:56:41 PDT
I do not know what you mean when asking for "technical feedback". I started drafting a response to you working group posting, but it's very difficult to get rid of the feeling that you are just trying to overcome a perceived obstacle, not to come up with a spec that can be implemented on multiple platforms with their distinct UX conventions. I'll give my answers, but please excuse me if these seem impatient in their turn. > It is reasonable for the application to accept text input to an element that is different than where mouse events are being dispatched. I don't really see what this has to do with keyboard events. They'll still got to a focused element whether mouse is locked or not, correct? This doesn't mean that mouse events need a custom target. >> - Why document.lockMouse() instead of navigator.lockMouse()? > Is there a reason navigator would be more appropriate? Sure, you wouldn't need to worry about which subframe's document to call it on. > Callbacks for the asynchronous success or failure of a call to lockMouse serve the need without adding unnecessary new events, and is a pattern already established in recent specs such as geolocation. Are there any issues with this? You haven't identified any inconsistencies. I'm not sure how to answer this. I said that some events are delivered as events, and others as callbacks. How is that "you haven't identified any inconsistencies"? > You do not believe a click event should be able to be dispatched from JS to another element? What I "believe" is that synthetic events do not trigger a default action per HTML5. It's implicit in the spec, but you can see bug 64580 comments 79 and 80 for a succinct explanation from Hixie. You can certainly dispatch a click event on a button, and you can handle it in a custom event handler, but the button itself won't do anything in response. > You can not today drag a job control in a single direction with unlimited input. OK, I better understand this use case now. It's really questionable if this kind of functionality should be available to web applications at all. > On the topic of acceleration and calibration, there seems to only be speculation that this is harmful. I remember seeing an FPS shooter game that didn't compensate for mouse acceleration, it was basically unplayable. It seems obvious that acceleration that's good for linear movement won't be as good when applied to controlling 3D view. > Users who desire no acceleration can configure it the same way they do with games today (system configuration, drivers, or in game) Are you really suggesting that users will manually tweak global system settings before playing a 3D game in a browser, and change them back afterwards?
Alexey Proskuryakov
Comment 18 2011-10-20 14:10:36 PDT
In addition, I find it extremely annoying that you landed a part of this patch today.
Vincent Scheib
Comment 19 2011-10-20 16:46:00 PDT
Alexey, your contributions to the spec discussion are welcome and I'm pleased to see they are not an obstacle. We announced this feature over a month ago and no concern around implementing the spec in webkit has been raised. I hope to continue discussing your finer points as we move forward breaking the work down to smaller patches to focus discussion. I'll continue to address your specification questions on the webevents mailing list to make them accessible for other parties interested in the spec. http://lists.w3.org/Archives/Public/public-webevents/2011OctDec/0070.html
Note You need to log in before you can comment on or make changes to this bug.