Bug 16491 - Add simpler and better event registration system
Add simpler and better event registration system
Status: RESOLVED LATER
Product: WebAppsWG
Classification: Unclassified
Component: DOM
unspecified
PC Windows 3.1
: P2 normal
: ---
Assigned To: Anne
public-webapps-bugzilla
:
Depends on:
Blocks: 20158
  Show dependency treegraph
 
Reported: 2012-03-23 08:24 UTC by Anne
Modified: 2014-02-13 11:41 UTC (History)
9 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Anne 2012-03-23 08:24:13 UTC
At TPAC and on the mailing list it was brought up to have something like

  target.on.{event}.add(handler)
  target.on.{event}.remove(handler)

To these event listeners it would be as if all events bubble. You could pass some kind of options object for constraints. Having event delegation natively was also brought up. Nobody has made a concrete proposal to date however.
Comment 1 Jonas Sicking 2012-03-23 23:46:26 UTC
It would be very nice if you could do something like:

target.on.add({ onclick: function(event) { ... },
                onmouseover: function(event) { ... },
                onmouseout: function(event) { ... },
                someOtherState: 4, });

This way could could actually do object-oriented, rather than having to have glue functions which forward to methods on objects.
Comment 2 Anne 2013-01-04 22:41:33 UTC
I think we should add something close to: http://api.jquery.com/on/
Comment 3 Glenn Maynard 2013-01-05 00:10:52 UTC
For reference, another event interface is Prototype's Event.on: http://prototypejs.org/doc/latest/dom/Event/

In particular, it returns an object which can later be used to remove the event handler, so you don't need to keep a reference to the function.  For example:

this.handler1 = element.on("click", function(e) { });
...
this.handler1.stop();

I found this handy when I used it, though these days I find this pattern to work fine:

foo = function()
{
    this.onclick = this.onclick.bind(this);
    foo.addEventListener("click", this.onclick, false);
    ...
    foo.removeEventListener("click", this.onclick, false);
}
foo.prototype.onclick = function(e) { ... }

(where by "fine" I mean it works well enough, even if it's a little uncosmetic, that I'm not desperately searching for something better and I'm not sure Prototype's approach is actually better).
Comment 4 Anne 2013-01-05 12:08:21 UTC
James Robinson suggested filtering on event data so UAs can optimize which events to actually fire (e.g. if you're only interested in WASD and space you'd only get events when those keys are pressed).
Comment 5 Olli Pettay 2013-01-05 12:20:42 UTC
Implementations tend to use events also internally
(default handling is the last phase) so UAs might not be able
to optimize too much.
Comment 6 Glenn Maynard 2013-01-05 18:23:38 UTC
I'm not sure if the suggestion is to allow optimizing out the whole event dispatch, or just the invocation of particular event listeners, but I don't think either is useful.

I can't easily benchmark how long firing an event from native code takes, but from JavaScript it's taking about .15ms Firefox and .06ms in Chrome, so optimizing out the actual firing of the event, even if it was possible, doesn't seem useful.  (https://zewt.org/~glenn/time-event-dispatch-1.html)

Dispatching an event with 10000 listeners takes 22ms (Chrome) to 25ms (Firefox) on my system, or about 2uS per handler.  Even in an ugly case with a hundred keypress handlers on window, one for each key, this doesn't seem like a useful optimization either.  (https://zewt.org/~glenn/time-event-dispatch-2.html)
Comment 7 Olli Pettay 2013-01-05 18:32:14 UTC
Performance depends quite heavily on the length of the event target chain,
but yes, in general event handling is quite fast these days.
(and getting faster in Gecko once we move to WebIDL bindings)
Comment 8 Anne 2013-02-11 10:02:47 UTC
For the dictionary:

* normalBubbles = false
* capture = false
* onlyTrusted = false
Comment 9 Glenn Maynard 2013-02-14 14:10:45 UTC
Recommend dropping onlyTrusted.  Even if it was frequently used (it seems extremely rarely useful), people don't need a special native feature to say "if(!e.isTrusted) return;".
Comment 10 Jonas Sicking 2013-02-15 07:47:02 UTC
I think there are at least 3 different phases that are useful to register for:

* capture-only: This is what you currently get if you pass "true" as 3rd argument
  of addEventListener. Useful for overriding listeners on the target.
* target-and-bubble: This is what you currently get by default for bubbling
  events. This is useful for "click" events and the like to see if an event
  was targetted at a given subtree.
