Intent to Ship/Intervene: Make mousewheel listeners of smoothscroll.js passive

258 views
Skip to first unread message

Dave Tapuska

unread,
Jun 1, 2017, 12:54:57 PM6/1/17
to blink-dev

Contact emails

dtap...@chromium.org

 

Spec

https://goo.gl/JeA7dA

 

Summary

Make addEventListener with type mousewheel from smoothscroll.js have passive event listeners by default.

 

Motivation

The smoothscroll.js code is largely broken and is preventing us from shipping scroll top interop. With the scroll top interop behavior activated sites fail to scroll at all with the wheel event. This library was corrected a few years ago but the web continues to clone the broken version and we have been blocked in shipping this long implemented fix for interop.


Smooth scroll has been enabled in Chrome for a number of releases so using custom smooth scrolling is not necessary anymore.


1400 of 470k of the top sites are using this broken code so activating the scroll top interop code isn't helpful when we break 0.3% of websites. The code has largely been placed inline in a sites common javascript files. So we propose examining the function name that matches the "ssc_wheel" name where the type of event is mousewheel and the target of the listener is the window.


Essentially pseudocode:

addEventListener(String event_type, Function f, EventListenerOptions? options) {


  if (!options && event_type == "mousewheel" && this == window && f.name == "ssc_wheel") {

    passive = true;

  }

}


There is no guarantee that this is the last straw in the scroll top interop shipping path but it eliminates one road block that we have been largely unable to overcome.


Interoperability and Compatibility Risk

Certain risk occurs if someone uses  the function name "ssc_wheel" they will magically get a passive event listener for mousewheel. We have examined the data available from http_archive and luckily ssc_wheel is not that common and is associated with this code.


Mousewheel is an non-standardized event; sad face.


The smoothscroll.js code looks for the chrome user agent so other vendors likely don't need to do this intervention.


There is also some risk that there are minified versions of the smoothscroll.js code that we are unable to detect. We will not know that until we ship the scroll top interop and receive bug reports.


Ongoing technical constraints

None

 

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes

 

OWP launch tracking bug

https://crbug.com/501568

 

Link to entry on the feature dashboard

https://www.chromestatus.com/feature/5749447073988608

 

Requesting approval to ship?

Yes


PhistucK

unread,
Jun 1, 2017, 1:05:37 PM6/1/17
to Dave Tapuska, blink-dev
Please, use the standard subject -
Intent to ship: ...

