Closed Bug 1501983 Opened 3 years ago Closed 2 months ago

Add onslotchange event handler

Categories

(Core :: DOM: Core & HTML, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
93 Branch
Tracking Status
firefox93 --- fixed

People

(Reporter: smaug, Assigned: soniasingla)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, good-first-bug)

Attachments

(1 file)

Blocks: shadowdom
Priority: -- → P3
Component: DOM → DOM: Core & HTML

Spec pull request: https://github.com/whatwg/html/pull/4129 . It looks like we ended up using GlobalEventHandlers.

We are also considering adding it to ShadowRoot, but there's no spec PR for that yet.

There is now and I suspect we'll merge it once an editorial matter is resolved: https://github.com/whatwg/dom/pull/785.

Keywords: good-first-bug

Hi :smaug , :annevk, i would like to work on it, can you help me getting started? thanks

Flags: needinfo?(bugs)
Flags: needinfo?(annevk)

To add support for the webidl event handler, add onslotchange to https://searchfox.org/mozilla-central/source/dom/webidl/EventHandler.webidl
(see https://html.spec.whatwg.org/#globaleventhandlers)

And for the DOM content attribute (https://html.spec.whatwg.org/#ix-event-handlers),
add new event message here https://searchfox.org/mozilla-central/source/widget/EventMessageList.h#458 and then
map the event name to attribute like this
https://searchfox.org/mozilla-central/rev/12770bd668c0a6bdaa8eb96ad9507c6febe8d23d/dom/events/EventNameList.h#226
(or if the handler should be supported on svg elements too, use EventNameType_All, not EventNameType_HTMLXUL)
(please test other browsers whether they support onslotchange attribute on svg elements too)

Once you've done that, run the relevant tests, since at least
some of the subtests of https://searchfox.org/mozilla-central/source/testing/web-platform/tests/html/webappapis/scripting/events/event-handler-all-global-events.html should now pass, so you may need to remove
https://searchfox.org/mozilla-central/source/testing/web-platform/meta/html/webappapis/scripting/events/event-handler-all-global-events.html.ini#26-36

Flags: needinfo?(bugs)

:championshuttler I was not able to find any patch for this on https://phabricator.services.mozilla.com/p/championshuttler and you didn't assign the bug to yourself ; are you working on it? I'm asking because I had proposed this task to Sonia Singla as part of her coding experience at Igalia. But if you still plan to work on it, that's perfectly fine... I just want to be sure to avoid duplicate effort.

Hey :fredw I was planning to work on it this weekend but if you proposed it for Sonia I will assign it to her np :)

Flags: needinfo?(annevk)
Assignee: nobody → soniasingla.1812

Make sure to add the entry to xpcom/ds/StaticAtoms.py, else build will fail :)

Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/85509bddf46f
Add onslotchange event handler.r=smaug
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 93 Branch

Documentation for FF93 can be tracked in https://github.com/mdn/content/issues/8624

My understanding is that this change adds the onslotchange global event handler, so now you can assign your handler functions on Window - that right? Why would you want to use Window.onslotchange rather than addEventListener on slotchange for the specific slot you're interested in?

Or to put it another way, we have this example https://github.com/mdn/web-components-examples/tree/master/slotchange
It has a template with two slots. We use this.shadowRoot.querySelectorAll('slot'); to get the list of all slots and then add our event listener to the slot that might change using slots[1].addEventListener('slotchange', function(e) {.

That makes sense - we're only interested in changes to that slot and we will only get those events and act on them.

How would we do this using the global event handler, and why would that be something people want to do? Is there some reason you'd want to globally monitor slot changes.

Apologies if this is a dumb question. The reason I ask is for the examples. It is easy to show a useful global example for events you want to handle at the global level, but not always so sensible when you really are only interested in specific elements (or slots in this case).

Flags: needinfo?(bugs)

@Hamish: I think there is a confusion: "global event handler" refers to the mixin https://html.spec.whatwg.org/multipage/webappapis.html#globaleventhandlers (see also https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers) from which inherits Window, Document, HTMLElement, SVGElement, MathMLElement and I guess more IDL interfaces (workers?). Additionally, this patch also implements the IDL attribute on ShadowRoot ( https://dom.spec.whatwg.org/#interface-shadowroot ) so you can just do this.shadowRoot.onslotchange = myHandler() too. Finally the onslotchange IDL attribute reflects the onslotchange content attribute on elements, so one can also just do <element onslotchange="myHandler()">. All of this does not really add more power for developers compared to addEventerlistener, but it makes it more consistent with how other events behave. More info on the blink-dev thread: https://groups.google.com/a/chromium.org/g/blink-dev/c/cagoIboJ6Oo/m/yje1mcIUBAAJ

Thanks Frederic - a consistency change makes sense. Also the explanation makes sense. I've been confused by some cases where the onXxx appeared to predate the global handler, and also where the global handler can in theory be called on an element, but in practice cannot (onsecuritypolicyviolation - because the violation occurs before the event can be added to the element - I "think").

Anyway, answered. Much appreciated.

fredw answered here, thanks :)

Flags: needinfo?(bugs)

I have marked this dev-doc complete. The work is still being reviewed, but can be tracked here: https://github.com/mdn/content/issues/8624#issuecomment-916640529

You need to log in before you can comment on or make changes to this bug.