UAs observing the presence of event listeners is bad

[foolip]

What happens if the UI process/thread thinks that a point only has non-canceling event listeners, but when the event reaches the main thread, a regular event listener that calls preventDefault() has been added? It's too late to actually prevent the scroll, but it'll look to the event listener like it can prevent it.

Perhaps create non-cancelable events when the UI process/thread actually isn't waiting for the result? Though this would make the presence of other no-op event handlers observable, ugh.

[annevk]

I was just thinking about this. A regular event listener could also be added during dispatch.

[RByers]

Good question, I hadn't fully thought through the implications here. I think it's OK though - at least for touch events. The rules for when a touchstart/touchmove is cancelable are currently implementation-defined (long standing spec bug that the spec has never fully reflected the implementations here). Basically I think the UA makes a decision about whether an event is cancelable before it is dispatched. If something changes during dispatch, its too late to revisit that decision. A similar scenario today: touchmove during scroll is uncancelable on most browsers, so what if one touchmove handler modifies the div to no longer be scrollable? Clearly that doesn't somehow change the behavior of that event for other handlers.

This does mean however that existing handler code could be confused by seeing uncancelable events (event though they were written to depend on cancelation). Should we consider not invoking these handlers in that case?

[foolip]

The rules for when a touchstart/touchmove is cancelable are currently implementation-defined (long standing spec bug that the spec has never fully reflected the implementations here).

Does not cancelable here mean that event.cancelable is false, or that it's true but event.preventDefault() still has no effect?

The spec says that touchstart, touchend and touchmove are cancelable in the normal sense, but does reality agree?

I don't know of a good solution to this yet. Creating non-cancelable events would make the presence of other event listeners observable, but skipping event listeners that are added too late seems equally weird—with a non-canceling event listener added earlier you'd be able to tell that your event listener is being ignored, which I think can never happen in the existing event model.

[annevk]

Implementation-defined is not "OK". We should figure out what the processing model is before we have to live with it forever.

[dtapuska]

Honestly it seems to me that really want we want is a subclass of events that are based on a flag (mayCancel) and then we are thinking of the consequences of the exact same messages delivered without the context of the flag being set and that is leading to complexity.

Should we think about the context of it like it is almost a different event type. ie; much like sending "touchmove_async" vs "touchmove". I'm not saying to do this; but I'm saying should we think of the solutions in that approach. That "touchmove" with mayCancel:true is fundamentally a different message than "touchmove" mayCancel:false.

Then we can reason about adding an event listener that wants cancelable events should not get non-cancelable events. (this would break the existing web wrt to touch today).

[annevk]

Dispatching a different type of event based on the listener violates the event model.

[dtapuska]

I never meant for us to actually do that.. just to think about it logically from that perspective. As it seems analogous to the goal we wish to achieve.

[foolip]

@annevk, how terrible is the "create non-cancelable events when the UI process/thread actually isn't waiting for the result" suggestion? Is it a non-starter that the presence of other event listeners becomes observable? Practically speaking it seems even somewhat useful, as you'd have an easy way to detect that you've succeeded in making all of your listeners non-canceling.

[annevk]

Well, it violates the invariants. How bad that is I'm not sure. I'm not really familiar with systems where they broke them.

[chutten]

If we think about it conceptually in the way @dtapuska is mentioning, then there's nothing for this case but for the "late" listener to go untriggered for this event type. This then operates as though the "synchronous event" is dispatched first and the "async" one second.

But the event is still the same event. A compliant UA could have preventDefault simply check if it is being called within a mayCancel handler and either log the warning or take action.

This ambiguity should be addressed in the draft.

As an author, I think I'd appreciate the feature being written in terms of generating virtual "async events" so that talking about timing will be clear from the spec and from the code.

[foolip]

Perhaps events can have a "deliver only to non-canceling listeners" flag which is set if there are only non-canceling listeners known at the time of creation, which causes canceling listeners to not be invoked at all. That would exclude late canceling listeners, but not late non-canceling listeners.

One might also think of it as a third cancelable state, a "not cancelable but could have" been in addition to true/false.

[RByers]

Sorry for the delay, finally able to get back to this (and working on a PR for the DOM spec for it).

