Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature test for passive event listener support #1894

Closed
RByers opened this issue Feb 19, 2016 · 30 comments
Closed

Add feature test for passive event listener support #1894

RByers opened this issue Feb 19, 2016 · 30 comments

Comments

@RByers
Copy link
Contributor

RByers commented Feb 19, 2016

For passive event listeners, feature detection is important but non-trivial.

I don't have much experience with Modernizr but I can try to post a feature detect for this if desired.

@patrickkettner
Copy link
Member

Yes please! Happy to guide you through it, too

On Feb 19, 2016, at 12:28 PM, Rick Byers notifications@github.com wrote:

For passive event listeners, feature detection is important but non-trivial.

I don't have much experience with Modernizr but I can try to post a feature detect for this if desired.


Reply to this email directly or view it on GitHub.

@paulirish
Copy link
Member

paulirish commented Mar 17, 2016

Here's the test (and use) for event listener options and its passive option:

// assume the feature isn't supported
var supportsPassive = false;
// create options object with a getter to see if its passive property is accessed
var opts = Object.defineProperty && Object.defineProperty({}, 'passive', { get: function(){ supportsPassive = true }});
// create a throwaway element & event and (synchronously) test out our options
document.addEventListener('test', function() {}, opts);

// Use our detect's results. passive applied if supported, capture will be false either way.
elem.addEventListener('touchstart', fn, supportsPassive ? { passive: true } : false); 

The Object.defineProperty && guard only neccessary for IE8 and below. Likely not neccessary for most folks.


And a more terse version, assuming a modern runtime:

var supportsPassive = false;
document.createElement("div").addEventListener("test", _ => {}, { get passive() { supportsPassive = true }});

elem.addEventListener('touchstart', fn, supportsPassive ? { passive: true } : false); 

@ryanseddon
Copy link
Member

The terse option seems better we can just wrap in a try catch for older browsers.

@foolip
Copy link
Contributor

foolip commented Mar 24, 2016

Since the listener argument is nullable and addEventListener does nothing in that case, one can avoid the temporary element altogether, so:

var supportsPassive = false;
try {
  document.addEventListener("test", null, { get passive() { supportsPassive = true }});
} catch(e) {}

@foolip
Copy link
Contributor

foolip commented Mar 24, 2016

(If brevity is a plus, one can also omit document. as window is also an EventTarget.)

@WebReflection
Copy link

IE8 has Object.defineProperty and it throws errors with non built-in objects. The && doesn't guard from throwing

@paulirish
Copy link
Member

Good call. Thx

Latest version is now over here: WICG/EventListenerOptions#30 (comment)

@mgol
Copy link

mgol commented Apr 8, 2016

@ryanseddon You can't test syntax using try-catch, the whole thing will just not parse.

@ryanseddon
Copy link
Member

@mgol yep, we did try { eval() } catch() {} for ES2015 syntax feature tests but this is different to those.

@RByers
Copy link
Contributor Author

RByers commented May 18, 2016

Should I do something to get this listed in https://modernizr.com/docs?

@patrickkettner
Copy link
Member

it'll be listed automatically once I cut a new release, which will probably be tonight

@RByers
Copy link
Contributor Author

RByers commented May 18, 2016

Great, thanks!

@patrickkettner
Copy link
Member

​thank YOU

@amritk
Copy link

amritk commented Jul 8, 2016

Any word on getting this into a release?

@KenjiBaheux
Copy link

From a quick look, I didn't find a mention of passive event listener in https://modernizr.com/docs
Is the release coming soon?

Thanks in advance!

@enjikaka
Copy link

enjikaka commented Aug 17, 2016

This isn't released yet. I dropped the

Modernizr.addTest('passiveeventlisteners', function() {
    var supportsPassiveOption = false;
    try {
      var opts = Object.defineProperty({}, 'passive', {
        get: function() {
          supportsPassiveOption = true;
        }
      });
      window.addEventListener('test', null, opts);
    } catch (e) {}
    return supportsPassiveOption;
  });

into our Modernizr build locally for now.

When will this be released? @patrickkettner @paulirish

