[cssom-view] Update {add,remove}Listener() to hook into DOM correctly #4408
Conversation
| 1. <a spec=dom>Add an event listener</a> with the <a spec=dom>context object</a> | ||
| and an <a spec=dom>event listener</a> whose | ||
| <a spec=dom for="event listener">type</a> is <code>change</code>, | ||
| and <a spec=dom for="event listener">callback</a> is |callback|. |
There was a problem hiding this comment.
This should probably specify what happens when callback is null explicitly, which is nothing?
Though I guess per https://dom.spec.whatwg.org/#add-an-event-listener step 2 we're alright, so maybe not worth it.
There was a problem hiding this comment.
Yeah, no need to check it multiple times.
| <a spec=dom for="event listener">callback</a> is |callback|, | ||
| and <a spec=dom for="event listener">capture</a> is false, | ||
| then <a spec=dom>remove an event listener</a> with | ||
| the <a spec=dom>context object</a> and that <a spec=dom>event listener</a>. |
There was a problem hiding this comment.
I wonder if we should fold this conditional into "remove an event listener".
There was a problem hiding this comment.
That would make sense to me.
There was a problem hiding this comment.
I think currently it does not because HTML doesn't need it, but it would effectively be a no-op for HTML so probably okay. Alternative we have this and an inner variant that HTML invokes directly.
There was a problem hiding this comment.
No strong opinion. But I don't think we need to block this PR on this tweak. We can always change again in a followup.
Fixes #4407.
Tests: web-platform-tests/wpt#18390
@annevk are some of the terms here not exported in DOM but should be?