This is a very good question. Since we explicitly say that the insertion points are not rendered (not just their contents), it's not obvious what the behavior should be when you add a shadow DOM subtree to an insertion point.

We need to fix this bug in spec.

My inclination is to spec it the way WebKit implements it now: you can certainly add a shadow DOM subtree to an insertion point, but you will never be able to see it, since the insertion point itself is never rendered.

WDYT?

That interpretation sounds good to me.

Is it worth specifying what happens when you have a <content> element outside of Shadow DOM (ie not an insertion point) with a ShadowRoot? And what happens when it gets moved inside the ShadowRoot?

+shinyak

As we discussed this with shinyak, we don't have any intentional explicit code which handles this case in current WebKit implementation. So just adapting the way WebKit implement is not appropriate.

I don't have any clear idea about which should be done in this case. IMO, my first interpretation is the best for us now if we don't have any alternative idea.

(In reply to comment #3)
> +shinyak
> 
> As we discussed this with shinyak, we don't have any intentional explicit code
> which handles this case in current WebKit implementation. So just adapting the
> way WebKit implement is not appropriate.
> 
> I don't have any clear idea about which should be done in this case. IMO, my
> first interpretation is the best for us now if we don't have any alternative
> idea.

Which one is the first idea?

It doesn’t matter if what WebKit does is deliberate or not… I believe it is just being an example of what the semantics should be.

I don’t like the suggested behavior in Comment 1 (if I understand it.) It seems inconsistent.

In the current WebKit implementation:
  1. A shadow root attached to InsertionPoint is simply ignored and not used at all. InsertionPoints still works as InsertionPoint.
  2. We don't throw any exception when attaching a shadowRoot to insertionPoints.

For 1). it sounds reasonable for me.
For 2). It might be worth discussing whether we should throw exception or not.

(In reply to comment #5)
> It doesn’t matter if what WebKit does is deliberate or not… I believe it is
> just being an example of what the semantics should be.
> 
> I don’t like the suggested behavior in Comment 1 (if I understand it.) It seems
> inconsistent.

Note that I've used a *active* InsertionPoint.
For inactive InsertionPoints, attaching a ShadowRoot does take effect.

(In reply to comment #6)
> In the current WebKit implementation:
>   1. A shadow root attached to InsertionPoint is simply ignored and not used at
> all. InsertionPoints still works as InsertionPoint.
>   2. We don't throw any exception when attaching a shadowRoot to
> insertionPoints.
> 
> For 1). it sounds reasonable for me.
> For 2). It might be worth discussing whether we should throw exception or not.
> 
> (In reply to comment #5)
> > It doesn’t matter if what WebKit does is deliberate or not… I believe it is
> > just being an example of what the semantics should be.
> > 
> > I don’t like the suggested behavior in Comment 1 (if I understand it.) It seems
> > inconsistent.

(In reply to comment #6)
> In the current WebKit implementation:
>   1. A shadow root attached to InsertionPoint is simply ignored and not used at
> all. InsertionPoints still works as InsertionPoint.

It's not ignored. Things just work exactly as expected. Since an active insertion point is not rendered, there's no way to render shadow dom subtree. It could totally be created and operated on. It's just never rendered.

>   2. We don't throw any exception when attaching a shadowRoot to
> insertionPoints.

Why would we throw an exception? It's similar to throwing an exception when you're trying to append a child to an element with style "display:none".

> 
> For 1). it sounds reasonable for me.
> For 2). It might be worth discussing whether we should throw exception or not.
> 
> (In reply to comment #5)
> > It doesn’t matter if what WebKit does is deliberate or not… I believe it is
> > just being an example of what the semantics should be.
> > 
> > I don’t like the suggested behavior in Comment 1 (if I understand it.) It seems
> > inconsistent.

I think we should not throw an exception, because we still have to deal with this case anyway:

<content id="a">
<div id="b">
<script>
var a = document.querySelector('#a');
var b = document.querySelector('#b');
var t = new ShadowRoot(a);
var u = new ShadowRoot(b);
a.parentNode.removeChild(a);
u.appendChild(a);
</script>

ie the ShadowRoot could be attached when the insertion point is inactive, and the insertion point could become active later.

It seems you guys gave me comment when I was writing the followings: :)

My suggestions are:

- For active InsertionPoints, attached ShadowRoots should not be used.
- For inactive InsertionPoint, attached ShadowRoots should take effect.
- It is okay to attach a ShadowRoot to either active or inactive InsertionPoints.
- It is okay to move a InsertionPoint inside the ShadowRoot or outside the
ShadowRoot. That might change its active/inactive status, but that should not
get rid of its attached ShadowRoots.


(In reply to comment #9)
> I think we should not throw an exception, because we still have to deal with
> this case anyway:
> 
> <content id="a">
> <div id="b">
> <script>
> var a = document.querySelector('#a');
> var b = document.querySelector('#b');
> var t = new ShadowRoot(a);
> var u = new ShadowRoot(b);
> a.parentNode.removeChild(a);
> u.appendChild(a);
> </script>
> 
> ie the ShadowRoot could be attached when the insertion point is inactive, and
> the insertion point could become active later.

So we don't have to change the current WebKit implementaton at all.

(In reply to comment #10)
> It seems you guys gave me comment when I was writing the followings: :)
> 
> My suggestions are:
> 
> - For active InsertionPoints, attached ShadowRoots should not be used.
> - For inactive InsertionPoint, attached ShadowRoots should take effect.
> - It is okay to attach a ShadowRoot to either active or inactive
> InsertionPoints.
> - It is okay to move a InsertionPoint inside the ShadowRoot or outside the
> ShadowRoot. That might change its active/inactive status, but that should not
> get rid of its attached ShadowRoots.
> 
> 
> (In reply to comment #9)
> > I think we should not throw an exception, because we still have to deal with
> > this case anyway:
> > 
> > <content id="a">
> > <div id="b">
> > <script>
> > var a = document.querySelector('#a');
> > var b = document.querySelector('#b');
> > var t = new ShadowRoot(a);
> > var u = new ShadowRoot(b);
> > a.parentNode.removeChild(a);
> > u.appendChild(a);
> > </script>
> > 
> > ie the ShadowRoot could be attached when the insertion point is inactive, and
> > the insertion point could become active later.

(In reply to comment #10)
> It seems you guys gave me comment when I was writing the followings: :)
> 
> My suggestions are:
> 
> - For active InsertionPoints, attached ShadowRoots should not be used.
> - For inactive InsertionPoint, attached ShadowRoots should take effect.
> - It is okay to attach a ShadowRoot to either active or inactive
> InsertionPoints.
> - It is okay to move a InsertionPoint inside the ShadowRoot or outside the
> ShadowRoot. That might change its active/inactive status, but that should not
> get rid of its attached ShadowRoots.
> 

The first bullet:
> For active InsertionPoints, attached ShadowRoots should not be used.
means:
  the ShadowRoot will not be rendered, 
  and the distribution algorithm for InsertionPoints will run,
right?

If so, I think these suggestions make sense.

Yes, 

> - For active InsertionPoints, attached ShadowRoots should not be used.

'Should' might be confusing, the correct sentence is:

For active InsertionPoints, distributed nodes (if exist) are rendered. The InsertionPoint element itself is not rendered. As a result, its attached shadow roots are not used.


(In reply to comment #12)
> (In reply to comment #10)
> > It seems you guys gave me comment when I was writing the followings: :)
> > 
> > My suggestions are:
> > 
> > - For active InsertionPoints, attached ShadowRoots should not be used.
> > - For inactive InsertionPoint, attached ShadowRoots should take effect.
> > - It is okay to attach a ShadowRoot to either active or inactive
> > InsertionPoints.
> > - It is okay to move a InsertionPoint inside the ShadowRoot or outside the
> > ShadowRoot. That might change its active/inactive status, but that should not
> > get rid of its attached ShadowRoots.
> > 
> 
> The first bullet:
> > For active InsertionPoints, attached ShadowRoots should not be used.
> means:
>   the ShadowRoot will not be rendered, 
>   and the distribution algorithm for InsertionPoints will run,
> right?
> 
> If so, I think these suggestions make sense.

I let this slip through. Sorry!

Post-discussion, it appears that no changes are necessary to the spec. Right? If I am wrong, please re-open.