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 support for passive event listeners #2871

Open
RByers opened this issue Jan 26, 2016 · 65 comments
Open

Add support for passive event listeners #2871

RByers opened this issue Jan 26, 2016 · 65 comments

Comments

@RByers
Copy link

@RByers RByers commented Jan 26, 2016

Blink has shipped support for EventListenerOptions and we expect to soon ship support for the passive option.

Ideally jquery users would be able to mark event listeners as passive, so they can get the same performance benefits. Also @scottgonzalez mentions it would be good for jquery to polyfill the capturing API as well.

Thoughts? Feel free to file any issues / questions with the spec here.

@dmethvin
Copy link
Member

@dmethvin dmethvin commented Jan 26, 2016

What kind of support are you looking for?

I think it would be really hard to expose either capture or passive via the classic .on() API which has been built with the assumption (by both us and users) of a single multiplexed native event handler. Special event hooks can take advantage of being able to capture events before bubbling jQuery handlers see them. All of jQuery's DOM manipulation knows about jQuery events so it can clone them and/or do teardown when the elements go away.

We could create a new API which is what #1735 is about, but there are several hard issues to solve if we want that to be the foundation for .on(). If these use cases are limited to scroll scenarios it seems like a plugin might be the best approach for now. That way we can avoid solving problems that don't (yet) exist for the general case.

Also, what is a reliable way to check for passive support? Do all non-passive implementations reliably throw an exception if they see an object there instead of a boolean? Are there (or might there be in the future) implementations that support { capture: true } but just ignore other options they don't understand?

@RByers
Copy link
Author

@RByers RByers commented Jan 26, 2016

What kind of support are you looking for?

I don't presume to know enough about event handling in jQuery and the typical usage patterns to have a valuable opinion on what the right design is. But at a high-level, the thing you want for maximizing scroll performance is some way for the code attaching the listener to disable the use of preventDefault.

This could even be just more syntax for the event name list. Eg. some knob that lets developers opt-in to passive-by-default for touchstart and touchmove events (wheel is currently harder due to the lack of equivalent to touch-action), then some escape hatch if necessary like .on("touchstart:withPreventDefault", handler).

Or maybe developers just have to resort to addEventListener if they want to opt-in to the performance benefits. I've talked with the developers of one popular library already who want this perf benefit and were OK (but not thrilled) about changing their .on() usage to addEventListener in order to get it. I reached out to @scottgonzalez a couple weeks ago (he's been part of the discussions over the past year about this problem / API in the pointer events working group) and he suggested it was worth filing an issue here to discuss.

Also, what is a reliable way to check for passive support?

The explainer shows the typical (if obscure) pattern of feature-detecting dictionary members:

var supportsCaptureOption = false;
document.createElement("div").addEventListener("test", function() {}, {
  get capture() {
    supportsCaptureOption = true;
    return false;
  }
});

@scottgonzalez
Copy link
Member

@scottgonzalez scottgonzalez commented Jan 26, 2016

I realize the API is tricky because of the data option that already exists. But I'd like the team to start by discussing how to embrace future native APIs. I'm very much in favor of adding this, but in general my requests for embracing new APIs quickly has been met with a lot of pushback and/or bikeshedding.

As far as an actual, usable API, my ideal solution would be to drop data (because it's silly) and add options (because it's useful). The native API is .addEventListener(type, listener, options). We could easily mimic that with .on(type, selector, listener, options). And since deprecating data probably won't happen, we could implement .on(type, selector, data, listener, options). Yes, it will suck to parse those options (perhaps we'll reach a new level of parameter hockey within jQuery), but it's doable since listener is required and acts as a delimiter between the two optional object parameters.

But again, let's start by limiting the discussion right now to whether this feature should make it into jQuery.

@dmethvin
Copy link
Member

@dmethvin dmethvin commented Jan 26, 2016

This could even be just more syntax for the event name list. Eg. some knob that lets developers opt-in to passive-by-default for touchstart and touchmove events (wheel is currently harder due to the lack of equivalent to touch-action), then some escape hatch if necessary like .on("touchstart:withPreventDefault", handler).

You can get pretty close to that today with special events: http://jsbin.com/bupesajoza/edit?html,js,output

The caveat is that all events attached to a specific element will behave like the first event, whether you provide the class or not. So it works fine for elements where there is only one touchstart handler per element.

@dmethvin
Copy link
Member

@dmethvin dmethvin commented Jan 26, 2016

I realize the API is tricky because of the data option that already exists.

