[webkit-reviews] review granted: [Bug 191333] PointerEvents should not require touch event listeners to be registered : [Attachment 354947] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Antoine Quint <graouts at apple.com>'s
request for review:
Bug 191333: PointerEvents should not require touch event listeners to be
registered
https://bugs.webkit.org/show_bug.cgi?id=191333

Attachment 354947: Patch

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




--- Comment #3 from Dean Jackson <dino at apple.com> ---
Comment on attachment 354947
  --> https://bugs.webkit.org/attachment.cgi?id=354947
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.