Automated tools are monitoring exact subject prefixes. :(


PhistucK

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHXv1w%3DNYx1181UKe%3DR_hTkK%2BHNgVpNK9BJdSjhBXeqb8XSfWQ%40mail.gmail.com.

Rick Byers

unread,
Jun 1, 2017, 1:34:24 PM6/1/17
to Dave Tapuska, blink-dev
I know this (changing behavior based on the name of an event listener function) is very unorthodox and worrying from a predictability perspective, but in this case I believe it's actually a good trade-off for the following reasons:
  • We've been at a deadlock on the scrollTop interop problem for years, and it's #14 on our top-starred blink bugs list.  This proposal seems better than all the options I had explored previously.
  • The only real alternative is probably to leave scrollTop behavior permanently different between Firefox and everyone else - a constant source of surprise to web developers.
  • Standardizing our scrollTop behavior isn't a great option because it has very surprising properties.  In particular, in the case of html,body{overflow:scroll} there's NO WAY to read or set the scroll position of the body element.  It's also known to be a web compat problem for browsers without WebKit in their UA string, so browsers like Firefox likely cannot switch. 
  • The 'mousewheel' event itself is non-standard (not specified formally anywhere), so we're already in "not following best practices" and implementation-defined territory
  • This "intervention" should be necessary only for browsers with the 'chrome' string in their user agent.  I.e. chromium and EdgeHTML.  There's no need for anyone else to implement this hack in order to gain interop
  • All this code exists on the web to work-around Chrome's lack of a smooth scrolling feature.  That was finally fixed over a year and a half ago.
  • This will improve scroll performance on a non-trivial portion of the web (with this intervention, these sites will get threaded scrolling instead of main-thread blocking scrolling).
  • The intervention is an extension of our earlier touch scrolling one.  There's some chance we'll be able to replace the name-specific hack with a more general "all mousewheel events on the window are passive by default" intervention in the future (and good performance reasons to want to try).
  • The conditions ('mousewheel', target==window, handler.name=='ssc_wheel') seem pretty unlikely to be hit in any case other than the code we're targeting.
Trying to apply the intervention design guidelines:
  • Predictable: although it's surprising, the behavior is 100% deterministic (and in the vast majority of cases code will not be affected)
  • Avoidable: Best practice for years has been to use the standardized 'wheel' event, long shipping virtually everywhere
  • Transparent: Like the other passive-event intervention, I assume we'd log a console message pointing to the chromestatus entry with details?  In particular, we should advise developers to just remove all this smooth scroll code entirely.
  • Justified by data: This intervention is a little weird because it's motivated primarily by interoperability, not performance.  But I do believe we have plenty of data to indicate that web compat requires we not simply break these pages.
All that said, given how close I've been to this issue, I'll recuse myself from my API OWNERS role here.

Dave, would you also ship ScrollTopLeftInterop at the same time as this?  Presumably if ScrollTopLeftInterop ultimately fails for other (not yet known) reasons, we'd back this hack out, right?

If this intent is accepted, can you please file an issue at WICG/interventions to track the fact that this is currently not written in any spec?  If we're successful in shipping ScrollTopLeftInterop then I think we'd want to work with (at least) the Edge team to try to standardize something here. But IMHO it's not worth the effort until we know whether or not it can work from a web compat perspective.

Rick


--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

Dave Tapuska

unread,
Jun 1, 2017, 3:43:18 PM6/1/17
to Rick Byers, blink-dev
I think this Intervention is generally tied to ScrollTopLeftInterop so if the other feature fails for some other reason then yes there is no reason for this intervention. Personally would like to see these both go in for M61.

Antonio Gomes

unread,
Jun 1, 2017, 5:28:50 PM6/1/17
to Dave Tapuska, Rick Byers, blink-dev
I am glad to see this attempt to ship ScrollTopLeftInterop. Thank you
Dave, Rick and the rest of the crew involved.

informal lgtm.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to blink-dev+...@chromium.org.
> To view this discussion on the web visit
> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHXv1wkswUK2xzd3ib2-DpaLh2T8-KKq3U2aocPTqEv2Q2dJJA%40mail.gmail.com.



--
--Antonio Gomes

Simon Pieters

unread,
Jun 2, 2017, 8:39:16 AM6/2/17
to Dave Tapuska, Rick Byers, blink-dev
On Thu, 01 Jun 2017 19:33:56 +0200, Rick Byers <rby...@chromium.org> wrote:

> I know this (changing behavior based on the name of an event listener
> function) is very unorthodox and worrying from a predictability
> perspective, but in this case I believe it's actually a good trade-off
> for
> the following reasons:
>
> - We've been at a deadlock on the scrollTop interop problem
> <https://groups.google.com/a/chromium.org/d/msg/blink-dev/75hVThfrJ08/JT3dDrNxI3sJ>
> for years
> <https://groups.google.com/a/chromium.org/d/msg/blink-dev/mJ5SlCj3CHc/PNSSYY5O_IQJ>,
> and it's #14 on our top-starred blink bugs list
> <https://bugs.chromium.org/p/chromium/issues/detail?id=157855>. This
> proposal seems better than all the options I had explored previously.
> - The only real alternative is probably to leave scrollTop behavior
> <https://dev.opera.com/articles/fixing-the-scrolltop-bug/> permanently
> different between Firefox and everyone else - a constant source of
> surprise
> to web developers.
> - Standardizing our scrollTop behavior isn't a great option because it
> has very surprising properties. In particular, in the case of
> html,body{overflow:scroll} there's NO WAY to read or set the scroll
> position of the body element. It's also known to be a web compat
> problem
> for browsers without WebKit in their UA string, so browsers like
> Firefox
> likely cannot switch.
> - The 'mousewheel' event itself is non-standard
> <https://developer.mozilla.org/en-US/docs/Web/Events/mousewheel> (not
> specified formally anywhere), so we're already in "not following best
> practices" and implementation-defined territory
> - This "intervention" should be necessary only for browsers with the
> 'chrome' string in their user agent. I.e. chromium and EdgeHTML.
> There's
> no need for anyone else to implement this hack in order to gain
> interop
> - All this code exists on the web to work-around Chrome's lack of a
> smooth scrolling feature. That was finally fixed over a year and a
> half
> ago <https://bugs.chromium.org/p/chromium/issues/detail?id=575>.
> - This will improve scroll performance on a non-trivial portion of the
> web (with this intervention, these sites will get threaded scrolling
> instead of main-thread blocking scrolling).
> - The intervention is an extension of our earlier touch scrolling one
> <https://developers.google.com/web/updates/2017/01/scrolling-intervention>.
> There's some chance we'll be able to replace the name-specific hack
> with a
> more general "all mousewheel events on the window are passive by
> default"
> intervention in the future (and good performance reasons to want to
> try).
> - The conditions ('mousewheel', target==window,
> handler.name=='ssc_wheel')
> seem pretty unlikely to be hit in any case other than the code we're
> targeting.
>
> Trying to apply the intervention design guidelines
> <https://github.com/wicg/interventions#intervention-design-guidelines>:
>
> - Predictable: although it's surprising, the behavior is 100%
> deterministic (and in the vast majority of cases code will not be
> affected)
> - Avoidable: Best practice for years has been to use the standardized
> 'wheel' event, long shipping virtually everywhere
> <https://developer.mozilla.org/en-US/docs/Web/Events/wheel#Browser_compatibility>
> - Transparent: Like the other passive-event intervention, I assume
> we'd
> log a console message pointing to the chromestatus entry with
> details? In
> particular, we should advise developers to just remove all this smooth
> scroll code entirely.
> - Justified by data: This intervention is a little weird because it's
> motivated primarily by interoperability, not performance. But I do
> believe
> we have plenty of data to indicate that web compat requires we not
> simply
> break these pages.
>
> All that said, given how close I've been to this issue, I'll recuse
> myself
> from my API OWNERS role here.
>
> Dave, would you also ship ScrollTopLeftInterop
> <https://www.chromestatus.com/features/6386758136627200> at the same time
> as this? Presumably if ScrollTopLeftInterop ultimately fails for other
> (not yet known) reasons, we'd back this hack out, right?
>
> If this intent is accepted, can you please file an issue at
> WICG/interventions to track the fact that this is currently not written
> in
> any spec? If we're successful in shipping ScrollTopLeftInterop then I
> think we'd want to work with (at least) the Edge team to try to
> standardize
> something here. But IMHO it's not worth the effort until we know whether
> or
> not it can work from a web compat perspective.
>
> Rick