@Bamieh
Copy link

Bamieh commented Oct 16, 2016

How about cleaning up the event listener after adding finishing the test?

Modernizr.addTest('passiveeventlisteners', function() {
    var supportsPassiveOption = false;
    try {
      var opts = Object.defineProperty({}, 'passive', {
        get: function() {
          supportsPassiveOption = true;
        }
      });
      window.addEventListener('test', null, opts);
      window.removeEventListener('test', null, opts);
    } catch (e) {}
    return supportsPassiveOption;
  });

@WebReflection
Copy link

@Bamieh ... cleaning a null listener ? I hope browsers are smart enough to realize it's pointless regardless :-)

@Bamieh
Copy link

Bamieh commented Oct 17, 2016

@WebReflection browsers will always disappoint you! no but seriously, nothing in the spec mentions testing the EventListener interface for null, hence the listener will stay registered. (1.3.1.)

@WebReflection
Copy link

Does it throw if it's triggered? What do specs say about dealing with null listener on dispatch? Anyway, i'm for cleaning up too, I just think in this case might be effectively unnecessary.

@Bamieh
Copy link

Bamieh commented Oct 17, 2016

Okay after further investigation,
window.addEventListener('test', null, opts)
does not add an event listener to the window object in the first place, hence no need to remove anything.
Only tested on chrome though.
getEventListeners(window) // test is not listed in the object.

@RByers
Copy link
Contributor Author

RByers commented Nov 22, 2016

nothing in the spec mentions testing the EventListener interface for null, hence the listener will stay registered. (1.3.1.)

Spec is here. Step 2: "If callback is null, terminate these steps". I.e. this is defined to be a no-op and AFAIK all browsers implement it as such.

@bgirard
Copy link

bgirard commented Apr 12, 2017

Looking at the spec here step 3
Let capture, passive, and once be the result of flattening more options.
will happen after the step 2 early return.

If I'm interpreting this correctly, this feature detection isn't spec compliant and likely most browsers aren't either given that this works in practice.

Perhaps the spec should be changed to explicitly allow for feature detection in this way?

@patrickkettner
Copy link
Member

hi @bgirard!
I think you are misreading them, but it could very well be my own misunderstanding.

@domenic @RByers @annevk is @bgirard correct that the .passive property should not be being accessed by browsers without a callback also being defined (according to the spec)?

@domenic
Copy link

domenic commented Apr 14, 2017

That's incorrect; it's accessed by the Web IDL bindings which convert the incoming object into a dictionary, and take place before any of the algorithm steps. I can appreciate that being pretty subtle though for those not familiar with Web IDL.

@patrickkettner
Copy link
Member

thanks for the quick response!

@bgirard
Copy link

bgirard commented Apr 14, 2017

Thanks for clarifying. That means that this snippet works, unsurprisingly.

What about the spec being misleading in the way that step 2 and 3 is ordered? The spec is describing the flattening behavior that is occurring in Web IDL. Should we open a spec issue to re-order these steps to make it explicit that flattening should occur before the early return on a null callback?

@domenic
Copy link

domenic commented Apr 14, 2017

The Web IDL conversion from object to dictionary and the "flattening" are orthogonal. Flattening operates on either a boolean or a dictionary; objects are never involved. There's no need to reorder the steps. It's not misleading to people who understand Web IDL, which again, I understand is not too many non-implementers. But you're looking at the algorithm for implementers, anyway.

@bgirard
Copy link

bgirard commented Apr 14, 2017

Ohh I understand now. Thank you for clarifying!

@WebReflection
Copy link

hey, if you assign a variable like this opts = {} you might as well know this is absolutely a redundant operation Object.prototype.__defineGetter__.call(opts, "passive", getter); 'cause the following one is exactly the same in 1/3rd of the code opts.__defineGetter__("passive", getter);

Moreover, this is not a "most-likely", this is a certain statement:

If a browser is lacking Object.defineProperty it is most-likely too old to support passive events

there is no browser on earth that would implement passive events without being compatible with ES5 so the code is just fine as it is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests