From b5c49ed2300f830adb76a44dd45683c1928229a5 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Fri, 28 Aug 2026 01:58:02 +0000 Subject: [PATCH] Bug 2018625 - Dispatch one touch event per distinct target rather than one per changed touch point. r=edgar PresShell::EventHandler::DispatchTouchEventToDOM() loops over every changed touch point and dispatches a separate DOM event for each one. When more than one touch point changes in a single widget event -- which happens constantly while pinching -- the page therefore receives several touchmove events in a row carrying identical touches and changedTouches lists, since both lists are derived from the full touch list of the widget event. Dispatch at most one event per distinct target instead. Both Blink and WebKit do the same, collecting the targets of the changed touch points into a set and dispatching one TouchEvent per entry, so this follows them. test_mouse_events_after_touchend.html and mouseevents-after-touchend.tentative.html released two touch points on the same target at once and expected two touchend events, so they now expect the single touchend that carries both of them. For the latter that is also what the WebDriver actions it performs should produce: actions belonging to one tick happen at the same moment (https://w3c.github.io/webdriver/#actions, example 11), so the two touch points are removed at the same moment. The test reports changedTouches and touches now so that a mismatch says which behaviour a browser has rather than only that the number of events differs. multi-touch-interactions.html gains two subtests and loses one. The one that starts failing hits the same assertion that already fails for touchend #1, that the previously received targetTouches for the target is not empty, so it is a pre-existing problem now reached for a second target rather than a new one. Differential Revision: https://phabricator.services.mozilla.com/D319010 --- dom/events/test/mochitest.toml | 2 + .../test_mouse_events_after_touchend.html | 17 ++++-- .../test/test_touchmove_once_per_target.html | 60 +++++++++++++++++++ layout/base/PresShell.cpp | 12 ++++ .../multi-touch-interactions.html.ini | 9 +-- .../mouseevents-after-touchend.tentative.html | 42 ++++++++----- 6 files changed, 116 insertions(+), 26 deletions(-) create mode 100644 dom/events/test/test_touchmove_once_per_target.html diff --git a/dom/events/test/mochitest.toml b/dom/events/test/mochitest.toml index 9e00370eed6c..4a3de3792c82 100644 --- a/dom/events/test/mochitest.toml +++ b/dom/events/test/mochitest.toml @@ -668,6 +668,8 @@ skip-if = [ ["test_text_event_in_content.html"] +["test_touchmove_once_per_target.html"] + ["test_unbound_before_in_active_chain.html"] ["test_unicode_input_on_windows_with_emulation.html"] diff --git a/dom/events/test/test_mouse_events_after_touchend.html b/dom/events/test/test_mouse_events_after_touchend.html index a55d8c7e4440..dcc296442975 100644 --- a/dom/events/test/test_mouse_events_after_touchend.html +++ b/dom/events/test/test_mouse_events_after_touchend.html @@ -221,18 +221,24 @@ SimpleTest.waitForFocus(async () => { await promiseFlushingAPZGestureState(); events = []; info("test_multi_touch: testing..."); + // Both touch points are on the same target, so they are reported by a + // single touchend whose changedTouches contains both of them. + let endedTouches = 0; const waitForTouchEnd = new Promise(resolve => { - let count = 0; function onTouchEnd(event) { - if (++count == 2) { - removeEventListener("touchend", onTouchEnd, {capture: true}); - requestAnimationFrame(() => requestAnimationFrame(resolve)); - } + endedTouches = event.changedTouches.length; + removeEventListener("touchend", onTouchEnd, {capture: true}); + requestAnimationFrame(() => requestAnimationFrame(resolve)); } addEventListener("touchend", onTouchEnd, {capture: true}); }); synthesizeTouch(child, [5, 25], 5); await waitForTouchEnd; + is( + endedTouches, + 2, + `Both touch points should be reported by the touchend ${desc}` + ); is( stringifyEvents(shiftEventsBefore(events)), stringifyEvents([]), @@ -242,7 +248,6 @@ SimpleTest.waitForFocus(async () => { stringifyEvents(events), stringifyEvents([ { type: "touchend", target: child }, - { type: "touchend", target: child }, ]), `Multiple touch should not cause mouse events ${desc}` ); diff --git a/dom/events/test/test_touchmove_once_per_target.html b/dom/events/test/test_touchmove_once_per_target.html new file mode 100644 index 000000000000..4096c24e985d --- /dev/null +++ b/dom/events/test/test_touchmove_once_per_target.html @@ -0,0 +1,60 @@ + + + + + +Test for bug 2018625: a touch event is dispatched once per target + + + + + + +
+ + + diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp index 8b1f69b52380..f78806ec1f88 100644 --- a/layout/base/PresShell.cpp +++ b/layout/base/PresShell.cpp @@ -9947,6 +9947,12 @@ void PresShell::EventHandler::DispatchTouchEventToDOM( nsEventStatus tmpStatus = nsEventStatus_eIgnore; WidgetTouchEvent* touchEvent = aEvent->AsTouchEvent(); + // A single widget event may carry more than one changed touch point. In + // that case only one DOM event should be dispatched per distinct target, + // carrying all the changed touch points, rather than one event per changed + // touch point. + AutoTArray, 4> dispatchedTargets; + // loop over all touches and dispatch events on any that have changed for (dom::Touch* touch : touchEvent->mTouches) { // We should remove all suppressed touch instances in @@ -9972,6 +9978,12 @@ void PresShell::EventHandler::DispatchTouchEventToDOM( } content = capturingContent; } + + if (dispatchedTargets.Contains(targetPtr.get())) { + continue; + } + dispatchedTargets.AppendElement(targetPtr); + // copy the event MOZ_ASSERT(touchEvent->IsTrusted()); WidgetTouchEvent newEvent(true, touchEvent->mMessage, touchEvent->mWidget); diff --git a/testing/web-platform/meta/touch-events/multi-touch-interactions.html.ini b/testing/web-platform/meta/touch-events/multi-touch-interactions.html.ini index 739ef7a4452e..4e8ec5d5c6ce 100644 --- a/testing/web-platform/meta/touch-events/multi-touch-interactions.html.ini +++ b/testing/web-platform/meta/touch-events/multi-touch-interactions.html.ini @@ -1,14 +1,11 @@ [multi-touch-interactions.html] expected: if (os == "android") and fission: [OK, TIMEOUT] - [touchstart #2: change in touches.length is valid] - expected: FAIL - [touchend #1: change in targetTouches.length is valid] expected: FAIL - [touchend #2: changedTouches is a subset of last received touches] - expected: FAIL - [touchend #3: change in targetTouches.length is valid] expected: FAIL + + [touchend #2: change in targetTouches.length is valid] + expected: FAIL diff --git a/testing/web-platform/tests/touch-events/mouseevents-after-touchend.tentative.html b/testing/web-platform/tests/touch-events/mouseevents-after-touchend.tentative.html index 3032fadf1936..72fb924172cb 100644 --- a/testing/web-platform/tests/touch-events/mouseevents-after-touchend.tentative.html +++ b/testing/web-platform/tests/touch-events/mouseevents-after-touchend.tentative.html @@ -37,7 +37,12 @@ addEventListener("load", t => { "touchend"]) { if (type == "touchend") { addEventListener(type, event => { - events.push({type: type, target: event.target}); + events.push({ + type: type, + target: event.target, + changedTouches: event.changedTouches.length, + touches: event.touches.length, + }); }, {capture: true}); } else { addEventListener(type, event => { @@ -59,6 +64,12 @@ addEventListener("load", t => { function stringifyEvent(event) { return `{ type: ${event.type}, target: ${ event.target.id || event.target.nodeName + }${ + event.changedTouches !== undefined + ? `, changedTouches: ${event.changedTouches}` + : "" + }${ + event.touches !== undefined ? `, touches: ${event.touches}` : "" }${ event.detail !== undefined ? `, detail: ${event.detail}` : "" }${ @@ -99,7 +110,7 @@ addEventListener("load", t => { assert_equals( stringifyEvents(events), stringifyEvents([ - { type: "touchend", target: child }, + { type: "touchend", target: child, changedTouches: 1, touches: 0 }, { type: "mousemove", target: child, detail: 0, button: 0, buttons: 0 }, { type: "mousedown", target: child, detail: 1, button: 0, buttons: 1 }, { type: "mouseup", target: child, detail: 1, button: 0, buttons: 0 }, @@ -123,7 +134,7 @@ addEventListener("load", t => { assert_equals( stringifyEvents(events), stringifyEvents([ - { type: "touchend", target: child }, + { type: "touchend", target: child, changedTouches: 1, touches: 0 }, ]) ); }, "Single tap whose touchstart is consumed should not cause a click"); @@ -143,7 +154,7 @@ addEventListener("load", t => { assert_equals( stringifyEvents(events), stringifyEvents([ - { type: "touchend", target: child }, + { type: "touchend", target: child, changedTouches: 1, touches: 0 }, ]) ); }, "Single tap whose touchend is consumed should not cause a click"); @@ -166,24 +177,24 @@ addEventListener("load", t => { // double click, therefore, it's fine either single click twice or // a set of a double-click. stringifyEvents([ - { type: "touchend", target: child }, + { type: "touchend", target: child, changedTouches: 1, touches: 0 }, { type: "mousemove", target: child, detail: 0, button: 0, buttons: 0 }, { type: "mousedown", target: child, detail: 1, button: 0, buttons: 1 }, { type: "mouseup", target: child, detail: 1, button: 0, buttons: 0 }, { type: "click", target: child, detail: 1, button: 0, buttons: 0 }, - { type: "touchend", target: child }, + { type: "touchend", target: child, changedTouches: 1, touches: 0 }, { type: "mousemove", target: child, detail: 0, button: 0, buttons: 0 }, { type: "mousedown", target: child, detail: 1, button: 0, buttons: 1 }, { type: "mouseup", target: child, detail: 1, button: 0, buttons: 0 }, { type: "click", target: child, detail: 1, button: 0, buttons: 0 }, ]), stringifyEvents([ - { type: "touchend", target: child }, + { type: "touchend", target: child, changedTouches: 1, touches: 0 }, { type: "mousemove", target: child, detail: 0, button: 0, buttons: 0 }, { type: "mousedown", target: child, detail: 1, button: 0, buttons: 1 }, { type: "mouseup", target: child, detail: 1, button: 0, buttons: 0 }, { type: "click", target: child, detail: 1, button: 0, buttons: 0 }, - { type: "touchend", target: child }, + { type: "touchend", target: child, changedTouches: 1, touches: 0 }, { type: "mousemove", target: child, detail: 0, button: 0, buttons: 0 }, { type: "mousedown", target: child, detail: 2, button: 0, buttons: 1 }, { type: "mouseup", target: child, detail: 2, button: 0, buttons: 0 }, @@ -209,12 +220,12 @@ addEventListener("load", t => { assert_equals( stringifyEvents(events), stringifyEvents([ - { type: "touchend", target: child }, + { type: "touchend", target: child, changedTouches: 1, touches: 0 }, { type: "mousemove", target: child, detail: 0, button: 0, buttons: 0 }, { type: "mousedown", target: child, detail: 1, button: 0, buttons: 1 }, { type: "mouseup", target: child, detail: 1, button: 0, buttons: 0 }, { type: "click", target: child, detail: 1, button: 0, buttons: 0 }, - { type: "touchend", target: child }, + { type: "touchend", target: child, changedTouches: 1, touches: 0 }, { type: "mousemove", target: child, detail: 0, button: 0, buttons: 0 }, { type: "mousedown", target: child, detail: 1, button: 0, buttons: 1 }, { type: "mouseup", target: child, detail: 1, button: 0, buttons: 0 }, @@ -238,12 +249,12 @@ addEventListener("load", t => { assert_equals( stringifyEvents(events), stringifyEvents([ - { type: "touchend", target: child }, + { type: "touchend", target: child, changedTouches: 1, touches: 0 }, { type: "mousemove", target: child, detail: 0, button: 0, buttons: 0 }, { type: "mousedown", target: child, detail: 1, button: 0, buttons: 1 }, { type: "mouseup", target: child, detail: 1, button: 0, buttons: 0 }, { type: "click", target: child, detail: 1, button: 0, buttons: 0 }, - { type: "touchend", target: child }, + { type: "touchend", target: child, changedTouches: 1, touches: 0 }, { type: "mousemove", target: child, detail: 0, button: 0, buttons: 0 }, { type: "mousedown", target: child, detail: 1, button: 0, buttons: 1 }, { type: "mouseup", target: child, detail: 1, button: 0, buttons: 0 }, @@ -265,11 +276,14 @@ addEventListener("load", t => { .pointerUp({sourceName: "touchPointer"}) .pointerUp({sourceName: "touchPointer2"}) .send(); + // Both pointers are released in the same tick, i.e. at the same moment, and + // both of them started on #child. Therefore a single touchend reporting + // both removed touch points is expected rather than one touchend per + // released pointer. assert_equals( stringifyEvents(events), stringifyEvents([ - { type: "touchend", target: child }, - { type: "touchend", target: child }, + { type: "touchend", target: child, changedTouches: 2, touches: 0 }, ]) ); }, "Multi tap should not cause mouse events");