To further our support for the Pointer Events spec on iOS, we need to implement capture as specified at https://w3c.github.io/pointerevents/#pointer-capture: the Element APIs, the implicit capture and the gotpointercapture and lostpointercapture events.
<rdar://problem/47605689>
Created attachment 360413 [details] Patch
Created attachment 360414 [details] Patch
Attachment 360414 [details] did not pass style-queue: ERROR: Source/WebCore/page/PointerCaptureController.h:46: The parameter name "implicit" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 25 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 360413 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=360413&action=review > Source/WebCore/dom/Element.h:504 > +#if ENABLE(POINTER_EVENTS) > + ExceptionOr<void> setPointerCapture(int32_t); > + ExceptionOr<void> releasePointerCapture(int32_t); > + bool hasPointerCapture(int32_t); > +#endif You should put: using PointerID = int32_t; and then use PointerID in the parameter types. > Source/WebCore/dom/Element.idl:104 > + [Conditional=POINTER_EVENTS, EnabledAtRuntime=PointerEvents, MayThrowException] void setPointerCapture (long pointerId); > + [Conditional=POINTER_EVENTS, EnabledAtRuntime=PointerEvents, MayThrowException] void releasePointerCapture (long pointerId); > + [Conditional=POINTER_EVENTS, EnabledAtRuntime=PointerEvents] boolean hasPointerCapture (long pointerId); Nit: remove spaces before ( > Source/WebCore/dom/PointerEvent.h:42 > + int32_t pointerId { 0 }; Ditto. Maybe this is where you do the using and it is included in Element.h? > Source/WebCore/dom/PointerEvent.h:82 > + int32_t pointerId() const { return m_pointerId; } Ditto. > Source/WebCore/dom/PointerEvent.h:104 > + int32_t m_pointerId { 0 }; Ditto. > Source/WebCore/page/PointerCaptureController.cpp:42 > +PointerCaptureController::PointerCaptureController() > +{ > +} > + = default > Source/WebCore/page/PointerCaptureController.cpp:52 > + // 2. If the Element on which this method is invoked is not connected ([DOM4]), throw an exception with the name InvalidStateError. Nit: Remove ([DOM4]) > Source/WebCore/page/PointerCaptureController.cpp:57 > + // 3. If this method is invoked while the document has a locked element ([PointerLock]), throw an exception with the name InvalidStateError. Nit: Remove ([PointerLock]) > Source/WebCore/page/PointerCaptureController.cpp:83 > + if (implicit == ImplicitCapture::No && iterator == m_activePointerIdsToCapturingData.end()) Do the implicit check separately, first. > Source/WebCore/page/PointerCaptureController.cpp:88 > + if (!hasPointerCapture(capturingTarget, pointerId)) > + return { }; Bit silly that you have to do the hash lookup again. Maybe make a private helper that takes capturingData directly.... > Source/WebCore/page/PointerCaptureController.cpp:109 > + auto& capturingData = iterator->value; > + return capturingData.pendingTargetOverride == capturingTarget || capturingData.targetOverride == capturingTarget; ... and does this bit. > Source/WebCore/page/PointerCaptureController.cpp:121 > + // When a pointer lock is successfully applied on an element, a user agent MUST run the steps as if the releasePointerCapture() > + // method has been called if any element is set to be captured or pending to be captured. > + for (auto& capturingData : m_activePointerIdsToCapturingData.values()) { > + capturingData.pendingTargetOverride = nullptr; > + capturingData.targetOverride = nullptr; > + } I don't really understand how this comment translates to this logic. > Source/WebCore/page/PointerCaptureController.cpp:124 > +void PointerCaptureController::touchEndedOrWasCanceledForIdentifier(int32_t pointerId) cancelled > Source/WebCore/page/PointerCaptureController.cpp:201 > + if (capturingData.pendingTargetOverride) > + capturingData.targetOverride = capturingData.pendingTargetOverride; > + else > + capturingData.targetOverride = nullptr; capturingData.targetOverride = capturingData.pendingTargetOverride; will work in all cases. > Source/WebCore/page/PointerCaptureController.h:46 > + ExceptionOr<void> releasePointerCapture(Element*, int32_t, ImplicitCapture = ImplicitCapture::No); Did this compile? > Source/WebCore/page/PointerCaptureController.h:58 > + Element* pendingTargetOverride; > + Element* targetOverride; Sure these shouldn't be RefPtr? What happens if the Element is removed while it is captured? Please test. > Source/WebCore/page/PointerCaptureController.h:62 > + HashMap<int32_t, CapturingData> m_activePointerIdsToCapturingData; m_capturingDataForPointerId? > LayoutTests/pointerevents/ios/pointer-events-implicit-capture-has-pointer-capture-in-pointer-down.html:17 > + ui.beginTouches({ x: 50, y: 50 }).then(() => test.done()); await ui.beingTouches(..); test.done(); (would need async in the parameter) > LayoutTests/pointerevents/ios/pointer-events-implicit-capture-release-exception.html:19 > + assert_true(target.hasPointerCapture(event.pointerId)); Why not put test.done() here? > LayoutTests/pointerevents/ios/pointer-events-implicit-capture-release.html:19 > + assert_false(target.hasPointerCapture(event.pointerId)); And here.
Committed r240634: <https://trac.webkit.org/changeset/240634>
Mass move bugs into the DOM component.