Bug 191333 – PointerEvents should not require touch event listeners to be registered

• Status: RESOLVED
• Resolution: FIXED
• Component: UI Events

[Dean Jackson]

We have code to make sure that we don't handle touch events if there are no listeners. Since the implementation of pointer events is currently piggy-backing on touch events, it means we require a touch listener to be registered in order to dispatch pointer events.

We need to remove this restriction, or probably duplicate it for pointer events.

[Radar WebKit Bug Importer]

<rdar://problem/45857523>

[Antoine Quint]

Created attachment 354947 [details]
Patch

[Dean Jackson]

Comment on attachment 354947 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=354947&action=review

> Source/WebCore/dom/EventNames.h:362
> +    std::array<std::reference_wrapper<const AtomicString>, 9> touchEventNames() const;

Rename this to touchAndPointerEventNames

> Source/WebKit/UIProcess/WebPageProxy.cpp:2327
> +        updateTrackingType(m_touchEventTracking.touchStartTracking, names.pointerdownEvent);
> +        updateTrackingType(m_touchEventTracking.touchMoveTracking, names.pointermoveEvent);
> +        updateTrackingType(m_touchEventTracking.touchEndTracking, names.pointerupEvent);

Maybe m_touchEventTracking should be renamed as well.

> LayoutTests/pointerevents/utils.js:10
> +    if (arguments.length !== 2 && arguments.length !== 3) {
> +        console.error(`target_test expected 2 or 3 arguments but got ${arguments.length}.`);
> +        return;
> +    }
> +
> +    const impliedOptions = arguments.length == 2;
> +

Don't use the horrible arguments object. Instead, do a ..args rest parameter.

> LayoutTests/pointerevents/utils.js:54
> +            inputType : "hand",

Strange this isn't "finger"

> LayoutTests/pointerevents/utils.js:106
> +        events = [events];

Wouldn't this already take a list?

> LayoutTests/pointerevents/utils.js:107
> +        return this._run(`uiController.sendEventStream('${JSON.stringify({ events })}')`);

nice. You could probably Array-ify the events inside the ${} though.

[EWS Watchlist]

Comment on attachment 354947 [details]
Patch

Attachment 354947 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10005279

New failing tests:
pointerevents/ios/pointer-events-dispatch-on-touch.html

[EWS Watchlist]

Created attachment 354954 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 354947 [details]
Patch

Attachment 354947 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10005930

New failing tests:
pointerevents/ios/pointer-events-dispatch-on-touch.html

[EWS Watchlist]

Created attachment 354967 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[EWS Watchlist]

Comment on attachment 354947 [details]
Patch

Attachment 354947 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10006304

New failing tests:
pointerevents/ios/pointer-events-prevent-default.html
pointerevents/ios/pointer-events-dispatch-on-touch.html

[EWS Watchlist]

Created attachment 354975 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Antoine Quint]

(In reply to Dean Jackson from comment #3)
> Comment on attachment 354947 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=354947&action=review
> 
> > Source/WebCore/dom/EventNames.h:362
> > +    std::array<std::reference_wrapper<const AtomicString>, 9> touchEventNames() const;
> 
> Rename this to touchAndPointerEventNames

Will fix when landing.

> > Source/WebKit/UIProcess/WebPageProxy.cpp:2327
> > +        updateTrackingType(m_touchEventTracking.touchStartTracking, names.pointerdownEvent);
> > +        updateTrackingType(m_touchEventTracking.touchMoveTracking, names.pointermoveEvent);
> > +        updateTrackingType(m_touchEventTracking.touchEndTracking, names.pointerupEvent);
> 
> Maybe m_touchEventTracking should be renamed as well.

Will fix when landing.

> > LayoutTests/pointerevents/utils.js:10
> > +    if (arguments.length !== 2 && arguments.length !== 3) {
> > +        console.error(`target_test expected 2 or 3 arguments but got ${arguments.length}.`);
> > +        return;
> > +    }
> > +
> > +    const impliedOptions = arguments.length == 2;
> > +
> 
> Don't use the horrible arguments object. Instead, do a ..args rest parameter.

You got it, will fix when landing.

> > LayoutTests/pointerevents/utils.js:54
> > +            inputType : "hand",
> 
> Strange this isn't "finger"
> 
> > LayoutTests/pointerevents/utils.js:106
> > +        events = [events];
> 
> Wouldn't this already take a list?

In practice we'll always use a single item, so it felt easier on call sites to pass a single item rather than an array that's a single item.

> > LayoutTests/pointerevents/utils.js:107
> > +        return this._run(`uiController.sendEventStream('${JSON.stringify({ events })}')`);
> 
> nice. You could probably Array-ify the events inside the ${} though.

I tried and failed, but I will try harder!

[Antoine Quint]

Committed r238274: <https://trac.webkit.org/changeset/238274>