1633450 - Mousemove effects appear unexpectedly on touch devices with synthMouseMove pref enabled

Status: RESOLVED FIXED

(Core :: DOM: Events, enhancement)

Description

[Kartikaya Gupta (email:[e-mail])]

Filing this per https://bugzilla.mozilla.org/show_bug.cgi?id=1545393#c8

Currently the layout.reflow.synthMouseMove pref is enabled on Android, which results in unexpected (from a user's point of view) effects on touch devices. These include hover/mouseover effects being "pinned" to the screen position of the last tap after scrolling. STR can be seen here:

https://github.com/mozilla-mobile/fenix/issues/8779
https://github.com/mozilla-mobile/fenix/issues/9860
https://github.com/mozsearch/mozsearch/pull/308#issue-408060050

I plan to turn this pref back off on Android/Fenix, but filing this bug per Makoto's request in case there are other things we can do in Gecko to turn the pref back on without having these unexpected effects.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Note that this effect can be reproduced on Windows touchscreen tablets too, so it affects current desktop releases.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/8)]

Well, in Pointer Events, pointer boundary events should be fired when the element underneath the pointer is changed by a layout change if and only if the pointer is hoverable. So, I think that we should enable the pref on Android but make PresShell stop dispatching it for touch. I'll take a look this too when I work on bug 1967878.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/8)]

Attachment: Bug 1633450 - part 1: Make `PresShell::RecordPointerLocation()` not store the mouse location caused by non-hoverable input device r=smaug!,#dom-core

The reason why we don't synthesize eMouseMove on Android is, changing
hover element for touch is odd. Pointer Events defines that pointer
boundary events at a layout change should be fired if the pointer is
stationary input source [1]. So, it might be true that we should not
synthesize eMouseMove at the position which caused by a touch.

However, then, we'll get the new failures of
pointerevent_after_target_appended.html and
pointerevent_after_target_removed.html when ?touch.

TEST-UNEXPECTED-FAIL |
/pointerevents/pointerevent_after_target_appended.html?touch |
mouse events from touch received before/after child attached at mousedown -
assert_equals: events received expected

"mouseover@parent,mouseenter@parent,mousedown@parent,
(child-attached),
mouseout@parent,mouseover@child,mouseenter@child,mouseup@child,
mousedown@child,mouseup@child,
mouseout@child,mouseleave@child,mouseleave@parent"

but got

"mouseover@parent,mouseenter@parent,mousedown@parent,
(child-attached),
mouseup@child,mouseout@parent,
mouseover@child,mouseenter@child,
mousedown@child,mouseup@child,
mouseout@child,mouseleave@child,mouseleave@parent"

TEST-UNEXPECTED-FAIL | /pointerevents/pointerevent_after_target_appended.html?touch |
mouse events from touch received before/after child moved at mousedown -
assert_equals: events received expected

"mouseover@child,mouseenter@parent,mouseenter@child,mousedown@child,
(child-moved),
mouseover@child,mouseenter@child,mouseup@child,
mousedown@child,mouseup@child,
mouseout@child,mouseleave@child,mouseleave@parent"

but got

"mouseover@child,mouseenter@parent,mouseenter@child,mousedown@child,
(child-moved),
mouseup@child,
mouseover@child,mouseenter@child,
mousedown@child,mouseup@child,
mouseout@child,mouseleave@child,mouseleave@parent"

TEST-UNEXPECTED-FAIL |
/pointerevents/pointerevent_after_target_removed.html?touch |
mouse events from touch received before/after child removal at mousedown -
assert_equals: events received expected

"mouseover@child,mouseenter@parent,mouseenter@child,mousedown@child,
(child-removed),
mouseover@parent,mouseup@parent,
mousedown@parent,mouseup@parent,
mouseout@parent,mouseleave@parent"

but got

"mouseover@child,mouseenter@parent,mouseenter@child,mousedown@child,
(child-removed),
mouseup@parent,
mouseover@parent,
mousedown@parent,mouseup@parent,
mouseout@parent,mouseleave@parent"

So, all of them means that mouse boundary events before mouseup get
lost and mouse boundary events are fired before the following
mousedown instead.

This currently affects to the touch screen on Windows 2-in-1 PCs and
Windows Tablets. This change could cause regressions on them, but
the behavior should be exactly same as Firefox for Android. Therefore,
I believe that this is acceptable regression on Windows since Android
is more important platform than Windows from touchable environment
point of view and the consistency is important for debugging the
behavior both for us and web developers.

1. https://w3c.github.io/pointerevents/#boundary-events-caused-by-layout-changes

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/8)]

