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

Invert 'mayCancel' to 'passive', making it consistently false by default #17

Closed RByers closed 8 years ago

RByers commented 8 years ago

When using the new API (that takes an options dictionary), perhaps mayCancel should be false by default to encourage the more performant behavior? This could eventually become a burden if EventListenerOptions ends up getting used by other things - eg. do we really want to force every mousedown handler to tell us it might preventDefault when there's little performance benefit to knowing that up-front? But I'd love this to be the default for touchstart (may help developers think about the perf implications of asking for a cancelable event).

This would also address #15.

foolip commented 8 years ago

What I suppose is not an option compat-wise is to change the behavior of existing calls to addEventListener, right?

RByers commented 8 years ago

Right, that would be a breaking change.

FWIW we're actually exploring whether we could do that in some very carefully controlled ways (eg. in place of our current touch ACK timeout for seriously badly performing sites), but that's a whole other can of worms - not something I'm prepared to seriously propose yet :-)

foolip commented 8 years ago

Good, just making it very explicit :)

RByers commented 8 years ago

Jonas Sicking says:

I definitely think that if we're going to have any chance of moving significant portions of the web to have better performance, we need things to be fast by default, and then opt-in to slower behavior.

And the more we can simply point to "these APIs are fast, use these, those APIs are slow, don't use those" the better. It should be very clear when you're falling off the fast path.

annevk commented 8 years ago

Wouldn't this break all kinds of form stuff?

foolip commented 8 years ago

Whatever the solution existing code should continue to behave the same way, but even with a new API it is a problem that a different default for mayCancel makes sense for different kinds of events.

RByers commented 8 years ago

Wouldn't this break all kinds of form stuff?

You're talking about existing calls to addEventListener that don't supply an options dictionary? No - the spec indicates that using the old-form is equivalent to explicitly specifying mayCancel: true. Updated the bug title since it was misleading.

but even with a new API it is a problem that a different default for mayCancel makes sense for different kinds of events.

Perhaps, if you can predict in advance what optimizations we might someday want to make based on knowing a listener won't cancel. I think Jonas's argument is that mayCancel:true is NEVER going to be faster than mayCancel:false so we should encourage developers to explicitly tell us in advance when they want mayCancel:true. Of course developers that want to avoid extra typing are just going to continue using the 2-argument form anyway (this debate probably only really gets interesting if we were to talk of a new short-form on API).

annevk commented 8 years ago

So what are you suggesting, that if you supply a dictionary rather than undefined, true, or false, as third argument, you get "false" as default value for this property, but true otherwise? I guess that might be okay, though need to think through the naming some more.

RByers commented 8 years ago

Let's re-open this as the confusing "default" behavior of mayCancel is one of the main outstanding warts in the specification. I've written the spec to match what we discussed here - mayCancel should default to false for new API forms but true for the existing forms.

The alternative I think is to give up on the 'fast by default' (I really don't think there's a lot we can do here given that the 2 argument form will likely never change behavior). Then we could rename mayCancel to neverCancels and make false the default consistently.

domenic commented 8 years ago

I don't think "fast by default" really gives you anything useful. What are the cases it helps with? I can think of two:

  1. You are lazy and want to type addEventListener("foo", {}) instead of addEventListener("foo", { mayCancel: false }).
  2. You are somewhat lazy, but also value clarity a bit, and want to type addEventListener("foo", { capture: true }) which is more clear than addEventListener("foo", true) but less clear than addEventListener("foo", { capture: true, mayCancel: false }).

This just seems really confusing, prioritizing the comfort of those writing the code over the clarity for future readers.

RByers commented 8 years ago

Yeah, I'm leaning towards that thinking as well. If we flip the meaning, anyone care to bikeshed on the name? eg:

domenic commented 8 years ago

Of those I like disableCancel personally.

RByers commented 8 years ago

Talking with @annevk I think we all agree we should really have a consistently-false default. So the main thing that remains is to debate the name. Anne also suggested 'passive', which might also help address @ojanvafai's #13.

I'll rewrite the spec using 'passive' for now and we can keep this issue open to debate the name further (I'll continue to add to my list of options). @smaug---- any preference?

RByers commented 8 years ago

New name (from #17) is, for the moment, "passive". I think this makes the sort of confusion described here less likely (especially with the default consistently false now, I doubt people will use it accidentally). I tried writing a note to explain this in more detail, but it just felt awkward. Let me know if you guys think we should do more...