I'm trying to figure out if the specific behavior we're talking about here belongs in the DOM spec or not (eg. it may belong to the TouchEvents spec).

Concretely, today all browsers have an optimization where if there are no touch event handlers for a point, then touch scrolling can start without waiting for the currently running task to complete. I think it's debatable whether this behavior is really part of the platform or not (certainly not spec'd anywhere today, but something developers need to be aware of nonetheless so probably should be). Should the DOM spec be saying something about this already (at least that the absence of handlers can impact event processing somehow)?

So today if I add a touch handler when a scroll has already started, I'll get touch events with cancelable=false and preventDefault will be ignored. Is this really fundamentally different from the scenario we're talking about here? If I add a new handler while a non-cancelling listener is running, then again that handler could just get a touch event with cancelable=false. If we can figure out how to properly describe the current behavior, then perhaps the new behavior is a simpler extension of that.

So to me the options are:

1. Don't try to specify this behavior in the DOM spec itself. The DOM spec doesn't say when a touch event will be cancelable and when it won't, that's up to the TouchEvents spec to define (still my problem of course).

2. Define the optimization here as part of DOM to apply consistently to all types of events: if no cancelable listeners are present at event dispatch time, then set Event.cancelable to false. As discussed this is uncomfortable because the presence of listeners "isn't supposed to alter the event". Alternately we could set only the internal flag indicating that preventDefault should be ignored (without actually changing cancelable).

3. Define an additional extra flag as @foolip suggests to prevent late-added canceling listeners from getting invoked in the first place. Personally I think the likely benefit in practice here is very small, and so doesn't justify the extra complexity.

Implementation-defined is not "OK". We should figure out what the processing model is before we have to live with it forever.

Agreed, I didn't mean to imply that there should be implementation-defined behavior here. Only that I believe we have the freedom to define the behavior that makes sense without breaking the web because behavior in this regard already differs between browsers and has changed over time (without, AFAIK, causing web compat issues).

[annevk]

Curious what @smaug---- thinks about the above. It sounds like touch events broke the event model badly.

[smaug----]

I think RByers means 'Event.cancelable to false' in 2, but definitely not that.
UAs should just dispatch non-cancelable events when it isn't possible to prevent any default handling, and it should be spec'ed clearly in the spec dispatching those events, so, in Touch Events spec.

(or am I missing something here?)

[annevk]

Yeah, I think I'm with you now. What touch events seem to do from the description is just check if the current scrolling state is active (or some such) and then dispatch non-cancelable events rather than cancelable events. That doesn't really observe event listeners and doesn't really require any changes to DOM.

[RByers]

I think RByers means 'Event.cancelable to false' in 2, but definitely not that.

Right, sorry - fixed.

UAs should just dispatch non-cancelable events when it isn't possible to prevent any default handling, and it should be spec'ed clearly in the spec dispatching those events, so, in Touch Events spec.

Agreed.

What touch events seem to do from the description is just check if the current scrolling state is active (or some such) and then dispatch non-cancelable events rather than cancelable events. That doesn't really observe event listeners and doesn't really require any changes to DOM.

Exactly.

So the interesting scenario is what happens when a finger goes down when there is only non-cancelling touchstart listeners. Conceptually, the UA determines that it's not currently possible to prevent any default handling, and immediately goes into "scrolling active state". Then dispatches a touchstart event with cancelable=false (and we don't need to worry about what it means for a canceling listener to be added from within the non-canceling listener callback).

Ok, this all sounds good and I've almost got a PR ready that captures this. The only remaining question I have is whether the DOM spec should have a note or something mentioning that user agents may use the lack of canceling listeners as a signal in performance optimizations. I think it would add clarity to the purpose of mayCancel but I don't think we need anything normative. Preferences? Better to just leave this out of the spec entirely?

[annevk]

A note might be okay, then we can also clarify, in that note, that this is not to be observable from script.

[RByers]

Ok, I've written this up as discussed, eg. see here. I think this works but feel free to re-open / provide other feedback on the PR if there are outstanding concerns / suggestions.

[RByers]

Sounds like there isn't the consensus here I thought there was. Re-opening.

[RByers]

@annevk said:

