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

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

Closed
domenic opened this issue Jul 13, 2015 · 12 comments

Comments

@domenic
Copy link
Collaborator

domenic commented Jul 13, 2015

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

@foolip
Copy link
Member

foolip commented Jul 13, 2015

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 #21 casting some doubt on whether overloading the third argument will be web compatible.

@domenic
Copy link
Collaborator Author

domenic commented Jul 13, 2015

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

@dtapuska
Copy link
Collaborator

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

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 RByers removed the spec label Jul 27, 2015
@RByers
Copy link
Member

RByers commented Jul 29, 2015

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

domenic commented Jul 29, 2015

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

@annevk
Copy link
Collaborator

annevk commented Jul 29, 2015

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

domenic commented Jul 29, 2015

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

this is bizarre.

@domenic
Copy link
Collaborator Author

domenic commented Jul 29, 2015

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

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

annevk commented Jul 30, 2015

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

RByers commented Sep 22, 2015

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

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

6 participants