Intent to Implement: modify addEventListener to accept EventListenerOptions

[Dave Tapuska]

Contact emails

dtap...@chromium.org, rby...@chromium.org

Spec

https://github.com/whatwg/dom/pull/82

https://github.com/RByers/EventListenerOptions

Summary

Add the ability to set different options when adding event listeners via add a dictionary to addEventListener API.

ie: add addEventListener(DOMString, EventListener, EventListenerOptions);

Motivation

Currently other platforms (Gecko) add additional booleans to the argument list to modify the behavior of the added event listeners. We'd like to turn this into a dictionary (EventListenerOptions) so that is a more flexible API for future work. The specifications above were largely targeted to be able to support passive listeners and we wish to implement this. (a separate Intent to Implement will follow later) I think EventListenerOptions as an API stands by itself since there have been requests to add additional booleans to addEventListener.

Compatibility Risk

Low risk; it is an additional API that would need to be called.

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

crbug.com/543685

Link to entry on the feature dashboard

https://www.chromestatus.com/features/5718574840676352

Requesting approval to ship?

No; will be behind experimental flag.

[Rick Byers]

I think landing this first (before adding any new options like 'passive') sounds like a good plan.  Presumably we'd also augment removeEventListener at the same time, right?

I'm not sure if it would make sense to ship this without shipping "passive" at the same time, but we can worry about that when we get to intent-to-ship.

Rick

[Dave Tapuska]

Yes; sorry addEventListener has a corresponding change to removeEventListener

ie;

void removeEventListener(DOMString type, EventListener listener, EventListenerOptions options);

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

[Boris Zbarsky]

On 10/15/15 3:27 PM, Dave Tapuska wrote:
> Compatibility Risk
>
> Low risk; it is an additional API that would need to be called.

No, it's a change to an existing API.

As a result, one compat risk I see here is that this code:

target.addEventListener("foo", someFunction, {});

currently adds a capturing listener. With the proposed
EventListenerOptions it would add a bubbling listener.

Chances are this sort of thing (presumably via someone somewhere up the
callstack passing an object that then finds its way to this third
argument, since passing an actual object literal there right now is
pretty weird) is not actually happening much, of course.

There's also a compat issue in reverse: code written to the new API will
add a capturing listener in browsers that don't support it yet but
probably a bubbling listener in ones that do support it. This can be
dealt with by feature-detecting whether the new API is supported, for
people who think to do that.

Just worth keeping in mind,
Boris

[Dave Tapuska]

Yes the API ultimately be a union (but cannot be for Runtime enabled flag).

void [add|remove]EventListener(DOMString type, EventListener listener, optional (EventListenerOptions or boolean) options);

If third argument converts to an object that will be forced into an EventListenerOptions object instead of being converted into a boolean as before. This is tracked as issue 21 

[Rick Byers]

Agreed, there's definitely some compat risk here.  I think the consensus from the previous discussions around this is that it's worth trying anyway (since the API is so much nicer than the less-risky alternatives).  But if/when we ship we should definitely keep our eyes out for compat problems and be prepared to disable shipping and revamp the API if we see serious issues.

There's also the larger risk (which this entry in the template is really intended to highlight) that we could ship this API in Chrome and fail to get interoperable support in other browsers.  Obviously we're not yet at the point of this risk being low enough to ship (e.g. not yet landed in the DOM spec), but I'm optimistic based on the discussions I've had with key folks at Mozilla, Microsoft and Apple that the risk will be low enough by the time we've got an implementation ready to ship.  In particular, I'm hopeful that we can ship this (along with the 'passive' option) by the end of the year.

Rick

Rick

[PhistucK]

How would you feature detect it?

addEventListener("click", function () { }, {a: 1}) does not throw.

Developers should avoid using this API in browsers that do not support it, because it will make useCapture be true even if that flag is not specified.

Honestly, I prefer a whole new method for that (thus, easier to feature detect) and that can be shorter, a la jQuery even, element.on("click", function () { }); and an overload, element.on({handler: function () { }, type: "click", otherOption: 1}). addEventListener was always too long. This follows the way the new DOM chose with remove(), append(), closest() and the rest.

☆PhistucK

[Rick Byers]

On Thu, Oct 15, 2015 at 4:47 PM, PhistucK <phis...@gmail.com> wrote:

How would you feature detect it?