Attachment: Bug 1633450 - part 2: Enable synthesized `eMouseMove` on Android r=smaug!,#dom-core

We should enable synthesized eMouseMove on Android too for consistency
between platforms.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/8)]

Attachment: Bug 1633450 - part 1: Make `PresShell::RecordPointerLocation()` forget the last mouse cursor position at `ePointerCancel` if the mouse cursor was for the compatibility mouse event of touch r=smaug!,#dom-core

When APZ starts handling pan or some other gesture, ePointerCancel
is dispatched to notify web apps of end of normal touch input.
Therefore, we can use this event to forget the last mouse cursor
position which was set by eMouseMove of the compatibility mouse
events after a single tap. Then, we can prevent unexpected :hover
state changes after the element underneath the mouse cursor is changed
by a scroll caused by a swipe.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/8)]

Attachment: Bug 1633450 - part 1: Make `PresShell::RecordPointerLocation()` forget the last mouse cursor position at `ePointerCancel` if the mouse cursor was for the compatibility mouse event of touch r=smaug!,#dom-core

When APZ starts handling pan or some other gesture, ePointerCancel
is dispatched to notify web apps of end of normal touch input.
Therefore, we can use this event to forget the last mouse cursor
position which was set by eMouseMove of the compatibility mouse
events after a single tap. Then, we can prevent unexpected :hover
state changes after the element underneath the mouse cursor is changed
by a scroll caused by a swipe.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/8)]

I realized that after enabling the synthesized mousemove on Android debug builds (except in the xorigin mode), test_bug426082.html starts failing frequently. There is an intermittent failure bug for the test. According to the latest log, it occurred only on Wayland builds mostly only on debug builds. So, I guess this is caused by a race.

The failures on Android are these lines. I logged the mouseover event whose default handler updates the :hover state. Then, after synthesizing mousedown on the <label>, mouseover is fired on <html>. (Before that, the test synthesizes mousemove on the <label> first, then, mouseover is fired on the <label> as expected.)

Currently, we don't dispatch mouse boundary events at handling eMouseDown. So, the unexpected mouseover is fired by a synthesized eMouseMove which is fired within the next animation frame. The position should be stored by the last eMouseDown by the root PresShell and the synthesized eMouseMove is fired at the stored position.

Even if I set the viewport, I couldn't fix this.

<meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=no">

Ikezoe-san, do you have any ideas what could happen around dispatching mousedown on Android?

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/8)]

Oh, perhaps, this is not an issue of the test. It passes in the verify mode. So, I guess a mousemove came from the parent process or somewhere else.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/8)]

Its previous test enables the async event dispatch!

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/8)]

Attachment: Bug 1633450 - part 3: Make the previous test of `test_bug426082.html` wait for no pending event before finish r=hiro!

With enabling the synthesize eMouseMove on Android,
test_bug426082.html starts failing frequently. The reason is, it
checks the :hover state of elements which is target of its synthesized
mouse events. However, an unexpected mousemove event may be fired
on the document.

The unexpected mousemove came from the parent process and its
preceding test, test_bug422132.html enables async event dispatching
and synthesizes wheel events. Therefore, this patch makes the test
wait for no pending mouse events with synthesizing a click before
finish.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/987eeaf2a167
https://hg.mozilla.org/integration/autoland/rev/589006a502fe
part 1: Make `PresShell::RecordPointerLocation()` forget the last mouse cursor position at `ePointerCancel` if the mouse cursor was for the compatibility mouse event of touch r=smaug,hiro
https://github.com/mozilla-firefox/firefox/commit/620ab054c6f0
https://hg.mozilla.org/integration/autoland/rev/1c859a558651
part 2: Enable synthesized `eMouseMove` on Android r=smaug
https://github.com/mozilla-firefox/firefox/commit/8205cbc1ed49
https://hg.mozilla.org/integration/autoland/rev/1d959c83f775
part 3: Make the previous test of `test_bug426082.html` wait for no pending event before finish r=hiro

Comment 12

[amarc]

https://hg.mozilla.org/mozilla-central/rev/589006a502fe
https://hg.mozilla.org/mozilla-central/rev/1c859a558651
https://hg.mozilla.org/mozilla-central/rev/1d959c83f775