21067 2013-02-21 02:38:00 +0000 [Shadow]: Provide an api to insertionParent 2013-08-07 08:51:16 +0000 1 1 1 Unclassified WebAppsWG HISTORICAL - Component Model unspecified PC All RESOLVED FIXED P2 normal --- 22715 1 sorvell dglazkov bugs dominicc erik.arvidsson esprehn hayato sorvell public-webapps-bugzilla oldest_to_newest 83468 0 sorvell 2013-02-21 02:38:00 +0000 Now that the shadowRoot element is exposed on a host element and insertion points have getDistributedNodes, it's possible to walk *down* the composed dom tree. However, it's not possible to walk *up* the composed dom tree. Since walking both directions is possible in normal dom, it should probably also be when using shadowDOM. This api is also one way to address this bug: https://www.w3.org/Bugs/Public/show_bug.cgi?id=21066. 84654 1 dglazkov 2013-03-19 19:13:09 +0000 Should it just be distributedParent? There must be better names. 84658 2 esprehn 2013-03-19 22:18:40 +0000 (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. 84659 3 bugs 2013-03-19 22:20:43 +0000 insertionParent? That is what XBL1 uses (internally). 84661 4 dglazkov 2013-03-19 22:23:20 +0000 (In reply to comment #3) > insertionParent? That is what XBL1 uses (internally). +1. 84764 5 hayato 2013-03-21 08:24:38 +0000 I think We should add the API to only Element and Text, rather than Node itself. 84813 6 dominicc 2013-03-22 00:43:34 +0000 Should it be available on ShadowRoot too? 84816 7 hayato 2013-03-22 02:22:55 +0000 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? 84817 8 esprehn 2013-03-22 02:29:23 +0000 (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). 84818 9 hayato 2013-03-22 02:36:16 +0000 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). 84822 10 hayato 2013-03-22 06:30:28 +0000 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'. 84823 11 hayato 2013-03-22 06:50:00 +0000 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'. 84825 12 bugs 2013-03-22 08:21:35 +0000 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. 84826 13 hayato 2013-03-22 08:28:13 +0000 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. 87793 14 hayato 2013-05-17 10:20:48 +0000 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. 87806 15 dglazkov 2013-05-17 16:37:58 +0000 (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). 87809 16 esprehn 2013-05-17 17:15:15 +0000 (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... 87814 17 sorvell 2013-05-17 17:29:32 +0000 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. 87863 18 sorvell 2013-05-18 01:27:39 +0000 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. 87900 19 hayato 2013-05-20 07:10:25 +0000 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. 88050 20 hayato 2013-05-22 05:46:16 +0000 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. 88077 21 sorvell 2013-05-22 17:18:11 +0000 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. 88114 22 hayato 2013-05-23 04:49:04 +0000 (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. 88252 23 hayato 2013-05-27 11:24:54 +0000 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. 91058 24 hayato 2013-07-22 04:57:09 +0000 FYI. webkitInsertionParent has been removed. https://chromiumcodereview.appspot.com/15757013 91442 25 hayato 2013-07-31 02:03:27 +0000 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. 91716 26 hayato 2013-08-07 05:50:59 +0000 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. 91717 27 hayato 2013-08-07 08:51:16 +0000 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.