Attachment 372765 Details for Bug 197005 – Patch

[patch] Patch

bug-197005-20190624173056.patch (text/plain), 21.19 KB, created by Antoine Quint on 2019-06-24 08:30:57 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antoine Quint

Created: 2019-06-24 08:30:57 PDT

Size: 21.19 KB

patch

obsolete

>Subversion Revision: 246729
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 4b8818d927b4f548ecbc3402102260fb33f1aa30..1ec0a0f020a024ca4adcca40c8112ca451d85355 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,33 @@
>+2019-06-24  Antoine Quint  <graouts@apple.com>
>+
>+        [Pointer Events WPT] Unskip imported/w3c/web-platform-tests/pointerevents/pointerevent_lostpointercapture_is_first.html
>+        https://bugs.webkit.org/show_bug.cgi?id=197005
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We were calling processPendingPointerCapture() at the wrong time, calling in after dispatching a PointerEvent rather than before.
>+        We now do this correctly in the consolidated PointerCaptureController::dispatchEvent() method, which we call for dispatching all
>+        PointerEvents, save for gotpointercapture and lostpointercapture since these should not yield the processing of the pending pointer
>+        capture per the spec.
>+
>+        This uncovered a couple of new issues. First, since we would now call processPendingPointerCapture() and dispatch a lostpointercapture
>+        event earlier, the alternative lostpointercapture dispatch when an element is removed (which is dispatched asynchronously on the
>+        document) would be dispatched *after* dispatching the event in processPendingPointerCapture(). We now check in processPendingPointerCapture()
>+        whether the event target is connected to fix this. This makes sure pointerevent_lostpointercapture_for_disconnected_node.html doesn't regress.
>+
>+        Finally, we must also call processPendingPointerCapture() when implicitly releasing pointer capture during handling of a "pointerup" event.
>+        This ensures that pointerevent_releasepointercapture_invalid_pointerid.html doesn't regress.
>+
>+        As a result of all these changes, we now pass imported/w3c/web-platform-tests/pointerevents/pointerevent_lostpointercapture_is_first.html reliably.
>+
>+        * page/PointerCaptureController.cpp:
>+        (WebCore::PointerCaptureController::dispatchEventForTouchAtIndex):
>+        (WebCore::PointerCaptureController::dispatchEvent): We now more closely adhere to the spec when determining what the pointer capture target is by
>+        only checking for the target override. We can now do this safely since we call processPendingPointerCapture() before and not after event dispatch.
>+        (WebCore::PointerCaptureController::pointerEventWasDispatched):
>+        (WebCore::PointerCaptureController::processPendingPointerCapture): Cache the pending target override to make sure that dispatching a "gotpointercapture"
>+        or "lostpointercapture" event during this function does not alter it until the next call is made when the next event is dispatched.
>+
> 2019-06-24  Antoine Quint  <graouts@apple.com>
> 
>         [Pointer Events] Respect pointer capture when dispatching mouse boundary events and updating :hover
>diff --git a/Source/WebCore/page/PointerCaptureController.cpp b/Source/WebCore/page/PointerCaptureController.cpp
>index f822abdc11db2fd87cd605939e2c0204ffbf7fc7..001b309298a1b36599983adcdfb5c40bf822704b 100644
>--- a/Source/WebCore/page/PointerCaptureController.cpp
>+++ b/Source/WebCore/page/PointerCaptureController.cpp
>@@ -174,8 +174,8 @@ bool PointerCaptureController::preventsCompatibilityMouseEventsForIdentifier(Poi
> #if ENABLE(TOUCH_EVENTS) && PLATFORM(IOS_FAMILY)
> void PointerCaptureController::dispatchEventForTouchAtIndex(EventTarget& target, const PlatformTouchEvent& platformTouchEvent, unsigned index, bool isPrimary, WindowProxy& view)
> {
>-    auto dispatchEvent = [&](const String& type) {
>-        target.dispatchEvent(PointerEvent::create(type, platformTouchEvent, index, isPrimary, view));
>+    auto dispatchOverOrOutEvent = [&](const String& type) {
>+        dispatchEvent(PointerEvent::create(type, platformTouchEvent, index, isPrimary, view), &target);
>     };
> 
>     auto dispatchEnterOrLeaveEvent = [&](const String& type) {
>@@ -200,10 +200,10 @@ void PointerCaptureController::dispatchEventForTouchAtIndex(EventTarget& target,
> 
>         if (type == eventNames().pointerenterEvent) {
>             for (auto& element : WTF::makeReversedRange(targetChain))
>-                element->dispatchEvent(PointerEvent::create(type, platformTouchEvent, index, isPrimary, view));
>+                dispatchEvent(PointerEvent::create(type, platformTouchEvent, index, isPrimary, view), element.ptr());
>         } else {
>             for (auto& element : targetChain)
>-                element->dispatchEvent(PointerEvent::create(type, platformTouchEvent, index, isPrimary, view));
>+                dispatchEvent(PointerEvent::create(type, platformTouchEvent, index, isPrimary, view), element.ptr());
>         }
>     };
> 
>@@ -213,19 +213,17 @@ void PointerCaptureController::dispatchEventForTouchAtIndex(EventTarget& target,
>         // https://w3c.github.io/pointerevents/#the-pointerdown-event
>         // For input devices that do not support hover, a user agent MUST also fire a pointer event named pointerover followed by a pointer event named
>         // pointerenter prior to dispatching the pointerdown event.
>-        dispatchEvent(eventNames().pointeroverEvent);
>+        dispatchOverOrOutEvent(eventNames().pointeroverEvent);
>         dispatchEnterOrLeaveEvent(eventNames().pointerenterEvent);
>     }
> 
>-    pointerEventWillBeDispatched(pointerEvent, &target);
>-    target.dispatchEvent(pointerEvent);
>-    pointerEventWasDispatched(pointerEvent);
>+    dispatchEvent(pointerEvent, &target);
> 
>     if (pointerEvent->type() == eventNames().pointerupEvent) {
>         // https://w3c.github.io/pointerevents/#the-pointerup-event
>         // For input devices that do not support hover, a user agent MUST also fire a pointer event named pointerout followed by a
>         // pointer event named pointerleave after dispatching the pointerup event.
>-        dispatchEvent(eventNames().pointeroutEvent);
>+        dispatchOverOrOutEvent(eventNames().pointeroutEvent);
>         dispatchEnterOrLeaveEvent(eventNames().pointerleaveEvent);
>     }
> }
>@@ -268,16 +266,21 @@ RefPtr<PointerEvent> PointerCaptureController::pointerEventForMouseEvent(const M
> 
> void PointerCaptureController::dispatchEvent(PointerEvent& event, EventTarget* target)
> {
>+    if (!target || event.target())
>+        return;
>+
>+    // https://w3c.github.io/pointerevents/#firing-events-using-the-pointerevent-interface
>+    // If the event is not gotpointercapture or lostpointercapture, run Process Pending Pointer Capture steps for this PointerEvent.
>+    processPendingPointerCapture(event);
>+
>+    // If the pointer capture target override has been set for the pointer, set the target to pointer capture target override object.
>     auto iterator = m_activePointerIdsToCapturingData.find(event.pointerId());
>     if (iterator != m_activePointerIdsToCapturingData.end()) {
>         auto& capturingData = iterator->value;
>-        if (capturingData.pendingTargetOverride && capturingData.targetOverride)
>+        if (capturingData.targetOverride)
>             target = capturingData.targetOverride.get();
>     }
> 
>-    if (!target || event.target())
>-        return;
>-
>     pointerEventWillBeDispatched(event, target);
>     target->dispatchEvent(event);
>     pointerEventWasDispatched(event);
>@@ -335,8 +338,10 @@ void PointerCaptureController::pointerEventWasDispatched(const PointerEvent& eve
>         // override for the pointerId of the pointerup or pointercancel event that was just dispatched, and then run Process Pending
>         // Pointer Capture steps to fire lostpointercapture if necessary.
>         // https://w3c.github.io/pointerevents/#implicit-release-of-pointer-capture
>-        if (event.type() == eventNames().pointerupEvent)
>+        if (event.type() == eventNames().pointerupEvent) {
>             capturingData.pendingTargetOverride = nullptr;
>+            processPendingPointerCapture(event);
>+        }
> 
>         // If a mouse pointer has moved while it isn't pressed, make sure we reset the preventsCompatibilityMouseEvents flag since
>         // we could otherwise prevent compatibility mouse events while those are only supposed to be prevented while the pointer is pressed.
>@@ -348,8 +353,6 @@ void PointerCaptureController::pointerEventWasDispatched(const PointerEvent& eve
>         if (event.type() == eventNames().pointerdownEvent)
>             capturingData.preventsCompatibilityMouseEvents = event.defaultPrevented();
>     }
>-
>-    processPendingPointerCapture(event);
> }
> 
> void PointerCaptureController::cancelPointer(PointerID pointerId, const IntPoint& documentPoint)
>@@ -409,19 +412,22 @@ void PointerCaptureController::processPendingPointerCapture(const PointerEvent&
> 
>     auto& capturingData = iterator->value;
> 
>+    // Cache the pending target override since it could be modified during the dispatch of events in this function.
>+    auto pendingTargetOverride = capturingData.pendingTargetOverride;
>+
>     // 1. If the pointer capture target override for this pointer is set and is not equal to the pending pointer capture target override,
>     // then fire a pointer event named lostpointercapture at the pointer capture target override node.
>-    if (capturingData.targetOverride && capturingData.targetOverride != capturingData.pendingTargetOverride)
>+    if (capturingData.targetOverride && capturingData.targetOverride->isConnected() && capturingData.targetOverride != pendingTargetOverride)
>         capturingData.targetOverride->dispatchEvent(PointerEvent::createForPointerCapture(eventNames().lostpointercaptureEvent, event));
> 
>     // 2. If the pending pointer capture target override for this pointer is set and is not equal to the pointer capture target override,
>     // then fire a pointer event named gotpointercapture at the pending pointer capture target override.
>-    if (capturingData.pendingTargetOverride && capturingData.targetOverride != capturingData.pendingTargetOverride)
>-        capturingData.pendingTargetOverride->dispatchEvent(PointerEvent::createForPointerCapture(eventNames().gotpointercaptureEvent, event));
>+    if (capturingData.pendingTargetOverride && capturingData.targetOverride != pendingTargetOverride)
>+        pendingTargetOverride->dispatchEvent(PointerEvent::createForPointerCapture(eventNames().gotpointercaptureEvent, event));
> 
>     // 3. Set the pointer capture target override to the pending pointer capture target override, if set. Otherwise, clear the pointer
>     // capture target override.
>-    capturingData.targetOverride = capturingData.pendingTargetOverride;
>+    capturingData.targetOverride = pendingTargetOverride;
> }
> 
> } // namespace WebCore
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 05057be3d841691393bcafe075bd7577c92de7c0..b419aae0642db8af8aa264a2c5ad278698405b80 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-06-24  Antoine Quint  <graouts@apple.com>
>+
>+        [Pointer Events WPT] Unskip imported/w3c/web-platform-tests/pointerevents/pointerevent_lostpointercapture_is_first.html
>+        https://bugs.webkit.org/show_bug.cgi?id=197005
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * platform/mac-wk1/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt: Removed.
>+        * platform/mac-highsierra-wk1/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt: Removed.
>+	    * platform/mac-highsierra/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt: Removed.
>+        Since we've fixed the issue with event dispatch in WK1, we can remove these platform-specific expectations.
>+        * platform/mac/TestExpectations: We no longer skip this test which works reliably.
>+        * pointerevents/mouse/pointer-capture.html: We modify this test to correctly expect the "gotpointercapture" event only once the next
>+        pointer event has been dispatched.
>+
> 2019-06-24  Antoine Quint  <graouts@apple.com>
> 
>         [Pointer Events] Respect pointer capture when dispatching mouse boundary events and updating :hover
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index 372878c914e08cc2b35236b1b40ba771b3693ace..71da9c0795fe0305205a77071704eb4fe6d20b16 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-06-24  Antoine Quint  <graouts@apple.com>
>+
>+        [Pointer Events WPT] Unskip imported/w3c/web-platform-tests/pointerevents/pointerevent_lostpointercapture_is_first.html
>+        https://bugs.webkit.org/show_bug.cgi?id=197005
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * web-platform-tests/pointerevents/pointerevent_lostpointercapture_is_first-expected.txt: Added.
>+        * web-platform-tests/resources/testdriver-vendor.js:
>+        (dispatchMouseActions): We need to disable dragMode for the eventSender or else the "pointermove" events in the test will
>+        not be dispatched as there is no mouseUp() call in the test's event sequence.
>+
> 2019-06-24  Antoine Quint  <graouts@apple.com>
> 
>         [Pointer Events] Respect pointer capture when dispatching mouse boundary events and updating :hover
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/pointerevents/pointerevent_lostpointercapture_is_first-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/pointerevents/pointerevent_lostpointercapture_is_first-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..bd7a81c36cc8e2270dc4eb9881d653b1ebe655d5
>--- /dev/null
>+++ b/LayoutTests/imported/w3c/web-platform-tests/pointerevents/pointerevent_lostpointercapture_is_first-expected.txt
>@@ -0,0 +1,18 @@
>+Pointer Events capture test - lostpointercapture order
>+
>+Test Description: This test checks if lostpointercapture is handled asynchronously and prior to all subsequent events. Complete the following actions:
>+Press and hold left mouse button over "Set Capture" button and move a little. "gotpointercapture" should be logged inside of the black rectangle
>+"lostpointercapture" should be logged inside of the black rectangle after pointerup
>+
>+Test passes if lostpointercapture is dispatched after releasing the mouse button and before any additional pointer events.
>+
>+
>+Pointer Events Capture Test
>+
>+The following pointer types were detected: mouse.
>+
>+The following events were logged: gotpointercapture@target0, lostpointercapture@target0.
>+
>+
>+PASS lostpointercapture is dispatched prior to subsequent events 
>+
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/resources/testdriver-vendor.js b/LayoutTests/imported/w3c/web-platform-tests/resources/testdriver-vendor.js
>index dcbbf1fb68585c6bdd9ab987f17f3a1b32955182..5adc0307d25672e5673bd717a9612ce715946fdb 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/resources/testdriver-vendor.js
>+++ b/LayoutTests/imported/w3c/web-platform-tests/resources/testdriver-vendor.js
>@@ -14,6 +14,8 @@ function dispatchMouseActions(actions)
> 
>     return new Promise(resolve => {
>         setTimeout(() => {
>+            eventSender.dragMode = false;
>+
>             for (let action of actions) {
>                 switch (action.type) {
>                 case "pointerMove":
>diff --git a/LayoutTests/platform/mac-highsierra-wk1/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt b/LayoutTests/platform/mac-highsierra-wk1/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt
>deleted file mode 100644
>index e0038c394190958bc7facf64ee37cc078d88099b..0000000000000000000000000000000000000000
>--- a/LayoutTests/platform/mac-highsierra-wk1/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt
>+++ /dev/null
>@@ -1,5 +0,0 @@
>-
>-FAIL Mouse down and capture to green. assert_array_equals: Received events: green received pointerover,green received pointerenter,green received pointermove,green received pointerdown,green received gotpointercapture lengths differ, expected 7 got 5
>-FAIL Mouse down at green and capture to blue. assert_array_equals: Received events: green received pointermove lengths differ, expected 11 got 1
>-FAIL Mouse down and capture to green, move to blue and release capture assert_array_equals: Received events: green received pointermove,green received lostpointercapture lengths differ, expected 12 got 2
>-
>diff --git a/LayoutTests/platform/mac-highsierra/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt b/LayoutTests/platform/mac-highsierra/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt
>deleted file mode 100644
>index 8ae0fa221a4940a8fc2a5e64051530324b1b3e4e..0000000000000000000000000000000000000000
>--- a/LayoutTests/platform/mac-highsierra/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt
>+++ /dev/null
>@@ -1,5 +0,0 @@
>-
>-FAIL Mouse down and capture to green. assert_array_equals: Received events: green received pointerover,green received pointerenter,green received pointermove,green received pointerdown,green received gotpointercapture,green received pointermove,green received pointerout,green received pointerleave,green received pointerover,green received pointerenter,green received pointermove lengths differ, expected 7 got 11
>-FAIL Mouse down at green and capture to blue. assert_array_equals: Received events: green received pointerout,green received pointerover,green received pointerenter,green received pointermove,green received pointermove,green received pointermove,green received pointermove lengths differ, expected 11 got 7
>-FAIL Mouse down and capture to green, move to blue and release capture assert_array_equals: Received events: green received pointerout,green received pointerover,green received pointerenter,green received pointermove,green received lostpointercapture,green received pointermove,green received pointerout,green received pointerleave,blue received pointerover,blue received pointerenter,blue received pointermove,blue received pointermove property 0, expected "green received pointerover" but got "green received pointerout"
>-
>diff --git a/LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt b/LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt
>deleted file mode 100644
>index e0038c394190958bc7facf64ee37cc078d88099b..0000000000000000000000000000000000000000
>--- a/LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt
>+++ /dev/null
>@@ -1,5 +0,0 @@
>-
>-FAIL Mouse down and capture to green. assert_array_equals: Received events: green received pointerover,green received pointerenter,green received pointermove,green received pointerdown,green received gotpointercapture lengths differ, expected 7 got 5
>-FAIL Mouse down at green and capture to blue. assert_array_equals: Received events: green received pointermove lengths differ, expected 11 got 1
>-FAIL Mouse down and capture to green, move to blue and release capture assert_array_equals: Received events: green received pointermove,green received lostpointercapture lengths differ, expected 12 got 2
>-
>diff --git a/LayoutTests/platform/mac/TestExpectations b/LayoutTests/platform/mac/TestExpectations
>index dab2ff787b78eeee866747db76629408a6c31b36..6aae42c460e7071a91f8aa248408ffebaad363a2 100644
>--- a/LayoutTests/platform/mac/TestExpectations
>+++ b/LayoutTests/platform/mac/TestExpectations
>@@ -1885,7 +1885,6 @@ imported/w3c/web-platform-tests/pointerevents/pointerlock/pointerevent_pointerlo
> imported/w3c/web-platform-tests/pointerevents/pointerlock/pointerevent_pointermove_in_pointerlock-manual.html [ Skip ]
> imported/w3c/web-platform-tests/pointerevents/pointerlock/pointerevent_pointermove_on_chorded_mouse_button_when_locked-manual.html [ Skip ]
> 
>-webkit.org/b/197005 imported/w3c/web-platform-tests/pointerevents/pointerevent_lostpointercapture_is_first.html [ Skip ]
> webkit.org/b/197007 imported/w3c/web-platform-tests/pointerevents/pointerlock/pointerevent_coordinates_when_locked.html [ Skip ]
> 
> webkit.org/b/192501 [ Debug ] animations/play-state-in-shorthand.html [ Pass Failure ]
>diff --git a/LayoutTests/pointerevents/mouse/pointer-capture.html b/LayoutTests/pointerevents/mouse/pointer-capture.html
>index 1f124a474c254ed6fcc6a1fc5bacef258c6a0bc2..89d2424acc9a824eb1e312cdea8125102f1ee7dc 100644
>--- a/LayoutTests/pointerevents/mouse/pointer-capture.html
>+++ b/LayoutTests/pointerevents/mouse/pointer-capture.html
>@@ -45,14 +45,11 @@ target_test({ x: "100px", y: "100px", width: "100px", height: "100px" }, (target
>         assert_true(target.hasPointerCapture(pointerId), "The target has pointer capture after calling setPointerCapture().");
>     });
>     eventSender.mouseDown();
>-    eventTracker.assertMatchesEvents([
>-        { type: "gotpointercapture" }
>-    ]);
>-    eventTracker.clear();
> 
>-    // Move the mouse oustide the target again, this time this yields a "pointermove" event since the target has pointer capture.
>+    // Move the mouse oustide the target again. This will yield a "gotpointercapture" event and a "pointermove" event since the target now has pointer capture.
>     eventSender.mouseMoveTo(250, 250);
>     eventTracker.assertMatchesEvents([
>+        { type: "gotpointercapture" },
>         { type: "pointermove", x: 250, y: 250 }
>     ]);
>     eventTracker.clear();

Flags:

dino: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 197005: 372743 | 372744 | 372747 | 372751 | 372754 | 372765