User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:36.0) Gecko/20100101 Firefox/36.0 Build ID: 20141020030200 Steps to reproduce: Run either the Pointer Lock Demo (http://mdn.github.io/pointer-lock-demo/ - linked from the MDN PointerLock API page), or the Pointer Lock Controls example from Three.js (http://threejs.org/examples/#misc_controls_pointerlock). Tested in Firefox Nightly, on a Mac. Actual results: The controls behave very erratically: tiny movements of the mouse result in huge movements on screen. If I log the values, then the mozMovementX and mozMovementY values are very large for small movements. Expected results: mozMovementX and mozMovementY report the difference between movements: and the pointer/camera moves in a natural way.

Looks fine in 36.0a1 (2014-10-27) Win 7 x64

I can verify this when e10s is active. Modified STR: 1. Get the latest Firefox Nightly channel (tested on 36.0a1 (2014-10-28) on Windows 7) 2. Navigate to about::support and click "Refresh Nightly.." to reset all prefs. 3. After the browser restarts, choose "Enable and Restart" on the "Would you like to help test e10s dialog?". 4. Navigate to https://dl.dropboxusercontent.com/u/40949268/dump/pointer-lock-demo/index.html 5. Open Web Console (Ctrl+Shift+K on Windows) 6. Click on the canvas, choose "Hide pointer" 7. Move the mouse cursor a tiny amount to try to move the ball by one pixel. 8. Observe the prints in console log. Observed: When the mouse cursor is moved by one pixel, the page prints something like "Got mouse movement of X: -386 and Y:-104" app.js:105 "Got mouse movement of X: -386 and Y:-102" app.js:105 "Got mouse movement of X: -388 and Y:-101" app.js:105 "Got mouse movement of X: -389 and Y:-98" app.js:105 "Got mouse movement of X: -391 and Y:-98" app.js:105 .... Expected: The page should print something like "Got mouse movement of X: 1 and Y:0" app.js:105 "Got mouse movement of X: 2 and Y:1" app.js:105 "Got mouse movement of X: 0 and Y:1" app.js:105 "Got mouse movement of X: 3 and Y:1" app.js:105 "Got mouse movement of X: 0 and Y:2" app.js:105 @Earle Castledine, any chance you had e10s enabled? To verify whether e10s is active, navigate to "about:config" and search for "browser.tabs.remote.autostart". If that says true, e10s is enabled.

Confirmed with e10s enabled.

Yep, sorry I forgot I had enabled it. Well spotted, by the way.

Thanks, good to know, now we are sure this is definitely just e10s specific. You had this on OSX, and I'm seeing it on Windows, so marking OS to all.

This looks like a dupe of bug 1044228.

They definitely are reporting issue with the same, but given that the STRs to verify that the bug is fixed, and that the user stories are different (1044228 just says pointer lock doesn't trigger, this bug says pointer lock does trigger, but values reported are bogus), I'd recommend keeping both open until both STRs are verified to have been fixed.

Relaying a suggestion from an email thread, perhaps we should disable pointer lock when e10s is on, until we can find time to fix this? At least that way apps get a clear error, instead of the API appearing to work but actually being broken.

This fixes the code that makes pointer lock mouse lock work, which involves repositioning the mouse on every user mouse move to the center of the window and then filtering these synthetic events in esm when they loop around. It's really a pretty awful implementation which needs to be re-written. Also tests for pointer lock are disabled pretty much everywhere because they are very prone to failure.

Comment on attachment 8576020 [details] [diff] [review] patch Review of attachment 8576020 [details] [diff] [review]: ----------------------------------------------------------------- Can you talk to handyman about how this will interact with bug 1075670? Hopefully it will make this patch a little simpler. Also, I'm worried about the security implications here. Is there a way we could only allow this if we're in fullscreen mode?

(In reply to Bill McCloskey (:billm) from comment #16) > Comment on attachment 8576020 [details] [diff] [review] > patch > > Review of attachment 8576020 [details] [diff] [review]: > ----------------------------------------------------------------- > > Can you talk to handyman about how this will interact with bug 1075670? > Hopefully it will make this patch a little simpler. Hmm, thought that landed. Will do. > Also, I'm worried about the security implications here. Is there a way we > could only allow this if we're in fullscreen mode? Don't think so, the api doesn't specify you have to be in fullscreen to use this. Single process definitely does not enforce a restriction like it.

(In reply to Jim Mathies [:jimm] from comment #17) > (In reply to Bill McCloskey (:billm) from comment #16) > Don't think so, the api doesn't specify you have to be in fullscreen to use > this. Single process definitely does not enforce a restriction like it. At first I thought maybe we could use the permission manager to handle this. TabParent could get the current URL and check if it has pointer lock permissions. However, we currently allow iframes to do pointer locking, so TabParent can't even say for sure what the right URL is. I'd really like to have pointer locking and this patch seems pretty nice. Maybe we should ask the security team for ideas. I found the original security review for non-fullscreen pointer locking here: https://bugzilla.mozilla.org/show_bug.cgi?id=822654

(In reply to Bill McCloskey (:billm) from comment #18) > (In reply to Jim Mathies [:jimm] from comment #17) > > (In reply to Bill McCloskey (:billm) from comment #16) > > Don't think so, the api doesn't specify you have to be in fullscreen to use > > this. Single process definitely does not enforce a restriction like it. > > At first I thought maybe we could use the permission manager to handle this. > TabParent could get the current URL and check if it has pointer lock > permissions. However, we currently allow iframes to do pointer locking, so > TabParent can't even say for sure what the right URL is. > > I'd really like to have pointer locking and this patch seems pretty nice. > Maybe we should ask the security team for ideas. I found the original > security review for non-fullscreen pointer locking here: > https://bugzilla.mozilla.org/show_bug.cgi?id=822654 I'm a little confused here, a are your security concerns specific to this patch and e10s?

Well, my concerns are related to sandboxing. With sandboxing, we assume that the content process has been compromised and we want to ensure that it still can't do anything bad. This patch would allow it to move the pointer to arbitrary places on the screen. That's a pretty serious exploit. I looked at how Chrome deals with this and they seem to restrict how pointer locking is used in iframes: http://www.chromium.org/developers/design-documents/mouse-lock I don't entirely understand the restrictions though. My fear is that if we land this for e10s then it will be forgotten for sandboxing, so it would be nice to get it right the first time.

I talked to Chris Pearce and he's okay with only allowing pointer locking for top-level frames or for fullscreen. However, it looks like my permission manager check idea won't work. I filed bug 1142256 about that. I guess we should just proceed without any security checking for now since bug 1142256 will be some work. I'll review this once we figure out how this relates to the screenX bug.

Comment on attachment 8588439 [details] [diff] [review] patch One minor change, in TabParent::RecvSynthesizeNativeMouseMove there was no need for the added chrome offset. Puppet widget is now returning the correct value for WidgetToScreenOffset.

I just noticed a comment in here that needed an edit as well.

Comment on attachment 8588444 [details] [diff] [review] patch v.2 Review of attachment 8588444 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/ipc/PBrowser.ipdl @@ +337,5 @@ > + * events. > + * > + * aPoint a synth point relative to the window > + */ > + sync SynthesizeNativeMouseMove(nsIntPoint aPoint); I think you can use LayoutDeviceIntPoint here by declaring it at the top of the file, similar to LayoutDeviceIntRect, which is already there. Then you can drop some casts elsewhere in the patch.

Updated per comments. The previous try push is still good for landing.

FIXED? Has this patch landed in the 40.0a1 (2015-04-08) Nightly? The the pointer lock is still broken there. An easy way to test is the bananabread demo. https://developer.mozilla.org/en-US/demos/detail/bananabread When you press ` to open the menu, the mouse cursor does not work with e10s enabled. Tested with Ubuntu 14.10.

No, it hasn't. It was merged to mozilla-central this morning, so it will be in tomorrow's nightly.

Jim, fyi I've been working on making these native event synthesization functions async and also wiring them up so they work in B2G (i.e. in content processes, which should work on e10s as well). Bug 1146349 has my work so far, and it's getting close to complete. If you have any other changes like this one pending please let me know.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #32) > Jim, fyi I've been working on making these native event synthesization > functions async and also wiring them up so they work in B2G (i.e. in content > processes, which should work on e10s as well). Bug 1146349 has my work so > far, and it's getting close to complete. If you have any other changes like > this one pending please let me know. Not working on anything else related to synth events, this bug was a one-off for this weird pointer lock implementation we have.

Verified working on Unity DT2 and the two links in the first comment. Great! (I did however see that on the three.js demo there is another e10s issue with the cursor not vanishing, filed 1152866.)

Thanks for fixing! It works in everything I've tried, including Em-DOSBox. Control via locked mouse works both in a normal tab and in full screen mode. The cursor was always properly hidden in a normal tab, but always failed to hide in full screen mode as Alon Zakai reports in 1152866.