Add EventListenerOptions and passive event listeners #82

RByers · 2015-09-22T16:10:30Z

My first crack at incorporating the ideas and discussion from my EventListenerOptions proposal into the spec. The result can be previewed here.

One thing I'm still not super happy with is the somewhat confusing defaults for mayCancel. On the one hand we agreed that we wanted the performance behavior (mayCancel:false) the be the default when using the new API form. But we of course can't change the existing behavior. Let's discuss that further here if desired.

annevk · 2015-09-22T17:35:09Z

How does this address e.g., WICG/EventListenerOptions#20? Also, it's very confusing to still have outstanding discussion elsewhere. Not really sure what to pay attention to now.

annevk · 2015-09-22T17:53:59Z

Basically, the way this is written I don't see much room for optimizations, unless you're doing something that's not allowed.

RByers · 2015-09-22T18:00:16Z

It's addressed by what @smaug--- said. I.e. whether or not a particular event is cancelable is not the purview of DOM, it's up to the individual specs. So a handler being added 'too late' is the same as other scenarios where a non-cancelable event is received by a handler, so already covered by the DOM spec. If we can get consensus here, then I'll add something to touch-events (w3c/touch-events/#6) that indicates that a TouchEvent may be uncancelable if, at the time it was first received, there are no mayCancel=true handlers for it. WDYT? Is there something more I should add here (perhaps expand on the note?).

Regarding discussion, I thought there were enough separate non-trivial issues (with non-trivial discussion history) that it was valuable to continue to use a separate issue tracker for them. But if you prefer I can try to close them all and merge all discussion into this PR. I don't have a preference on whether minor editorial discussion occurs here or in separate issues. But I defer to you on all of this - what's easiest for you at this stage?

annevk · 2015-09-23T07:05:23Z

Well, what I said in WICG/EventListenerOptions#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.

I don't really care where we settle on the design, but I'd like to settle on the design first. (I asked for the processing model before because the design wasn't clear. Now the processing model is clear, but the design seems wrong.)

RByers · 2015-09-23T13:45:35Z

Ok, I suggest we continue this conversation in WICG/EventListenerOptions#20 since it's (IMHO) a continuation of the same debate.

annevk · 2015-11-10T10:06:23Z

@RByers what is the plan for this feature?

RByers · 2015-11-10T14:11:39Z

@annevk Sorry for the delay, we're still working on it (I've just bee caught up in some other things). @dtapuska is actively working on the blink implementation. I expect to be able to make the spec work here my top priority in a couple weeks.

RByers · 2015-11-30T22:25:04Z

Ok @annevk, I've finally reworked this pull request according to your suggestions in WICG/EventListenerOptions#20. WDYT?

annevk · 2015-12-15T10:09:49Z

dom.bs

+  just a boolean, in which case it determines the value of the <b>capture</b> option.
+
+  When set to true,
+  the <var>capture</var> option prevents <b>callback</b> from being invoked when


<var>capture</var> is no longer a variable so this seems wrong.

How about the <b>capture</b> member of <var>options</var>? Or do you prefer simply the capture option?

I think "<var>options</var>' <code>capture</code> member" is what is typically used.

RByers · 2015-12-15T19:51:16Z

Thanks for the review! I've updated the pull request based on your feedback. I assume I should avoid squashing all the commits until the review is done and we're ready to merge.

annevk · 2015-12-16T09:22:24Z

Feel free to rebase and squash once you're confident it's in a good shape and I'll just do another pass. Might be easier.

RByers · 2015-12-16T17:10:24Z

Ok I think the only thing left is what exactly we want to cover in the "observing event listeners" section. Let's keep iterating on that, then once you're happy I'll rebase/squash (which I think will cause all the line-by-line discussions to disappear from GitHub :-( ).

annevk · 2016-01-01T16:25:12Z

dom.bs

- {{Event/preventDefault()}} was invoked
- while the {{Event/cancelable}} attribute
- value is true, and false otherwise.
+ {{Event/preventDefault()}} was used successfully to indicate cancellation.


(Adding more nitpicky comments. Feel free to ignore for now.) s/used/invoked/ We don't want to omit mentioning when it returns false. The original text did that correctly.

Whoops, must have lost this by accident somewhere along the line. Fixed.

annevk · 2016-01-01T16:29:48Z

dom.bs

+<h3 id=observing-event-listeners>Observing event listeners</h3>
+
+<p>In general, developers do not expect the presence of an <a>event listener</a> to be
+observable.  The impact of an <a>event listener</a> is determined by its <b>callback</b>.


Throughout, there should only be a single space after a period.

annevk · 2016-01-05T08:54:39Z

dom.bs

+
+   <li>Clear <var>event</var>'s <a>in passive listener flag</a>.
+
+   <li>If the call to {{EventListener/handleEvent()}} threw any exception, <a>report the exception</a>.


Are you sure this is correct? Reporting the exception is synchronous. Seems like you could circumvent the passive restriction by storing the event somewhere and then throwing.

If reporting the exception is synchronous, then is it OK to just clear the passive listener flag afterwards (updated)? That would definitely be preferable, I was just thinking that wouldn't be possible.

As far as I can tell that's fine. Where do you think it would break?

I just wasn't thinking about it properly (thinking in terms of an algorithm with a 'throw' statement). Thanks for catching this, I agree it's better and I don't anticipate any implementation issue.

annevk · 2016-01-05T08:56:22Z

