New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Need "slotchange" event #288

Closed
dglazkov opened this Issue Jul 22, 2015 · 106 comments

Comments

Projects
None yet
@dglazkov
Contributor

dglazkov commented Jul 22, 2015

For developers, it will be good to have an event that is fired whenever slotting algorithm runs within a shadow tree. Maybe runs and produces changes compared to the previous result? Don't know. Also don't know if this is v1 or v2.

@dglazkov dglazkov added the shadow-dom label Jul 22, 2015

@annevk

This comment has been minimized.

Member

annevk commented Jul 22, 2015

I vote v2 or at least not the MVP we can ship when we're ready aligning with all decisions made thus far.

@hayatoito

This comment has been minimized.

Member

hayatoito commented Jul 22, 2015

For interoperability, should we finish the following task (mentioned in #128):

Enumerate all DOM operations that will trigger the slotting algorithm. This will be defined as a synchronous action.

to guarantee when such an event is fired?

I also vote v2. I'd like to be careful about exposing the timing of running the slotting algorithm to developers.

@sedwards2009

This comment has been minimized.

sedwards2009 commented Jul 23, 2015

In my web components I use a MutationObserver to watch for changes under the host node. It seems to do what you are asking.

@sorvell

This comment has been minimized.

sorvell commented Jul 23, 2015

MutationObserver doesn't work since a slot may be the child of an element with a shadowRoot. In this case, the child list of the element may not change but the elements that are rendered in place of the element's slot do change.

@sorvell

This comment has been minimized.

sorvell commented Jul 23, 2015

Having worked with Shadow DOM a lot, not having this feature in Chrome's existing implementation is very cumbersome. I'd really like to see this api included in v1 if possible.

If it's ever important to be able to manage your children (menu, tabs, stack of pages, carousel, etc), then you need to know when the elements distributed to your slots have changed.

MutationObservers are quite cumbersome to use for this: To use MO, you have to do something like: (1) MO your own childList, (2) whenever a slot is added to you, (3) find that slot's host element (recurse up the tree to find the shadowRoot.host) and MO that host's childList, and (4) repeat step 2 for that element. This is just to see childList changes. To get the actual distribution changes to a given slot, you have to observe attributes on every one of these children and then, perhaps, do a dirty checking scheme to detect what the changes are.

The platform has all this information as a result of distributing. For the user to do this correctly is both complicated and requires significant duplication of effort.

@annevk

This comment has been minimized.

Member

annevk commented Jul 23, 2015

@sorvell you'd need to propose a concrete processing model for this event and get buy-in from everyone. I'd really rather not put Shadow DOM back in a state where not everything has buy-in from everyone. Shipping across browsers is more important.

@sorvell

This comment has been minimized.

sorvell commented Jul 23, 2015

I definitely do not want to jeopardize buy-in. The momentum that Shadow DOM has right now is great.

I'm hoping this is a relatively easy addition, especially with the simplified slots model that has been adopted.

I think it could be a non-bubbling event fired when MutationObservers are. Perhaps it could just be an addition to MutationObservers, and therefore decoupled from the Shadow DOM spec.

@esprehn

This comment has been minimized.

esprehn commented Jul 23, 2015

Preferably this would just be an async notification that your distribution changed, perhaps even just MutationObserver can have a type of "distributedChildList" ?

@hayatoito

This comment has been minimized.

Member

hayatoito commented Oct 16, 2015

Let me add label v1 to this issue to get more attention.

@hayatoito hayatoito added the v1 label Oct 16, 2015

@rniwa

This comment has been minimized.

Contributor

rniwa commented Oct 16, 2015

This is about getting notified when the final distribution is changed or when assigned nodes are changed? If former, I'm opposed to it at least in v1.

@JanMiksovsky

This comment has been minimized.

JanMiksovsky commented Oct 17, 2015

Generally speaking, I'm excited to see Shadow DOM v1 ship sooner rather than later, even if it means living without the ability to detect changes in slot assignment as discussed here.

That said, I want to make clear that, without this feature, I believe it's prohibitively difficult to build components that can be composed as people expect. We would like people to be able to build custom elements that are every bit as reliable as the standard HTML elements, and have proposed a set of criteria for evaluating that level of quality called the Gold Standard checklist. One of the checklist items, Content Reprojection explicitly encourages component authors to properly handle reprojected content, even when that content changes. The example described on that page — counting the number of children in the final distribution — is trivial, but this issue has come up in practice time and again.

As a more practical example, the Basic Web Components project includes a <basic-carousel> component that implements a sophisticated carousel UI, with full keyboard support, screen reader support, touch/trackpad swiping, etc. People have tried to compose <basic-carousel> into their own components, with mixed results. Among other things, the carousel can show a set of dots along the bottom of the carousel, with one dot corresponding to each assigned node. (Demo) The component does an enormous amount of work — very much along the lines of @sorvell's solution using MutationObserver — but there are still cases the component doesn't handle properly. One result is that the number of dots can fail to update properly. So while this carousel can be useful as-is, it's quite brittle when composed into other components.

Again, I'm not saying Shadow DOM v1 should be held up for this feature. (I'd rather having something than nothing.) All I want to do is make plain the very real limitations of not having this feature.

@rniwa

This comment has been minimized.

Contributor

rniwa commented Oct 17, 2015

@JanMiksovsky : Do you want an event to detect when the "distributed" (i.e. after unwrapping / flattening slots) nodes are changed or when assigned nodes are changed? Also, do you want this event be synchronous or asynchronous?

@JanMiksovsky

This comment has been minimized.

JanMiksovsky commented Oct 19, 2015

As a component author, I want to be careful to focus on the scenarios I encounter while writing production components, and leave questions about the actual API design to browser engineers. With that in mind, I only want to call out the component requirements I see here that bear on this question.

In our basic-carousel example, the carousel wants to know how many children (e.g., img elements) it is showing, in order that it might show the correct number of dots. As I understand the distributed/assigned distinction, I believe that basic-carousel wants to know about changes to the set of nodes distributed to its default slot.

If basic-carousel only learned when the set of nodes assigned its default slot had changed, that would seem insufficient to support scenarios in which basic-carousel is composed into other components. For example, some users of basic-carousel want to compose a basic-carousel instance into a component of their own. Let’s call their new component outer-carousel. They create a default <slot> for outer-carousel and place that slot inside the composed basic-carousel instance. Here, the only node assigned to basic-carousel’s default slot would always be outer-carousel’s slot. If basic-carousel only hears about direct assignment changes to its own slot, it would never learn when a new image has been added to outer-carousel.

(If I’ve misunderstood things here, or there’s some other way in which basic-carousel could achieve its composability objectives, please let me know.)

Regarding timing, I don’t think basic-carousel presents any requirement for synchronous timing; asynchronous notification would be fine. Ideally, it would learn about distribution changes before the changes have rendered (so that there’s no visible lag between the appearance of a new image in the carousel and the appearance of a new dot for it). But beyond that, I don’t see any special timing needs.

@rniwa

This comment has been minimized.

Contributor

rniwa commented Nov 17, 2015

After discussing with @JanMiksovsky in person today, I'm pretty convinced that we need to add this event. I think the question is the timing (sync vs. async vs. end-of-mirotask/nanotask) as well as the name. I would like to stay away from the term "distribution" as much as possible.

@hayatoito

This comment has been minimized.

Member

hayatoito commented Nov 17, 2015

+1. I'm super positive to add this feature.

@hayatoito

This comment has been minimized.

Member

hayatoito commented Nov 17, 2015

I'm also very welcome any concrete proposal if someone can write it.

I need a help from an expert, if we are going to use MutationObserver.

@sorvell

This comment has been minimized.

sorvell commented Nov 17, 2015

It's great to hear 2 major vendors being open to this feature.

The Polymer library recently added support for this type of notification. It is a lot of code to manage and the performance degrades in complex cases due to the need to manage sets of Mutation Observers.

Polymer is fine with an asynchronous notification, ideally with the same basic behavior as Mutation Observers.

@rniwa

This comment has been minimized.

Contributor

rniwa commented Nov 19, 2015

If this follows MutationObserver timing, then it should probably be a new MutationRecord type.

@travisleithead

This comment has been minimized.

Member

travisleithead commented Nov 19, 2015

+1 for adding to MutationObserver records

@sorvell

This comment has been minimized.

sorvell commented Jan 28, 2016

This was discussed at the recent custom elements F2F meeting. I believe there was general agreement that this a critical feature needed for webcomponents 'v1' implementations and that

  • the timing should be consistent with MutationObservers (microtask)
  • it is ok if the notification does not include information about the changes to the distributed nodes list.
@rniwa

This comment has been minimized.

Contributor

rniwa commented Jan 28, 2016

Here's a concrete API proposal:

Add boolean slotAssignments = false to MutationObserverInit. We'll also add a mutation record of type slotAssignment.

On a mutation record of this type: target returns the slot element whose list of distributed nodes have been mutated; addedNodes and removedNodes always returns an empty array. previousSibling, nextSibling, attributeName, attributeNamespace, and oldValue all always return null.

We can add a new step to each DOM mutation such as the concept insert to queue a mutation record of type slotAssignment. This would result in each slot synchronously getting a new mutation record of type slotAssignment whenever a slot assignment has changed. This is observable if the mutation observer's takeRecords() is called. If there are multiple mutations that affect slot assignments, each such mutation queues a new record in this model.

Alternatively, we can store the list of distributed nodes for each slot element at the beginning of each micro task, and queue a mutation record if the list of nodes had changed at the end of the micro task.

@sorvell

This comment has been minimized.

sorvell commented Jan 29, 2016

Would the user facing api by like this?

new MutationObserver(function() {
  // react to slot changes...
}).observe(slotElement, {slotAssignment: true});

If so, this seems fine. Thanks!

@ajklein

This comment has been minimized.

ajklein commented Jan 29, 2016

Is there a reason not to just use an event for this? Creating a new MutationRecord type that contains none of the usual MutationRecord fields seems odd. The trickiest part of using an event is getting it to fire at microtask timing, but I suspect we already have enough spec machinery to do that.

@rniwa

This comment has been minimized.

Contributor

rniwa commented Jan 29, 2016

Yeah, now that I'm thinking about this more, it seems more natural to add an event on HTMLSlotElement whenever the distributed nodes change.

@hayatoito

This comment has been minimized.

Member

hayatoito commented Jan 29, 2016

I'm afraid that an event will be too spammy if we define the timing of "distributed node change" naively.

The trickiest part is how we can define the timing of "distribution nodes change" so that we do not have an interoperability issue.

In the current spec, UA can delay the calculation of distributed nodes until it is actually demanded as long as developers can not see the difference.

@rniwa

This comment has been minimized.

Contributor

rniwa commented Jan 29, 2016

We can just check that at the end of each micro task.

@hayatoito

This comment has been minimized.

Member

hayatoito commented Jan 29, 2016

That might be unacceptable because it could be super expensive.

To make the topic more understandable, suppose that if we had an imaginary "ComputedStyle Change" event, when should we dispatch such a "ComputedStyle change" event?

UA do not want to calculate ComputedStyle at the end of each micro task, I think.

Does someone have any idea? I am aware that supporting this feature request, "Slotting change event", is very important for developers. I need a workable solution.

@rniwa

This comment has been minimized.

Contributor

rniwa commented Jan 29, 2016

No, checking this condition is pretty easy & efficient since you can determine whether a node belongs to a given slot or not in O(1). Essentially, each shadow root needs to store the names of all slots its tree contains in a hash set/map. When its host' children changes, it can notify the shadow root that the slot assignments have changed for a specific name / default slot (keep repeating this process if the slot's parent also has a shadow root if you wanted to get notified of changes in distributed nodes).

Now, at the end of micro task, you visit all those shadow roots that have been notified of such potential changes and those that do have relevant event listeners. If the slot element had been inserted or removed from a given shadow root, you may need to resolve the first slot for each slot name in O(n). Otherwise, dispatch the event for each slot for which the shadow root had been notified of the change in its node assignments.

Having said that, I'm not opposed to running this at the end of the task (before painting essentially) although that might be painfully late for some use cases. Exposing style resolution timing is not acceptable for us.

@annevk

This comment has been minimized.

Member

annevk commented Apr 7, 2016

So you want to fire slotchange event when a slot element is newly inserted into a shadow root or removed from a shadow root? I'm not sure if that makes sense. The event I implemented in WebKit wouldn't get fired in those cases.

That makes sense. You want to perform slotting though so you know if the next insert/remove changes things.

Then, that could affect the distributed nodes of another slot element S2 to which S1 had been assigned. If there is such a slot S2, we'd check whether S2 is assigned to another slot for the same reason, and so on and so forth.

And all of them get an event, right? Thank you for mentioning that.

@rniwa

This comment has been minimized.

Contributor

rniwa commented Apr 7, 2016

So you want to fire slotchange event when a slot element is newly inserted into a shadow root or removed from a shadow root? I'm not sure if that makes sense. The event I implemented in WebKit wouldn't get fired in those cases.

That makes sense. You want to perform slotting though so you know if the next insert/remove changes things.

Right.

Then, that could affect the distributed nodes of another slot element S2 to which S1 had been assigned. If there is such a slot S2, we'd check whether S2 is assigned to another slot for the same reason, and so on and so forth.

And all of them get an event, right? Thank you for mentioning that.

Yup.

@hayatoito

This comment has been minimized.

Member

hayatoito commented Apr 11, 2016

For insert, it seems these two are significant:

