[Pointer Events WPT] Unskip imported/w3c/web-platform-tests/pointerevents/pointereven...
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Jun 2019 19:00:17 +0000 (19:00 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Jun 2019 19:00:17 +0000 (19:00 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197005

Reviewed by Dean Jackson.

LayoutTests/imported/w3c:

* 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.

Source/WebCore:

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.

LayoutTests:

* 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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246752 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/pointerevents/pointerevent_lostpointercapture_is_first-expected.txt [new file with mode: 0644]
LayoutTests/imported/w3c/web-platform-tests/resources/testdriver-vendor.js
LayoutTests/platform/mac-highsierra-wk1/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt [deleted file]
LayoutTests/platform/mac-highsierra/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt [deleted file]
LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/pointerevents/pointerevent_mouse_capture_change_hover-expected.txt [deleted file]
LayoutTests/platform/mac/TestExpectations
LayoutTests/pointerevents/mouse/pointer-capture.html
Source/WebCore/ChangeLog
Source/WebCore/page/PointerCaptureController.cpp

index c291dd4..5a16976 100644 (file)
@@ -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 Dean Jackson.
+
+        * 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  Greg Doolittle  <gr3g@apple.com>
 
         Web Inspector: AXI: Audit: image label test is throwing spurious errors on elements with existing alt attr, but no value: <img alt>
index 372878c..bceb500 100644 (file)
@@ -1,5 +1,17 @@
 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 Dean Jackson.
+
+        * 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
         https://bugs.webkit.org/show_bug.cgi?id=198999
         <rdar://problem/51979477>
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 (file)
index 0000000..bd7a81c
--- /dev/null
@@ -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 
+
index dcbbf1f..5adc030 100644 (file)
@@ -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 (file)
index e0038c3..0000000
+++ /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 (file)
index 8ae0fa2..0000000
+++ /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 (file)
index e0038c3..0000000
+++ /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
-
index dab2ff7..6aae42c 100644 (file)
@@ -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 ]
index 1f124a4..89d2424 100644 (file)
@@ -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();
index 9d44f8e..8f2a1e8 100644 (file)
@@ -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 Dean Jackson.
+
+        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  Greg Doolittle  <gr3g@apple.com>
 
         Web Inspector: AXI: Audit: image label test is throwing spurious errors on elements with existing alt attr, but no value: <img alt>
index f822abd..001b309 100644 (file)
@@ -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