Attachment 372744 Details for Bug 197005 – Patch

[patch] Patch

bug-197005-20190624140156.patch (text/plain), 16.58 KB, created by Antoine Quint on 2019-06-24 05:01:58 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antoine Quint

Created: 2019-06-24 05:01:58 PDT

Size: 16.58 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..b3b6473c6bc3fdd1c6fe906552099048f2eb8274 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);
>         } else {
>             for (auto& element : targetChain)
>-                element->dispatchEvent(PointerEvent::create(type, platformTouchEvent, index, isPrimary, view));
>+                dispatchEvent(PointerEvent::create(type, platformTouchEvent, index, isPrimary, view), element);
>         }
>     };
> 
>@@ -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..819846ac2fd7ce9241f770b8dfe24b5fa9dbc874 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+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.
>+        Since we've fixed the issue with event dispatch in WK1, we can remove this WK1-specific test expectation.
>+        * platform/mac/TestExpectations: We no longer skip this test which works reliably.
>+
> 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-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 ]

Actions: View | Formatted Diff | Diff

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