Skip to content
This repository has been archived by the owner on Mar 19, 2018. It is now read-only.

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

Closed
RByers opened this issue Jul 9, 2015 · 14 comments
Closed

Comments

@RByers
Copy link
Member

RByers commented Jul 9, 2015

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.

@RByers RByers added the spec label Jul 9, 2015
@foolip
Copy link
Member

foolip commented Jul 9, 2015

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

@RByers
Copy link
Member Author

RByers commented Jul 9, 2015

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
Copy link
Member

foolip commented Jul 9, 2015

Good, just making it very explicit :)

@RByers
Copy link
Member Author

RByers commented Jul 9, 2015

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.

RByers added a commit that referenced this issue Jul 9, 2015
Addresses issue #17 and issue #15
@RByers RByers closed this as completed Jul 9, 2015
@annevk
Copy link
Collaborator

annevk commented Jul 10, 2015

Wouldn't this break all kinds of form stuff?

@foolip
Copy link
Member

foolip commented Jul 10, 2015

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 RByers changed the title Should mayCancel be false by default? Should mayCancel be false by default when using the new API variant? Jul 10, 2015
@RByers
Copy link
Member Author

RByers commented Jul 10, 2015

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
Copy link
Collaborator

annevk commented Jul 11, 2015

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
Copy link
Member Author

RByers commented Sep 22, 2015

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
Copy link
Collaborator

domenic commented Sep 22, 2015

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
Copy link
Member Author

RByers commented Sep 22, 2015

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

  • passive
  • fast
  • noCancel
  • neverCancels
  • disableCancel
  • cantCancel
  • preventCancel
  • suppressCancel

@domenic
Copy link
Collaborator

domenic commented Sep 22, 2015

Of those I like disableCancel personally.

@RByers
Copy link
Member Author

RByers commented Sep 24, 2015

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 added a commit to RByers/dom that referenced this issue Sep 24, 2015
This gives us a consistently 'false' default that is simpler.
WICG/EventListenerOptions#17
@RByers
Copy link
Member Author

RByers commented Sep 24, 2015

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...

@RByers RByers changed the title Should mayCancel be false by default when using the new API variant? Invert sense of Should mayCancel be false by default when using the new API variant? Sep 24, 2015
@RByers RByers changed the title Invert sense of Should mayCancel be false by default when using the new API variant? Invert 'mayCancel' to 'passive', making it consistently false by default Sep 24, 2015
@RByers RByers closed this as completed Dec 17, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants