Closed Bug 1278268 Opened 5 years ago Closed 5 years ago

Update widget wheel synthesization functions to move the mouse cursor if needed

Categories

(Core :: Panning and Zooming, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox49 --- wontfix
firefox50 --- fixed

People

(Reporter: kats, Assigned: kats)

Details

(Keywords: arch, Whiteboard: [gfx-noted])

Attachments

(4 files)

(In reply to Botond Ballo [:botond] from bug 1276107 comment #39)
> (In reply to Kartikaya Gupta (email:kats@mozilla.com) from bug 1276107 comment #37)
> > Another, perhaps better, option to avoid the footgun is to modify the
> > wheel-synthesization functions in the widget code to move the mouse to
> > provided coordinates before synthesizing the wheel. I'm not 100% sure if
> > that would work though.
> 
> Discussed this with Kats; this will be explored in a follow-up.

This bug is the follow-up.
Sadly after two days of tinkering with this I am still unable to get it to work on Windows. It looks like if you inject a mousemove followed by a mousewheel into the Windows APIs you still get the mousewheel message back before the mousemove. What a disaster.
I even tried throwing in some NS_DispatchToMainThread calls to spin the event loop and wasn't successful.

The next option is to provide a guarantee that any move-synth call will fire a mousemove event. That way even no-op moves will fire a mousemove event and we don't need to track the coordinates in JS to know if it's a no-op or not.
Turns out windows itself does send us the mousemove event, but in the widget code we run into the condition at [1] and drop it. I have patches that seem to be working on all platforms now. Try pushes coming shortly.
Keywords: arch
Whiteboard: [gfx-noted]
Comment on attachment 8761569 [details]
Bug 1278268 - Ensure that no-op mousemove synthesizations on Windows still fire a mousemove event.

https://reviewboard.mozilla.org/r/58676/#review55760

looks ok to me.
Attachment #8761569 - Flags: review?(jmathies) → review+
Comment on attachment 8761567 [details]
Bug 1278268 - Fix a latent bug in a test where we try to scroll something before it's layerized.

https://reviewboard.mozilla.org/r/58672/#review55906
Attachment #8761567 - Flags: review?(botond) → review+
Comment on attachment 8761568 [details]
Bug 1278268 - Back out cset 8c896f70f97b from bug 1276107 as we have a better fix in hand.

https://reviewboard.mozilla.org/r/58674/#review55912
Attachment #8761568 - Flags: review?(botond) → review+
Comment on attachment 8761570 [details]
Bug 1278268 - Simplify some test code to use the moveMouseAndScrollWheelOver function.

https://reviewboard.mozilla.org/r/58678/#review55914
Attachment #8761570 - Flags: review?(botond) → review+
Thanks for figuring out a proper solution to this footgun, and doing this cleanup!
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/38d10c62b33b
Fix a latent bug in a test where we try to scroll something before it's layerized. r=botond
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d17d61ee826
Back out cset 8c896f70f97b from bug 1276107 as we have a better fix in hand. r=botond
https://hg.mozilla.org/integration/mozilla-inbound/rev/acc080f11d72
Ensure that no-op mousemove synthesizations on Windows still fire a mousemove event. r=jimm
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f6385cec917
Simplify some test code to use the moveMouseAndScrollWheelOver function. r=botond
https://hg.mozilla.org/mozilla-central/rev/9d17d61ee826
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
I don't think this needs uplifting.
You need to log in before you can comment on or make changes to this bug.