Should it just be distributedParent? There must be better names.

(In reply to comment #1)
> Should it just be distributedParent? There must be better names.

I dunno, if the method on <shadow> is getDistributedNodes() then distributedParent seems correct unless we want to rename that method too.

insertionParent? That is what XBL1 uses (internally).

(In reply to comment #3)
> insertionParent? That is what XBL1 uses (internally).

+1.

I think We should add the API to only Element and Text, rather than Node itself.

Should it be available on ShadowRoot too?

I'd like to defer adding the API to ShadowRoot until we have a use case for that.

(In reply to comment #6)
> Should it be available on ShadowRoot too?

(In reply to comment #7)
> I'd like to defer adding the API to ShadowRoot until we have a use case for
> that.
> 
> (In reply to comment #6)
> > Should it be available on ShadowRoot too?

There is one though. If I'm walking up the tree and I run into shadow I need to know if I've been reprojected with a <shadow> to continue traversing upwards.

ShadowRoot should probably have both "host" and "insertionParent" (or whatever we're calling it).

Thank you for the use case.
Okay. Let me add the API to the ShadowRoot too.

(In reply to comment #8)
> (In reply to comment #7)
> > I'd like to defer adding the API to ShadowRoot until we have a use case for
> > that.
> > 
> > (In reply to comment #6)
> > > Should it be available on ShadowRoot too?
> 
> There is one though. If I'm walking up the tree and I run into shadow I need
> to know if I've been reprojected with a <shadow> to continue traversing
> upwards.
> 
> ShadowRoot should probably have both "host" and "insertionParent" (or
> whatever we're calling it).

As for ShadowRoot.insertionParent, let me raise an question.

Suppose a shadow host has two multiple shadow roots as follows:

  [older_shadowroot]
    - <div id=div1>

  [younger_shadowroot]
    - <shadow id=shadow1>


1. What should be 'div1.insertionParent'?

   I am sure that should be 'div1.insertionParent == shadow1', right?
   It's consistent with <content> element case.

2. What should be older_shadowroot.insertionParent?

   I am wondering that should be 'older_shadowroot.insertionParent == shadow1' or not.


I feel some strangeness for 'div1.insertionParent == older_shadowroot.insertionParent == shadow1'.

Note that we can not traverse every nodes on eventPath by using only insertionParent.

That means we cannot emulate event.path by insertionParent.

The reason is the reprojection. We can not traverse all intermediate insertion points, through which the node is reprojected. Since insertionParent is stateless.

To emulate event.path, we need an additional parameter, the original event target node, like:

element.parentOnEventPath(eventTarget)


(In reply to comment #10)
> As for ShadowRoot.insertionParent, let me raise an question.
> 
> Suppose a shadow host has two multiple shadow roots as follows:
> 
>   [older_shadowroot]
>     - <div id=div1>
> 
>   [younger_shadowroot]
>     - <shadow id=shadow1>
> 
> 
> 1. What should be 'div1.insertionParent'?
> 
>    I am sure that should be 'div1.insertionParent == shadow1', right?
>    It's consistent with <content> element case.
> 
> 2. What should be older_shadowroot.insertionParent?
> 
>    I am wondering that should be 'older_shadowroot.insertionParent ==
> shadow1' or not.
> 
> 
> I feel some strangeness for 'div1.insertionParent ==
> older_shadowroot.insertionParent == shadow1'.

For events XBL1 has .originalTarget but I really hope we don't add similar
thing for web components. As little as possible of the shadow DOM should be
exposed outside.

Agreed. I'd like to avoid adding API unless there is a strong use case for that.

(In reply to comment #12)
> For events XBL1 has .originalTarget but I really hope we don't add similar
> thing for web components. As little as possible of the shadow DOM should be
> exposed outside.

event.path() was landed in blink as described in
https://www.w3.org/Bugs/Public/show_bug.cgi?id=21066#c25.

Although we still have to spec event.path(), I'd like to know whether we still need insertionParent or not because event.path() might be sufficient.


(In reply to comment #13)
> Agreed. I'd like to avoid adding API unless there is a strong use case for
> that.
> 
> (In reply to comment #12)
> > For events XBL1 has .originalTarget but I really hope we don't add similar
> > thing for web components. As little as possible of the shadow DOM should be
> > exposed outside.

(In reply to comment #14)
> event.path() was landed in blink as described in
> https://www.w3.org/Bugs/Public/show_bug.cgi?id=21066#c25.
> 
> Although we still have to spec event.path(), I'd like to know whether we
> still need insertionParent or not because event.path() might be sufficient.
> 
> 
> (In reply to comment #13)
> > Agreed. I'd like to avoid adding API unless there is a strong use case for
> > that.
> > 
> > (In reply to comment #12)
> > > For events XBL1 has .originalTarget but I really hope we don't add similar
> > > thing for web components. As little as possible of the shadow DOM should be
> > > exposed outside.

Okay. I will wait to add insertionParent to the spec until webdevs (Polymer teams and others) have concluded that event.path is not enough.

If that happens, I think we need to go back to re-evaluate the opportunities for a better solution (and maybe even nix event.path in the process).

(In reply to comment #15)
> ...
> 
> Okay. I will wait to add insertionParent to the spec until webdevs (Polymer
> teams and others) have concluded that event.path is not enough.
> 
> If that happens, I think we need to go back to re-evaluate the opportunities
> for a better solution (and maybe even nix event.path in the process).

As a webdev, event path is not enough. It doesn't address Google Feedback's use case of wanting to walk up the ancestor chain to figure out containing blocks, event.path would require dispatching fake events on every node and is crazy expensive (and does rewriting and other magic).

It also doesn't allow positioning an object easily, since you have no trivial way to walk up the tree and decide who your positioned ancestor is without dispatching fake events...

event.path solves the use cases outlined in the bug where it was requested: https://www.w3.org/Bugs/Public/show_bug.cgi?id=21066. In that thread, it was noted that insertionParent does not provide enough information to construct the event.path.

If there is a general need to be able to construct the composed ancestor tree by traversing up from a given node, insertionParent is insufficient for that purpose for the same reason. See, for example, https://www.w3.org/Bugs/Public/show_bug.cgi?id=21066#c16.

It seems to me that reprojection makes insertionParent not a useful concept and we should remove it and address general 'traversing up the composed tree' via some alternate api, if necessary.

So, we can remove event path from the list of reasons we need an api to 'for a given node, find its ancestors in the composed tree'.

However, as Elliot points out, there are other reasons we need this information (see above). He also separately mentioned the need to discover if a node visible in an overflow scrolling block.

One option is that we could address each of these use cases independently and add api to address them, as we've done with event.path(). That seems impractical and that leaves us with needing the general api.

As previously mentioned, reprojection makes insertionParent insufficient for constructing the list we want; however, we probably do not need actual insertion points in the list of ancestors. Instead we can live with 'ancestors in the composed *render* tree'.

Given this, we can replace insertionParent with renderParent or composedParent and this would point to the parent of the last insertion point to which a node is distributed. This should be enough to generate the necessary list.

If we decide we do need insertion points in the list of ancestors, we could change insertionParent to insertionPath and this would be a list of insertion points to which the node is distributed.

Thank you the comments. Let me summarise the discussion so far:

- We need event.path() to address the use case described in https://www.w3.org/Bugs/Public/show_bug.cgi?id=21066

- We don't need insertionParent, which doesn't meet any requirement of any use cases. We can remove insertionParent.

- We still need another API to address the use case, 'Traversing up the composed tree'. The API's name might be either 'renderParent' or 'composedParent'. We might need a list of insertionPoints which are skipped.

If there is no objection to remove insertionoParent in a few days, let me remove the experimental support from blink. I am assuming no one is currently using that.

I'd like to provide an alternative which meets a requirement.
That should be renderParent (or composedParent?), which skips insertion points.

I am not sure whether renderParent is good name or not. Any suggestions?


(In reply to comment #19)
> Thank you the comments. Let me summarise the discussion so far:
> 
> - We need event.path() to address the use case described in
> https://www.w3.org/Bugs/Public/show_bug.cgi?id=21066
> 
> - We don't need insertionParent, which doesn't meet any requirement of any
> use cases. We can remove insertionParent.
> 
> - We still need another API to address the use case, 'Traversing up the
> composed tree'. The API's name might be either 'renderParent' or
> 'composedParent'. We might need a list of insertionPoints which are skipped.

I prefer composedParent over renderParent; however, I am wondering about the alternative of simply changing insertionParent to insertionPath. In this case, insertionPath would be the list of insertion points to which a node was distributed. Given this, I believe the full event.path could be constructed. Hayato-san, I respect your knowledge of this topic, do you agree about this?

While the event path tree is not necessary for the use cases in this thread, I'm not sure it's never necessary, and it seems odd to have event.path and traversal via 'composedParent' produce a different tree. I'd rather err on the side of providing more information.

(In reply to comment #21)
> I prefer composedParent over renderParent; however, I am wondering about the
> alternative of simply changing insertionParent to insertionPath. In this
> case, insertionPath would be the list of insertion points to which a node
> was distributed. Given this, I believe the full event.path could be
> constructed. Hayato-san, I respect your knowledge of this topic, do you
> agree about this?

Agreed. If we can return the list of insertion points, we can construct full event.path in theory.

At the same time, I'm afraid that might require some amount of code in the consumer side.
Let me emulate eventPath using insertionPath():

This is a pseudo code.

function eventPath(node) {
  if (!node)
     return [];
  var insertionPoints = node.insertionPath();
  if (insertionPoints.isEmpty) {
    if (node.isShadowRoot)
      if (node.toShadowRoot.isYoungestShadowRoot)
        return [node] ++ eventPath(node.shadowHost);
      else
        return [node];
    else
      return [node] ++ eventPath(node.parentNode));
  else {
    return [node] ++ insertionPoints ++ eventPath(insertionPoints.last.parentNode);
  }
}

... and I am now feeling there might be a bug in this eventPath function. I'm not sure. :)
The point is that it might not be easy to emulate eventPath in the client side.


> 
> While the event path tree is not necessary for the use cases in this thread,
> I'm not sure it's never necessary, and it seems odd to have event.path and
> traversal via 'composedParent' produce a different tree. I'd rather err on
> the side of providing more information.

Thank you for the info. I appreciate that. Let's continue discussion and iterate cycle until we have a good spec.

I think we need further discussion to define a good alternative APIs.

At least, looks like no one needs the current implementation of insertionParent. Let me remove it from the blink.

FYI. webkitInsertionParent has been removed.

https://chromiumcodereview.appspot.com/15757013

Let me try to spec the insertionPath and implement it in blink to get more feedbacks.
See the discussion in http://code.google.com/p/chromium/issues/detail?id=234031.

I feel that API should be named, 'getDestinationInsertionPoints()' because:

- The spec is using the terms of *distributed nodes* and *destination insertion points*.

  - https://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html#dfn-distributed-nodes
  - https://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html#dfn-destination-insertion-points

- We already have the API, getDistributedNodes().


I also feel that if the destination insertion points has an insertion point whose root node is a user-agent shadow root, we should discard the entire list and return an empty NodeList instead.

I've updated the spec.
https://dvcs.w3.org/hg/webcomponents/rev/77db857bafaa

Let me close this issue.
If you have any concerns, feel free to re-open this issue and add a comment.