addEventListener("click", function () { }, {a: 1}) does not throw.

Developers should avoid using this API in browsers that do not support it, because it will make useCapture be true even if that flag is not specified.

See here for how to feature-detect (basically you detect the read of the dictionary property).  Discussion of why this is guaranteed to work is here.

Honestly, I prefer a whole new method for that (thus, easier to feature detect) and that can be shorter, a la jQuery even, element.on("click", function () { }); and an overload, element.on({handler: function () { }, type: "click", otherOption: 1}). addEventListener was always too long. This follows the way the new DOM chose with remove(), append(), closest() and the rest.

Discussion of that option is here.  The consensus of DOM experts was that if we're going to change the name, we'd like to do it in conjunction with a larger overhaul of the eventing pattern.  But given the short term incremental gains we're trying to achieve with passive event listeners, we really don't want to block it on such a larger change to the API (will make adoption harder, etc.).

[Dave Tapuska]

Feature detection is done via getters on the dictionary. See https://github.com/RByers/dom/commit/e39129c90adb14cf6602a6dca1fbc70f1b8083b8

If the getter isn't called then it isn't supported.

On Thu, Oct 15, 2015 at 4:47 PM, PhistucK <phis...@gmail.com> wrote:

[PhistucK]

It would have been good to include this in the intent, as it is a compatibility risk and feature detection is very important.

While it does address my concerns, this feature detection seems kind of complex to me, enough that it might encourage browser detections instead. I may be totally wrong, of course.

☆PhistucK

[Elliott Sprehn]

Can we add a UseCounter to detect when a non-boolean is passed as the third argument as a first step?

Recently we've found some crazy stuff like people calling removeEventListener() without any arguments, or calling addEventListener without a second argument far more often than we expected.

This should be easy to measure the impact of how much content will possibly break.

[Rick Byers]

On Thu, Oct 15, 2015 at 5:24 PM, Elliott Sprehn <esp...@chromium.org> wrote:

Can we add a UseCounter to detect when a non-boolean is passed as the third argument as a first step?

Recently we've found some crazy stuff like people calling removeEventListener() without any arguments, or calling addEventListener without a second argument far more often than we expected.

This should be easy to measure the impact of how much content will possibly break.

I'm fine with that, but note that it will only be an upper bound on the possible breakage.  In many cases an accidentally-capturing listener switching to be a non-capturing listener won't necessarily cause any problems (and is arguably more likely to be what the developer intended, perhaps fixing a problem).  So even if we get a non-trivial amount of usage, we might still need to do something else (eg. attempt shipping) to better determine the real compat risk.

[Elliott Sprehn]

On Thu, Oct 15, 2015 at 2:34 PM, Rick Byers <rby...@chromium.org> wrote:

On Thu, Oct 15, 2015 at 5:24 PM, Elliott Sprehn <esp...@chromium.org> wrote:

Can we add a UseCounter to detect when a non-boolean is passed as the third argument as a first step?

Recently we've found some crazy stuff like people calling removeEventListener() without any arguments, or calling addEventListener without a second argument far more often than we expected.

This should be easy to measure the impact of how much content will possibly break.

I'm fine with that, but note that it will only be an upper bound on the possible breakage.  In many cases an accidentally-capturing listener switching to be a non-capturing listener won't necessarily cause any problems (and is arguably more likely to be what the developer intended, perhaps fixing a problem).  So even if we get a non-trivial amount of usage, we might still need to do something else (eg. attempt shipping) to better determine the real compat risk.

Yeah that seems fine, still if this returned something like 20% that might make us rethink our plan. :)

- E

[Rick Byers]

Yes indeed!  I expect this to be <<1%, and probably only a fraction of those to actually cause any behavior change.  If we get evidence that it's dramatically worse than that, then I agree it's worth rethinking.

- E

[Elliott Sprehn]

Btw, I didn't mean to imply we shouldn't implement this behind a flag in parallel to adding the UseCounter.

- E 

[Philip Jägenstedt]

The issue that Boris raised is one I'm also somewhat concerned about, and we discussed it earlier:

https://github.com/RByers/EventListenerOptions/issues/21

Nevertheless, LGTM to implement behind a flag, and I second Elliott's suggestion to measure the problem in V8EventTargetCustom.cpp, to at least have some idea before shipping. The use counters in that file are the ones that gave the surprising results Elliott mentioned.

Philip