Skip to content

Commit

Permalink
Make PresShell flush pending synthetic mousemove before dispatchi…
Browse files Browse the repository at this point in the history
…ng `mousedown` or `mouseup

`inert-iframe-hittest.html` expects that `:hover` state should be updated
immediately after a pointer down.  The state is updated by
`EventStateManager::NotifyMouseOver` [1] when we received a real or synthetic
`eMouseMove`.  Therefore, the hover state may have not set yet if no refresh
occurs between the pointer down and checking the result.

Additionally, some WPT for UI Events and Pointer Events expect that `mouseover`
or `mouseout` should be fired before `mouseup`/`pointerup` if a preceding
`mousedown` or `pointerdown` event listener removes the target.  So, this
patch may fix intermittent failures of them if there are.

Therefore, we should flush pending synthetic `mousemove` event for keeping the
event order as same as they happen.  However, simply flushing it causes
unexpected pointer capture state change because
`PointerEventHandler::ProcessPointerCaptureForMouse` handles synthetic
`mousemove` but it should not cause pointer boundary events at the moment (the
pointer boundary events should be fired when the pointer is actually moved, note
that this is different rule from the rules of mouse boundary events).
Therefore, this patch changes
`PointerEventHandler::ShouldGeneratePointerEventFromMouse`.

However, this blocks `lostpointercatpure` event when the capturing content has
already been removed from the DOM tree.  The path is,
`EventHandler::HandleEventWithPointerCapturingContentWithoutItsFrame` stops
dispatching `pointerup` nor `pointercancel` in this case, but
`EventStateManager::PostHandleEvent` is the only handler of implicit pointer
capture release.  Therefore, we need to make it dispatch lostpointercatpure`
event if the canceling event caused `ePointerUp` or `ePointerCancel`.

Finally, this patch fixes a bug of `browser_ext_browserAction_popup_preload.js`.
It tests the pre-loading which starts when `mouseover` before `mousedown` on a
widget.  However, it does not correctly emulate the user input.
* Synthesizing only `mouseover` does not update the internal pointer info and
does not cause dispatching related events.
* Synthesizing `mousedown` without `mousemove` cause `mouseover` before
`mouseup` because at dispatching `mousedown`, `PresShell` stores the cursor [2]
and the mouse boundary events will be dispatched before `mouseup`.

`ext-browserAction.js` assumes the events are fired as this order,
`mouseover` -> `mousedown` -> `mouseup`, but this would not work if the
preceding `mousemove` of `mousedown` is not correctly fired.  I'm not sure
whether `mousemove` is always fired before `mousedown` on any environments,
but it should be rare and it affects only this kind of tricky code.  For now,
fixing this in the test side must be enough.

1. https://searchfox.org/mozilla-central/rev/9a5bf21ea2dd04946734658f67f83f62ca76b0fa/dom/events/EventStateManager.cpp#4874,4913-4914
2. https://searchfox.org/mozilla-central/rev/9a5bf21ea2dd04946734658f67f83f62ca76b0fa/layout/base/PresShell.cpp#6756

Differential Revision: https://phabricator.services.mozilla.com/D193870

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1864654
gecko-commit: 5e411c1e0424f542714c947b1c82020e13e211a9
gecko-reviewers: smaug, dom-core, extension-reviewers, edgar, robwu
  • Loading branch information
masayuki-nakano authored and moz-wptsync-bot committed Jan 24, 2024
1 parent 0a8ff7c commit 785113a
Showing 1 changed file with 81 additions and 0 deletions.
81 changes: 81 additions & 0 deletions uievents/mouse/mouseover-at-removing-mousedown-target.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<meta name="timeout" content="long">
<meta name="variant" content="?duration=16"> <!-- 60fps -->
<meta name="variant" content="?duration=42"> <!-- 24fps -->
<title>Check whether `mouseup` events are fired after pending boundary events</title>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script src=/resources/testdriver.js></script>
<script src=/resources/testdriver-actions.js></script>
<script src=/resources/testdriver-vendor.js></script>
<style>
div#parent {
width: 100%;
height: 50px;
background-color: gray;
}
div#child {
width: 100%;
height: 40px;
background-color: lime;
}
</style>
</head>
<body>
<div id="parent"><div id="child"></div></div>
<script>
"use strict";

const searchParams = new URLSearchParams(document.location.search);
const duration = parseInt(searchParams.get("duration"));

async function runTest(t) {
const parent = document.getElementById("parent");
const child = document.getElementById("child");
const mouseEvents = [];
function onMouseOverOrUp(event) {
// Ignore events before `mousedown` to make this test simpler.
if (mouseEvents[0]?.startsWith("mousedown")) {
mouseEvents.push(`${event.type}@${event.target.localName}${event.target.id ? `#${event.target.id}` : ""}`);
}
}
try {
child.getBoundingClientRect(); // flush layout
child.addEventListener("mousedown", event => {
event.target.remove();
mouseEvents.push("mousedown@div#child");
}, {once: true});
document.addEventListener("mouseover", onMouseOverOrUp, {capture: true});
document.addEventListener("mouseup", onMouseOverOrUp, {once: true, capture: true});
const actions = new test_driver.Actions(duration);
await actions.pointerMove(10, 10, {origin: child})
.pointerDown({button: actions.ButtonType.LEFT})
.pointerUp({button: actions.ButtonType.LEFT})
.send();
await new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve)));
assert_equals(
mouseEvents.toString(),
"mousedown@div#child,mouseover@div#parent,mouseup@div#parent",
t.name
);
} finally {
document.removeEventListener("mouseover", onMouseOverOrUp, {capture: true});
parent.appendChild(child);
}
}

// This test tries to detect intermittent case that mouseout might be fired
// after a while from a DOM tree change. Therefore, trying same test 30 times.
for (let i = 0; i < 30; i++) {
promise_test(async t => {
await runTest(t);
// Make things stabler to start next test.
await new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve)));
}, `mouseover should be fired before mouseup if mousedown target is removed (${i})`);
}
</script>
</body>
</html>

0 comments on commit 785113a

Please sign in to comment.