* target-only: This is useful for load events, or for checking that an element,
  and not its descendents is currently being hovered.


So rather than having a "capture: true/false" property, I think it might be better to have a "phase: 'target'/'capture'/'targetbubble'" property. That also makes the API easy to expand as we find use cases for more phase combinations, like 'bubble' or 'targetcapture'.

An alternative would be to use something like:

phase: ["bubble", "target"]

But that seems like annoyingly much typing.
Comment 11 Glenn Maynard 2013-02-15 15:27:13 UTC
I'd suggest simply mode: "bubble" mode: "capture", replacing the boolean with an enum.  If use cases for letting people specify arbitrary phases show up, adding mode: "both" and letting people check eventPhase manually seems cleaner than specifying a set in the listener itself.

I'm not sure if it's worth having a dedicated "AT_TARGET only" mode.  It doesn't seem so common that we need a shortcut for simply "if(e.eventPhase != Event.AT_TARGET) return".

I'm assuming we're only talking about API convenience and not optimizing event dispatch.

(And as Anne pointed out on IRC, capture listeners do run on the target :)
Comment 12 Jonas Sicking 2013-02-15 19:15:18 UTC
Currently the event system has come up with both the distinction between bubbling and non-bubbling events, as well as the distinction between bubbling and capturing listeners, as a way to enable people to not have to write "if(e.eventPhase != Event.X) return" at the beginning of the listener.

So at least previously we've gone to great lengths to avoid that.

