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

Why do we believe making mayCancel false as a default is at all backward-compatible? #22

Closed domenic closed 9 years ago

domenic commented 9 years ago

This breaks every piece of code in the world that currently does e.preventDefault(). Why do we think this is OK?

foolip commented 9 years ago

I don't think this is anyone's intention. I guess you're saying that if the third argument is missing, it should be treated like the default dictionary values { capture: false, mayCancel: false }, while the current behavior is { capture: false, mayCancel: true }?

Ways around this:

  1. Prose says that if the third argument is boolean, the listener's may cancel flag will be true.
  2. Invert name and meaning of mayCancel so that false is also the current default.
  3. A new function name.

There's also https://github.com/RByers/EventListenerOptions/issues/21 casting some doubt on whether overloading the third argument will be web compatible.

domenic commented 9 years ago

Hmm, I hadn't considered 1. It is pretty crazy though. Overall I would prefer 2.

dtapuska commented 9 years ago

2 - leads to a double negative which can be confusing to understand. But maybe it is just me...

ie; you could call it willNotCancel

And then the default is false.

tabatkins commented 9 years ago

No, negative argument names are a confusing anti-pattern.

We either need to come up with a positive name of the opposite meaning, or leave it as it is, with prose making it clear that the default dictionary value (true) is used by default when the dict isn't passed.

RByers commented 9 years ago

The fundamental issue here is that we have two notions of "default". One is what we want the behavior to be for developers opting-in to the new API form but not specifying mayCancel explicitly, and one is what we need the behavior to be for code unaware of the new API form. I.e. why do we think the "default" behavior for the existing API should be the same as the "default" behavior for a new API form?

I don't think we want to invert the meaning because when the developer has opted into the new API we still want mayCancel=false to be the default behavior (#17).

If we consider the boolean form of addEventListener to be legacy, is it really that weird to say that it sets up an EventListenerOptions with mayCancel=true for legacy compatibility reasons? I'll try to specify this precisely as part of #19. Any objections?

domenic commented 9 years ago

It is weird to say that addEventListener(s, f) is different than addEventListener(s, f, {}). Very, very weird.

annevk commented 9 years ago

Idea of sorts...

Change: mayCancel -> passive.

 aEL(s, f) // passive = false
 aEL(s, f, {}) // passive = false
 aEL(s, f, {capture:false}) // passive = true

Once you start using the dictionary the default flips.

domenic commented 9 years ago
 aEL(s, f, {}) // passive = false
 aEL(s, f, {capture:false}) // passive = true

this is bizarre.

domenic commented 9 years ago

I'm not sure what the motivation here is anyway for making mayCancel false the default. It is to save characters so you can type {} instead of { mayCancel: false }? Why do we want to encourage that kind of obtuse coding?

tabatkins commented 9 years ago

@domenic No, I imagine it's so that you automatically opt into the "better" model if you're using the newer API variant. (Legacy code continues to use the legacy behavior.) Most code isn't trying to cancel anything, and as we add optimizations for non-cancelling events of various types, it's better if most stuff automatically works better.

This does mean that, right now, without very many options in the dictionary, there may be code that just does aEL(..., ..., {}) just to opt into the new API, which is indeed weird and not worth encouraging, but I also don't think it's something to fear or worth objecting to.

It is weird that aEL(a, b) and aEL(a,b,{})` end up being different, but that's legacy upgrades for you.

(I might call the new option cancelable rather than mayCancel to describe the behavior of the event rather than the programmer.)

this is bizarre.

Agree that making the behavior change based on whether some keys are in the dictionary or not is bizarre and unwanted. You also need to hack around WebIDL by specifying the type as any and handling the dictionary in prose; using IDL properly won't let you do this bizarre behavior.

annevk commented 9 years ago

using IDL properly won't let you do this bizarre behavior

Euhm yes it would. You just don't define defaults. Mutation observers does something like this. Where the state of some fields depends on others to make the API more convenient to use.

RByers commented 9 years ago

I've written the spec as discussed. I think it works, but I don't love it. Let's discuss further in #17.