Well, what I said in #20 (comment) is that what touch events do today doesn't require any changes in DOM. What you are suggesting here is that they observe listeners, which is something we want to avoid. ... Now the processing model is clear, but the design seems wrong.

See above where I said:

Concretely, today all browsers have an optimization where if there are no touch event handlers for a point, then touch scrolling can start without waiting for the currently running task to complete.

This is unfortunate but necessary in practice for good scrolling performance with touch events. I think there are only three fundamental design choices here:

1. Leave things as they are, with scroll performance being a fundamental disadvantage of the web compared to native mobile platforms (relying on evangelism and tooling to try to get developers to break all JS up into <16ms chunks)
2. Try to deprecate / replace event models that permit scroll blocking.
3. Specify the existing behavior of all browsers (which observe listener presence) and carefully expose a small API to allow developers to turn it off on a handler-by-handler basis.

To me, the only option consistent with how the web evolves in practice (and so likely to have broad impact) is 3. I'd be happy to be convinced otherwise.

[annevk]

It's confusing since when I asked if we were breaking the event model before I was told we are not. Unfortunately, I'm having a hard time articulating why observing listeners is a bad idea. I've asked @Hixie to chime in, though maybe @smaug---- could do so as well.

[Hixie]

A UA observing listeners is bad because it means that if you happen to do target.addEventListener() with a no-op listener, you change the behaviour. A no-op listener should be a no-op. Trying to debug which random library added which random event listener to which random node and thus caused performance to tank is a terrible, terrible developer experience.

[RByers]

Yep, I agree this sucks but it's been the reality of touch (and wheel) event handling on most browsers since the introduction of threaded scrolling and mobile browsers generally (eg. see my posts here and here). Today the impact is limited only to performance and we try to provide performance tools to help developers reason about it. But their choice today is "listen and hurt performance or don't listen". The thinking here is that we can let developers opt-out of this annoying choice - let them listen without having the side effect of hurting performance. Isn't getting more developers to opt-into that (with tools/APIs to measure the performance improvement) exactly the way to solve the "terrible developer experience" you mention @Hixie?

In the long-run, if we can get enough frameworks to opt-in to this behavior, perhaps we can someday consider making it the default and solve this "adding no-op listener can hurts performance" problem once and for all. But just doing nothing and continuing to pretend in the specs that this issue doesn't exist is not going to help.

Bottom line, it sounds like we're both have the same goal here - decrease the risk that developers will hurt performance by adding event listeners. We continue to see a very real problem here in practice (a big part of the "third party web is killing performance, block it all" debate). So, if you don't think we should do what I propose here, then how do you suggest we solve this problem?

[Hixie]

So to summarise:

• with no event handlers, scrolling should be fast
• with an event handler that cancels the event, scrolling shouldn't happen
• modulo those constraints, scrolling should be as fast as possible
• if existing code needs to change to make scrolling fast, doing so should be easy

Is that right?

@annevk, do you have an alternative proposal for how to address those requirements?

[annevk]

I haven't been able to think of something that gives the same granularity of opt-in control. Just a global or per scrolling context flag of sorts (e.g., some new CSS feature).

[RByers]

Thanks @Hixie, that's a great summary.

Just a global or per scrolling context flag of sorts (e.g., some new CSS feature).

Yeah, a global won't work because the biggest problems in practice come from third-party scripts (eg. analytics libraries). We need the browser to mediate between multiple competing interests, and we need tooling that can point the finger at specific scripts (eg. "scrolling is slow due to the handlers added by foo.js").

My original proposal was a new CSS property (scroll-blocks-on) but we realized that violated the 4th constraint in that it wasn't composable. Multiple scripts may want to influence the scroll behavior for the document element, CSS provides no such composition mechanism (and even if it did, people argued that would be "spooky action at a distance" - hard to reason about and adopt incrementally).

[Hixie]

Yeah I don't see how a CSS feature could work here. The key point I think is the first two bullet points. They apply to existing pages, so whatever we do here has to be a change to an existing feature, not a new feature.

[RByers]

Good point. I guess I've been ignoring getting the spec caught up to what implementations have already been doing, that's probably the real source of contention here. Note that for good performance, there's actually a more stringent version of your first constraint that we use in practice:

• with no event handlers at the point where input occurred, scrolling should be fast

I.e. if you have long document with a touch/wheel handler only on some image carousel in the middle, scrolling at other points in the document should be nearly as fast as if there were no handlers on the page at all.

[RByers]

@annevk and I chatted about this on IRC and came to an agreement. Tentative conclusions:

• Observing event handler presence for perf reasons is unfortunate but acceptable
• Observing for the sake of making an event uncancelable is discouraged but some events (touch/wheel) may be specified to do so.
• New events should be designed to avoid this (like pointer events), reach out to public-script-coord if you think you have a use case.
• Add a non-normative section "Observing event listeners" after interface EventTarget that spells this out (kind of like "Action versus Occurrence").

[RByers]

Any feedback on the section I've added? Some wordsmithing definitely required (pull requests happily accepted).

[annevk]

"This section is non-normative." is not needed. However, if you write non-normative language, you cannot use RFC2119 terms, such as "may" or "should", so you need to rephrase things.

Change public-script-coord to public-script-coord@w3.org.

Directly adjacent to the first paragraph, I would add something like, "That is, a developer adding a no-op event listener would not expect that to have any side effects."

I would also explain that the sensor APIs that require this and other legacy event-based APIs such as touch, are unfortunate in their design that the only way to implement them efficiently requires observing listeners. I think calling that out as an API design flaw sends the right message.

[RByers]

"This section is non-normative." is not needed. However, if you write non-normative language, you cannot use RFC2119 terms, such as "may" or "should", so you need to rephrase things.

Ok, thanks. I'm a little confused by this sentence:

All diagrams, examples, and notes in this specification are non-normative, as are all sections explicitly marked non-normative. Everything else in this specification is normative.