I actually think that that was a good thing. Being able to set a flag at the time of registration, rather than adding additional code in the listener, both increases code readability of the listener (the listener can be more focused on doing whatever action needs to be done rather than when to do it) as well as code readability at the listener registration site (it's easy to see what types of events the listener will listen to).

FWIW, one of the other big features of the new event registration system is the Selector filter, which also just avoids having to type "if(!e.target.matchesSelector('...')) return"
Comment 13 Glenn Maynard 2013-02-15 20:34:18 UTC
The distinction between bubbling and capturing listeners is useful, since otherwise you'd be firing every single even handler up the DOM tree twice.

But we don't need to try to eliminate every possible condition you might short-circuit your event listener with.  Maybe somebody would want to do something weird like "if(e.eventPhase != e.AT_TARGET)", but they should use script for that.  The same applies to checking e.isTrusted, or shiftKey, or countless other possible filters.

> FWIW, one of the other big features of the new event registration system is the Selector filter, which also just avoids having to type "if(!e.target.matchesSelector('...')) return"

(I think matchesSelector was renamed to just "matches".)

No, delegation is more than that.  You want to know if any node between yourself (the delegate) and the target matches the selector.  That is, given

<div class=delegate>
    <div class=box>
        <span>hello</span>
    </div>
</div>

If you put a listener on .delegate to listen to clicks on .box, it should still fire if you click within the span, even though the target itself doesn't match the selector.  In other words, starting at the target, search up through parent nodes to find the first one that matches the selector.  (This is http://api.jquery.com/closest.)

Also, you'll want to know what element actually matched the selector.  jQuery sets currentTarget to the matched element, and sets an additional property delegateTarget which is the original currentTarget (the element the event listener is bound to).
Comment 14 Jonas Sicking 2013-02-16 05:43:14 UTC
(In reply to comment #13)
> The distinction between bubbling and capturing listeners is useful, since
> otherwise you'd be firing every single even handler up the DOM tree twice.

So? If one of the calls always returns immediately then it still behaves correctly.

I do agree that the distinction is useful, my point is that your argument is inconsistent. We shouldn't allow filtering such that the listener is only notified at the target since that can easily be implemented using logic in the listener.

On the other hand you are saying that we should allow filtering based on capturing vs. bubbling phase as to avoid starting the handler unnecessarily.

> But we don't need to try to eliminate every possible condition you might
> short-circuit your event listener with.  Maybe somebody would want to do
> something weird like "if(e.eventPhase != e.AT_TARGET)", but they should use
> script for that.  The same applies to checking e.isTrusted, or shiftKey, or
> countless other possible filters.

I agree that we can't create every possible filter that people will want to do. But we also shouldn't have no filters as per your own argument above.

The trick is to find which filters are common enough that people will want to filter on them.

FWIW, I think that if we support an "at target" phase, then we can remove the "normalBubbles" flag.

> No, delegation is more than that.  You want to know if any node between
> yourself (the delegate) and the target matches the selector.  That is, given
> 
> <div class=delegate>
>     <div class=box>
>         <span>hello</span>
>     </div>
> </div>
> 
> If you put a listener on .delegate to listen to clicks on .box, it should
> still fire if you click within the span, even though the target itself
> doesn't match the selector.  In other words, starting at the target, search
> up through parent nodes to find the first one that matches the selector. 
> (This is http://api.jquery.com/closest.)
> 
> Also, you'll want to know what element actually matched the selector. 
> jQuery sets currentTarget to the matched element, and sets an additional
> property delegateTarget which is the original currentTarget (the element the
> event listener is bound to).

This is quite interesting! This makes a lot of sense. I wonder if we should even make the firing order such that the listener fires along with listeners registered on the .box.
Comment 15 Glenn Maynard 2013-02-16 06:07:49 UTC
(In reply to comment #14)
> So? If one of the calls always returns immediately then it still behaves
> correctly.
> 
> I do agree that the distinction is useful, my point is that your argument is
> inconsistent. We shouldn't allow filtering such that the listener is only
> notified at the target since that can easily be implemented using logic in
> the listener.

It's not inconsistent.  The overwhelming majority of listeners don't want to receive the event during the capturing phase, and almost zero listeners want to receive events during both.  It's a distinction that almost every event listener cares about, so much so that we usually don't want to have to think about it at all.  The other criteria are all vanishingly rare by comparison.

That doesn't mean that no other filters are useful to have at an API level, but I disagree with the argument that the existance of the capture criteria implies we should necessarily avoid the need for "if(e.something) return" checks in less ubiquitous cases.

> I agree that we can't create every possible filter that people will want to
> do. But we also shouldn't have no filters as per your own argument above.

I'm not saying we should have no filters.  I do think we should have a tighter criteria than "somebody, somewhere might want to filter on this", though.  So, I don't think being able to specify a set of event phases (["capture", "target"]) (for example) is worthwhile.  My intuition, at least, is that filtering on isTrusted is also not worth it, at least for now.

I don't think we're fundamentally disagreeing.  I just think we should be conservative in adding API-level filters until we know that they're worthwhile, especially since this API is already proposing a pretty big bulk of new features.

> This is quite interesting! This makes a lot of sense. I wonder if we should
> even make the firing order such that the listener fires along with listeners
> registered on the .box.

I hadn't thought of that, and it's interesting: it means delegated listeners can be effectively equivalent to listeners on the event itself, while having the benfits of delegation.  It would add some complexity to the basic event dispatch model and I'm not sure how to polyfill it, so we should think about this carefully, but at first glance I like it.
Comment 16 Olli Pettay 2013-02-16 11:02:21 UTC
(In reply to comment #14)
> 
> I wonder if we should
> even make the firing order such that the listener fires along with listeners
> registered on the .box.
Why would you do that? (and I'm wondering how to not regress event dispatching
performance too badly)
Comment 17 Glenn Maynard 2013-02-16 19:51:01 UTC
It means that delegated events would fire as if they were actually dynamically registered on the real target.  For example, if you have a block like this:

<div onclick="console.log('outer'); event.stopPropagation();">
    <div onclick="console.log('inner'); event.stopPropagation();"</div>
</div>

and you want to use delegates for both click events, you could say this:

<div class=delegate>
    <div class=outer>
        <div class=inner></div>
    </div>
</div>

<script>
querySelector(".delegate").addEventListener("click", function(e) {
    console.log("outer");
    e.stopPropagation();
}, { delegate: ".outer" });

querySelector(".delegate").addEventListener("click", function(e) {
    console.log("inner");
    e.stopPropagation();
}, { delegate: ".inner" });
</script>

and you'd always get the same event order as the first one, without having to carefully match up the registration order with the nesting order of the DOM nodes.  

Aside: I like the idea of making this a simple extension to addEventListener, as I tried out above, but the options block really wants to come before the function, not after it.  I'm not sure if an addEventListener(type, options, function) overload is as natural as addEventListener(type, function, options), unfortunately.

> and I'm wondering how to not regress event dispatching performance too badly

I assume it would be bad to have to check the filters of each node's parents as you walk the tree, since that becomes n^2.  Precalculating the list of matching delegates would avoid this, but precalculation doesn't really match the model, and it would still require walking up the tree an extra time.  Hmm.  Combined with the difficulty of polyfilling, it might not be worth it.
Comment 18 Anne 2014-02-13 11:41:32 UTC
This does not seem high priority given the other things we have going on. JavaScript will prolly provide this in due course.