Webcomponents: slotchange event should not bubble

[sorvell]

According to the spec, the slotchange event bubbles. The spec also says that slotchange should re-fire at a slot's assignedSlot. Both of these behaviors together result in some odd an unexpected behavior: if a slot is assigned to another slot, the inner slot will see a node being assigned twice. See the console output here: http://jsbin.com/jexabaf/edit?html,output.

I propose that the re-firing at assignedSlot is ok, but the bubbling behavior should be removed.

Note, Canary's current implementation appears to follow the spec (slotchange bubbles). Safari's implementation does not (slotchange does not bubble).

[treshugart]

Looking at the main issue (https://github.com/w3c/webcomponents/issues/288) around the slotchange event, all evidence seems to point to it being decided that it should _not_ bubble. I could have missed another issue where this was changed, though. It would seem Safari is correct here. cc @rniwa @hayatoito

[hayatoito]

Either is fine to me. I am okay to make it a non-bubble event in DOM Standard, and update the Blink's implementation.

I guess I and @annevk missed the rough consensus in the #288 when we defined a slotchange event in https://github.com/whatwg/dom/pull/229/. It has been a bubble event from the beginning in DOM Standard.

@annevk , are you okay to make it a non-bubble event? Was it intentional to make it a bubble event?

[annevk]

It was somewhat intentional so folks could make easier use of it with event delegation. I don't feel strongly though. @inscriber thoughts?

[rniwa]

It was somewhat intentional so folks could make easier use of it with event delegation. I don't feel strongly though.

I think the problem here is the slot is inside another slot because bubbling events can get into a shadow tree when its bubbling target is assigned to a slot.

For example, if a slot (A) outside a shadow tree (X) gets a new assignee, slotchange event fired on (A) will bubble into another shadow tree (Y) attached on an ancestor of (A) if there is another slot (B) in (Y). Then, we will fire the second slotchange event on (B). So if you have an event listener added on (B), you end up observing slotchange twice once for when it was fired on (A) and another for itself. You can get arbitrarily many duplicate slotchange events as you nest more shadow trees.

One solution for this problem is to make a single bubbling slotchange event fired only at the slot to which a node was assigned. Because this event will bubble, every slot which gets affected by this slot assignment will get notified. This has a nice feature that one can still observe all slot changes in a shadow tree at its ShadowRoot just like Blink's current implementation, and simplify the spec & implementation since we don't have to find "ancestor" slots.

[rniwa]

I don't have a strong preference (with a slight preference towards firing a single bubbling event) but making this a bubbling event and then firing on every slot that got affected is simply wrong. That just doesn't make any sense because of the duplication problem.

[hayatoito]

Blink has already shipped slotchange events. It might be difficult to change the behavior significantly. I think making it to non-bubble events is fine in Blink. The compatibility risk is low enough, I guess.

BTW, I'm curious that what's criteria in general to decide a bubble or non-bubble for an event? It looks there is always a use case where bubbling might be useful for web developers, I guess. Isn't it true?

Even if we adopt a bubble event here, users can ignore _non-essential_ events by checking "event.currentTarget != event.target" in their event listeners, if they want to, in general.

[rniwa]

@hayatoito: I would think that making the event not bubble would be a lot risker for you because event listeners on ancestors of slot will no longer receive slotchange event.

[hayatoito]

Yeah, thanks. I think I am aware of the meaning of this change. I will send a PSA to blink-dev if we decide to change it to a non-bubble event. I hope that's acceptable.

[hayatoito]

I do not have a strong opinion, but the workaround, checking "event.target != event.currentTarget" in event listeners, could be okay for web developers, I prefer the status quo.

@sorvell, WDYT?

[rniwa]

I don't think the status quo is an acceptable behavior. Getting multiple events for a single slot change would be extremely confusing for authors. I would object to keeping this event bubble and be fired on all the "ancestor" slots.

[bicknellr]

EDIT: Sorry, I didn't refresh before posting this, it might not be relevant anymore.

I think this might just be a Canary bug?

Here's the output I'm seeing:

Event {isTrusted: true, type: "slotchange", target: slot#slot, currentTarget: slot#slot, eventPhase: 2…} "bubbled" false
Event {isTrusted: true, type: "slotchange", target: slot#slot, currentTarget: slot#innerSlot, eventPhase: 3…} "bubbled" true
Event {isTrusted: true, type: "slotchange", target: slot#innerSlot, currentTarget: slot#innerSlot, eventPhase: 2…} "bubbled" false

(eventPhase: 2 is AT_TARGET, 3 is BUBBLING_PHASE)

I don't think the second event should happen in this situation:

• The a was assigned to #slot, so the first event is certainly meant to occur.
• The 'signal a slot change' algorithm also causes 'signal a slot change' to run on the slot's assigned slot (#slot's assigned slot is #innerSlot), so the third event makes sense.
• The second event - with target #slot, current target #innerSlot, and in the _bubbling_ phase - doesn't make sense either way its shown state is interpreted:
  
  
  
  • This can't be the (correct) bubbling phase of the first event, because these slots are in different ShadowRoots and the event is _not_ composed.
  
  
  • This can't be the (correct) result of 'signal a slot change' propagating 'slotchange' because it does not have target #innerSlot and is in the bubbling phase.
  
  

[annevk]

@smaug---- thoughts?

[hayatoito]

@bicknellr

The second event is correct. It is the bubbling phase of the first event. This behavior is intentionally designed. The definition of _composed_ event might be complex that you think, I guess.

The first event's event path will be: #slot => #innerSlot => #inner's shadow => #inner => #host's shadow.

Note: #host does not receive an event.

A _composed_ event does not mean the event is scoped in the _one_ node tree.
A _composed_ event means the event is scoped in (including) descendant node trees.

For example, that means:

• We don't exclude a #innerSlot from an event path because it is a shadow-including descendant of #slot's root.
• We exclude a host node from an event path because it is not a shadow-including descendant of #slot's root.

[hayatoito]

BTW, I am okay to make slotchange a non-bubble event, because my preference is not a strong preference. Either is fine to me. It looks most of us prefer a non-bubble slotchange event.

Note that if we update the example slightly so that _MID_ node would be inserted as the parent of #slot, the event path would be:

#slot => MID => #innerSlot => #inner's shadow => #inner => #host's shadow.

In this case, if we make a slotchange event a non-bubble event, #innerSlot does not receive a slotchange event which occurred on #slot, at bubbling phase.
I guess #innerSlot might want to be notified because #slot is a descendant of #innerSlot in the flat tree. I know that slotchange event is designed only for a _change of directly assigned children_, but we can know that change happens on _descendants_ via bubbling phase of slotchange events. There is no use case for that? (I am aware that we can use CAPTURE phase, but that is not popular, I guess)

[hayatoito]

Ah, I have a second thought on this issue.

@rniwa's proposal, https://github.com/w3c/webcomponents/issues/571#issuecomment-248266363
might make sense.

I would hear feedback from Polymer folks about this proposal. If that is okay, +1 for the proposal.

• Update the spec so that at most one slotchange event is dispatched on at most one slot element per one DOM mutation.
• Let slotchange events be bubbling events (the status quo)

I am assuming that, per the current spec, multiple slot elements on which a slotchange event happens, as a result of _one_ DOM mutation, are always _on the same event path_. If that is not correct, please correct me.

@sorvell
Does this cover use cases? If you are interested in only the change of directly assigned nodes (or change of re-distribution, including fallback contents) of a slot, this proposal would not make sense...

[bicknellr]

A composed event does not mean the event is scoped in the one node tree.
A composed event means the event is scoped in (including) descendant node trees.

Ok, yeah, I thought that composed meant the event was restricted to nodes in the same tree - thanks for the correction. In this case, I'd also support @rniwa's proposal in the comment you mentioned. If a single bubbling event already reaches all (and only) affected slots, then the additional propagation behavior in 'signal a slot change' seems redundant.

[hayatoito]

If a single bubbling event already reaches all (and only) affected slots, then the additional propagation behavior in 'signal a slot change' seems redundant.

Thanks. Yeah, that's exactly what I want to achieve here.
If we can agree on this basic idea, I'm fine to try to update DOM Standard, and update the Blink's implementation too. I hope no one will have a concern to change the behavior of slotchange events in Blink.

[bicknellr]

I think its worth mentioning that composed: false not meaning "this event does not propagate through the composed tree" is kind of odd. Is there a compelling use case against making composed: false limited to the tree in which it was dispatched?

One case that @sorvell brought up yesterday when I asked about this was the ability to determine which slot an event from a non-shadow child propagated through by merely listening to events on that slot. The counter-argument to this is that you could listen on the shadow-root's host and check the result of composedPath() for the slot.

Anecdotally, I think that the situations in which an event isn't either (1) meant only for the object controlling a the tree where the event was dispatched (i.e., a custom element dispatching events on itself to inform a component wrapping it) or (2) acceptably handled by any ancestor in the composed tree are rare.

If it's improper for an event to propagate to ancestor trees of the target's root because exposing that event is potentially 'unsafe' (event name conflicts, carries internal information), then why isn't it also improper for those events to propagate into descendant trees given the contents of those trees are similarly unknown and potentially unsafe for those same events?

EDIT: Relevance: if composed: false didn't propagate to other trees, the current definition of 'signal a slot change' would be preferable.

[rniwa]

If it's improper for an event to propagate to ancestor trees of the target's root because exposing that event is potentially 'unsafe' (event name conflicts, carries internal information), then why isn't it also improper for those events to propagate into descendant trees given the contents of those trees are similarly unknown and potentially unsafe for those same events?

The key difference is that the author of a component has a control over what gets used in its shadow tree. So if it uses another component with its own shadow tree, the outer component's shadow tree is fully aware of that fact.

[hayatoito]

If it's improper for an event to propagate to ancestor trees of the target's root because exposing that event is potentially 'unsafe' (event name conflicts, carries internal information), then why isn't it also improper for those events to propagate into descendant trees given the contents of those trees are similarly unknown and potentially unsafe for those same events?

I thought this problem a while ago, and I have chosen the current design after some thoughts. I am not sure I can explain what I thought in the past well, but let me try:

• For an ancestor tree, the internal structure of a descendant tree should not be a concern because it is the implementation detail of a component.
• For a slot in a descendant tree, which elements in the ancestor tree are slotted into it should be a concern. It is not the implementation details of other components. It should not be hidden.

In other words:

• A boundary of [shadowHost (ancestor tree) ===> shadow-root (descendant tree)] would be a separation of a concern.
• A boundary of [slot (descendant tree) ===> slotted elements (ancestor tree)] should be _open_. Slots should have a right to listen an event which happens on slotted elements.

That matches the rest of Shadow DOM's basic design, I think.

• [Access from an ancestor tree to a descendant tree]: should be banned, basically.
• [Access from a descendant tree to an ancestor tree]: should be okay.

[hayatoito]

I would like to move this issue forward.

Is everyone okay to change the spec as I explained in https://github.com/w3c/webcomponents/issues/571#issuecomment-248556202 ?

I have just roughly implemented the new behavior in Blink, locally, to confirm that the new behavior can cover all use cases.
As long as I can confirm using web platform tests, all use cases in web platform tests are fully satisfied.
https://github.com/w3c/web-platform-tests/blob/master/shadow-dom/slotchange.html

Given that, I am now 90% confident that we can do this change.
Since Blink has already shipped slotchange events, sooner is better, if we were in favor of the new behavior.

[rniwa]

Since Blink has already shipped slotchange events, sooner is better, if we were in favor of the new behavior.

WebKit has also shipped slotchange event, and our behaviors are incompatible at the moment so there shouldn't be too much compat risk involved given any code that depends on the current WebKit behavior or the current Blink behavior wouldn't function well on the other engine.

[annevk]

FWIW, I'm good either way I think. I haven't quite verified for myself yet that the proposed change has no bad side effects, but hope to do so as part of the DOM PR.

[hayatoito]

Thanks. Let me try to make a PR to DOM Standard.