Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 639227 Force document level scroll blocking event listeners to be passive
Starred by 7 users Project Member Reported by kenjibaheux@chromium.org, Aug 19 2016 Back to list
Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 626086
issue 626101
issue 627104

Blocking:
issue 669901
issue 625675



Sign in to add a comment
Status: Started
Blocking: 625675
Blockedon: 627104
Comment 4 by rbyers@chromium.org, Aug 22 2016
Blocking: 639433
Comment 5 by rbyers@chromium.org, Aug 23 2016
Blockedon: 626101
Comment 6 by rbyers@chromium.org, Aug 23 2016
Blockedon: 626086
Components: Blink>Input
Labels: -M-54 M-55
Additional resources:
 - W3C tracking: https://github.com/WICG/interventions/issues/18
 - Overview doc: https://docs.google.com/document/d/1mlUcJZHLyz1fip7CNEiU0w7JgcxBZUilvOwg0i9ZvQc/edit

Quick status update:
 - currently launched as a 50% finch trial in dev-channel.  Results indicate a 50% improvement in scroll start times in the 99th percentile, and substantial improvements across the board.  This is a huge win for scroll performance!
 - aiming to expand to a trial in beta in M54
 - working on DevRel outreach and devtools improvements
 - hoping to ship 100% in M55 (or M56)
Comment 9 by phistuck@gmail.com, Sep 5 2016
I think this will break (relatively) smooth sticky elements. They might suddenly lag and look bad (there is already a small jump that looks bad, but this might make things even worse).
I think this should be gated on position: sticky,  issue 231752 .
Do you have an example?
Note that the AMP carousel is currently (subtly) broken by this intervention.  Filed issue here: https://github.com/ampproject/amphtml/issues/4820
Comment 13 by kbr@chromium.org, Oct 20 2016
Blocking: -639433
Comment 14 by kbr@chromium.org, Oct 20 2016
Blockedon: 639433
This will definitely break a lot of existing code. Previously if you wanted to suppress scrolling (which sometimes *is* what you wanted) - you registerd a listener and handled the event. What else should you have done instead?

With the relatively new "passive" event listener API there was a way to improve performance for certain cases, but still the old implementation worked. Now all those simple listeners that prevent scrolling (their only purpose) will stop working. You are breaking the one best API you had for this purpose. 

OK - people can rewrite their pages and add a "passive:false", they need to rewrite their code every other month anyway since the web platform team decided that backwards compatibility is not as important as new features and a "faster web". 
But signs are that you actually have plans to actively ignore the "passive:false" flags in a future version - what should we do then?
To quote: " However some sort of intervention may still be necessary if lots of slow sites still end up opting-out of passive listeners." - It's great that you want to make the web faster, but most devs prefer correctly working sites over better performance. It's not the job of the UA to work against the devs. The UA cannot be smarter than *all* the devs. If devs are dumb and their sites are slow - let them starve to death, but don't break sites that are carefully crafted, use the new API correctly and are happy if they loose a bit of responsiveness if this means that their site is not broken. With this and the upcoming proposed changes you cannot choose anymore, which is a bad thing.
Yes specifying passive: false is the main footgun and we don't want many people to use that. We believe most actions that authors intend already are covered by specifying touch-action.

In order for it to work in IE & Edge you would have needed to use touch-action. And that still works fine today. The real question is what percentage of sites don't use touch-action yet preventDefault that register and event listener on the main document. From the data we've gathered it is a small percentage of sites.


Comment 17 by woxxom@gmail.com, Oct 25 2016
The bug title doesn't say which events.
All 100 of them https://developer.mozilla.org/docs/Web/Events ?

Summary: Force document level scroll blocking event listeners to be passive (was: Force document level event listeners to be passive)
Updated bug title. It is documented in the design doc.
Comment 19 by woxxom@gmail.com, Oct 25 2016
So it means that in order to block/alter scrolling intentionally (on a desktop) by overriding mouse "wheel" event, we'll have to draw a transparent element over the entire document window and use it to attach an event listener.
Cc: rbyers@chromium.org
woxxom@ we aren't intending to go ahead with making wheel events passive until there is a declarative way (like touch-action) for wheel events. The intervention was only sent for touch events; however this bug tracks both touch and wheel.
Project Member Comment 21 by bugdroid1@chromium.org, Nov 11 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e913a6698f089e11e89fcc04ee19d34dac82d30a

commit e913a6698f089e11e89fcc04ee19d34dac82d30a
Author: dtapuska <dtapuska@chromium.org>
Date: Fri Nov 11 19:44:59 2016

Add use counter when touch-action isn't used when preventDefault'd.

Keep track of the lack of usage of touch-action on pages.

BUG= 639227 
TBR=isherman@chromium.org