The extra arg to the public API is messy but it's the least of the problems. I didn't even mention it because solving the architectural issues and legacy expectations would be much harder. If you shoehorn capture and passive into the existing API you have to define how it interacts with all the existing plumbing, worrying about how much it will break. If you create a new API you start with an empty slate and can say, for example if you want, that these events are never cloned or internally tracked during manipulation and also not subject to the special events system or jQuery's special triggering logic.

@RByers
Copy link
Author

@RByers RByers commented Jan 26, 2016

You can get pretty close to that today with special events: http://jsbin.com/bupesajoza/edit?html,js,output

Interesting, thanks.

Anyway I'll leave it to you jQuery experts to figure out what, if anything, you want to do here. Let me know if there's any questions about or feedback on the API / browser behavior. When we ship passive support in Chrome (currently targeting the upcoming m50 release), you can expect to see a bunch of evangelism with hard data about the performance benefits seen in practice. I can believe it's premature to worry about it much until then, given that early adopters have some OK options for working around jQuery here.

@timmywil
Copy link
Member

@timmywil timmywil commented Jan 27, 2016

Thanks for opening this. I'd like to see the numbers on the performance benefits before implementing, tho.

@gibson042
Copy link
Member

@gibson042 gibson042 commented Jan 27, 2016

Plumbing-wise, this seems very feasible (just extra data to pass to addEventListener and some extra code in jQuery.Event.prototype.preventDefault) and degrades gracefully (worse performance, and successful effects from calls you promised not to make).

Porcelain-wise, five arguments is ugly but @scottgonzalez's suggestion works for extending the .on( events [, selector ] [, data ], handler ) signature. However, it doesn't work as well for the .on( events [, selector ] [, data ] ) signature, for which passive-but-not-data-bound handlers will instead have to look like .on( events [, selector ], undefined, options ) (explicitly undefined data).

Still, I think it can work. +1 from me.

@dmethvin
Copy link
Member

@dmethvin dmethvin commented Jan 27, 2016