Should it be changed (since there aren't actually any sections explicitly marked non-normative as far as I can see?).

Directly adjacent to the first paragraph, I would add something like, "That is, a developer adding a no-op event listener would not expect that to have any side effects."

Done. Along with re-wording for 'may' and to avoid redundancy.

I would also explain that the sensor APIs that require this and other legacy event-based APIs such as touch, are unfortunate in their design that the only way to implement them efficiently requires observing listeners. I think calling that out as an API design flaw sends the right message.

Ok, sounds good. How does it look now?

[annevk]

We should maybe change that paragraph, yes. Or consistently markup non-normative sections I guess. Meh. Follow up issue?

Review of sorts:

• don't -> do not
• it's -> its
• xref for "event listener" would be good, maybe also <b>callback</b>?
• async -> asynchronous or maybe even "in parallel" (xref to HTML)
• passive should be about the passive concept, not the dictionary member
• you still use "should"

[RByers]

We should maybe change that paragraph, yes. Or consistently markup non-normative sections I guess. Meh. Follow up issue?

Filed whatwg/dom#83

[RByers]

don't -> do not
it's -> its
xref for "event listener" would be good, maybe also callback?

done

async -> asynchronous or maybe even "in parallel" (xref to HTML)

"asychronous scrolling" is starting to become a recognized term (and we're working in Houdini to try to define it more precisely). But replacing "scrolling can be allowed to start asynchronously" with "scrolling can be allowed to start in parallel" with a link to the HTML definition sounds good to me.

passive should be about the passive concept, not the dictionary member

How's this? I expanded the description of the member slightly to define the concept. Perhaps it deserves a more thorough definition somewhere?

you still use "should"

Damn, giving advice without using these terms is hard! Sorry.

[annevk]

It still seems like you're conflating model and API.

The bit where the term "event listener" is defined is the model. The dictionary stuff is the API (which I think we want as part of the EventTarget interface section and I don't think we want to define the dictionary members as you've done, though I guess they should be mentioned in in the domintro block).

The non-normative prose should only talk about the model and perhaps use the API in examples.

Mentioning the dictionary type in prose is also wrong. We don't really have access to that. We just know it contains capture/passive.

I wonder if @heycam can confirm that (dictionary or boolean) actually works.

[RByers]

Thanks Anne. I've been trying to better understand the intention and style here (also related to my previous questions about the role of the notes with summaries of the APIs). This comment helps a lot - thanks. I'll attempt to rework things next week based on what you've said. I'm also happy to read up on advice elsewhere (eg. previous codereviews) if there's anything you can point me to.

Concretely it sounds like maybe after "Each event listener consists of a type (of the event), callback, and capture and passive variables" I should try to define in prose what these variables (at least capture and passive) are about, defining the concepts there, right? Then I can summarize the use of the options in the Note below. So I should merge all of section 3.6 (EventListenerOptions) into 3.7 (EventTarget) with all the IDL at the top, then the model, the note summarizing the API usage, and the finally the API details / algorithms, right?

[heycam]

I wonder if @heycam can confirm that (dictionary or boolean) actually works.

Yes, that's fine, per the table at http://heycam.github.io/webidl/#dfn-distinguishable.

[annevk]

Yeah, that sounds about right. Generally the prose defining things doesn't really go into explaining things much since that can lead to interpretations that do not match what the algorithms actually require. So I tend to go light on prose and have notes and/or additional sections that elaborate a little bit on the design when it's not super clear.

[RByers]

For the record, @annevk and I have been discussing this issue further here. His opinion is that "observable" is mainly about being "script observable without timers".

There is at least one subtle script-observable behavior change (at least in Chrome and Safari where I tested): if there are no touch listeners present and a listener is added mid-gesture, no events are sent for the already touching finger. I built a demo of this here. You could argue however that this is a bug we could fix (or at least reduce to a small race window), though not sure we'd ever get Safari to change.

[smaug----]

Blink has also that odd wheel event handling where the existence of a cancelling listener can be detected even by cross origin parent document.

[RByers]

@smaug---- I think that case is orthogonal. In that case it's not the presence of the listener that changes anything, but the cancelling of the event. Yeah it's bad that cancelling the event inside the frame affects behavior in the parent document, but that has nothing to do with observing the presence/absence of a listener itself.

[RByers]

@annevk said:

I must be missing something since I thought we had a very clear observable case to go from and therefore observing even more for this passive stuff was okay...

I believe my observability arguments were all around performance. See discussion here and here.

I know it usually makes sense to consider performance effects as non-observable. But when it's a sharp cliff (no listeners -> site scrolls well, add one no-op listener -> scrolling is terrible), then the observability problem becomes just as bad as if it was a traditional script-observable logic issue.

Back to Hixie's argument for the problem here:

A UA observing listeners is bad because it means that if you happen to do target.addEventListener() with a no-op listener, you change the behaviour. A no-op listener should be a no-op. Trying to debug which random library added which random event listener to which random node and thus caused performance to tank is a terrible, terrible developer experience.

That's the status quo today. But if we give developers the tooling to ensure all their touch listeners are passive, then they can avoid the problem entirely. It's only when a site mixes passive and non-passive listeners for a touch or wheel event that listener presence is script-observable outside timing (and performance-observable only when there are non-passive listeners).

Perhaps we should consider a follow-up API that lets a site owner opt-in to forcing all touch listeners to be passive? Perhaps ultimately (years down the road) we can even move the web towards passive being the default for touch and wheel listeners and solve the problem once and for all.

[annevk]

And what is the plan again for touch events (any other events?) if all listeners are passive? Will there be a timing difference when the event gets delivered too?

[RByers]

My plan for touch and wheel events is to send the events as uncancelable when all listeners are passive. This won't change the script-observable timing, but can change the user-visible timing.

Eg. most importantly some sites today are probably (accidentally) depending on the existence of wheel listeners to cause scroll-linked effects (JS-based sticky / headers) to appear synchronized with scrolling. Making all those listeners passive shouldn't change what JS sees (except for the cancelable property of the Event of course), but could dramatically impact the user experience. In particular when scrolling becomes smoother and JS isn't fast enough to keep up then the disconnection between the scrolling and JS-driven effects often looks worse than when scrolling was just slowed down to match the JS frame rate.

Once we have passive listeners into the DOM spec, I plan to work with the TECG to describe this in the TouchEvents spec. Once we have that, I'll talk to Gary and Travis about getting something similar into the UI Events spec for wheel.

[RByers]

We've attempted to balance the concerns and capture the guidance around this in the spec here. AFAIK we've reached consensus around this (though may want to still discuss some details, eg. when I modify the touch events spec for this).