Bug 175346 – Default passive touch event listeners on the root

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Dean Jackson]

Interacting/scrolling pages on iOS is often a terrible experience because the page has poorly performing synchronous touch event handlers. This proposed intervention makes document-level listeners passive by default (meaning they can't call preventDefault to stop scrolling).

https://github.com/WICG/interventions/issues/35
https://docs.google.com/document/d/1II7oSIpd8pK91V5kEM3tDLKcIj398jOJn8Niqy6_loI/edit

https://github.com/whatwg/dom/issues/365

[Dean Jackson]

<rdar://problem/33164597>

[Rick Byers]

See also https://github.com/w3c/touch-events/issues/74

If WebKit ships the same behavior as Chromium then I will argue for updating the touch events spec to define it.

[Rick Byers]

More details (and some of the debate) here: https://groups.google.com/a/chromium.org/d/msg/blink-dev/BW3qrkisqIs/v5Au-HVTAwAJ

In particular, note the detailed behavior description:
Touch Scroll blocking (touchstart, touchmove) event listeners are treated as passive="true" if added to a root node (window, document, body) and if AddEventListenerOptions passive is not specified.

Are you planning on doing the same (window, document and body) or just "document"?

Also be sure not to apply this to "touchend" (just start/move).  Cancelling touchend is important for suppressing click events etc. (eg. sites relying on old fastclick libraries).

[Simon Fraser (smfr)]

We'll follow you.

[Dean Jackson]

*** Bug 164016 has been marked as a duplicate of this bug. ***

[Dean Jackson]

Created attachment 318847 [details]
Patch

[Dean Jackson]

Created attachment 318849 [details]
Patch

[Sam Weinig]

Comment on attachment 318849 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        now in the DOM specification:

This doesn't seem to be in the DOM spec yet.  It still has passive as having a default value of false.

> Source/WebCore/ChangeLog:19
> +        If the event listener explicitly defines "passive" to false in their
> +        options dictionary, then the behaviour is overridden.

This sentence took me a while to figure out what you mean by "the behavior".

> Source/WebCore/dom/EventTarget.cpp:72
> +    std::optional<bool> passive = options.passive;

You could use auto here.

[Simon Fraser (smfr)]

Comment on attachment 318849 [details]
Patch

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

>> Source/WebCore/ChangeLog:19
>> +        options dictionary, then the behaviour is overridden.
> 
> This sentence took me a while to figure out what you mean by "the behavior".

Yeah. Which behavior; the "passive by default", or do you override the request for passive=false?

> Source/WebCore/ChangeLog:24
> +        NOTE: Any fallout from this bug should be collected in:
> +        https://bugs.webkit.org/show_bug.cgi?id=175869
> +        Please do not revert this change just because a site is broken. We'll
> +        gather the issues and see if we can evangelise or detect via code.

Seem weird to put this in the changelog.

> Source/WebCore/dom/EventTarget.cpp:79
> +        if (auto* node = toNode()) {
> +            if (node->isDocumentNode() || node->document().documentElement() == node || node->document().body() == node)
> +                passive = true;
> +        } else if (toDOMWindow())
> +            passive = true;

Maybe flip these around because the toDOMWindow check is cheaper than the node-related checks.

> LayoutTests/fast/events/touch/ios/passive-by-default-on-document-and-window.html:98
> +<div style="height: 500vh">
> +    This is a tall element to make sure there is something to scroll.
> +</div>

Lame. Just style the body with height.

> LayoutTests/fast/events/touch/ios/passive-by-default-overridden-on-document-and-window.html:98
> +<div style="height: 500vh">
> +    This is a tall element to make sure there is something to scroll.
> +</div>

Lame.

[David Tapuska]

Comment on attachment 318849 [details]
Patch

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

> Source/WebCore/ChangeLog:15
> +        https://docs.google.com/document/d/1II7oSIpd8pK91V5kEM3tDLKcIj398jOJn8Niqy6_loI/edit

Any devtools warnings? I know we generate them in chromium to help developers.

[Dean Jackson]

Comment on attachment 318849 [details]
Patch

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

>> Source/WebCore/ChangeLog:12
>> +        now in the DOM specification:
> 
> This doesn't seem to be in the DOM spec yet.  It still has passive as having a default value of false.

Fixed.

>> Source/WebCore/ChangeLog:15
>> +        https://docs.google.com/document/d/1II7oSIpd8pK91V5kEM3tDLKcIj398jOJn8Niqy6_loI/edit
> 
> Any devtools warnings? I know we generate them in chromium to help developers.

We'll do that in a separate bug.

>>> Source/WebCore/ChangeLog:19
>>> +        options dictionary, then the behaviour is overridden.
>> 
>> This sentence took me a while to figure out what you mean by "the behavior".
> 
> Yeah. Which behavior; the "passive by default", or do you override the request for passive=false?

I made this more clear - you get a non-passive listener.

>> Source/WebCore/ChangeLog:24
>> +        gather the issues and see if we can evangelise or detect via code.
> 
> Seem weird to put this in the changelog.

I want to make sure that, in 3 months time, when we autospade a regression, we know what to do with it.

>> Source/WebCore/dom/EventTarget.cpp:72
>> +    std::optional<bool> passive = options.passive;
> 
> You could use auto here.

Fixed.

>> Source/WebCore/dom/EventTarget.cpp:79
>> +            passive = true;
> 
> Maybe flip these around because the toDOMWindow check is cheaper than the node-related checks.

Fixed.

>> LayoutTests/fast/events/touch/ios/passive-by-default-on-document-and-window.html:98
>> +</div>
> 
> Lame. Just style the body with height.

Fixed.

[Dean Jackson]

Committed r221092: <http://trac.webkit.org/changeset/221092>

[Rick Byers]

See https://bugs.webkit.org/show_bug.cgi?id=182521 for discussion of the fall-out of shipping this without also shipping touch-action.

[Lucas Forschler]

Mass move bugs into the DOM component.