Plumbing-wise, this seems very feasible (just extra data to pass to addEventListener

It's a lot more plumbing than that if we support capture. So let's not consider capture in this proposal at all.

and some extra code in jQuery.Event.prototype.preventDefault

The way I understood @RByers document, if you attach with passive: true then .preventDefault() is just ignored; perhaps a message goes to the console saying you made a mistake. So we can just pass it through and let the browser yell at the user. Also since passive seems to be strictly a performance optimization we should probably just let people set the flag even if it's not supported?

Porcelain-wise

This is probably a relatively rare need, correct? Whatever the API to convey the new data, it doesn't necessarily have to support all signatures or options. It could be a new jQuery.EventOptions object in the data position that we could check for with instanceof to eliminate ambiguity about what it is.

@RByers
Copy link
Author

@RByers RByers commented Jan 27, 2016

@gibson042
Copy link
Member

@gibson042 gibson042 commented Jan 28, 2016

It's a lot more plumbing than that if we support capture. So let's not consider capture in this proposal at all.

Yes. To make it absolutely explicit, this ticket is purely limited to passive: true.

if you attach with passive: true then .preventDefault() is just ignored; perhaps a message goes to the console saying you made a mistake. So we can just pass it through and let the browser yell at the user.

I'm pretty sure we also need to skip our own preventDefault to avoid lying about isDefaultPrevented.

Also since passive seems to be strictly a performance optimization we should probably just let people set the flag even if it's not supported?

Yes. That it doesn't work would only be visible when people call preventDefault, which the passive flag is an explicit promise to avoid.

This is probably a relatively rare need, correct? Whatever the API to convey the new data, it doesn't necessarily have to support all signatures or options. It could be a new jQuery.EventOptions object in the data position that we could check for with instanceof to eliminate ambiguity about what it is.

Please let's not do that. It would be clumsy for both us and our users, and the introduction of an inheritance-based pattern that is decidedly un-jQuery.

@KenjiBaheux
Copy link

@KenjiBaheux KenjiBaheux commented Apr 26, 2016

Rick released a comparison video showing the potential of passive event listeners on CNN.com and it's pretty compelling: https://www.youtube.com/watch?v=NPM6172J22g

I'm doing some deep dives of CNN and similar websites (e.g. bloomberg) to identify candidates for Passive Event Listeners but early take suggests that having Passive event listeners supported in jQuery would really help a lot.

@dmethvin
Copy link
Member

@dmethvin dmethvin commented Apr 27, 2016

Is this fast-tracked on several major browsers? We can add plumbing here but it seems like it would be of limited use unless enough browsers support it. Some links to browser tickets tracking implementation would be helpful.

@RByers
Copy link
Author

@RByers RByers commented Apr 30, 2016

Tracking bug for Safari is here. They've told me they're interested and they've contributed to the design, but who knows if/when they'll make implementing it a priority. Gecko tracking bug is here. Again priority isn't clear.

We're hearing a bunch of demand from web developers though (mostly in response to my video tweet). So there's a bit of a chicken and egg problem here. I think the other engines are waiting to see the extent to which developers adopt this, and many of the developers we talk to don't have an easy way to use it because jQuery doesn't expose it yet. But some of the developers we're talking to aren't using jQuery and can get the benefit right away (and we've got the metrics to let us report what perf improvements such sites are having in aggregate), so that'll probably fuel interest.

@dmethvin
Copy link
Member

@dmethvin dmethvin commented Apr 30, 2016

Just to be clear, the plugin that I mentioned above already lets you take advantage of passive event listeners, as long as you always want all the events of a particular type on a particular element to either be passive or non-passive. If you aren't ever attaching cancellable touchstart events on a page you can remove the .noPreventDefault class check completely and force all of them to be non-cancellable.

What more do you need at the moment to be able to use passive event listeners with jQuery? The lack of an .on() implementation shouldn't be a blocker. Describe your use cases and we can see what we can do to get you going.

@gibson042
Copy link
Member

@gibson042 gibson042 commented May 1, 2016

The DOM spec is making increasing use of event listener options (passive late last year, invoke-once last month, and delegation likely soon). I think we should get on board with the trend by adding a final options argument to .on in 3.1, deprecating selector and data and opening an avenue by which passive could be expressed (though actually implementing it would require a significant amount of complexity in registering and updating our run-all-the-things handler).

@dmethvin
Copy link
Member

@dmethvin dmethvin commented May 1, 2016

(though actually implementing it would require a significant amount of complexity in registering and updating our run-all-the-things handler)

That's the messy part and a major change, which is why I'm looking for a way to satisfy this sooner than jQuery 4.0 (or later). We'll need at least a separate addEventListener() per unique combination of options+capture, and at that point it becomes easier to just do one aEL() per jQuery event, especially once we land @jbedard's lower-overhead jQuery.event.fix() code. That will most likely change our private-not-private internal data structure, which is so well known it is used by dev tools so they can report the jQuery handlers for an event.

@RByers
Copy link
Author

@RByers RByers commented May 2, 2016

Just to be clear, the plugin that I mentioned above already lets you take advantage of passive event listeners, as long as you always want all the events of a particular type on a particular element to either be passive or non-passive.

Thanks Dave, I agree this looks like it should be enough to ensure people aren't blocked. I'm reading up on JQuery's event customization support to better understand it, but in the interim can you clarify what would happen if some code did both $(document).on("touchstart.noPreventDefault", handler) and $(document).on("touchstart", handler)?

I'm thinking this pattern is probably even better supported in a framework agnostic way (perhaps there's no need for a jQuery-specific plugin). Eg. in WICG/EventListenerOptions#39 I propose creating a generic library which just fires events named touchstarted and touchmoved (itself implemented on passive listeners). Or we could even go as far as WICG/EventListenerOptions#38 and have a library which makes all touch listeners on a page passive by default.

Actually, given jQuery's push on pointer events maybe touchstart and touchmove listeners added by jQuery should always be passive and jQuery users should just be required to annotate special elements on their page with touch-action - exactly as for pointer events? Even in the short term you could perhaps have some global option to enable this new "touch doesn't block scrolling" mode, as a stepping stone to the full transition to pointer events. @scottgonzalez wdyt?

Describe your use cases and we can see what we can do to get you going.

I think we really need to hear from some end developers using jQuery who are trying to get the perf benefit of passive touch listeners in their code. /cc @tylerbrandt from Optimizely and @CRHain88 who works on CNN.

@CRHain88
Copy link

@CRHain88 CRHain88 commented May 2, 2016

We're using a third-party library for an image carousel, which is using jQuery to manage touchstart, touchend, touchmove, and touchcancel. If we were to use native javascript, we'd have inconsistent implementation of how to create event listeners. I think we all agree that's not ideal. It also gets messy when a touchmove and mousemove listener need to do the same thing.

How would touchstart.noPreventDefault play with the ability to create namespace events? I like @gibson042's idea of adding an options argument. It seems like it would closely resemble and match the native implementation.

Is that enough information re: our use case?

@dmethvin
Copy link
Member

@dmethvin dmethvin commented May 2, 2016

How would touchstart.noPreventDefault play with the ability to create namespace events?

You can have multiple namespaces on an event, if that is what you're asking. So the third-party library would just need to include the plugin above and add .noPreventDefault to it. Or, someone could write a shim for .on() that takes the extra arg and does essentially the same thing. No matter the API approach, for now the discussion above regarding no mixing of event types still applies. That is, the first one to attach a jQuery touchstart event on a specific element defines whether it is noPreventDefault. You can't mix both on the same element.

@dmethvin
Copy link
Member

@dmethvin dmethvin commented May 2, 2016

@RByers in the interim can you clarify what would happen if some code did both $(document).on("touchstart.noPreventDefault", handler) and $(document).on("touchstart", handler)?

If they were done in that order, the second handler would also ignore preventDefault since the two handlers would both be multiplexed off the addEventListener created by the first one. If they were done in reverse order, the .noPreventDefault would have no effect.

@scottgonzalez
Copy link
Member

@scottgonzalez scottgonzalez commented May 2, 2016

Even in the short term you could perhaps have some global option to enable this new "touch doesn't block scrolling" mode

Any flag that globally changes behavior generally becomes a nightmare for plugin developers because they can't tell what behavior to expect.

@arschmitz Does jQuery Mobile have calls to event.preventDefault() that originate from touchstart or touchmove? I expect if there are any, they'd be indirect uses through vmouse, but any existence at all would be problematic for a global flag.

@gibson042
Copy link
Member

@gibson042 gibson042 commented May 3, 2016

I feel like my position has been misconstrued. I @scottgonzalez proposed that the interface for adding passive listeners would be .on( eventTypes, listener, { passive: true } ) (and I observed that the options parameter would be valuable even without passive support, since it better matches addEventListener(type, callback, options)). I would not be willing to tolerate observable effects that depend upon registration order, though—we could reattach passive dispatchers upon non-passive listener registration, or maybe automatically go passive by event type, or employ some other solution, but must not privilege the first registration.

@arschmitz
Copy link
Member

@arschmitz arschmitz commented May 3, 2016

@scottgonzalez jQuery Mobile does not call preventDefault in response to touchstart/move older versions do but not current ones. HOWEVER... we are working on adopting hammer.js to replace our gesture events which polyfills touch-action to an extent and calls preventDefault to prevent scrolling where needed. Hammer does not use jQuery any longer so won't be an issue but used to in v1.x

I would really like to see support added for event listener options / passive events in general though jQuery Mobile has always battled issues with scrolling performance on android devices and this would directly help some of our cases.

I have no problem writing and maintaining a plugin if need be as @dmethvin suggested for use in mobile if need be but would really prefer to just see it in core.

@dmethvin
Copy link
Member

@dmethvin dmethvin commented May 3, 2016

This can all be done but it involves a major rewrite of the event system with breaking changes because of the way it affect internals. I don't see that happening until at least jQuery 4 and who knows when that will occur. I am looking for ways to satisfy the need to access passive event listeners via jQuery sooner than a year or more from now.

I would not be willing to tolerate observable effects that depend upon registration order, though

That should definitely not happen in a correct full implementation of all the native options. I like your suggestion above that turns .on() into a polyfill essentially, assuming the spec gets far enough that we don't get screwed by shipping something that changes later. That again points to a longer delay in delivering this.

Any flag that globally changes behavior generally becomes a nightmare for plugin developers because they can't tell what behavior to expect.

Right, there already is a global behavior of doing what addEventListener is doing which makes blocking the default. It just takes a few lines of code like the plugin above to make non-blocking always be used if that's what people want, again with the caveat that the quick solution I mentioned above can't mix options on the same event+element combination. I think there should be a way to do it with more complex special-event-hook code, essentially defining a new touchstart-unblocked event if that's needed, but I can't tell from the discussion so far whether that's needed for the current use cases.

@kurtextrem

This comment has been minimized.

@mgol
Copy link
Member

@mgol mgol commented Apr 30, 2018

@batata004

This comment has been minimized.

@batata004

This comment has been minimized.

@trenshaw
Copy link

@trenshaw trenshaw commented Feb 5, 2019

@batata004

This comment has been minimized.

@neel-radica

This comment has been minimized.

@dmethvin
Copy link
Member

@dmethvin dmethvin commented Mar 28, 2019

I am going to ask people to refrain from posting in this ticket until they have read the discussion above and understand that this feature cannot be implemented without serious breaking changes.

idiazroncero added a commit to B-Prod/Bootstrap-Offcanvas that referenced this issue May 6, 2019
… to addEventListener

provoking many issues on event handling (i.e., e.preventDefalt() stopped to work).

In this library, it resulted on weird scroll behavior on mobile, offcanvas appearing / disappearing, console warnings and wroing handling of offcanvas status (open/close).

See this comment for a good overview of the problem: https://stackoverflow.com/a/39187679

For the time being, jQuery hasn't shipped a solution yet, so .on() is still unable to pass the parameters to the native addEventListener. See this issue to track the status of jQuery:  jquery/jquery#2871

The only solution is to override touchmove event before loading jquery:
See https://stackoverflow.com/questions/39152877/consider-marking-event-handler-as-passive-to-make-the-page-more-responsive
and https://stackoverflow.com/questions/46094912/added-non-passive-event-listener-to-a-scroll-blocking-touchstart-event

The code has been inserted on bootstrap.offcanvas.coffee and needs to be loaded after jQuery and before the library.
@hades200082

This comment has been minimized.

@batata004

This comment has been minimized.

@mgol

This comment has been minimized.

@batata004

This comment has been minimized.

@dmethvin
Copy link
Member

@dmethvin dmethvin commented Jun 1, 2019

We have this on the roadmap for 4.0 and abusive comments don't help. Just a few comments up I asked people to refrain from non-productive comments but it hasn't helped. If it continues we can just lock the ticket I guess.

Given the comments, I think this ticket has turned into a Google Search destination for people wondering why some [Violation] message shows up in the Chrome console. I know it's frustrating to have Chrome giving warnings like this. In many cases it does not even affect the usability of a site. The Chrome warning is only saying that it may affect performance, not that it does.

If your site is actually having performance issues that require passive listeners to fix, I already posted a simple fix above that will do that without the need for other breaking changes that will come with the 4.0 release. That plugin could be simplified to force all touchstart listeners on the page to passive. It would just go right after loading jQuery.

jQuery.event.special.touchstart = {
  setup: function( _, ns, handle ){
    this.addEventListener("touchstart", handle, { passive: true });
  }
};

With this in place, if you try to call event.preventDefault() inside the handler Chrome gives you a warning that your passive listener can't do that, which means you really needed a non-passive listener after all. Now you'll need to figure out why. Whatever the reason, it's not jQuery.

For more complicated cases where you need a mix of passive and non-passive listeners, 4.0 will be the ticket. Note that existing jQuery plugins that will inevitably break, and you will need to update them. If you are maintaining a web site and haven't written all the code yourself, be prepared for that. It may involve contacting the creators of jQuery plugins for any tooltip, carousel, lightbox, and maybe even browser add-in tools.

"We can't help it - it's how jQuery works and their team are claiming it's too much work to fix it" ?

The problem isn't that mixing passive and non-passive listeners is too much work to fix in jQuery, although it is a major change. The problem is that it may end up being too much work to fix on your web site. There are no touchstart events attached inside jQuery itself so every one of them generating these warnings is somewhere else outside the jQuery library. When we add the ability in 4.0 to mix types the default will still be non-passive listeners so any performance problem won't disappear until you fix either the plugins you depend upon or your own code. I foresee another round of abusive comments once 4.0 ships, this time aimed at plugin authors.

@hades200082

This comment has been minimized.

@dmethvin

This comment has been minimized.

@hades200082

This comment has been minimized.

@timmywil
Copy link
Member

@timmywil timmywil commented Jun 1, 2019

We understand this is an important feature. The feature is coming and it's planned for 4.0. Most of the comments here aren't relevant to the development of the feature, so I'm locking the ticket. The workaround Dave posted will allow users to take advantage of the passive option until 4.0 is released. We can't say for sure when 4.0 will be ready. We are all volunteers that work on jQuery when we can make the time, but we'll go as fast as we can.

@jquery jquery locked as off-topic and limited conversation to collaborators Jun 1, 2019
@timmywil timmywil changed the title Consider adding support for passive event listeners Add support for passive event listeners Jun 1, 2019
discoursereviewbot referenced this issue in discourse/discourse Mar 26, 2020
Previously we would consider a user "present" and "last seen" if the
browser window was visible.

This has many edge cases, you could be considered present and around for
days just by having a window open and no screensaver on.

Instead we now also check that you either clicked, transitioned around app
or scrolled the page in the last minute in combination with window
visibility

This will lead to more reliable notifications via email and reduce load of
message bus for cases where a user walks away from the terminal
@mgol
Copy link
Member

@mgol mgol commented Mar 22, 2021

I added the Behavior change label as it's unlikely to be possible to implement without any breaking change.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests