(no longer active) Reporter
	Description • 12 years ago

See this test case: http://people.mozilla.org/~eakhgari/pointerlock.html

Open the web console, and move the mouse around and click.  This happens both when pointer lock is in effect and when it's not.

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 1 • 12 years ago

It looks like onclick the mozMovementX,Y values are something very close to the MouseEvent.screenX/Y coords. It's not exactly the same, maybe due to the menubar height?

	(no longer active) Reporter
	Comment 2 • 12 years ago

(In reply to Chris Pearce (:cpearce) from comment #1)
> It looks like onclick the mozMovementX,Y values are something very close to
> the MouseEvent.screenX/Y coords. It's not exactly the same, maybe due to the
> menubar height?

Yeah, and they need to be calculated the same way that they are calculated for mousemove.  I'll try to take a look at this tomorrow.

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 3 • 12 years ago

I think mozMovement{X,Y} should be 0 for mouse down/up/click events, that's what Chrome appears to do.

I am running a patch through Try that makes this change now:
https://tbpl.mozilla.org/?tree=Try&rev=9c95957c365c

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 4 • 12 years ago

I think the pointer lock spec agrees, it's not entirely clear that movement{X,Y} should be non-zero only for mousemove events, but I think that's the implication.

"The members movementX and movementY must provide the change in position of the pointer, as if the values of screenX/Y were stored between two subsequent mousemove events eNow and ePrevious and the difference taken movementX = eNow.screenX-ePrevious.screenX."

http://dvcs.w3.org/hg/pointerlock/raw-file/default/index.html#extensions-to-the-mouseevent-interface

	(no longer active) Reporter
	Comment 5 • 12 years ago

(In reply to Chris Pearce (:cpearce) from comment #3)
> I think mozMovement{X,Y} should be 0 for mouse down/up/click events, that's
> what Chrome appears to do.

No, I believe that's the wrong thing to do.  movement{X,Y} need to have the real values for mousedown/up same way was mousemove.  In my tests with Chrome, if you move the mouse really fast and click immediately in a way that a mousemove event is skipped, a mousedown event will be raised with non-zero movementX/Y.

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 6 • 12 years ago

(In reply to Ehsan Akhgari [:ehsan] from comment #5)
> No, I believe that's the wrong thing to do.  movement{X,Y} need to have the
> real values for mousedown/up same way was mousemove.

But that means if script wants to observe all mouse movements it must listen for mouse movement in events other than mousemove? That seems to me to break the contract/abstraction of the mousemove event.

It seems to me that in the case where the mouse have moved before the click comes in, we should dispatch a mousemove event before dispatching the mousedown/up/click events.

	David Humphrey (:humph)
	Comment 7 • 12 years ago

CC'ing Vincent for thoughts on reading of the spec.

	(no longer active) Reporter
	Comment 8 • 12 years ago

(In reply to Chris Pearce (:cpearce) from comment #6)
> (In reply to Ehsan Akhgari [:ehsan] from comment #5)
> > No, I believe that's the wrong thing to do.  movement{X,Y} need to have the
> > real values for mousedown/up same way was mousemove.
> 
> But that means if script wants to observe all mouse movements it must listen
> for mouse movement in events other than mousemove? That seems to me to break
> the contract/abstraction of the mousemove event.
> 
> It seems to me that in the case where the mouse have moved before the click
> comes in, we should dispatch a mousemove event before dispatching the
> mousedown/up/click events.

That's a good point...  I mean, maybe compatibility with Chrome isn't too much of a concern.  The spec is really vague here.

	Vincent Scheib (scheib@chromium.org)
	Comment 9 • 12 years ago

(In reply to Ehsan Akhgari [:ehsan] from comment #8)
> (In reply to Chris Pearce (:cpearce) from comment #6)
> > (In reply to Ehsan Akhgari [:ehsan] from comment #5)
> > > No, I believe that's the wrong thing to do.  movement{X,Y} need to have the
> > > real values for mousedown/up same way was mousemove.
> > 
> > But that means if script wants to observe all mouse movements it must listen
> > for mouse movement in events other than mousemove? That seems to me to break
> > the contract/abstraction of the mousemove event.
> > 
> > It seems to me that in the case where the mouse have moved before the click
> > comes in, we should dispatch a mousemove event before dispatching the
> > mousedown/up/click events.
> 
> That's a good point...  I mean, maybe compatibility with Chrome isn't too
> much of a concern.  The spec is really vague here.

My initial understanding was that all system cursor movement would be presented via mousemove events. On my primary development platform (linux) that appears to be the case for Chrome. That assumption is wrong and or a bug has been introduced recently, I just verified on Chrome canary windows the movement can be lost (though on Chrome 19 I have not discovered that).

IMHO if browsers are able to provide all movement in mousemove events it would be best. I'm not aware of why that shouldn't be possible.

The specification made the assumption that all movement would be delivered via mousemove events. If we believe implementations can always provide mousemove events, I agree the specification should explicitly state that movementX/Y should only be non zero for mousemove events - it would significantly simplify application code.

I'm willing to do what's necessary to fix Chrome to always provide movement via mousemove events.

Here's one of my test pages, btw, that makes loss in pointer motion easy to see manually:
http://scheib.github.com/HTMLMisc/PointerLockAndFullscreen.html
Click on the pink box to align the system cursor and the HTMLCursor. Move frantically without having the mouse locked. The cursors should remain aligned. 

BTW Firefox nightly linux is currently drifting when fullscreen.

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 10 • 12 years ago

Patch, with test.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 11 • 12 years ago

Comment on attachment 633382 [details] [diff] [review]
Patch: Only report MouseEvent.movement{X,Y} values for mousemove events.

Why should movement point be available only on mousemove events?
In general we can't guarantee that the page gets all the mousemove events
(they may go to an iframe for example).

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 12 • 12 years ago

(In reply to Olli Pettay [:smaug] from comment #11)
> Comment on attachment 633382 [details] [diff] [review]
> Patch: Only report MouseEvent.movement{X,Y} values for mousemove events.
> 
> Why should movement point be available only on mousemove events?
> In general we can't guarantee that the page gets all the mousemove events
> (they may go to an iframe for example).

Olli: What I said in comment #6; we'll break the abstraction of the mousemove event if we report mouse moves in events other than mousemove. Authors will have to listen for all up/down/click/move events in order to capture all movements, and that doesn't seem obvious.

If there's unreported movement when a mousedown/up/click is processed, we'd be better to dispatch a mousemove event to report it before dispatching the click (admittedly my patch doesn't do that).

Either way we should clarify the spec... The pointerlock discussion was held in public-webapps right?

	Vincent Scheib (scheib@chromium.org)
	Comment 13 • 12 years ago

public-webapps is the right place for Pointer Lock spec discussion, though I'm the editor and actively listening here.

	Alex Keybl [:akeybl]
	Comment 14 • 12 years ago

Would we ever consider backing out bug 633602 if this was left unfixed in FF14? If not, we'll untrack for release.

	(no longer active) Reporter
	Comment 15 • 12 years ago

Is there any update on this, Chris?

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 16 • 12 years ago

Don't think this blocks basecamp. If I understand correctly you should only be looking at these properties after getting a mouselock, and since you can't do that on B2G (right?) no pages should be looking at these properties.

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 17 • 12 years ago

(In reply to Alex Keybl [:akeybl] from comment #14)
> Would we ever consider backing out bug 633602 if this was left unfixed in
> FF14? If not, we'll untrack for release.

We can ship without this being fixed.

(In reply to Ehsan Akhgari [:ehsan] from comment #15)
> Is there any update on this, Chris?

I've been working on other things.

I was waiting for Olli to reply to my comment 12, and am considering writing a patch to flush mouse movements with a mousemove event before dispatching mouse up/down/click events.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 18 • 12 years ago

(In reply to Chris Pearce (:cpearce) from comment #12)
> If there's unreported movement when a mousedown/up/click is processed, we'd
> be better to dispatch a mousemove event to report it before dispatching the
> click (admittedly my patch doesn't do that).

As far as I know, we don't report all the mousemoves. There are some tricky cases like moving the window, but
not mouse etc.

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 19 • 12 years ago

(In reply to Jonas Sicking (:sicking) from comment #16)
> Don't think this blocks basecamp. If I understand correctly you should only
> be looking at these properties after getting a mouselock, and since you
> can't do that on B2G (right?) no pages should be looking at these properties.

MouseEvent.mouseMovement{X,Y} are specified to report valid values, regardless of whether the mouse is locked or not.

We also report mouseMovement{X,Y} on B2G for the mouse events generated by a user touch & drag actions, so an app could still rely on mousemove when running on B2G.

	Alex Keybl [:akeybl]
	Comment 20 • 12 years ago

We're shipping this in FF14, I see no reason to track for upcoming releases in that case.

	(no longer active) Reporter
	Comment 21 • 12 years ago

Chris, are you working on a fix for this?  I'm planning to work around this bug in Emscripten since clicking can be completely broken in full-screen mode, but that would not be ideal since we may lose some real mouse movement information with that workaround.

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 22 • 12 years ago

I was planning on updating my patch to flush unreported mouse moves before sending mousedown/up/click, but I'm not sure when that will be, maybe a week to two. If you want to take the bug and work on it in the meantime, that's fine with me.

	Vincent Scheib (scheib@chromium.org)
	Comment 23 • 12 years ago

I've updated the spec to be clear that movement should be delivered via mousemove events.

http://dvcs.w3.org/hg/pointerlock/raw-file/default/index.html#extensions-to-the-mouseevent-interface
diff: http://dvcs.w3.org/hg/pointerlock/log/default/index.html

	(no longer active) Reporter
	Comment 24 • 12 years ago

(In reply to Vincent Scheib from comment #23)
> I've updated the spec to be clear that movement should be delivered via
> mousemove events.
> 
> http://dvcs.w3.org/hg/pointerlock/raw-file/default/index.html#extensions-to-
> the-mouseevent-interface
> diff: http://dvcs.w3.org/hg/pointerlock/log/default/index.html

Great, thanks a lot!

Chris, this matches what your patch here does in Gecko right?

	(no longer active) Reporter
	Comment 25 • 12 years ago

(In reply to Ehsan Akhgari [:ehsan] from comment #24)
> (In reply to Vincent Scheib from comment #23)
> > I've updated the spec to be clear that movement should be delivered via
> > mousemove events.
> > 
> > http://dvcs.w3.org/hg/pointerlock/raw-file/default/index.html#extensions-to-
> > the-mouseevent-interface
> > diff: http://dvcs.w3.org/hg/pointerlock/log/default/index.html
> 
> Great, thanks a lot!
> 
> Chris, this matches what your patch here does in Gecko right?

Ping?

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 26 • 12 years ago

(In reply to Ehsan Akhgari [:ehsan] from comment #25)
> Ping?

Oops, sorry!

> (In reply to Ehsan Akhgari [:ehsan] from comment #24)
> > (In reply to Vincent Scheib from comment #23)
> > > I've updated the spec to be clear that movement should be delivered via
> > > mousemove events.
> > > 
> > > http://dvcs.w3.org/hg/pointerlock/raw-file/default/index.html#extensions-to-
> > > the-mouseevent-interface
> > > diff: http://dvcs.w3.org/hg/pointerlock/log/default/index.html
> > 
> > Great, thanks a lot!

This is good. Thanks Vincent.

> > Chris, this matches what your patch here does in Gecko right?

I think my patch here has the effect of *discarding* mouse movement between the last mouse move and the mouse down. This may or may not be a negligible amount of movement. I'm probably not going to get to fixing this for a week or two, since I've got lots of blocking-basecamp+ stuff to work on first.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)
	Comment 27 • 11 years ago

Bumping this up in priority; Chris, any chance of updating your patch?  Or can you describe what needs to be done?

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 28 • 11 years ago

Vlad: My priority queue is overflowing to the point that it's leaking. I recommend you find someone else to tackle this.

	Julien Wajsberg [:julienw]
	Comment 29 • 7 years ago

This is still happening: http://jsbin.com/yezipiz/8

	Sylvestre Ledru [:Sylvestre]
	Comment 30 • 6 years ago

No assignee, updating the status.

	Sylvestre Ledru [:Sylvestre]
	Comment 31 • 6 years ago

No assignee, updating the status.