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

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

Closed
rik opened this issue Apr 14, 2016 · 9 comments
Closed

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

rik opened this issue Apr 14, 2016 · 9 comments

Comments

@rik
Copy link

rik commented Apr 14, 2016

I think this is a continuation of #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
Copy link
Collaborator

annevk commented Apr 14, 2016

That would make event classes way too magical.

@annevk annevk closed this as completed Apr 14, 2016
@RByers
Copy link
Member

RByers commented Apr 14, 2016

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

@rik
Copy link
Author

rik commented Apr 15, 2016

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

@tabatkins
Copy link
Collaborator

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

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

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

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

@RByers
Copy link
Member

RByers commented May 24, 2016

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

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

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.

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