Skip to content
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

Method for detecting finally-distributed nodes. #611

Closed
trusktr opened this issue Nov 27, 2016 · 35 comments
Closed

Method for detecting finally-distributed nodes. #611

trusktr opened this issue Nov 27, 2016 · 35 comments

Comments

@trusktr
Copy link
Contributor

trusktr commented Nov 27, 2016

(continuing from #288 (comment))

Currently, slot.assignedNodes({flatten: true}) doesn't tell the full story; it tells which nodes are possibly distributed at the given point (the slot) in a treeofnode trees, but it doesn't take into consideration the fact that nodes might be distributed even deeper into the tree of node trees if the context slot is assigned to a deeper slot; i.e. it does tell us if the returned nodes are actually distributed to that point.

It'd be nice to have a method distributedNodes() that answers the question: which nodes are distributed here, i.e. which nodes render relative to this point.

What about an Element.prototype.distributedNodes or HTMLSlotElement.prototype.distributedNodes property that returns children that are distributed to the context, and returns an empty array otherwise?

Which of the following would make more sense?

  1. slot.distributedNodes() which returns elements that are finally distributed and render relative tothe slot's parent. (i.e. the slot is not assigned anywhere else, so it is the furthest point where nodes are finally distributed).
  2. el.distributedNodes() which returns nodes that render relative to el (distributed via a child slot element of el).
@hayatoito
Copy link
Contributor

You can use slot.assignedSlot to know the slot is assigned to another slot or not.
https://dom.spec.whatwg.org/#dom-slotable-assignedslot

@trusktr
Copy link
Contributor Author

trusktr commented Nov 28, 2016

@hayatoito Not with closed trees.

In a tree of node trees where all the trees are mode open, it is possible to implement slot.distributedNodes() like so:

  • Let slot be the context object.
  • If (slot.assignedSlot)
    • return []
  • else
    • return slot.assignedNodes({flatten:true})

Basically, if the slot is assigned, then obviously nodes aren't finally distributed there. Only a final distribution point returns anything.

This doesn't work with closed trees because assignedSlot returns null even if the slot is assigned which means we can not tell if the nodes returned from slot.assignedNodes({flatten:true}) are actually rendered at that point in the tree of node trees; that's where I'm currently having trouble figuring out the least hacky way to accomplish this.

An official method for this would be nice because we wouldn't need to make a hack when dealing with the closed trees.

@hayatoito
Copy link
Contributor

hayatoito commented Nov 28, 2016

Basically, we will not provide any APIs which can tell us information about a structure of a closed tree from outside.

@trusktr
Copy link
Contributor Author

trusktr commented Nov 28, 2016

@hayatoito If a method like slot.distributedNodes() returns an empty array, it doesn't reveal the structure of the closed tree. The empty array result only means one of two possible things:

  1. Either no nodes were distributed from the parent light tree to the current tree
  2. or nodes have been distributed into a child tree.

This is no more revealing than slot.assignedNodes.

Having a way to get finally-distributed nodes is important for libraries like A-Frame if you wish for such libraries to promote ShadowDOM by being ShadowDOM-compatible. Libraries like that -- which render to WebGL or somewhere outside of DOM rendering -- need to be able to determine final-distribution of nodes in order to know how to render things.

If the platform doesn't have such a feature, then library authors (who are willing to take the time) will have to spend time coming up with a hack in order to achieve this with closed trees. Libraries will contain uglier code.

On the other hand, it would be nice for a library author to simply call something like slot.distributedNodes() to make life easy; and it doesn't reveal the closed tree structure.

Plus, as you know, ShadowDOM is not a security feature. It can be hacked to achieve this, but it's ugly that it has to be done.

I believe that for most cases -- for libraries that rely on DOM rendering -- this isn't an issue. But for libraries that need to know final-distribution (i.e. know flat the flat tree), closed trees are a temporary road block. Having something like slot.distributedNodes() is only going to make easier what we're already going to do as library authors.

Here is a list of libraries that (I believe) would benefit from such a feature if they want to be ShadowDOM-compatible:

@hayatoito
Copy link
Contributor

hayatoito commented Nov 28, 2016

  1. or nodes have been distributed into a child tree.

That is exactly what I thought as an example of leaking information; a closed shadow tree has a slot.
Such a leak is inconsistent with rest of APIs.

Suppose <video> elements in Blink, we never leaks an internal structure of <video> via any web-facing APIs.

@trusktr
Copy link
Contributor Author

trusktr commented Nov 28, 2016

@hayatoito

leaking information; a closed shadow tree has a slot.

  1. Is that really leaking any vital information? A shadow tree can accept my children. What can I do with that info? Can you prove why it is bad to know that a tree has a slot? What wrongdoing can be done with that info? The owner of the tree that contains the child shadow tree -- knowing that the child tree has a slot -- can not fool around with that child tree, except...
  2. Element.prototype.attachShadow can be hijacked to gain access to every single closed shadow tree in the window, so the structure of any shadow tree can be determined and manipulated.
  3. ShadowDOM is not a security feature.

Suppose

True, but <video> is native, and hijacking attachShadow doesn't work in that case. For now, I'm talking about Custom Elements.

I've been thinking about how to achieve this for a while now, at first with slottedCallback idea. I think slotchange is fine, and I can hack up what I want now, but it's a hack. The idea in this issue cleans it up a lot. slottedCallback however allows for more insight than the idea here.

If my library owns custom elements inside of each tree in a tree of shadow trees, then I can access slots in each tree, and on slotchange I can see which nodes are distributed across all the trees in order to determine their deepest positions. This is difficult compared to an official solution as simple as the algorithm above, but it will achieve what I want.

@trusktr
Copy link
Contributor Author

trusktr commented Nov 28, 2016

The most important point is this: library authors who write libraries that that expose custom elements for defining scene graphs that render in custom ways (f.e. to WebGL), and who wish for their libraries to be ShadowDOM-compatible, are going to find a way to do this the hard way or (if browser APIs allow) the easy way.

I'm simply suggesting that having the easy way would be really convenient. In the meantime, I'm going to do it the complicated (and as far as I know not pretty) way.

@hayatoito
Copy link
Contributor

hayatoito commented Nov 28, 2016

"Not leaking an internal structure" is a design goal for closed shadow trees from the beginning.
We have never agreed that we would change this goal. As long as the situation has not changed, we are unlikely to provide any API which are against the design goal. Such an API is simply inconsistent with the rest of the APIs, and should be considered as a bug in the platform.

If this design goal does not meet requirements, you can always use open shadow trees.

@trusktr
Copy link
Contributor Author

trusktr commented Nov 28, 2016

@hayatoito

Please use open shadow trees if this design goal does not meet requirements.

My library does not use ShadowDOM at the moment, and has no plans to use ShadowDOM. My library aims to be ShadowDOM-compatible. It is not I who is making a design decision on whether to use "open" or "closed" shadow trees. The end user of my library (or of any of the libraries mentioned above) can take any of those libraries and decide to use open or closed trees to distribute the elements as the end-user sees fit.

I can not possibly know if the end user will use open or closed trees, but any of the above libraries should work regardless of what the end user chooses, just like with native elements.

It would be great if you could solve the following challenge to experience what I mean:

Make three custom elements with the given attributes that will be used to render to a 2D canvas context:

  1. <x-scene width="" height="">
  2. <x-transform x="" y="">
  3. <x-circle radius="">

Then write the following markup which should (approximately) render a circle at position 80,80 with radius 10 in a canvas that is created and placed as a sibling to the x-scene element:

<x-scene>
  <x-transform x="20" y="50">
    <x-transform x="60" y="30">
      <x-circle radius="10">
      </x-circle>
    </x-transform>
  </x-transform>
</x-scene>

Once that is working, then write this markup:

<x-scene>
  <x-transform x="20" y="50">
    <x-circle radius="10">
    </x-circle>
  </x-transform>
</x-scene>

and then this JavaScript:

var t = document.querySelector('x-transform')
var r = t.attachShadow({mode:'closed'})
r.innerHTML = `
  <x-transform x="60" y="30">
    <slot></slot>
  </x-transform>
`

The results should be the same, but the second example will distribute the circle into the second transform.

That shadow root example is simple though. It is easy to get that working just with slot.assignedNodes({flatten:false}): the shadow root's <x-transform> element can call slot.assignedNodes() on the slot in order to determine what to render to the canvas, simple.

Now, imagine we have two layers of shadow roots. Write the following markup,

<x-scene>
  <x-circle radius="10">
  </x-circle>
</x-scene>

and the following javascript:

var s = document.querySelector('x-scene')
var r = s.attachShadow({mode:'closed'})

r.innerHTML = `
  <x-transform x="20" y="50">
    <slot></slot>
  </x-transform>
`

var t = r.querySelector('x-transform')
r = t.attachShadow({mode:'closed'})

r.innerHTML = `
  <x-transform x="60" y="30">
    <slot></slot>
  </x-transform>

Now, the slot element of the first shadow root gets automatically distributed to the slot of the second shadow root. This means that the x-circle element in the root tree gets "ultimately-distributed" (not assigned) to the slot in the second shadow tree.

The x-transform element in the first shadow tree cannot simply call slot.assignedNodes() anymore because it will be misleading information: the x-circle is not being rendered relative to that location anymore.

What is your solution to that, so that the x-circle is recognized to be rendered relative to the x-transform element in the second shadow tree?

@hayatoito
Copy link
Contributor

hayatoito commented Nov 28, 2016

I can not possibly know if the end user will use open or closed trees, but any of the above libraries should work regardless of what the end user chooses, just like with native elements.

You can tell users the limitation of your library; only open shadow roots are fully supported.
As far as I know, Polymer does it; Polymer does not support closed shadow roots.

If you have any further question, I think stackoverflow is a good place to discuss that.

@trusktr
Copy link
Contributor Author

trusktr commented Nov 28, 2016

You can tell users the limitation of your library; open shadow roots are only fully supported.
As far as I know, Polymer does it; Polymer does not support closed shadow roots.

Of course, I know I can just do that. I could also just tell users "we don't support ShadowDOM, don't use ShadowDOM with this library", just like A-Frame stated. Limitations are not ideal. People don't like limitations.

But that's not what I'm aiming for. I'm aiming for "This library is fully compatible with the latest awesome DOM standards like ShadowDOM, etc." and let them use ShadowDOM the way that they see fit and the way that it is publicly documented to work.

I want to promote ShadowDOM by fully supporting it in my library, which I am currently going to do by hacking a solution.

I'm not saying that what I want to do is not possible; it is possible, but it is not clean.

@trusktr
Copy link
Contributor Author

trusktr commented Nov 28, 2016

Not everyone always reads every piece of documentation. If I support only open shadow trees when browsers give an API for creating both open and closed trees, someone is going to try using the library with closed trees and it is going to cause headaches for the end user, and possibly headaches for the library author when the end user starts asking questions that wouldn't need to be asked if the library just worked with open or closed trees.

@trusktr
Copy link
Contributor Author

trusktr commented Nov 28, 2016

When I tried A-Frame as an end user, I was disappointed that I could not write Custom Element components with ShadowRoots. It means I can't create A-Frame components the way that I want to as an end user.

These sorts of limitations are what any custom-element library author should aim not to have.

I don't see what possible harm can come from knowing that "some child tree has a slot". It would be helpful if you can elaborate on that.

@trusktr
Copy link
Contributor Author

trusktr commented Nov 28, 2016

(not sure if you read emails first: I fixed typo in the last message, I meant "disappointed that I could not write")

@trusktr
Copy link
Contributor Author

trusktr commented Nov 30, 2016

@HAYATO, check this out. Custom Elements in my library observe slot elements in order to mark those elements' shadowParent (i.e. the parent of the slot they are distributed to), and adds distributed children to a shadowChildren list of the parent the slot. On slotchange of a slot, this code is ran:

        _handleDistributedChildren(slot) {
            const diff = this._getDistributedChildDifference(slot)

            for (const addedNode of diff.added) {

                // We do this because if the given slot is assigned to another
                // slot, then this logic will run again for the next slot on
                // that next slot's slotchange, so we remove the distributed
                // node from the previous shadowParent and add it to the next
                // one. If we don't do this, then the distributed node will
                // exist in multiple shadowChildren lists when there is a
                // a chain of assigned slots. For more info, see
                // https://github.com/w3c/webcomponents/issues/611
                if (addedNode._shadowParent)
                    addedNode._shadowParent._shadowChildren.delete(addedNode)

                addedNode._shadowParent = this
                this._shadowChildren.add(addedNode)
            }

            for (const removedNode of diff.removed) {
                removedNode._shadowParent = null
                this._shadowChildren.delete(removedNode)
            }
        }

where _getDistributedChildDifference returns an object containing added and removed properties which are lists of nodes that were distributed and undistributed, respectively, and looks like this:

        _getDistributedChildDifference(slot) {
            let previousNodes

            if (this._slotElementsAssignedNodes.has(slot))
                previousNodes = this._slotElementsAssignedNodes.get(slot)
            else
                previousNodes = []

            const newNodes = slot.assignedNodes({flatten: true})

            // save the newNodes to be used as the previousNodes for next time.
            this._slotElementsAssignedNodes.set(slot, newNodes)

            const diff = {
                removed: [],
            }

            for (let i=0, l=previousNodes.length; i<l; i+=1) {
                let oldNode = previousNodes[i]
                let newIndex = newNodes.indexOf(oldNode)

                // if it exists in the previousNodes but not the newNodes, then
                // the node was removed.
                if (!(newIndex >= 0)) {
                    diff.removed.push(oldNode)
                }

                // otherwise the node wasn't added or removed.
                else {
                    newNodes.splice(i, 1)
                }
            }

            // Remaining nodes in newNodes must have been added.
            diff.added = newNodes

            return diff
        }

When a chain of assigned slots exists (i.e. slot1 assigned to slot2, slot2 assigned to slot3, etc), then the that code will run multiple times, once per slotchange of each slot in the chain.

Because there's not a way to get finally-distributed nodes, I have to guard against the fact that a previous slot in the chain might have added the distributed node to shadowChildren of the previous slot's parent.

If there were a distributedNodes() method, then that defensive check would not be needed and the code would look like this:

        _handleDistributedChildren(slot) {
            const diff = this._getDistributedChildDifference(slot)

            for (const addedNode of diff.added) {
                addedNode._shadowParent = this
                this._shadowChildren.add(addedNode)
            }

            for (const removedNode of diff.removed) {
                removedNode._shadowParent = null
                this._shadowChildren.delete(removedNode)
            }
        }

furthermore, this logic wouldn't have to fire once per slot if there was an event that only fired on finally-distributed children. Right now, if there is a chain of 4 slots across 4 shadow tree, then this logic will fire four times from shallowest to deepest slot.

TLDR: I've gotten it to work, but it's ugly due to the fact that there's no possible way to know if the nodes returned from from slot.assignedNodes({flatten: true}) are finally-distributed to the current slot or not, so instead I am relying on the fact that the slotchange event will fire from shallowest to deepest slot.

Is that order ot slotchange events something I can rely on? Or is it just Chrome's behavior and could possibly be different in other browsers?

@rniwa
Copy link
Collaborator

rniwa commented Dec 5, 2016

I think changing the behavior of assignedNodes({flatten: true}) to return [] or null when the slot is assigned to another slot might make sense.

@travisleithead @annevk : any opinions here?

@rniwa rniwa reopened this Dec 5, 2016
@hayatoito
Copy link
Contributor

I think that is a breaking change since this feature was already shipped in Blink.
That also affects the behavior of ::slotted(), which was also shipped.

@rniwa
Copy link
Collaborator

rniwa commented Dec 6, 2016

I highly doubt that anyone is relying on this particular behavior. If this turns out be a breaking change, then we can add a new option.

@hayatoito
Copy link
Contributor

hayatoito commented Dec 6, 2016

Yeah, we should use a new option, if we want the new behavior. However, this new behavior still reveals the existence of slot elements in a closed shadow tree, indirectly, I think.

@trusktr
Copy link
Contributor Author

trusktr commented Dec 6, 2016

add a new option

I agree a new option may be better than breaking the current behavior.

I was thinking about it, and there are three possible cases that might be nice to satisfy, two of which are already supported, when elements pass through a slot chain:

  1. Supported: Run logic on elements specifically assigned to a slot, most likely at the beginning of a slot chain (using assignedNodes()). I'm not sure when I would face this case yet.
  2. Supported: Run logic on elements that pass through a slot regardless of whether they are further distributed down the chain or not (using assignedNodes({flatten: true})). I'm not sure when I would face this case yet.
  3. Not supported: Run logic on elements only if they are finally-distributed on the slot, at the end of a slot chain (using assignedNodes({someOtherOption: true}) and maybe require flatten:true as well?). This case I encounter the need-to-know in practice in order to avoid running logic along a whole slot chain, and instead want to run logic only on the end of a chain.

@trusktr
Copy link
Contributor Author

trusktr commented Dec 6, 2016

this new behavior still reveals the existence of slot elements in a closed shadow tree, indirectly

This is true. What are the specific downsides of this?

@rniwa
Copy link
Collaborator

rniwa commented Dec 6, 2016

Given you can already detect whether an element is assigned to a slot or not by checking a child element's bounding rect, etc... beyond the most simple case (e.g. shadow tree just contains a single default slot), I don't think there is any concern here.

@trusktr
Copy link
Contributor Author

trusktr commented Dec 6, 2016

You can already detect whether an element is assigned to a slot or not by checking a child element's bounding rect, etc

That seems rather complicated and indirect. What happens when 3D transforms are applied? Have an example?

@rniwa
Copy link
Collaborator

rniwa commented Dec 6, 2016

That seems rather complicated and indirect. Also, what happens when 3D transforms are applied?Have an example?

I'm suggesting that author do this. I'm saying that there is no concern to add this feature because one could already detect whether an element is assigned to some slot or not with a pretty decent accuracy.

@trusktr
Copy link
Contributor Author

trusktr commented Dec 6, 2016

Can you provide a code sample of this? If it is not 100% accurate, then it isn't viable.

@rniwa
Copy link
Collaborator

rniwa commented Dec 6, 2016

Can you provide a code sample of this? If it is not 100% accurate, then it isn't viable.

I think you're misunderstanding what I stated. Please go re-read what I wrote before making a further reply. All I'm saying is that we should add this new option because there is no new information leak.

@trusktr
Copy link
Contributor Author

trusktr commented Dec 6, 2016

@rniwa You said

you can already detect whether an element is assigned to a slot or not by checking a child element's bounding rect

I'm just asking if you have an example (or link) on how to do that. I would love to know!

@rniwa
Copy link
Collaborator

rniwa commented Dec 6, 2016

you can already detect whether an element is assigned to a slot or not by checking a child element's bounding rect

I'm just asking if you have an example (or link) on how to do that. I would love to know!

I know how to do it (heuristically) but I don’t want to derail the discussion here.

@trusktr
Copy link
Contributor Author

trusktr commented Dec 17, 2016

I know how to do it (heuristically) but I don’t want to derail the discussion here.

@rniwa If you email me that I would be super grateful.

@annevk
Copy link
Collaborator

annevk commented Sep 6, 2017

@rniwa I think a new feature like that would be better discussed in a focused issue. I tend to agree with @hayatoito that it's better not to violate the theoretical boundaries. I'm closing this issue as I don't think it'll lead us anywhere fruitful at this point.

@annevk annevk closed this as completed Sep 6, 2017
@trusktr
Copy link
Contributor Author

trusktr commented Sep 7, 2017

However, this new behavior still reveals the existence of slot elements in a closed shadow tree, indirectly, I think.

Also, this is possible now anyways, Shadow DOM isn't a security feature. I don't think this argument provides something that is more useful than the one proposed here.

@annevk
Copy link
Collaborator

annevk commented Sep 7, 2017

It's an encapsulation feature and preserving that encapsulation is good.

@trusktr
Copy link
Contributor Author

trusktr commented Sep 17, 2017

How does the HTML engine render the visuals? Does it traverse the composed tree to do that rendering?

@annevk
Copy link
Collaborator

annevk commented Sep 18, 2017

You mean layout or rendering engine? If so, yes.

@trusktr
Copy link
Contributor Author

trusktr commented Sep 19, 2017

Then this is proof that we need to do this in userland too, if we want to implement custom rendering. (for example, WebGL rendering).

I can currently do it, it's just not as easy as it could be.

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

No branches or pull requests

4 participants