I'm excited that this issue is finally (hopefully) getting resolved!
Non-owner LGTM to ship this and ScrollTopLeftInterop together.

--
Simon Pieters
Opera Software

Chris Harrelson

unread,
Jun 5, 2017, 8:38:07 PM6/5/17
to Simon Pieters, Dave Tapuska, Rick Byers, blink-dev
LGTM1 to try to ship this. It's ugly, but there seems to be no alternative and allowing this interop fail indefinitely is not acceptable. Good luck!

Chris

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/op.y07hffxcidj3kv%40simons-mbp.lan.


Timothy Dresser

unread,
Jun 6, 2017, 7:54:51 AM6/6/17
to Chris Harrelson, Simon Pieters, Dave Tapuska, Rick Byers, blink-dev
What's the plan for messaging what's happening to site owners? Should triggering this intervention log a message telling devs to update their version of smoothscroll.js, or to remove smoothscroll.js?

Will we want to mention that if you're not using smoothscroll.js, you shouldn't use the function name "ssc_wheel", or do we think the probability of that is sufficiently low that we don't care?

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

Dave Tapuska

unread,
Jun 6, 2017, 9:25:42 AM6/6/17
to Timothy Dresser, Chris Harrelson, Simon Pieters, Rick Byers, blink-dev
Yes it *should* (doesn't today) log a intervention log and point at the chrome status entry.

dave.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHTsfZCy7XO6zKYnO5ZMxyobodB5U-vLWH-f_zW32h5%3DN4dUDQ%40mail.gmail.com.

Philip Jägenstedt

unread,
Jun 6, 2017, 1:50:55 PM6/6/17
to Dave Tapuska, Timothy Dresser, Chris Harrelson, Simon Pieters, Rick Byers, blink-dev
Dave, it looks like https://goo.gl/JeA7dA doesn't describe the processing model, can you copy the pseudocode into it?

Regarding "some risk that there are minified versions of the smoothscroll.js code that we are unable to detect", that actually seems quite likely. However, I don't quite see how we would find these cases ahead of time. When/if the first report of breakage comes in, perhaps there'll be some clues in there about how to search httparchive for more instances of the same.

LGTM2

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

Dave Tapuska

unread,
Jun 6, 2017, 1:57:55 PM6/6/17
to Philip Jägenstedt, Timothy Dresser, Chris Harrelson, Simon Pieters, Rick Byers, blink-dev
pseudo-code has been added to doc

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAARdPYeKvxv35xs0T%2BCKwNGktGVEtYihYx2oXwk%2BjBm-08yOXA%40mail.gmail.com.

TAMURA, Kent

unread,
Jun 8, 2017, 8:00:47 PM6/8/17
to Dave Tapuska, Philip Jägenstedt, Timothy Dresser, Chris Harrelson, Simon Pieters, Rick Byers, blink-dev
LGTM3.


Reply all
Reply to author
Forward
0 new messages