WICG / EventListenerOptions

An extension to the DOM event pattern to allow authors to disable support for preventDefault
Other
1.17k stars 140 forks source link

Add simpler feature detection mechanism? #31

Open RByers opened 8 years ago

RByers commented 8 years ago

There's lots of concern in #12 about the complexity of dictionary-member based feature detection. Perhaps we should be designing something simpler, ideally as a general pattern since dictionary-based APIs are being used / extended everywhere on the web now.

I originally had a getSupportedListenerOptions() API that was easier to use, which we "fixed" in #16. But maybe we should really be thinking of a better pattern? Some have suggested EventTarget.supportsPassive. This is kind of similar to DOMTokenList.supports and CSS @supports.

Suggestions?

jacobrossi commented 8 years ago

Just thinking out loud here... could we expose the actual dictionaries somewhere as interface objects?

dtapuska commented 8 years ago

Perhaps EventTarget.supportsEventListenerOptions should actually be used no? There are some cases where you'd like to test whether something supports passive or not. But I think the bulk of the concern is does this API possibly take a dictionary?

jacobrossi commented 8 years ago

[Expanding last comment] maybe not exposed on the global, to avoid pollution. But I like a more general: if(EventListenerOptions.passive) kind of test.

RByers commented 8 years ago

Perhaps EventTarget.supportsEventListenerOptions should actually be used no? There are some cases where you'd like to test whether something supports passive or not. But I think the bulk of the concern is does this API possibly take a dictionary?

Yes, in this case. But we've got some other options we may want to add where you really need to know if that option is supported (eg, once). So it's probably worth thinking here of what a good general pattern for specific dictionary members is.

RByers commented 8 years ago

Just thinking out loud here... could we expose the actual dictionaries somewhere as interface objects? Maybe not exposed on the global, to avoid pollution. But I like a more general: if(EventListenerOptions.passive) kind of test.

Yeah that's really what getSupportedOptions was about but it was clunky. How about this?

if (EventTarget.EventListenerOptions && EventTarget.EventListenerOptions.passive)
tabatkins commented 8 years ago

Why not on the global, tho? I mean, we pollute the global with the interface name already; putting the dict on there (which is usually just InterfaceNameDict or InterfaceNameOptions) doesn't seem much worse.

jacobrossi commented 8 years ago

I'm actually ok with the global, but I've heard concerns from others in the past.

I think hanging off EventTarget could also work, but want to make sure this is a pattern that's going to work for the other dictionary uses.

RByers commented 8 years ago

Yeah. I'm happy with global in this case. As of Feb 1, the token EventListenerOptions occurred exactly never in the Alex top 500k.

phistuck commented 8 years ago

we pollute the global with the interface name already

Do we? undefined in Chrome 49.

I think EventTarget.EventListenerOptions is good, regardless of any existing (or nonexistent) global EventListenerOptions definition in the wild. It does not belong there, it belongs to event targets. Regarding passive support on a certain EventTarget subclass, WheelEvent.EventListenerOptions can have a different dictionary, or maybe WheelEvent.supportedEventListenerOptions.passive will be true.

tabatkins commented 8 years ago

@phistuck Not all dictionaries have an associated global interface, or rather, exactly one associated global interface. I don't think the pattern of sticking it on an interface works in general.

If certain events support a different set of options, they should be using a different dict. I suggested very intentionally that we avoid anything that requires or encourages browsers to write custom logic to implement the support-detection, because long experience shows that always leads to incomplete or lying support tests. Just having IDL dictionaries show up on the global with their associated keys set to something truthy does this well, similar to how @supports works.

tabatkins commented 8 years ago

Do we? undefined in Chrome 49.

The interface name (unless it's [NoInterfaceObject]. Dictionaries are currently not exposed, so they obviously don't show up.

RByers commented 8 years ago

I think EventTarget.EventListenerOptions is good, regardless of any existing (or nonexistent) global EventListenerOptions definition in the wild. It does not belong there, it belongs to event targets.

Unfortunately it doesn't really belong to event targets, it belongs to event listeners and they don't have any representation in JS today. EventTarget is probably the closest thing.

@tabatkins I like your argument that this needs to be automatic to be reliable. So I guess the idea would be that we'd add some sort of WebIDL custom attribute to the dictionary definition to opt in to this? Do you think we'd want to block on that, or should we consider defining a one-off for EventListenerOptions and then trying to get consensus on the general pattern in parallel?

tabatkins commented 8 years ago

I think all dictionaries should do this by default, and we should allow [NoInterfaceObject] on dictionaries for the rare cases when we want to opt out, same as interfaces.

We should also do this as a one-off regardless, probably. We don't add arguments to APIs that often, but this one in particular already has several possibilities for addition lined up.

RByers commented 8 years ago

Ok so to be concrete, the one-off proposal (using today's WebIDL) is something like:

Option A:

interface EventTarget {
  ...
  static readonly attribute EventListenerOptions EventListenerOptions;
}

With the attribute defined to return a dictionary with all members set to true (at least until we define new non-boolean options).

tabatkins commented 8 years ago

Unless we start trying to use the detection for something else, like giving out the default values, we should stick with true as the value. This is intended for feature detection, and true makes that easier.

RByers commented 8 years ago

Sure, but what if we add a string option like class (as Anne proposed). We want to re-use the dictionary type, right? I'd be fine just defining that case to be "true" or " " or something similarly truthy.

tabatkins commented 8 years ago

What I mean is that the object that IDL dictionaries define on the global is purely for feature-testing purposes. It is not an instance of the dictionary in question (any more than the object that IDL interfaces define on the global is an instance of the interface). It just contains all the keys the engine supports enough to include in their internal IDL, set to a truthy value so you can do trivial property-existence checks.

jacobrossi commented 8 years ago

I think all dictionaries should do this by default, and we should allow [NoInterfaceObject] on dictionaries for the rare cases when we want to opt out, same as interfaces.

+1

RByers commented 8 years ago

What I mean is that the object that IDL dictionaries define on the global is purely for feature-testing purposes. It is not an instance of the dictionary in question (any more than the object that IDL interfaces define on the global is an instance of the interface). It just contains all the keys the engine supports enough to include in their internal IDL, set to a truthy value so you can do trivial property-existence checks.

Ah, so that would be kinda awkward to define in WebIDL, right? But in JavaScript that would be:

Option B)

  window.EventListenerOptions = {capture:true, passive:true}
tabatkins commented 8 years ago

I mean, the idea is that WebIDL just says that's what happens for dictionaries. Nothing needs to be defined manually; any time you define a dictionary type, it defines that sort of object on the global.

RByers commented 8 years ago

Yeah I know, I'm just trying to come up with a path for shipping this for EventListenerOptions ASAP (since we're actively getting people to adopt it) without necessarily blocking on the WebIDL question. But perhaps it's time to start a discussion with the WebIDL experts - I'll do that.