Should the command event really be composed?

[jakearchibald]

What is the issue with the HTML Standard?

https://html.spec.whatwg.org/multipage/form-elements.html#attr-button-command-custom

Step 5.5 fires a command event with composed set to true, meaning it leaks in/out of shadow boundaries.

Given that commandFor can only point to elements in the same root, it seems like this feature is encapsulated within a root, but then the composed event crosses root boundaries, which seems odd.

For example, if, within a shadow root, I have a button which has a custom command pointing to another element in the shadow root, why should things outside that root get to see what the command is, and cancel them? This seems like a leak of my (the owner of the shadow root) implementation details.

cc @mfreed7 as you might know the background here.

[jakearchibald]

Additionally, what the command --foo means to me (the author of the shadow root) might mean something completely different to the owners of the parent roots, so they may act wrongly to a capturing listener of the command event.

[annevk]

Yeah this seems bogus. Pretty much only UI events are meant to be composed.

cc @keithamus @lukewarlow

[keithamus]

We had a discussion about this in the matrix here: https://matrix.to/#/!AGetWbsMpFPdSgUrbs:matrix.org/$_lNM7kzsK1cY9w4eLFNM0dlDxN_IU2xqlvuXR3UaWh0

@smaug---- commented:

My main question is that if invoke isn't composed, doesn't that lead the user of the component to then usually rely on click?

to which I replied:

Right. FWIW I like the idea of making it composed, I think it makes sense especially if we consider the idea of having built ins dispatch invoke events to announce their own internal behaviours (such as a summary opening the details) - it seems to explain those better.

[jakearchibald]

I think it makes sense especially if we consider the idea of having built ins dispatch invoke events to announce their own internal behaviours (such as a summary opening the details)

It seems really weird to me that the command would compose whereas the toggle event of the <details> does not.

[jakearchibald]

(fwiw I think input events composing was a mistake, but I understand it in relation to mouse/keyboard events)

[jakearchibald]

@smaug---- I agree with @keithamus's original thoughts on this. It's ok to know a custom element was clicked (and other mouse/keyboard actions), because we don't want to create dead zones like you get with iframes, so it's right that those events compose. But, parent roots shouldn't know about specific interactions within the shadow root, unless the shadow root chooses to expose them.

If a user agent chose to implement <select> using a <button>, commands, and popovers, it would be really weird if commands, custom or otherwise, suddenly started leaking out of this implementation. They'd have to stop propagation to avoid this.

It feels a bit like how input events leak the internals of a shadow DOM by default. It seems like the wrong choice there.

If I had a button in my shadow root that issued a command to open a popover, it feels like that should be internal by default, and something I'd expose via custom events if I wanted to expose it.

[smaug----]

I'd expect command event to work (in the future) with something similar to referenceTarget, and in such case propagating the command out of shadow DOM would very much make sense - someone outside the shadow DOM is triggering the command and that someone should be able to see the event.
And even now it tells about the command one is trying to do (it is like exposing an API) and then the embedder can prevent that specific command.
(and if the event wasn't composed, then .source retargeting wasn't needed)

Do we need some finer grain API here, that in some cases commands would stay within a shadow DOM and in some cases they wouldn't?

[jakearchibald]

Ohh, so you're talking about a case where the button is in the light DOM, but is redirected into a shadow DOM by some new feature like referenceTarget? You can already cross boundaries by assigning directly to button.commandForElement.

My thoughts on this:

• It seems ok if the tree that contains the <button commandfor> can hear the event.
• It's obviously right that the tree that contains the target can hear the event.
• It seems odd that other trees can hear the event.

But, the current behaviour, where all command events leak, doesn't seem right.

If composed was false, I could fire an event on my custom element allowing external listeners to call preventDefault() if that's the behaviour I wanted. This seems better than requiring everyone to set stopPropogation listeners in their custom elements, because that seems like the common case.

[jakearchibald]

Should we remove composition here then look for ways to deal with the rarer cases where the commends should leak out of shadow roots?

[lukewarlow]

Cc @keithamus I'm not sure what the process for this would be on the chrome side (as it's shipped) but I'm neutral on this topic so if the consensus ends up being that we should remove it then I'd be happy to update the WebKit implementation.

Cc @mfreed7 for Google's thoughts too

[smaug----]

I don't think the consensus is that we should remove composed, since that would break command handling from other subtrees. Do we need similar event propagation logic what events with .relatedTarget have? I could live with that.
https://dom.spec.whatwg.org/#concept-event-dispatch "Otherwise, if parent is relatedTarget, then set parent to null. "

[jakearchibald]

that would break command handling from other subtrees

Can you cover the use-cases in a bit more detail here? It seems like:

• The common case is where the source and the target are in the same root. Leaking command events is bad here, because it exposes internal workings of a shadow root, and allows outside trees to alter how commands work. There may also be a clash of what particular command names mean.
• You can manually assign a commandForElement from another root, but if the developer can do this, they already have access to the target, so they can listen for the command event there.
• The proposed referenceTarget feature would let the shadow root owner direct commands from the host element to an element within another the shadow root without having a reference to that element.

<button command="--foo" commandFor="custom-thing">…</button>
<custom-thing id="custom-thing">…</custom-thing>

In the above case, if the implementation of custom-thing uses referenceTarget, and the command event wasn't composed, the root containing the button wouldn't see the command event. The owner of the button didn't choose to have the command retargeted, and they don't have visibility into that decision, so it's weird that the event is lost in this case.

referenceTarget will cause this issue in other places too, such as popoverTarget (details).

I don't think we should make the general usages weird/leaky just because of referenceTarget, especially as that feature isn't fully developed yet, and it impacts more than just command.

---

Because the problem is referenceTarget specific, a solution tailored to referenceTarget seems ok. As in, the command event (and other related events like popover toggle) can flow through retargeted boundaries.

<component-a>
  <template shadowrootmode="closed">
    <button command="--foo" commandFor="component-b">…</button>
    <component-b id="component-b">
      <template shadowrootmode="closed" shadowrootreferencetarget="inner-thing">
        <div id="inner-thing"></div>
      </template>
    </component-b>
  </template>
</component-a>

In the example above, the command would be seen within component-b's shadow root, since that's where the ultimate target is, but it would also be seen in component-a's shadow root, since it was retargeted from there.

We'd need to consider cases where retargeting was between multiple shadow roots.

---

Either way, it feels like we should remove compose from this event, and work on the referenceTarget-specific issues as part of that feature work. I don't think we should solve this just for command, nor should we end up having to default to composed events everywhere due to referenceTarget.

[jakearchibald]

@smaug---- I guess you still think that composed is the right thing here for now? I think I understand the problem, but I'm not sure composed-everywhere is the solution, and I'm worried that the longer we leave this, the harder it will be to change.

(it's absolutely not a hill I want to die on, so I'm happy to defer to what you think)