"Insert" means this timing, https://dom.spec.whatwg.org/#concept-node-insert, right?
I am assuming that we do nothing at the timing of insertion steps (https://dom.spec.whatwg.org/#concept-node-insert-ext).

  • The node is being inserted into a parent node that also has a shadow root
  • An inclusive-descendant of node is a slot element and node's (new) root is a shadow root

It looks we should remove the condition of "node's (new) root is a shadow root" from there.

e.g.
The following might not be a practical example, but it could happen:

document tree:

<x-foo>
  <slot id=s1>
    <div id=inserted></div> <--- inserted
  </slot>
</x-foo>

x-foo' shadow tree:

<slot id=s2></slot>

#s2's "slotchange" should be fired if #inserted is inserted as a child of #s1.
#inserted's root is not a shadow root in this case.

In addition to insert and remove, we have to check the change of element's "slot" attribute and slot's "name" attribute.

Let me take another look later.

@annevk

This comment has been minimized.

Member

annevk commented Apr 15, 2016

@hayatoito "insertion steps" run at roughly the same time as "insert", they're part of the same atomic operation anyway.

@annevk

This comment has been minimized.

Member

annevk commented Apr 15, 2016

@hayatoito your example is a case I missed by the way, that would be a "node is inserted into a parent that is a slot element". If nothing is assigned to the node, that would be the fallback that will get assigned.

@annevk

This comment has been minimized.

Member

annevk commented Apr 15, 2016

Here is some pseudo-code I plan to adopt for the DOM Standard next week. Hopefully folks can take a look at it and hopefully it's understandable. I'm assuming that slotables have pointers to slots and slots hold a list of pointers to slotables and both need updating during mutations. Flattening is not calculated as that does not seem necessary. CSS and the assignedNodes() method can handle flattening by themselves.

insert

  The node is being inserted into a parent node that also has a shadow root
    -> let slot be findSlot(node)
    -> if slot is non-null:
      -> run assignSlotables(slot) // this is naive, but good enough for spec
      -> slotchange(slot)

  The node is being inserted into a parent node that is a slot
    -> assignSlotables(parent)
    -> if node's assigned slot is non-null, slotchange(node's assigned slot)

  An inclusive-descendant of node is a slot element
    -> for each _slot_ in node's tree // a new slot changes everything
      -> let current be _slot_'s assigned nodes
      -> assignSlotables(_slot)
      -> let new be _slot_'s assigned nodes
      -> if current != new and _slot_ was not just inserted, slotchange(_slot_)

remove

  Node has an assigned slot
    -> slotchange(node's assigned slot) // recursive
    -> run assignSlotables(node's assigned slot)
    -> Assert: node's assigned slot is null

  Node has a slot element as inclusive-descendant
    -> for each _slot_ in node's tree:
      -> let current be _slot_'s asigned nodes
      -> assignSlotables(_slot_)
      -> let new be _slot_'s assigned nodes
      -> if current != new, slotchange(_slot_)
    -> for each _slot_ in oldParent's tree // spec calls this parent
      -> let current be _slot_'s asigned nodes
      -> assignSlotables(_slot_)
      -> let new be _slot_'s assigned nodes
      -> if current != new, slotchange(_slot_)

slotable's slot attribute change
  -> if value == oldValue, return
  -> if node's assigned slot is non-null:
    -> assignSlotables(node's assigned slot)
    -> slotchange(node's assigned slot)
  -> Assert: node's assigned slot == null
  -> let slot be findSlot(node)
  -> if slot is non-null
    -> assignSlotables(slot)
    -> slotchange(slot)

slot's name attribute change
  -> for each slot element _slot_ in slot element's tree
    -> let current be _slot_'s asigned nodes
    -> assignSlotables(_slot)
    -> let new be _slot_'s assigned nodes
    -> if current != new, slotchange(_slot_)

Obviously there's a bit of copypasta here that can be (and will be in the spec) reduced with extra algorithms.

@hayatoito

This comment has been minimized.

Member

hayatoito commented Apr 18, 2016

Thanks. I have reviewed "insert" part. Let me review other parts later.

It looks that there is an assumption that we preserve the result of assgnSlottables(slot) somewhere.
In other words, before running each algorithms, we can assume that each slot's "assingned nodes" are always up-to-date, right?

insert

  The node is being inserted into a parent node that also has a shadow root
    -> let slot be findSlot(node)
    -> if slot is non-null:
      -> run assignSlotables(slot) // this is naive, but good enough for spec
      -> slotchange(slot)

How slotchange(slot) can be defined? I think that should be recursive, like:

function slotchanged(slot):
  enqueue_slotchange_event_if_it_is_not_enquened_for(slot);
  let slot2 = fildSlot(slot):
  if slot2 != null:
     slotchanged(slot2)

Does this match your expectation?

Ops. I am afraid that this slotchanged(slot) is still not enough because a slot might be a fallback content of another slot. That makes the situation complex. We need additional check:

function slotchanged(slot):
  enqueue_slotchange_event_if_it_is_not_enquened_for(slot);
  let slot2 = findSlot(slot):
  if (slot2 != null):
     slotchanged(slot2)
  else if (slot's parent is slot) and (slot's parent's assigned nodes are empty):
     slotchanged(slot's parent)

  The node is being inserted into a parent node that is a slot
    -> assignSlotables(parent)
    -> if node's assigned slot is non-null, slotchange(node's assigned slot)

The node is never assigned to the parent slot, according to the definition.
It can be a member of distributed nodes, as a fallback content, but it is not assigned to the parent, by definition. Thus, these should be:

  The node is being inserted into a parent node that is a slot
    -> if parent's assigned nodes are empty, slotchange(parent)

  An inclusive-descendant of node is a slot element
    -> for each _slot_ in node's tree // a new slot changes everything

This should be in tree order (which is implicit?) to make sure that assignSlottables(slot-A) is called before assignSlottables(slot-B-which-is-a-descendant-of-A) is called. That is required to handle the fallback contents correctly.

@hayatoito hayatoito closed this Apr 18, 2016

@hayatoito hayatoito reopened this Apr 18, 2016

@annevk

This comment has been minimized.

Member

annevk commented Apr 18, 2016

Yes. A slot has assigned nodes (a list of slotables). A slotable has an assigned slot ( a slot).

The assign slotables algorithm updates both those internal variables. (Therefore the node can end up being assigned to the parent when it is inserted into a slot. I don't see a reason to special case fallback content. It should just end up being assigned if there only is fallback content.)

The "slotchange" algorithm (going to call it signal a slot change most likely) is indeed recursive, but does not need "findSlot", it can simply use the assigned slot pointer for recursion.

@hayatoito

This comment has been minimized.

Member

hayatoito commented Apr 18, 2016

I don't see a reason to special case fallback content. It should just end up being assigned if there only is fallback content.

Yeah, I am feeling that "Not considering fallback contents as assigned nodes" is making the algorithms around here complicated. However, that's the conclusion in #317.

I am open to either option:

  1. Preserve the status quo. "assigned nodes" do not include the fallback contents. Instead, we might want to have a helper function for the spec so that we can write the algorithms easier.
  2. Let the definition of "assigned nodes" include the fallback contents too.

@rniwa , WDTY?
2 is a relatively huge change for the spec. I have to update a lot of places... :(

@annevk

This comment has been minimized.

Member

annevk commented Apr 18, 2016

Oh, I missed that assignedNodes() only includes fallback in the flattened case. Okay, then yes, I should make the changes as you suggested and review again. I don't think we need to change the current model.

@annevk

This comment has been minimized.

Member

annevk commented Apr 18, 2016

The fallback case means remove needs to run "slotchange(parent)" when a node is removed from a parent that is a slot with assigned nodes being empty.

@hayatoito

This comment has been minimized.

Member

hayatoito commented Apr 18, 2016

Yes, that's exactly same to what I am about to say for "remove part"

Remove:
  Node has a parent slot whose assigned nodes are empty
    -> slotchange(parent)
@annevk

This comment has been minimized.

Member

annevk commented Apr 18, 2016

By the way, let slot2 = findSlot(slot): in your algorithm earlier on for "slotchange" can just be let slot2 be slot's assigned slot. No need to run findSlot().

annevk added a commit to whatwg/dom that referenced this issue Apr 18, 2016

Shadow: define slotchange event
Fixes w3c/webcomponents#288.

This defines the slotchange event in response to remove/insert
operations, changes to a slot’s name attribute, and changes to an
element’s slot attribute.

This also introduces the assigned nodes concept for slots, and assigned
slot concept for slotables.

Slotables now also have a name, rather than a “get name”.

The slotchange event dispatches just after mutation observers.
@annevk

This comment has been minimized.

Member

annevk commented Apr 18, 2016

I posted a PR at whatwg/dom#229 that I'd appreciate feedback on. I think there's some potential for more cleanup to what I did and I'm going to study it more now, but it should meet all requirements as-is.

annevk added a commit to whatwg/dom that referenced this issue Apr 19, 2016

Shadow: define slotchange event
Shadow: define slotchange event

Fixes w3c/webcomponents#288.

This defines the slotchange event in response to remove/insert operations, changes to a slot’s name attribute, and changes to an element’s slot attribute.

This also introduces the assigned nodes concept for slots, and assigned slot concept for slotables.

Slotables now also have a name, rather than a “get name”.

The slotchange event dispatches just after mutation observers have their callbacks invoked. The slotchange event is not yet marked scoped as scoped events are not yet defined in the DOM Standard.

Note: although the original issue did not mention it, this also suppresses signaling when slots are removed from a tree. Not just when they are inserted.

dstockwell pushed a commit to dstockwell/chromium that referenced this issue Apr 22, 2016

yuzus Commit bot
Support slotchange event
This CL implements slotchange event,
which is added to the microtask queue whenever its assigned nodes
or its fallback nodes change.
In order to achieve it, this CL separates assignment, distribution and fallback
calculations in SlotAssignment.
Also note that slotchange event gets fired at most once per a microtask.

See the spec issue here: w3c/webcomponents#288
See the spec here: https://dom.spec.whatwg.org/#mutation-algorithms

BUG=595287

Review URL: https://codereview.chromium.org/1899653002

Cr-Commit-Position: refs/heads/master@{#389056}

tabatkins added a commit to tabatkins/dom that referenced this issue Apr 28, 2016

sync upstream (#1)
* Add more legacy event types for createEvent()

Approximately as requested at
https://bugzilla.mozilla.org/show_bug.cgi?id=1251198#c7. This is the
list of events supported in createEvent() by at least two of Firefox,
Chrome, and IE 11. The one exception is I omitted MutationEvent,
which all three support, because apparently spec-land has decided to
deny its existence in the hope that it will go away.

Bikeshed does not know about all of the added interface names,
hopefully that will sort itself out over time.

* Meta: improve pull request instructions for DOM

See whatwg/fetch#276 for context.

* Enable an event listener to be invoked just once

* Editorial: web compatibility typically remains relevant

Fixes whatwg#210.

* Shadow: define attachShadow() for custom elements

* Meta: make it easier to reference participate in a tree

PR: whatwg#216

* Define node document for new Text nodes

Fixes whatwg#224 and part of whatwg#212. Also fix part of whatwg#209 by stating these
algorithms can rethrow exceptions.

* Use a single concept for attribute changes

This setup is still a little sketchy I think, but not more so than the
insertion and removing steps.

* SVGEvent is only Gecko, Blink also has SVGEvents

As pointed out by zcorpan in whatwg#227.

* Set createDocument()'s content type based on namespace

Fixes whatwg#217.

PR: whatwg#218

* Make document.createEvent("touchevent") sometimes throw

Browsers typically disable touch events on non-touch devices, and there exists web content that detects this difference using document.createEvent().

Fixes whatwg#227.

* Shadow: define slotchange event

Shadow: define slotchange event

Fixes w3c/webcomponents#288.

This defines the slotchange event in response to remove/insert operations, changes to a slot’s name attribute, and changes to an element’s slot attribute.

This also introduces the assigned nodes concept for slots, and assigned slot concept for slotables.

Slotables now also have a name, rather than a “get name”.

The slotchange event dispatches just after mutation observers have their callbacks invoked. The slotchange event is not yet marked scoped as scoped events are not yet defined in the DOM Standard.

Note: although the original issue did not mention it, this also suppresses signaling when slots are removed from a tree. Not just when they are inserted.

* Editorial: update custom element cross-spec references

* Editorial: fix a few cross-linking missteps

* Editorial: make "is" and "prefix" optional in "create an element"

* Use "create an element" in createHTMLDocument

Takes care of part of whatwg#212.

* Editorial: align exception language with IDL

* Editorial: introduce more shadow-including terms for CSS

Fixes whatwg#225.

* Editorial: distributed -> flattened

* Meta: export more terms

Fixes whatwg#233.

* Editorial: add "shadow host" and "assigned" as terms

This makes a couple of non-null checks read better and enshrines a term
we had already been using.

* Editorial: flip non-null/otherwise conditions

PR: whatwg#234

* Shadow: <slot> is now defined in HTML

* Remove passive as event listener key

This changes makes passive no longer contribute to the uniqueness of
an event listener. It therefore also no longer needs to be supported
as part of removeEventListener().

Fixes WICG/EventListenerOptions#27.

PR: whatwg#236

* Meta: point out event's timeStamp is likely to change

See whatwg#23 for details.

* Add [CEReactions] annotations to mutating methods

Part of w3c/webcomponents#186, and furthering whatwg/html@27aa7bc.

Linking [CEREactions] will happen once tabatkins/bikeshed#677 is fixed.

* Editorial: check stop propagation flag at start of invoke

* Editorial: deduplicate Veli Şenol

* Editorial: define defaults for EventListenerOptions

Although this is also done in prose, this nonetheless simplifies the
prose a bit and makes it clearer to those skimming the standard what is
going on (although skimming is not recommended).

Fixes whatwg#239.

* Meta: link to Japanese translation

See
triple-underscore/triple-underscore.github.io#1
for more details.

dstockwell pushed a commit to dstockwell/chromium that referenced this issue May 27, 2016

Rewrite Shadow DOM distribution engine to support partial synchronous…
… distribution for v1

This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.

The summary of the changes is:

1. Make Blink be more spec compliant, regarding slotchange event.

2. Get significant performance improvements.

   Now SlotAssignment has |slotname|-to-|slot| HashMap, via DocumentOrderedMap.
   This HashMap is updated in each insertion/removal/renaming of slots.

   Though DOM Standard [2] requires synchronous calculation of slot assignments
   in each DOM mutation to fire a slotchagne event at the end of microtask,
   this CL does not calculate it synchronously, as an important optimization,
   if we can be sure to delay it. Instead, we do a minimum necessary check to
   detect a slotchange, using the characteristics of slot assignments.

   Especially, appending a child to a shadow host, which happens a lot in
   Polymer, becomes much faster.

3. Make HTMLSlotElement a much smaller footprint element.

   The following member variables are removed:
     m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes, m_oldFallbackNodes,
     m_distributionState and m_assignmentState.

4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
   already detected at early stage.

See the bug 610961 for details to know the basic ideas behind the scene.

The results of micro benchmarks are:

- PerformanceTests/ShadowDOM/v1-distribution.html => About 1.8x faster
  Time:
    Before: avg 3190.3 runs/s
    After:  avg 5885.6 runs/s

- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.0x faster
  Time:
    Before: avg  51.6 runs/s
    After:  avg 306.4 runs/s

- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.4x faster
  Time:
    Before: avg 1647.4 runs/s
    After:  avg 5645.0 runs/s

This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.

- PerformanceTests/ShadowDOM/v1-distribution.html
  JS Heap:
    Before: avg 21357790.4 bytes
    After:  avg  3136606.4 bytes

- PerformanceTests/ShadowDOM/v1-host-child-append.html
  JS Heap:
    Before: avg 5860745.6 bytes
    After:  avg 4165614.4 bytes

- PerformanceTests/ShadowDOM/v1-slot-append.html
  JS Heap:
    Before: avg 13256016 bytes
    After:  avg  3540172.8 bytes

Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107
- This CL also fixes the following bugs. These tests no longer fail.
  - crbug.com/610588
  - imported/wpt/shadow-dom/HTMLSlotElement-interface.html

The future works are:

1. There is a still false-positive case for slotchange event.

   e.g. A preceding slot is inserted together with a following slot.  This would
   not be a significant issue, but should be fixed. As of now, we must pay a
   performance penalty to remove this kind of false-positive.

2. Add more layout tests and add W3C Web Platform tests to be upstreamed.

3. Optimize the performance more and more.

    e.g. It might be possible to optimize
    HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
    structure.

4. Support SlotAssignment for non shadow trees. e.g. a document tree.

    This is a low-priority task because a document tree is unlikely to have a
    slot.

5. Isolate NeedsRecalDistribution flag for v0 and v1.

    Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
    to support both together in one page.

6. DocumentOrderedMap might not be optimized for our purpose.

    e.g. DocumentOrderedMap has to travers DOM Tree if conflicts happen.
    Just using Document::compareDocumentPosition() might be better, given that
    conflicts is unlikely to happen.

Links:

[1] slotchange event: w3c/webcomponents#288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
    4.2.2 Shadow tree  4.2.2.1 Slots
      4.2.2.2 Slotables
      4.2.2.3 Finding slots and slotables
      4.2.2.4 Assigning slotables and slots
      4.2.2.5 Signaling slot changes
    4.2.3 Mutation algorithms

BUG=531990,595287,610588,610961,579107

Review-Url: https://codereview.chromium.org/1995203002
Cr-Commit-Position: refs/heads/master@{#396443}
@trusktr

This comment has been minimized.

trusktr commented Nov 25, 2016

As an end-user, having a slotchange event feels against the grain of MutationObserver movement. Using MutationObserver would follow new and recommended patterns (not considering performance).

With the current slotchange event, I am running into the automatic requirement that I must now calculate myself which nodes were added or removed (which may be O(n)), but, otherwise, why on earth would I even care about the slotchange event? Why not use MutationObserver with an option to opt-in to distribution changes just like childList? Performance of an opt-in feature is O(1) when not opting-in (a conditional check).

@rniwa said,

I think the use case matters here. It looks like the all use cases listed here are about updating the UI / rendering to accommodate changes in the distributed nodes, and not so much about updating DOM / API surface for component users. So it's actually desirable for those updates to not take place until the end of the current task.

I do that myself by deferring my render logic to an animation frame. But, not all users may be adept enough to do that, and those users may not necessarily care about animation anyways. In fact, those users probably will never touch MutationObserver.

@annevk said,

If you do your own bookkeeping, you can replay. This is similar to attribute records without old values. But instead here we don't even provide a way to get to the old value (you have to store it yourself) and new value you have to get yourself too through a method.

What are the use cases where people want to listen to slotchange but don't want to see what the changes were in the distributed nodes? In my case (coincidentally?) I must detect which nodes were distributed or undistributed, otherwise slotchange isn't useful for me. For perspective, my lib is tracking the flat tree in order to render it to WebGL, so knowing which nodes are assigned/distributed is required.

@rniwa said,

With slotchange, only thing we can observe is that the list of distributed nodes have changed. It doesn't tell us which nodes are added or removed into/from where.

Because of this, I'm opposed to using mutation observers unless the list of nodes that got inserted and removed are also provided.

Yes, please. I myself at least don't have any other reason to listen to slotchange other than to get the list of nodes that got inserted and removed. Is it coincidence that the first time I need to use slotchange I also need to calculate which nodes were added/removed?

But @hayatoito and I both agree that doing so would be prohibitively expensive in both Blink and WebKit so I don't think this is an option.

If we use MutationObserver, with an opt-in option, how is that prohibitively more expensive than having to do it manually in JavaScript? It's not expensive at all if users don't opt-in similarly to childList, O(1).

@treshugart said,

We haven't come up with a use case warranting knowing the exact changes yet for a slot change, but we also haven't really used the event in anger yet.

For my lib to be able to keep track of the final flat tree (for rendering in WebGL), this is needed. When nodes are assigned into slots, they have new parents with respect to the WebGL render tree (i.e. they are transformed relative to their slot parent). My lib keeps track of two things: when nodes are "possibly distributed" in which case they are not rendered relative to their original parents, and when those same nodes are "assigned", in which case they are rendered relative to their slot parents. So, when adding a shadow root to a host, the host's children automatically become "possibly distributed" and will not be rendered unless they happen to be distributed to a slot, and the rendering happens relative to that new location.

For more perspective, I believe that if a library like http://aframe.io (which renders Custom HTML Elements to WebGL via Three.js) wants to be compatible with ShadowDOM, then that library will also have to know how nodes are distributed (i.e. keep track of the flat tree), and the most obvious way to do that is keeping track of nodes that are assigned/unassigned to slots. Using MutationObserver for this would be much cleaner than making us do it in user-land.

The problem is, we need some way to determine the final position of a distributed node. For example, if a slot S1 is distributed to a slot S2, which is distributed to S3, which is distributed to S4, then a node assigned to S1 would be finally distributed to S4 and would render relative to the parent of S4. This is what want to keep track of so that I can render to WebGL. If all the shadow trees are open, then this is no problem: I can check slot.assignedSlot.assignedSlot.assignedSlot.etc in a loop until I get null, in which case I'll be able to tell the final assignment of a node. But some trees are closed. Hijacking attachShadow can help work around the closed trees.

Besides slot.assignedNode(), what if there was something like slot.distributedNodes() to find the final distribution? It would return an array that contains only nodes that are rendered relative to that slot. In the above example, S1, S2, and S3 would all return empty arrays, while S4 would return an array with nodes which are rendered relative to that slot's parent.

insert

  The node is being inserted into a parent node that also has a shadow root
    -> let slot be findSlot(node)
    -> if slot is non-null:
      -> run assignSlotables(slot) // this is naive, but good enough for spec
      -> slotchange(slot)

Does that slotchange(slot) happen before the connected/disconnected events/callbacks of the node that is being inserted into the parent node? I think that since distribution is a less-likely event than connected/disconnected (i.e. all nodes have connected/disconnected events, but not all nodes are distributed), then it would be convenient for the slotchange event to fire first, which would make it easier to prevent some otherwise-normal connection/disconnection logic when the condition of distribution is detected, rather than canceling or reversing that logic after distribution is detected and it happens that both happened in the same tick synchronously.

@trusktr

This comment has been minimized.

trusktr commented Nov 27, 2016

Revised my previous comment.

I have a clearer picture after some testing. slot.assignedNodes() basically gives the literally assigned nodes on the context slot, while slot.assignedNodes({flatten: true}) gives the distributed nodes at the context slot. However, slot.assignedNodes({flatten: true}) returns nodes even if the nodes have been further distributed to a deeper node. This is similar to my previous slot.distributedNodes() idea, except that it doesn't return an empty array for the intermediate slots.

It might be nice to have a method (f.e. slot.distributedNodes() or slot.assignedNodes({distributed: true})) that when called on the slots S1 through S4 of my example will return distributed nodes only when called on S4. Does that make sense?

For example, here's a jsfiddle showing S1 through S4 assigned nodes flattened: https://jsfiddle.net/trusktr/9bs9mryt/

The output looks like this:

slot change on s1: [p]
slot change on s2: [p]
slot change on s3: [p]
slot change on s4: [p]

If we had a new method for getting strictly finally distributed nodes (slot.distributedNodes() or slot.assignedNodes({distributed: true})), the output would look like this:

slot change on s1: []
slot change on s2: []
slot change on s3: []
slot change on s4: [p]

where we see the final position of the <p> tag is at S4. If we were to unassign S3 from S4, then the output would be:

slot change on s1: []
slot change on s2: []
slot change on s3: [p]
slot change on s4: []

Get what I mean? I think that I believe that that would be useful for my case where I want to know the final positions where nodes are distributed.

I believe that I can currently achieve this by hijacking attachShadow so that I can always have shadow root references and by extension always have slot references, and then hack some way to find final distributions during slotchange events. Would love ideas on if there's some clean way to do it.

Made a new issue for the idea: #611

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment