-
Notifications
You must be signed in to change notification settings - Fork 28
Default to passive: true on root document touch scroll blocking event listeners #35
Comments
/cc @teddink, @smaug----, @RByers, @foolip, @BenjaminPoulain PTAL as I value feedback from everyone. |
So how is this different to #18 ? |
By placing this in the wicg we are hoping that we can solicit feedback from our colleagues at Microsoft and Apple. #18 covers more than just this root document intervention it also covers making it by default for everything and making events non-blocking during fling. So this is a concrete implementation of a example used in #18 . |
Doing it in this venue makes sense, but showing the changes as a patch in DOM would I think make it easier to review. |
Thanks for filing this specific concrete issue Dave. Note that there's also an active intent thread on blink-dev. We'll do the cross-vendor discussion here and bring any conclusions to the intent discussion. |
And to be concrete, the precise effect of this change is:
|
Is that any Window, Document or HTMLBodyElement, or some specific ones? |
In Blink the code is here. |
I guess that's if the target is a Node, and using the "node document". That's unambiguous to me, thanks. @smaug----, thoughts on that? |
I've filed this WebKit bug in the hopes that we can get some public signal on whether this is something WebKit would ever consider doing. |
hmm, target.document == target? What does that mean? Or target.document.documentElement == target? Is this only for events dispatched to Window, Document or Body or what? |
And does this mean any document, or only documents which have .defaultView != null ? |
target is instance of EventTarget to which the addEventListener is being called on. It has nothing to do with dispatch target at all. |
Sure, but what exactly is I think what you mean is something more like (based on looking at the Blink implementation):
|
Yes there was an implicit predicate if target is a node on 2, 3, 4. |
My point was that |
#35 (comment) is correct AFAICT. @smaug---- @bzbarsky @annevk, any concerns with this? Just to spell out the obvious, yes, it does mean that @dtapuska, can you poke more people working on Edge to get feedback here? |
I'm very much confused. |
Yes, it's the context object. I think a DOM PR would be good, and perhaps publishing the output somewhere if other vendors want to see this shipped before committing, see blink-dev. |
My initial impression is that it seems rather hairy to bake a dependency on the Window object and HTML's body element in |
I agree it's hacky to special case the Window object etc. Our ultimate goal is to treat all all context objects the same - passive would default to @annevk do you think it could be reasonable to add a generic hook to DOM for getting the default If you think that's plausible then we can file a DOM spec issue and start working on a PR. |
@BenjaminPoulain; do you have any feelings on whatwg/dom#365 ? Olli has promised it's on his todo list; I really value other vendors input in this space. |
Maybe I missed a merge request but is there a property exposed anywhere to query and/or change the default behavior? Or is it all in w3c/touch-events#75? Note that I have not worked in WebCore for a few years. It would be good to contact Beth Dakin for an official position. |
Note that this is on track to ship in Chrome 56 - hitting stable in a few weeks.
Sorry I missed this question. No there's no way to change the default, just override it by specifying |
https://bugs.webkit.org/show_bug.cgi?id=175346 <rdar://problem/33164597> Reviewed by Sam Weinig. Source/WebCore: Make any touchstart or touchmove event listeners passive by default if they are on the document, window, body or document element targets. This follows the "intervention" first implemented by Chrome/Blink: WICG/interventions#35 https://docs.google.com/document/d/1II7oSIpd8pK91V5kEM3tDLKcIj398jOJn8Niqy6_loI/edit whatwg/dom#365 If the event listener explicitly defines "passive" to false in their options dictionary, then they'll still get a non-passive listener. 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. Tests: fast/events/touch/ios/passive-by-default-on-document-and-window.html fast/events/touch/ios/passive-by-default-overridden-on-document-and-window.html * dom/EventNames.h: (WebCore::EventNames::isTouchScrollBlockingEventType const): Added this helper to identify the types of touches we want to check for. * dom/EventTarget.cpp: (WebCore::EventTarget::addEventListener): Check for the event being one of the touch-types that we care about, and the target being one of the Node/Window types we care about. If so, tell the event listener to be passive. * dom/EventTarget.h: Use an optional for the passive member. (WebCore::EventTarget::AddEventListenerOptions::AddEventListenerOptions): * dom/EventTarget.idl: Change "passive" to not have a default value, so we can detect if it was explicitly set to false. LayoutTests: * fast/events/touch/ios/passive-by-default-on-document-and-window-expected.txt: Added. * fast/events/touch/ios/passive-by-default-on-document-and-window.html: Added. * fast/events/touch/ios/passive-by-default-overridden-on-document-and-window-expected.txt: Added. * fast/events/touch/ios/passive-by-default-overridden-on-document-and-window.html: Added. * fast/events/touch/ios/tap-with-active-listener-on-window.html: Explicitly set passive to false. * fast/events/touch/ios/touch-event-regions/document.html: Ditto. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@221092 268f45cc-cd09-0410-ab3c-d52691b4dbfc
These behaviors aren't backed by any spec, but these issues are linked: WICG/interventions#35 WICG/interventions#64
What is the status of getting this behavior into a spec? In whatwg/dom#365 (comment) it was noted that we have tests for this in WPT, but seemingly no spec, so I've renamed the test in web-platform-tests/wpt#29039. |
These behaviors aren't backed by any spec, but these issues are linked: WICG/interventions#35 WICG/interventions#64
…e, a=testonly Automatic update from web-platform-tests Make 2 passive-by-default tests tentative (#29039) These behaviors aren't backed by any spec, but these issues are linked: WICG/interventions#35 WICG/interventions#64 -- wpt-commits: cc6fee62b855f6f93ac9c56bf763f91607d75146 wpt-pr: 29039
(As noted in #72, we intend to archive this repository and are thus triaging and resolving all open issues) Standardization of this particular intervention is tracked in whatwg/dom#365 and Domenic added a comment summarizing the latest status. There's some wpt work going on in web-platform-tests/wpt#34464 now and I think we can close this issue here. |
Chromium/Blink would like to propose an intervention where touch scroll blocking event listeners (touchstart and touchmove) added to Document Level objects would default the AddEventListenerOptions passive field to true.
Document Level objects are defined to be the window, document and body.
Touch scroll blocking event listeners are touchstart and touchmove.
Empirically we have determined that over 80% of touch event listeners invoked don't end up preventing scrolling. There are two opt outs that developers can use:
See more details in Document Level Passive Event Listeners Intervention
and #18
The text was updated successfully, but these errors were encountered: