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

Should preventDefault not be exposed on passive event handlers? #34

Closed rik closed 8 years ago

rik commented 8 years ago

I think this is a continuation of https://github.com/WICG/EventListenerOptions/issues/3. I haven't seen this suggestion made for that problem.

element.addEventListener('touchstart', function(e) {
  console.log(e.preventDefault); // logs undefined
  e.preventDefault(); // TypeError: e.preventDefault is not a function
}, {passive: true});

The error message could be special-cased to add the reason why e.preventDefault is not a function.

annevk commented 8 years ago

That would make event classes way too magical.

RByers commented 8 years ago

Whoa, yeah we never do magic like that. If we wanted it to fail, throwing would be the way.

rik commented 8 years ago

I don't understand what you mean by magic in this context. Could you expand a bit?

tabatkins commented 8 years ago

Changing what type of event object you get based on the options passed to the event listener registration.

As Rick said, if we did want to enforce this more generally, we'd have .preventDefault() exist but throw. But that doesn't seem to be particularly useful here.

laukstein commented 8 years ago

Chrome 53.0.2745.0 canary for e.preventDefault returns native function preventDefault(), also seems e has no parameter to track the EventListener option passive: true.

try...catch wouldn't prevent the exception either:

Unable to preventDefault inside passive event listener invocation. chromium...Event.cpp#239

Any backward compatibility for it? Old browsers will consider {passive: true} as true and e.preventDefault() wouldn't have exception...

dtapuska commented 8 years ago

You need to feature detect. You can certainly use Modernizr to help you out.

target.addEventListener(type, handler, Modernizr.passiveeventlisteners ? {passive:true} : false);

RByers commented 8 years ago

Also note that preventDefault never throws an exception, it just generates a console warning. If you're using a passive listener, then you shouldn't be trying to call preventDefault (since it will just be ignored).

laukstein commented 8 years ago

Still, why not to list EventListener options in event? It would help to prevent any future warnings.

The best practice in any feature (new spec) is to handle backward compatibility, all polyfills and hacks must be avoided as much as possible.

dtapuska commented 8 years ago

That would be a little weird because the Event would mutate over the dispatch. ie; think about adding two event listeners with different options and dispatch a single event to them. At each point of the dispatch of the single event the options would be different. If someone held onto the event and didn't explicitly copy it and did event delegation after some point they would see it mutate.