Review-Url: https://codereview.chromium.org/2475443004
Cr-Commit-Position: refs/heads/master@{#431618}

[modify] https://crrev.com/e913a6698f089e11e89fcc04ee19d34dac82d30a/third_party/WebKit/Source/core/events/AddEventListenerOptionsResolved.cpp
[modify] https://crrev.com/e913a6698f089e11e89fcc04ee19d34dac82d30a/third_party/WebKit/Source/core/events/AddEventListenerOptionsResolved.h
[modify] https://crrev.com/e913a6698f089e11e89fcc04ee19d34dac82d30a/third_party/WebKit/Source/core/events/Event.cpp
[modify] https://crrev.com/e913a6698f089e11e89fcc04ee19d34dac82d30a/third_party/WebKit/Source/core/events/Event.h
[modify] https://crrev.com/e913a6698f089e11e89fcc04ee19d34dac82d30a/third_party/WebKit/Source/core/events/EventTarget.cpp
[modify] https://crrev.com/e913a6698f089e11e89fcc04ee19d34dac82d30a/third_party/WebKit/Source/core/events/RegisteredEventListener.h
[modify] https://crrev.com/e913a6698f089e11e89fcc04ee19d34dac82d30a/third_party/WebKit/Source/core/events/TouchEvent.cpp
[modify] https://crrev.com/e913a6698f089e11e89fcc04ee19d34dac82d30a/third_party/WebKit/Source/core/events/TouchEvent.h
[modify] https://crrev.com/e913a6698f089e11e89fcc04ee19d34dac82d30a/third_party/WebKit/Source/core/frame/UseCounter.h
[modify] https://crrev.com/e913a6698f089e11e89fcc04ee19d34dac82d30a/third_party/WebKit/Source/core/input/TouchEventManager.cpp
[modify] https://crrev.com/e913a6698f089e11e89fcc04ee19d34dac82d30a/third_party/WebKit/Source/core/input/TouchEventManager.h
[modify] https://crrev.com/e913a6698f089e11e89fcc04ee19d34dac82d30a/third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp
[modify] https://crrev.com/e913a6698f089e11e89fcc04ee19d34dac82d30a/tools/metrics/histograms/histograms.xml

Blockedon: -639433
Labels: -M-55 M-56
Status: Fixed
Blocking: 669901
I posted my opinion on this on Github (https://github.com/WICG/interventions/issues/18#issuecomment-276531695), but I thought it best to CC on your internal tracker.

See also https://github.com/cytoscape/cytoscape.js/issues/1702

> And just to make sure we're all clear on the benefit - we're talking about giving all users the experience on MANY major websites seen on the right of this video: https://www.youtube.com/watch?v=NPM6172J22g. Given the huge positive response that video has gotten from users, we're willing to accept a little bit of hacks / compat pain here.

In my opinion, It should be the responsibility of those major websites to update their code to opt-in to new browsers features. If these websites are major, then they can certainly afford the dev resources to do so.

It's my understanding that a large part of the spirit of the web is that backwards compatibility is baked-in. Have a really old page from 1996? It should still work. Have a new webapp in 2017? That should work too.

This change can break pages and webapps that rely on touch events for the benefit of making sites with video, like CNN, faster. I don't think it's a fair trade-off to put speed over correctness --- regardless of how nice the speed is.

Please reconsider making this an opt-in feature rather than an opt-out one. The feature itself is a good idea, but I strongly disagree with changing well-established, default behaviour.

Thanks for your time and consideration.
This will break pretty much every web app that is specifically preventing scrolling when used on a touch-capable device or computer.

There's close to ten thousand activity pages we created for a a UK education company a year ago that override swipe events at the document level in order to turn pages, so thanks for the extra business you're pushing our way by breaking the web standards in much the same way we swear at IE.

Welcome back to the days or browser detection in order to patch this console spam (and soon-to-be bug).

Why browser detection? Well - we still need to support old browsers such as IE8/9 (clients need them unfortunately), and does anyone want to risk passing an invalid argument to something that has a history of falling down when being passed invalid arguments...

Looks like there's been almost no developer feedback or information on this outside the people who actually follow Chromium dev directly, so I'll repeat what's been said above (and by everyone that's just found out about this):

Make it opt-in, not opt-out.
I'm deeply sorry for the frustration this has caused you.  We've long tried the "opt-in" approach but have learned that on average developers don't make the effort to opt-in to better performance.  In particular, in this case we started pushing passive touch listeners heavily back in June (https://developers.google.com/web/updates/2016/06/passive-event-listeners) including during the Google I/O Chrome keynote, and outreach to a large number of frameworks and other major sites would we knew could benefit.  They almost all told us "can't you just detect this and do it for me automatically so I don't have to change my code?".  As you can see in the graph at https://developers.google.com/web/updates/2017/01/scrolling-intervention, we had almost zero impact on real-world scroll performance via this approach.

So we believe that when only tiny number of sites are negatively impacted, and a huge number are positively impacted, we should prefer a "fast by default" policy instead.

Browser detection is not necessary - simply apply touch-action (https://developer.mozilla.org/en-US/docs/Web/CSS/touch-action) as you already would need to do to make your page work with Edge/pointer events.  Then it will work correctly in Chrome and the console spam will go away.
I understand the motivation here, but you just broke a few parts of my web app. What happened to "don't break the web"?

In particular, drag-drop reordering implementations based on ember-sortable (https://github.com/jgwhite/ember-sortable/issues/130) as well as custom-built solutions. In addition, virtual scrolling implementations in our online editor are affected.

I'm unhappy that I have to fun a fire-drill this weekend to rebuild behavior that was working fine. I hope this is not part of your standard operating procedure in the future.
Why isn't touch-action: none sufficient to set on the the sortable-items css to fix the ember sortable issue?
Sign in to add a comment