(Sorry, I was about to merge, but then I spotted that. Not sure why I didn't catch it earlier.)

annevk · 2016-01-05T17:34:16Z

The only other thing that looks a bit odd is the wrapping. I can clean that up I suppose.

RByers · 2016-01-05T18:21:48Z

The only other thing that looks a bit odd is the wrapping. I can clean that up I suppose.

Looks like the pattern is to use ~100 except in <pre> and <code> blocks, right? I'm fixing that now.

This introduces an EventListenerOptions dictionary which can be used to explicitly specify options to addEventListener and removeEventListener. This also introduces a "passive" option, which disables the ability for a listener to cancel the event. See https://github.com/RByers/EventListenerOptions/blob/gh-pages/explainer.md for a high-level overview, and https://github.com/RByers/EventListenerOptions/issues?q=is%3Aissue for most of the debate that went into the design.

RByers · 2016-01-05T18:32:46Z

Ok, re-wrapped all lines I touched to 100 columns (except <pre> and <code> blocks).

This introduces an EventListenerOptions dictionary which can be used to explicitly specify options for addEventListener() and removeEventListener(). This also introduces a "passive" option, which disables the ability for a listener to cancel the event. See https://github.com/RByers/EventListenerOptions/blob/gh-pages/explainer.md for a high-level overview, and https://github.com/RByers/EventListenerOptions/issues?q=is%3Aissue for most of the debate that went into the design. PR: #82

annevk · 2016-01-06T11:45:33Z

Landed as 253a21b.

annevk · 2016-01-06T11:45:40Z

Thank you!

RByers mentioned this pull request Sep 22, 2015

Rework spec into the DOM standard WICG/EventListenerOptions#19

Closed

RByers mentioned this pull request Sep 22, 2015

UAs observing the presence of event listeners is bad WICG/EventListenerOptions#20

Closed

RByers force-pushed the event-listener-options branch from f440630 to 29925e7 Compare September 25, 2015 14:28

RByers force-pushed the event-listener-options branch from cddfc43 to 7e05d12 Compare November 30, 2015 22:20

RByers force-pushed the event-listener-options branch from 7e05d12 to 49ba2a3 Compare November 30, 2015 22:30

RByers changed the title ~~Add EventListenerOptions~~ Add EventListenerOptions and passive event listeners feature Dec 2, 2015

RByers changed the title ~~Add EventListenerOptions and passive event listeners feature~~ Add EventListenerOptions and passive event listeners Dec 2, 2015

annevk reviewed Dec 15, 2015
View reviewed changes

RByers force-pushed the event-listener-options branch from d4322fe to c01304d Compare December 17, 2015 18:29

annevk reviewed Jan 1, 2016
View reviewed changes

RByers force-pushed the event-listener-options branch from b2fa94e to 530c38b Compare January 4, 2016 18:46

annevk reviewed Jan 5, 2016
View reviewed changes

RByers force-pushed the event-listener-options branch from 530c38b to d17e7f8 Compare January 5, 2016 15:27

RByers force-pushed the event-listener-options branch from d17e7f8 to 0ad4e56 Compare January 5, 2016 18:32

annevk closed this Jan 6, 2016

RByers deleted the event-listener-options branch September 27, 2016 16:28

annevk mentioned this pull request Aug 17, 2017

Editorial: simplify EventListenerOptions processing #492

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add EventListenerOptions and passive event listeners #82

Add EventListenerOptions and passive event listeners #82

RByers commented Sep 22, 2015

annevk commented Sep 22, 2015

annevk commented Sep 22, 2015

RByers commented Sep 22, 2015

annevk commented Sep 23, 2015

RByers commented Sep 23, 2015

annevk commented Nov 10, 2015

RByers commented Nov 10, 2015

RByers commented Nov 30, 2015

annevk Dec 15, 2015

RByers Dec 15, 2015

annevk Dec 16, 2015

RByers commented Dec 15, 2015

annevk commented Dec 16, 2015

RByers commented Dec 16, 2015

annevk Jan 1, 2016

RByers Jan 4, 2016

annevk Jan 1, 2016

RByers Jan 4, 2016

annevk Jan 5, 2016

RByers Jan 5, 2016

annevk Jan 5, 2016

RByers Jan 5, 2016

annevk commented Jan 5, 2016

annevk commented Jan 5, 2016

RByers commented Jan 5, 2016

RByers commented Jan 5, 2016

annevk commented Jan 6, 2016

annevk commented Jan 6, 2016


		<li>Clear <var>event</var>'s <a>in passive listener flag</a>.

		<li>If the call to {{EventListener/handleEvent()}} threw any exception, <a>report the exception</a>.

Add EventListenerOptions and passive event listeners #82

Add EventListenerOptions and passive event listeners #82

Conversation

RByers commented Sep 22, 2015

annevk commented Sep 22, 2015

annevk commented Sep 22, 2015

RByers commented Sep 22, 2015

annevk commented Sep 23, 2015

RByers commented Sep 23, 2015

annevk commented Nov 10, 2015

RByers commented Nov 10, 2015

RByers commented Nov 30, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RByers commented Dec 15, 2015

annevk commented Dec 16, 2015

RByers commented Dec 16, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk commented Jan 5, 2016

annevk commented Jan 5, 2016

RByers commented Jan 5, 2016

RByers commented Jan 5, 2016

annevk commented Jan 6, 2016

annevk commented Jan 6, 2016