. In the composed tree, the parent chain looks like 3, 2, 9, 6, 5, 1, I believe.
If I am following the algorithm at http://w3c.github.io/webcomponents/spec/shadow/#event-paths correctly, we start with the event path being [3, 2], then we see that the destination insertion points of
is not empty and that none of them are shadow insertion points, so we set the path to [3, 2, 7, 10](note that the order of the insertion points here is "rootmost first", unlike normal event path behavior).
Now we start off at the and start walking the parent chain. We add "9" to the path, then we add "8" to the path. Then CURRENT is a shadow root, so we skip to its shadow host, which is "6". We keep walking: "5", "4", skip to "1". So the final path is:
3,2,7,10,9,8,6,5,4,1
I have ignored for now issues dealing with multiple shadow trees and whatever scenario comment 13 is talking about.
I have confirmed that in the above simple testcase Chrome in fact has the event path I just described.
What I don't quite understand, if we posit that we do want to put all the insertion points in the path, is why the path is not the somewhat more intuitive:
3,2,10,9,8,7,6,5,4,1
?
comment: 51
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c51
Boris Zbarsky wrote on 2014-03-21 21:39:31 +0000.
Created attachment 1457
Simple testcase
comment: 52
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c52
Dimitri Glazkov wrote on 2014-03-21 22:19:31 +0000.
(In reply to Boris Zbarsky from comment #50)
Thanks for the test case -- I turned it into a jsbin:
http://jsbin.com/newir/1/edit
What I don't quite understand, if we posit that we do want to put all the
insertion points in the path, is why the path is not the somewhat more
intuitive:
3,2,10,9,8,7,6,5,4,1
Here's how the reasoning works. Since insertion points aren't in the composed tree, but are in event path, we have to come up with a mental model of how they are added.
Here's the mental model that the spec ascribes:
During distribution, first 2 is distributed into insertion point 7. So the path is now 3,2,7.
Then, the result of that distributed into 10, so the path is now 3,2,7,10.
In other words, the spec follows the distribution sequence and builds the path accordingly.
FWIW, I am fine for insertion points to not receive any events. I'll be sad about it for a day, and then I'll get over it.
comment: 53
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c53
Dimitri Glazkov wrote on 2014-03-21 22:40:38 +0000.
The desired property here is composition symmetry. If you group chunks of the path by trees (let's mark them as 1, 2, and 3), you get:
1:[3,2] 2:[7] 3:[10,9,8] 2:[6,5,4] 1:1
Notice how if I only had trees 2 and 3 (for example, I am an author of a component that created shadowroot#4), I have a view into the contiguous fragment of the same event path:
2:[7] 3:[10,9,8] 2:[6,5,4]
Similarly, if the main document turns out to be not a document at all, but just another shadow root, and we add insertion point in it, its inner event path will not change, only the outer parts of it will. Which means that the authors of components don't have to worry about how the composition affects them.
comment: 54
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c54
Dimitri Glazkov wrote on 2014-03-21 23:47:42 +0000.
(In reply to Olli Pettay from comment #47)
(In reply to Steve Orvell from comment #43)
Let's go back to this case in
https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c39 and review the
counter-argument.
<content select=".header"></content>
<content select=".content"></content>
Why? Add a listener to the shadow host? You can easily filter out events in the
listener, for example by using the content.getDistributedNodes().
Or you can add listeners to the distributed nodes themselves.
Neither option is acceptable.
If a listener is added to the shadowRoot, then each handler must start at
event.target and walk up the tree checking if the element is in the specific
content's list of distributed nodes. This has a non-trivial cost in code and
performance.
Making propagation path more complicated has also performance cost, and
makes event propagation inconsistent.
I agree on this point. The key here is deciding whether we should carry this burden as a platform or pass it on to developers.
Also, it is rather trivial cost in code, and makes code less unexpected,
since
propagation path doesn't have inconsistencies.
A good general exercise here is to assume that each shadow root chunk is written by different people at different times. As a rule, these developers should not need to have any insight into how these chunks are composed. In other words, if I as a developer have to defensively anticipate what will happen outside of my shadow tree, we failed the main objective of this whole shadow DOM exercise -- composition.
Having all insertion points included in the event path satisfies this constraint very well. As a developer, I have a consistent, simple way to find out what events are being dispatched in the parts of the outer tree that are distributed to my insertion points.
Having no insertion points included in the event path also satisfies this constraint, and it's pretty clear that our guinea pig developer (Steve) already worked around this problem by simply putting a div around the insertion point.
Having only final insertion point included in event path is craziness, because now I, as a developer of my component may or may not receive an event depending on how some other dude/dudette composed it in their app. That's composition fail.
So, to keep on moving, we should just zap the insertion points altogether from the event path.
comment: 55
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c55
Boris Zbarsky wrote on 2014-03-22 00:43:49 +0000.
I think the difference in mental model here is in whether insertion points get redistributed themselves or not. So where you came up with:
1:[3,2] 2:[7] 3:[10,9,8] 2:[6,5,4] 1:1
I had come up with:
1:[3,2] 3:[10,9,8] 2:[7,6,5,4] 1:1
based on the staying put when redistribution into 10 happens.
So in my mental model, you distribute stuff into an insertion point, then you make the insertion point go away, hoisting its kids up to be kids of its parent, then you look for shadow trees attached to that parent. But in your model it sounds like you distribute stuff into the insertion point, then you look for shadow trees attached to its parent and distribute the entire insertion point into those as needed, and then you hoist things out of insertion points. At least if I understand your model correctly
I think my model makes a lot more sense when thinking about what happens when 10 only accepts some of the kids of 6. At that point it becomes clear that 7 itself is not actually being distributed under 10...
comment: 56
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c56
Boris Zbarsky wrote on 2014-03-22 00:47:53 +0000.
I guess we're no longer using this stuff for CSS inheritance at least (if I read http://dev.w3.org/csswg/css-scoping/#inheritance right), so we can just worry about the event behavior...
comment: 57
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c57
Jonas Sicking wrote on 2014-03-22 04:17:52 +0000.
First off, I think we should look at this from the point of view of use cases.
What is the use case for adding an event listener to an insertion point? A user can't ever click an insertion point because they are not rendered.
However it does seem nice and intuitive if an event listener attached to an insertion point ends up receiving events targetted at any of the nodes inserted into that insertion point. As well as those nodes descendants of course.
And for the sake of consistency, this should always happen, no matter if there is shadow DOM attached to the parent of the insertion point or not. And no matter if there is shadow DOM attached to one of the nodes inserted into that insertion point.
Assuming that we can do this without creating large performance problems, and without creating too weird edge cases, do people agree that this is a laudable goal?
So I read comment 50 and also ended up with the same question as bz. Why not the more intuitive propagation path of 3,2,10,9,8,7,6,5,4,1?
So I started writing a comment why Dimitri's logic in comment 52 didn't make sense.
And instead writing the logic that would lead to bz's intuitive propagation path in comment 50 (3,2,10,9,8,7,6,5,4,1).
However I realized that I couldn't come up with a logical set of rules that would lead to that path.
While it seems to make sense to have that come before the , I couldn't motivate why that should be.
And, more importantly, it only seems to make sense if the '7' insertion point is an immediate child of the '6' shadow host. Let's insert an element between the '6' and the '7' nodes, such that we get the following tree:
In this case no node is inserted into multiple insertion points, which makes the whole thing much simpler. The '2' node is inserted into the '7' insertion point, and the 'a' node is inserted into the '10' insertion point.
In this case it's pretty clear that an event fired at the '3' node would get a propagation path of 3,2,7,a,10,9,8,6,5,4,1
So it makes sense to me that if you simply remove the 'a' and insert the '7' directly into the '6', that you'd get the same propagation path but with the 'a' removed. I.e. 3,2,7,a,10,9,8,6,5,4,1
I've read through the initial comments in this bug but I don't see the inconsistencies that are being discussed there. I.e. all of the following appears true:
- No node ever receives the same event twice.
- Any time an event reaches a node, it also reaches the node's parent.
- During the capturing phase, a node always receives an event before its children.
- During the bubbling phase, a node always receives an event after its children.
- The only way to receive an event multiple times in the same shadow tree is by
registering listeners on multiple nodes, where some of the nodes are ancestors of
other nodes.
All of the above appears to be true both both with and without the shadow DOM spec.
The only way that I can see that you could receive the same event multiple times is if you register event listeners in multiple different shadow trees. In that case it doesn't seem surprising that you can receive an event multiple times. It's something that happens even in the very simple event model that XBL1 uses.
Is the above wrong? Or is there some other inconsistency that I'm missing? Or is the concern about performance at this point?
Examples would be very helpful.
comment: 58
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c58
Jonas Sicking wrote on 2014-03-22 04:31:38 +0000.
For what it's worth, if I create a tree which illustrates how event propagation happens using the currently defined event path, and the example at 1 I get something like this:
1 http://w3c.github.io/webcomponents/spec/shadow/#distribution-results
comment: 59
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c59
Olli Pettay wrote on 2014-03-22 13:34:25 +0000.
(In reply to Jonas Sicking from comment #57)
Is the above wrong? Or is there some other inconsistency that I'm missing?
Or is the concern about performance at this point?
The inconsistency comes from the non-linear event propagation path in the
final flattened tree.
comment: 60
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c60
Olli Pettay wrote on 2014-03-22 14:18:27 +0000.
(In reply to Dimitri Glazkov from comment #54)
So, to keep on moving, we should just zap the insertion points altogether
from the event path.
This would be fine to me. A lot less surprising propagation path.
comment: 61
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c61
Boris Zbarsky wrote on 2014-03-22 15:01:14 +0000.
non-linear event propagation path in the final flattened tree.
I'm still thinking about Jonas' example, but do you mind explaining clearly the issue here? At least in my simple example, the nodes in the final flattened tree (so ignoring the insertion points and shadow roots, which are not present in the flattened tree) see the event in the order 3,2,9,6,5,1, which seems like the expected order...
comment: 62
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c62
Olli Pettay wrote on 2014-03-22 16:26:42 +0000.
Created attachment 1458
example
This shows a case when dispatching two events ends up creating
a diamond shape, because the first insertion point is cherry picked form the
flattened tree to the next item in the propagation path after the target.
Such behavior is really odd when we otherwise operate on trees and lists.
(And in my mind insertion points are there in the flattened tree. Otherwise events couldn't go through them at all.
But it is actually not a Shadow DOM terminology.)
DOM:
[click me 1]
[click me 2]
attached to sh1:
attached to sh2:
propagation paths are
- SPAN[id=s1], CONTENT[id=ip1], CONTENT[id=ip2.1], SPAN[id=sh2], DIV[id=sh1]
- SPAN[id=s2], CONTENT[id=ip1], CONTENT[id=ip2.2], SPAN[id=sh2], DIV[id=sh1]
comment: 63
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c63
Olli Pettay wrote on 2014-03-22 17:00:48 +0000.
In other words, atm union of two event paths is a tree.
You can rely on that if there is a target T in the path, the next
target T' in the path is always the same for event type foo.
comment: 64
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c64
Jonas Sicking wrote on 2014-03-22 17:45:04 +0000.
Ah, I think I see the issue indeed.
In Olli's example the "tree" that the DOM events code sees is this (I took the liberty of adding id's to the shadowroots for clarity):
[click me 1]
[click me 2]
Note that the node shows up twice!
I.e. if you fire an event on s1 the event path is
s1,ip1,ip2.1,sr2,sh2,sr1,sh1
And if you fire an event on s2 the event path is
s2,ip1,ip2.2,sr2,sh1,sr1,sh1
I.e. in both cases does the event go through ip1 (which is expected given that both s1 and s2 are inserted into ip1), but ip1's parent is different in the two events.
I'm curious, what does the spec say the event path is if you target an event on the ip1 node? Or on insertion points in general?
I still don't feel convinced either way of what the right thing to do here is.
I don't think including just the innermost insertion point is a good solution since it introduces other types of inconsistencies. I.e. it would mean that adding a shadow DOM to sh1 means that suddenly ip1 doesn't see any events. This will prevent any type of encapsulation.
And preventing any and all events to go through insertion points seems like breaking a useful feature.
comment: 65
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c65
Olli Pettay wrote on 2014-03-22 18:11:26 +0000.
It is probably best to not add any insertion points to the path.
You can add listener to the shadow host, or to the parent of the insertion point
or to the child nodes of shadow host or only to those child nodes which
are currently distributed to insertion point etc.
That gives already plenty of flexibility without need to change
event path creation significantly.
comment: 66
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c66
Dimitri Glazkov wrote on 2014-03-23 16:02:54 +0000.
I turned Olli's snippet into the jsbin here: http://jsbin.com/gakid/1/edit
I added the wrapper around ip2.*, and Steve's workaround works well there. It doesn't work as well around ip1, since that makes ip1 a grandchild of sh2. In that particular case, I can listen to the shadow root and get the same result...
I am almost sure that the wrapper workaround is a reasonable solution, but I need to play with this some more.
comment: 67
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c67
Hayato Ito wrote on 2014-03-24 04:50:29 +0000.
Thank you for all guys for comments for the event path.
I am feeling that I have to explain the background of the current event path calculation algorithms and the rationale of that.
I've designed the current event path algorithms so that we can achieve the following goal:
Objective A) In each node tree (forget other node trees), the event path should be a linear path.
That means:
- If a node on the event path has the parent node in the node tree, the parent node should also be on the event path. Only exception is the root node of a node tree.
- In each node tree, the child node must come before its parent node on the event path.
From the view of developers whose concern is only one node tree where they live, they see the behavior of the event path doesn't change at all as long as their concern is one node tree. They shouldn't care of other node trees.
The following is not the goal of the current algorithm:
Objective B) Define a reasonable total order between nodes which are in different node trees on the same event path.
I'd like to point out the following fact because I guess every one here doesn't notice this fact:
- Think about a tree of trees. Node trees that are involved in one even path can form a tree of node trees. Node trees can't be linear here.
I suggest you guys to think about this fact carefully. You can easily create this case by modifying the example in comment #50 as follows:
-
Make
a shadow host and let it host the following shadow tree.
In this case:
- Event path will be [3, 2, 7, 10, 9, 8, 6, 13, 12, 11, 5, 4, 1].
- There are 4 node trees which are involved in this event path.
- A node tree whose root node is #4 have two child node trees, a node tree whose root node is #8 and a node tree whose root node is #11.
It'd be difficult to define a reasonable total order between nodes in this case. Can you define a reasonable total order in this case?
DFS pre-oder? BFS-post-order? Neither can be a reason strong enough to abandon the current simple one-pass algorithm.
That's the reason why objective B) is not the goal.
The current algorithm uses one-pass algorithm and can achieve the objective A) at the same time.
I think simple one-pass algorithm is important here for the efficiency.
I'm not saying that we should include insertion points in the event path. That's orthogonal.
However, comment #63 can't be a strong reason not to include insertion points because the claim of 'union of two event paths is a tree' is not correct according to the current spec.
The current spec doesn't define one super node tree, which are composed of nodes which participate in a node tree in the same tree of trees.
The spec doesn't define ancestor/descendant relationships between nodes if they are in different node trees. That's intentional.
That means a tree used in 'union of two event paths is a tree' can't have a clear definition. Nodes in different node trees can't be a tree.
They live in separated worlds and should be considered as disconnected.
One more thing:
If we would exclude all insertion points from event path, we must have exceptional rule for insertion points which have fallback elements.
Example:
In this case, event path for "#n4" can't be [n4, n2, n1]. That should be [n4, n3, n2, n1]. We have to have an exceptional rule.
comment: 68
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c68
Hayato Ito wrote on 2014-03-24 05:49:48 +0000.
As I mentioned that in comment #9, I'd like to note the following again:
If we exclude all shadow roots and insertion points from the current event path, the event path will be equivalent with "inclusive ancestors of the target node in the composed tree" with the following exceptional cases:
- The target node itself is an insertion point.
- If fallback elements are distributed to a insertion point, we must include the insertion points to the event path so that we can achieve objective A).
comment: 69
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c69
Dimitri Glazkov wrote on 2014-03-25 16:16:52 +0000.
Thank you so much for explaining the design goals, Hayato-san!
It was illuminating for me. I'd forgotten about the fallback content in insertion points.
comment: 70
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c70
Jonas Sicking wrote on 2014-03-26 22:34:05 +0000.
I agree with comment 67 and comment 68. Order within a tree is much more important than order between nodes in different trees.
And in comment 50 bz also suggests an event path where the total order of nodes is arguably "non-sensical" since it jumps back and forth between two trees.
I'd even argue that the order of the non-insertion-point nodes makes a lot of sense as it's currently defined by spec since it matches rendering order. This means that if you click an icon inside a button inside a toolback that the event goes first to the icon, then the button then the toolbar as it's bubbling out. Even if the nodes are spread out over multiple shadow DOM trees.
I don't see a way to accomplish that if we also want to have a simple total order between nodes?
That said, it's still not clear to me what the right answer is on the topic of when/if to fire events on insertion points.
I do quite like the current feature that you can listen on an insertion point to get notified about all the events that are inserted in that insertion point. And it feels quite intuitive for authors that that is the case.
While you can generally wrap a around the insertion point and add your listeners there, that can affect rendering. For example if some of the nodes involved in the rendering are rendered as tables/tablecells/tablerows.
It also will affect nested insertion points if the parent of the insertion point that you are wrapping has a binding. Now it's the span that will get distributed rather than all the nodes that were inserted in the insertion point.
comment: 71
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c71
Dimitri Glazkov wrote on 2014-03-27 16:18:13 +0000.
(In reply to Jonas Sicking from comment #70)
I don't see a way to accomplish that if we also want to have a simple total
order between nodes?
That said, it's still not clear to me what the right answer is on the topic
of when/if to fire events on insertion points.
Maybe we can resolve this if we reduce the overall problem to this question: is introducing more complex total order in event path net positive or net negative?
Cons:
- increased complexity
- (please add more)
Pros:
- more intuitive for the consumer localized to any given tree
- useful to listen for events coming out of elements, distributed to insertion points
- fallback content is handled consistently
comment: 72
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c72
Olli Pettay wrote on 2014-03-27 17:19:28 +0000.
(In reply to Dimitri Glazkov from comment #71)
Cons:
- increased complexity
Slower event dispatch since event target chain creation becomes
more complicated because ... see the next point
- (please add more)
Inconsistent targeting from target T to T' in the event target chain,
which makes event dispatch less intuitive when one needs to think about the
whole event dispatch, not only local shadow tree.
And because of .stop*Propagation() for example one needs to think about the whole
path in some cases, not only local parts. Same with default handling. If you use in the
shadow trees and click somewhere, which link to follow? The first shadow tree
can affect to the behavior of the nested shadow trees.
Pros:
- more intuitive for the consumer localized to any given tree
- useful to listen for events coming out of elements, distributed to
insertion points
- fallback content is handled consistently
(I'm not saying pros aren't pros)
I guess XBL1 doesn't have this problem since it doesn't have explicit insertion points,
so event propagation follows always the natural path.
comment: 73
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c73
Jonas Sicking wrote on 2014-03-27 17:59:28 +0000.
XBL1 these days do have explicit insertion points. I.e. we leave the insertion points in the tree. The reason XBL1 doesn't have this problem simply because it optimizes simplicity over usefulness.
For example, if you have an XBL1 binding for a button that looks like
<-- xbl1 equivalent of shadowroot, sort of
And content like:
text here
then clicking on the icon or the text in the button does not fire any events in the XBL1 shadow content at all. However if you click a few pixels away so that you hit the borders added by the two divs now the event does fire on the shadow content.
From an author point of view this is arguably very inconsistent. Normally with the DOM, it doesn't matter if the user clicks on the border of an element, or on any of the elements inside it. This makes a lot of sense from a UI point of view too when it comes to things like click events.
However with XBL1 that model is broken and if you want to implement a UI widget that has insertion points you are basically out of luck when it comes to catching all UI actions inside your widget as I can tell.
You could overlay any inserted content with absolutely positioned elements, but that means that those events don't reach the content inside the insertion point that the user clicked which is really bad too.
So while XBL1 is simple implementation-wise, it seems to me that it creates a lot of complexity for authors.
Or am I misremembering the XBL1 event model?
comment: 74
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c74
Olli Pettay wrote on 2014-03-27 18:25:30 +0000.
(In reply to Jonas Sicking from comment #73)
XBL1 these days do have explicit insertion points.
Oh, right, the recent change to make anonymous content to look more like
shadow dom.
I'm talking about the classical xbl1 which Gecko had for 10+ years
then clicking on the icon or the text in the button does not fire any events
in the XBL1 shadow content at all.
Yes it does. Event propagates to the insertion point.
comment: 75
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c75
Boris Zbarsky wrote on 2014-03-27 19:07:16 +0000.
In XBL1, the event path is just along the composed tree.
comment: 76
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c76
Jonas Sicking wrote on 2014-03-27 19:38:24 +0000.
So then XBL1 also has exact same the complex weaving of nodes from different trees. I.e. the event path for "normal" (non-insertion-point, non-shadow-root) nodes is the same for XBL1 and shadow DOM.
So it sounds like we're back to that the remaining question to solve is if we should have insertion points in the event path or not? Or is there other things too?
I guess we could also argue over if the shadow-root should be in the event path or not, but I don't see any disadvantages of having it in the event path, and don't think I've seen anyone in here argue that it shouldn't be in the event path. Or did I miss that?
I agree that if you look at the event path across different events and across different disconnected trees, then keeping insertion points in the event path creates a more complex understanding.
However reasoning about the order of events within a single tree, even when looking across multiple different events, is still quite simple. I.e. it's no different from normal DOM event propagation (again, unless I'm missing something).
And if you look at the order of events across multiple trees, then you'll always see a somewhat complex view, even in XBL1. But it definitely gets more complex when you have insertion points in there too.
So the question is, does the ability to listen to events from nodes inserted into an insertion point warrant that extra complexity?
comment: 77
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c77
Olli Pettay wrote on 2014-03-27 20:25:44 +0000.
(In reply to Jonas Sicking from comment #76)
So then XBL1 also has exact same the complex weaving of nodes from different
trees.
No. In XBL if you have target T, the next target in the chain is always T'
comment: 78
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c78
Hayato Ito wrote on 2014-03-28 04:25:50 +0000.
(In reply to Jonas Sicking from comment #76)
I guess we could also argue over if the shadow-root should be in the event
path or not, but I don't see any disadvantages of having it in the event
path, and don't think I've seen anyone in here argue that it shouldn't be in
the event path. Or did I miss that?
Good point. I've thought it too.
My comment,
If we exclude all shadow roots and insertion points from the current event path, the event path will be equivalent with "inclusive ancestors of the target node in the composed tree.
implies that. :)
If we remove insertion points, we might want to remove shadow roots too from the event path.
I'm not saying that we should remove insertion points (and shadow roots) from the event path.
I still don't see a strong disadvantage of having these in the event path.
So the question is, does the ability to listen to events from nodes inserted
into an insertion point warrant that extra complexity?
I've seen 'extra complexity' of having insertion points in even path several times in this discussion.
However, I'd like to figure out a real issue what this complexity would cause.
From the experience of implementing the event path in blink, this didn't cause any extra burden for implementors.
Therefore, I'd like to know what is a real issue for web developers by having insertion points in the event path.
From the discussion, it looks the advantages outweigh the disadvantages, if exists, for web developers.
I'd like to note that the original motivation of including insertion points in the event path is we thought it's useful for web developers.
comment: 79
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c79
Hayato Ito wrote on 2014-03-28 04:32:46 +0000.
(In reply to Hayato Ito from comment #78)
From the experience of implementing the event path in blink, this didn't
cause any extra burden for implementors.
I'm not saying that this will apply to all user-agent's implementations.
I'd like to share my experience in blink, hoping it might be useful for the discussion.
comment: 80
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c80
Jonas Sicking wrote on 2014-03-28 05:52:39 +0000.
So then XBL1 also has exact same the complex weaving of nodes from different
trees.
No. In XBL if you have target T, the next target in the chain is always T'
I was referring to "normal" nodes here. Both XBL1 and Shadow DOM alternates "normal" nodes from different shadow trees in the event path. I thought this was one of the sources of complexity that you were worried about, but it appears that I misunderstood?
So it indeed then seems like the only contentious part here is weather to include insertion points in the event path or not.
comment: 81
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c81
Olli Pettay wrote on 2014-03-28 11:12:42 +0000.
(In reply to Hayato Ito from comment #79)
I'm not saying that this will apply to all user-agent's implementations.
I'd like to share my experience in blink, hoping it might be useful for the
discussion.
(IIRC when shadow dom event dispatch was added to webkit/blink, its
event dispatch speed regressed significantly. At least there was a time
when doing perf testing I noticed chrome's event dispatch speed regressed quite a bit)
It wouldn't be an implementation issue for Gecko, but perf would regress a bit.
comment: 82
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c82
Olli Pettay wrote on 2014-03-28 11:27:22 +0000.
The setup the spec has oddly prioritizes top most shadow trees over the
nested ones.
Using https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c62 as an example.
if p1.onclick = function(e) { e.stopPropagation(); }, then nothing
in the shadow tree 2 will see the event during bubble phase.
And since shadow tree 1 is there first also during capture phase,
p1.parentNode.addEventListener("click", function(e) { e.stopPropagation() }, true); would also prevent shadow tree 2 to see the event.
Why the shadow tree which has the final insertion has lower priority than
the first shadow tree?
And yet default handling doesn't have the same setup.
If you have elements in shadow tree 1 and in shadow tree 2, default
handling goes via the deepest most first, not the one in shadow tree 1.
(There are so many cases which just start to behave oddly when event path
doesn't follow the normal descendant->ancestor path)
comment: 83
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c83
Dimitri Glazkov wrote on 2014-03-28 18:13:01 +0000.
(In reply to Olli Pettay from comment #81)
(In reply to Hayato Ito from comment #79)
I'm not saying that this will apply to all user-agent's implementations.
I'd like to share my experience in blink, hoping it might be useful for the
discussion.
(IIRC when shadow dom event dispatch was added to webkit/blink, its
event dispatch speed regressed significantly. At least there was a time
when doing perf testing I noticed chrome's event dispatch speed regressed
quite a bit)
FWIW, this code has been extensively rewritten many times and we have not seen this particular codepath being a perf issue in real world.
I am not saying that we haven't done our share of poorly-written code when we first started exploring the problem, but the algorithm we settled on and what's in spec today is O(N). You shouldn't see any dramatic perf problems.
comment: 84
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c84
Boris Zbarsky wrote on 2014-03-28 18:57:18 +0000.
Just to note the obvious, O(N) is a necessary but not sufficient condition for non-sucky event dispatch performance. The constant matters a lot.
comment: 85
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c85
Dimitri Glazkov wrote on 2014-03-28 19:00:15 +0000.
(In reply to Boris Zbarsky from comment #84)
Just to note the obvious, O(N) is a necessary but not sufficient condition
for non-sucky event dispatch performance. The constant matters a lot.
Yup :) I similarly don't want us to speculate about performance problems without any actual data to back it up.
comment: 86
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c86
Olli Pettay wrote on 2014-03-28 19:20:15 +0000.
So, just to clarify, the complains I have are about the case when
we redistribute.
After chatting with sicking and still feel that
either we need to remove insertion points from the path
or do effectively comment 55.
In other words
Replace the "If the destination insertion points of CURRENT is not empty: .."
part with something like where you have the same list of insertion points as now, but if an insertion point foo has the same parent as what is a shadowroot's host, you put insertion point after that shadowroot in the event path.
comment: 87
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c87
Olli Pettay wrote on 2014-03-28 19:33:07 +0000.
(In reply to Olli Pettay from comment #86)
In other words
Replace the "If the destination insertion points of CURRENT is not empty: .."
part with something like where you have the same list of insertion points as
now, but if an insertion point foo has the same parent as what is a
shadowroot's host, you put insertion point after that shadowroot in the
event path.
Except that this can also lead to diamond shapes if you compare two paths.
(but otherwise this feels more logical than the current spec behavior)
comment: 88
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c88
Dimitri Glazkov wrote on 2014-03-28 19:41:52 +0000.
(In reply to Olli Pettay from comment #86)
In other words
Replace the "If the destination insertion points of CURRENT is not empty: .."
part with something like where you have the same list of insertion points as
now, but if an insertion point foo has the same parent as what is a
shadowroot's host, you put insertion point after that shadowroot in the
event path.
I thought Hayato provided a pretty good rationale in comment 67 for why this idea is not good? Did you have a chance to read that comment?
comment: 89
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c89
Olli Pettay wrote on 2014-03-28 20:06:13 +0000.
It full fills (A)
(well, if we don't want to put the shadowroot, we can just cut it from the path.)
So I don't see what the issue is.
And the path for the example would be
3,2,10,9,6,7,13,12,5,1
(assuming we don't put shadowroots there.)
comment: 90
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c90
Boris Zbarsky wrote on 2014-03-28 20:37:17 +0000.
We put together an etherpad to look at some examples, at https://etherpad.mozilla.org/RKukNtHHlO
I think my intuition, when formalized, would boil down to changing http://w3c.github.io/webcomponents/spec/shadow/#event-paths to the following:
- Let PATH be the empty ordered list of nodes
- Let CURRENT be NODE
- Add CURRENT to PATH
- Set INSERTION_POINTS to be the destination insertion points of NODE.
- If INSERTION_POINTS is not empty:
- Set INS to the last element of INSERTION_POINTS.
- Remove the last element from INSERTION_POINTS.
- Set CURRENT to INS.
- Add CURRENT to PATH.
- Repeat while CURRENT exists:
- If CURRENT is a shadow root:
- If NODE and CURRENT are in the same node tree and EVENT is one of the
events which must be stopped:
- Stop this algorithm
- If INSERTION_POINTS is not empty:
- Set INS to the last element of INSERTION_POINTS.
- While INS is a shadow insertion point:
- Remove the last element from INSERTION_POINTS.
- Add INS to PATH.
- Add the older shadow root relative to the root node of INS to
PATH.
- Remove the last element from INSERTION_POINTS.
- Add INS to PATH.
- Let CURRENT be the shadow host which hosts CURRENT
- Add CURRENT to PATH
- Otherwise:
- Let CURRENT be the parent node of CURRENT
- If CURRENT exists:
- Add CURRENT to PATH
Take the shadow insertion point bit with a grain of salt. For example, I don't understand why the spec has the "If SHADOW-ROOT is not the oldest shadow root" clause: it seems like if "destination insertion points" contains a shadow insertion point that means ipso facto that its root was not the oldest shadow root.... In any case, I modeled this above as basically the newer shadow tree being applied to the old shadow root, with distribution happening into shadow insertion points instead of normal insertion points. Hopefully that understanding of how multiple shadow trees and shadow insertion points work is correct.
Basically, the upshot is that insertion points here are kept with their parent in the dispatch path as opposed to being kept with their distributed nodes.
It's not obvious that this is any simpler spec-wise than what's in the spec right now (and in fact it looks a bit more complicated, though mostly because of the shadow handling). What do people think of the effects on the mental model for consumers of shadow DOM, though?
comment: 91
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c91
Jonas Sicking wrote on 2014-03-28 20:50:34 +0000.
I would be fine with this approach assuming that it doesn't cause any perf issues.
comment: 92
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c92
Dimitri Glazkov wrote on 2014-03-28 21:10:56 +0000.
(In reply to Boris Zbarsky from comment #90)
Basically, the upshot is that insertion points here are kept with their
parent in the dispatch path as opposed to being kept with their distributed
nodes.
I am still walking through examples, and it looks like the modification preserves the invariant I care about -- that inside of a given shadow tree, the author sees a consistent even path -- is preserved. Both spec and this proposal result in a matryoshka nesting, which is good.
comment: 93
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c93
Olli Pettay wrote on 2014-03-28 21:16:06 +0000.
(In reply to Jonas Sicking from comment #91)
I would be fine with this approach assuming that it doesn't cause any perf
issues.
It shouldn't be any slower than what is in the spec.
(removing insertion points from the path would be a bit faster).
This approach is fine to me. It has the diamond issue as I said, but
at least it would make event propagate in the final tree in somewhat expected order.
comment: 94
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c94
Dimitri Glazkov wrote on 2014-03-28 23:39:49 +0000.
(In reply to Olli Pettay from comment #82)
The setup the spec has oddly prioritizes top most shadow trees over the
nested ones.
Using https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c62 as an example.
if p1.onclick = function(e) { e.stopPropagation(); }, then nothing
in the shadow tree 2 will see the event during bubble phase.
And since shadow tree 1 is there first also during capture phase,
p1.parentNode.addEventListener("click", function(e) { e.stopPropagation() },
true); would also prevent shadow tree 2 to see the event.
Why the shadow tree which has the final insertion has lower priority than
the first shadow tree?
I tried to apply this thinking to both the spec'd and newly proposed event paths and I am stuck. Please help.
Let's take example 2 from https://etherpad.mozilla.org/RKukNtHHlO.
Suppose that I am an author of sh1 and sh2 is actually some component that I found on the internet. I have no idea how it works. The author of this component, however, calls e.stopPropagation() in ip2. Let's say I don't know this. That's what composition is about, right? :)
In spec'd event path, I see this the resulting event path as [x1, ip1]. Which sort of makes sense -- I clicked on x1, and I should at least hear something coming out of ip1, because I know that x1 is distributed there.
In new proposal, I see [x1] only. That seems bad. Suddenly, even though x1 is distributed into my ip1, some force of nature grabbed the emergency brake and my event just disappeared.
Am I crazy? At least to me, the spec'd event path actually makes sense, whereas the new proposal results in mysterious silence.
comment: 95
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c95
Jonas Sicking wrote on 2014-03-28 23:43:31 +0000.
Dimitri, isn't the same thing true in the current spec if you are the author of sh2 and sh1 is something you found on the internet?
comment: 96
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c96
Dimitri Glazkov wrote on 2014-03-28 23:44:39 +0000.
(In reply to Jonas Sicking from comment #95)
Dimitri, isn't the same thing true in the current spec if you are the author
of sh2 and sh1 is something you found on the internet?
That doesn't work that way :) I can't find something that uses me. Only the other way round.
comment: 97
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c97
Boris Zbarsky wrote on 2014-03-29 01:36:44 +0000.
I can't find something that uses me.
You can, because of capturing listeners. :(
If in example 2 a capturing listener on ip2 or sr2 calls stopPropagation, the author of sh1 will see the event hit capturing listeners on ip1 but not hit any listeners at all on x1.
In fact, even in the spec's setup calling stopPropagation in a capturing listener on ip2 will mean that sr1 gets the event in the capture phase but then ip1 and x1 never see it.
The only sane way to avoid composition issues with stopPropagation is to make stopPropagation tree-local or something... or something. Speaking of complexity.
comment: 98
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c98
Boris Zbarsky wrote on 2014-03-29 01:39:44 +0000.
And even with bubbling listeners, in the spec setup if ip2 calls stopPropagation in the bubble phase, you will see the event during the bubble phase at ip1 but not at sh2 or sr1... Basically, stopPropagation is the mind-killer. It is the little-death that brings total obliteration of composition. :(
comment: 99
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c99
Hayato Ito wrote on 2014-03-31 08:02:48 +0000.
The following has been on my radar for a long time:
- Whether calling event.stopPropagation() should affect the event dispatching in other node trees or not.
AFAIR, I think this is the first time when this topic is being discussed in the Shadow DOM spec bugzilla.
In the current spec, calling event.stopPropagation() in a node tree affects the event dispatching in other node trees, as you notice.
In the past, I thought that we shouldn't make event.stopPropagation() affect the behavior of event dispatching in node trees other than the current node tree. I thought that an event dispatching in each node tree must be completely independent. However, no one has taken care of this topic so far, except for me.
This is the typical situation where it's difficult to satisfy both worlds - the tree of trees world and the composed tree world.
I prefer the current rule. Calling an event.stopPropagation stops an entire event dispatching, including event dispatching in other node trees. It's simple and easy to implement and might be useful for some situations.
However it's worth discussing. I think we haven't have enough opinions from web developers about this topic.
comment: 100
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c100
Olli Pettay wrote on 2014-04-11 13:46:05 +0000.
Hayato, Dimitri, I'd like to understand what are the issues in the approach
explained rather clearly in comment 90.
As far as I see it makes event path more logical, and consistent when comparing to event propagation in DOM in genenal, than the current spec
since the top most shadow dom isn't handled in a special way, and yet
it keeps event handling within all the shadow doms consistent.
event.stopPropagation() should have the same behavior it has now in non-shadow-DOM. During capture phase things closer to the root node have
priority over those closer to the target, and in case of bubble phase vice-versa.
The spec doesn't current give that behavior , but the approach from comment 90
does.
comment: 101
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c101
Hayato Ito wrote on 2014-04-11 14:26:41 +0000.
Okay. I've taken a look at the algorithm in comment #90 closely.
I think the algorithm in comment #90 doesn't work as intended because it doesn't consider the case where a distribution happens at more than one node in the event path.
In the while loop of "Repeat while CURRENT exists:", CURRENT can have non-empty destination insertion points. In that case, we should consider the destination insertion points of CURRENT again as the algorithm does it at step 4 and 5.
Therefore, INSERTION_POINTS can't be a global variable. I guess we need a stack of INSERTION_POINTS or something like.
Although I've not tried to fix the algorithm, the algorithm might become more complex than that of #comment 90 to make ti work-as-intended.
comment: 102
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c102
Boris Zbarsky wrote on 2014-04-11 15:51:18 +0000.
Good catch. You're correct that we'd need to make INSERTION_POINTS a stack of lists, pushing onto it when we hit a CURRENT with nonempty insertion points and popping off it when we remove the last item of the currently-top list.
comment: 103
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c103
Boris Zbarsky wrote on 2014-04-11 15:53:26 +0000.
Though actually, maybe that's not the right ordering. I'll need to sit down with some concrete cases to see what the right behavior here is.
I agree that it will make the algorithm more complicated. :(
comment: 104
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c104
Hayato Ito wrote on 2014-04-11 21:24:02 +0000.
(In reply to comment #103)
Though actually, maybe that's not the right ordering. I'll need to sit down
with some concrete cases to see what the right behavior here is.
I agree that it will make the algorithm more complicated. :(
Yeah, I guess it might be tough task to define the algorithm nicely so that the relative order of insertion points in the same destination insertion points gets reversed in the event path.
That requires a kind of a non-trivial technique and I'm afraid that makes the algorithm complicated one.
comment: 105
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c105
Hayato Ito wrote on 2014-04-21 09:10:24 +0000.
Let me announce that I've just added a non-normative section, 'Event Paths Example', to the spec, hoping that it would be helpful for everyone to understand the current spec and the problem space we must handle.
http://w3c.github.io/webcomponents/spec/shadow/#event-paths-example
I think it is worth while to share a non-trivial example used in the section so that we don't miss a non-trivial case further.
comment: 106
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c106
Hayato Ito wrote on 2014-05-09 08:24:58 +0000.
I've tried to examine whether an alternative idea proposed in this discussion work well for the example 1 or not. However, I am afraid that a proposal is not well described so I can't figure out how a proposal work for the example.
I appreciate if someone would help me to make it clear how an alternative algorithm work for the example.
Let me close this issue if no one could have an alternative algorithm which works well for the example.
comment: 107
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c107
Olli Pettay wrote on 2014-05-09 09:14:17 +0000.
Just a note that the general consensus during the f2f (if I managed to follow the discussion correctly remotely - I did miss some of it) seemed to be that the setup in the spec isn't what is wanted, but the alternative more
normal child-parent propagation.
I'll try to write the algorithm still during this weekend.
As far as I see, at least currently, it just needs to be recursive so that we incorporate all the shadow trees in the path.
comment: 108
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c108
Hayato Ito wrote on 2014-05-09 10:03:31 +0000.
Thank you. I appreciate that.
(In reply to Olli Pettay from comment #107)
Just a note that the general consensus during the f2f (if I managed to
follow the discussion correctly remotely - I did miss some of it) seemed to
be that the setup in the spec isn't what is wanted, but the alternative more
normal child-parent propagation.
Yep, I thought that. However once I tried to make it a concrete algorithm, I realized that there are a lot of unclear points. Therefore I need your help to understand the alternative.
Let's solve this long discussion together.
comment: 109
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c109
Hayato Ito wrote on 2014-08-25 03:28:47 +0000.
I am wondering whether we can close this bug or not so that we can focus higher priority bugs.
We can reopen this bug anytime later if we feel we need a further discussion.
comment: 110
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c110
Olli Pettay wrote on 2014-08-25 13:47:52 +0000.
We're not going to implement the event handling currently in the spec.
And the WG agreed that it is not the right solution.
There is now a WIP algorithm
https://bugzilla.mozilla.org/show_bug.cgi?id=887541#c13
comment: 111
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c111
Hayato Ito wrote on 2014-08-26 04:05:09 +0000.
Thank you for letting me know that you have WIP algorithm.
I am strongly interested in the result of the WIP algorithm if we apply that to the example tree of trees used in '5.3 Event Paths Example' 1.
I am curious what is difference between the current one and WIP's one. Then, I would like to know which is better.
If WIP algorithm is ready to be applied, please let me know that.
1 http://w3c.github.io/webcomponents/spec/shadow/#event-paths-example
comment: 112
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c112
Olli Pettay wrote on 2014-08-28 13:03:41 +0000.
The example is hard to read. Showing the final flattened tree next to it would be useful.
comment: 113
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c113
Olli Pettay wrote on 2014-08-28 13:29:36 +0000.
[D, C, I, M, L, P, R, Q, O, N, K, J, H, G, U, T, S, F, E, X, W, V, B, A]
becomes
[D, C, M, L, R, Q, O, P, N, K, J, I, H, G, U, T, S, F, E, X, W, V, B, A](still verifying. I guess I should write a test)
The main thing is that 'I' is in the more natural place in the chain.
comment: 114
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c114
Olli Pettay wrote on 2014-08-28 19:59:13 +0000.
Looks like the wip algorithm needs still small tweak to cover event propagation
N->K-J-I-H
comment: 115
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c115
Olli Pettay wrote on 2014-08-28 21:37:20 +0000.
https://bugzilla.mozilla.org/show_bug.cgi?id=1059989#c2 does some tweaking
and the result would be
D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,E,X,W,V,B,A
comment: 116
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c116
Hayato Ito wrote on 2014-08-29 02:49:13 +0000.
Olli, thank you for the explanation.
Now I can say I understand the intention clearly. It's great to design the concrete algorithm. I've enjoyed reading and running the WIP algorithm in my head. It seems promising.
Aa far as I understand, the result between the current one and WIP one differ only if a re-distribution happens.
If re-distribution occurs, they put non-final destination insertion points in different places.
I don't have a strong opinions about which is better because both satisfy the main goal which I explained in '5.3 Event Paths Example':
http://w3c.github.io/webcomponents/spec/shadow/#event-paths-example
That means if we focus on one node tree and forget all other node trees, the event path would be seen as if the event happened only on the node tree which we are focused on. This is an important aspect in a sense that hosting shadow trees doesn't have any effect to the event path within the node tree the shadow host participate in as long as the event is not stopped somewhere in the descendant trees.
For example, from the view of the document tree 1, the event path would be seen as [D, C, B, A]. From the view of the shadow tree 2, the event path would be seen as [I, H, G, F, E]. The similar things also apply to other node trees.
It is also worth pointing out that if we exclude all insertion points and shadow roots from an event path, the result would be equivalent to the inclusive ancestors of the node on which the event is dispatched, in the composed tree.
From the view of difficulty in implementing, I don't have any concerns because both algorithm seem simple enough. That's great.
I am not sure how many developers will notice the difference between them and how important the difference is.
I'd like to hear opinions from developers about what use cases would tell the difference between them and when the difference matters.
comment: 117
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c117
Jonas Sicking wrote on 2014-08-29 02:57:28 +0000.
Yes, my understanding is that there is only a difference when redistribution happens. And the difference is only in where insertion points are located. All non-insertion-point elements will always have the same relative order in both algorithms.
comment: 118
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c118
Olli Pettay wrote on 2014-09-19 11:17:45 +0000.
Hayato, have you had chance to look at
the proposed algorithm in https://bugzilla.mozilla.org/show_bug.cgi?id=1059989
comment: 119
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c119
Hayato Ito wrote on 2014-09-22 08:46:31 +0000.
No. It seems the algorithm has been updated after I checked last time.
Let me take a look. Thank you for letting me know that.
Hayato, have you had chance to look at
the proposed algorithm in
https://bugzilla.mozilla.org/show_bug.cgi?id=1059989
comment: 120
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c120
Hayato Ito wrote on 2014-09-30 06:23:10 +0000.
I've taken a look at the algorithm in https://bugzilla.mozilla.org/show_bug.cgi?id=1059989#c10.
It looks the algorithm became more complex than before. That made it difficult for me to audit the correctness, however, it seems it will work for the example case, 5.3 Event Paths Example, as intended at least.
One suggestion: Could you avoid to use a term of projected? The projected is not defined in the spec.
comment: 121
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c121
Olli Pettay wrote on 2014-09-30 09:32:40 +0000.
Sure. Could it be just OLDER-SHADOW-ROOT?
comment: 122
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c122
Hayato Ito wrote on 2014-10-01 06:36:50 +0000.
(In reply to Olli Pettay from comment #121)
Sure. Could it be just OLDER-SHADOW-ROOT?
I meant we might want to update the following sentence in the algorithm, as an example:
Let SHADOW-POOL-HOST be the shadow insertion point into which CURRENT shadow root is projected.
Because the spec doesn't define what 'projected' means for a shadow root, I think we cay say, instead, if we want to make it more explicit:
Let SHADOW-POOL-HOST be the shadow insertion point in the younger shadow tree relative to the shadow tree whose root is CURRENT shadow root.
We can update other sentences where projected is used, in a similar way.
BTW, I don't have much worry about optimizing (and correcting?) the proposed algorithm as long as the result will be an equivalent because now I understand the intention of the proposal and it's feasibility.
Rather, I'd like to hear early feedbacks from developers, especially polymer developers and users, to know use cases where the difference of resulting event path between the current one and the proposed one matters. I have no idea when it matters.
I appreciate any feedbacks and opinions so that we can decide.
comment: 123
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c123
Olli Pettay wrote on 2014-10-07 19:39:03 +0000.
(In reply to Hayato Ito from comment #122)
Rather, I'd like to hear early feedbacks from developers, especially polymer
developers and users, to know use cases where the difference of resulting
event path between the current one and the proposed one matters. I have no
idea when it matters.
Have you asked Polymer devs, or hopefully other devs too? (we should not focus too much on feedback
from one script library only.) Any feedback yet?
I appreciate any feedbacks and opinions so that we can decide.
(I could also remind what was discussed during the last Webapps f2f ;) )
comment: 124
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c124
Hayato Ito wrote on 2014-10-08 05:56:09 +0000.
I'm afraid that this discussion thread became too long and it would be hard for new comers to follow the entire discussion.
Let me summarize the current situation here, hoping it would make it easy for developers to compare both and give us feedbacks easily.
There are two proposals about how event path should be.
It would be a little hard for me to explain each briefly. Therefore, let me use an concrete example to explain the difference between them here.
Let's use an example of http://w3c.github.io/webcomponents/spec/shadow/#event-paths-example.
(Sorry about that this example is a bit complex, but it is necessary to know the difference)
- The current one:
See http://w3c.github.io/webcomponents/spec/shadow/#dfn-event-path-calculation-algorithm for details.
The result event path for the example will be (as the spec explains):
[D,C,I,M,L,P,R,Q,O,N,K,J,H,G,U,T,S,F,E,X,W,V,B,A]
- The new one
See https://bugzilla.mozilla.org/show_bug.cgi?id=1059989 for details.
The result event path for the example will be:
[D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,E,X,W,V,B,A]
Some notes:
Both satisfy the original goal which the spec explained as follows:
That means if we focus on one node tree and forget all other node trees, the event path would be seen as if the event happened only on the node tree which we are focused on. This is an important aspect in a sense that hosting shadow trees doesn't have any effect to the event path within the node tree the shadow host participate in as long as the event is not stopped somewhere in the descendant trees.
For example, from the view of the document tree 1, the event path would be seen as [D, C, B, A]. From the view of the shadow tree 2, the event path would be seen as [I, H, G, F, E]. The similar things also apply to other node trees.
It is also worth pointing out that if we exclude all insertion points and shadow roots from an event path, the result would be equivalent to the inclusive ancestors of the node on which the event is dispatched, in the composed tree.
I don't worry about the difficulty of implementation in both. I think there is no significant difference between them in a term of time complexity in a sense of big-O notation.
Please feel free to add additional information.
comment: 125
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c125
Dimitri Glazkov wrote on 2014-10-08 14:07:10 +0000.
With my developer hat only, my experience is that this mostly does not matter. This is one of those platform changes that platform developers agonize over and web developers never notice.
I think we should do make this change and move on.
comment: 126
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c126
Olli Pettay wrote on 2014-10-08 14:25:55 +0000.
(In reply to Dimitri Glazkov from comment #125)
With my developer hat only, my experience is that this mostly does not
matter.
This would be my guess too
This is one of those platform changes that platform developers
agonize over and web developers never notice.
Well, web developers do notice the inconsistencies in API behavior eventually.
Inconsistencies in APIs tend to show up later, often when it is too late
to fix them.
("load" event propagation as an example - now we need to have a special event propagation path for one specific event type. It complicates the platform and
people tend to not remember the special case.
But it was too late to fix "load" already in 2006.)
comment: 127
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c127
Steve Orvell wrote on 2014-10-08 18:35:08 +0000.
I think the new proposal is good.
All the composition use cases in this thread are satisfied. It also makes some sense that the order of the insertion points in the event path is the reverse of the destination insertion points list. Similarly, it makes sense that we get subtrees contiguously (e.g. P, O, N and I, H, G).
comment: 128
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c128
Hayato Ito wrote on 2014-10-09 01:59:02 +0000.
Thank you for the feedbacks.
Now I am feeling that there is no blocking issue to adapt the new proposal, right?
If someone has an objection or find a flaw in the new proposal, please let us know that.
There might be a hidden bug or room for optimization in the new proposal, but it doesn't matter as long as the result event path is equivalent, I think.
That might be an implementation detail and we can optimize or fix the bug later.
Let me update the spec and make sure it will work actually.
comment: 129
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c129
Hayato Ito wrote on 2014-10-16 09:32:18 +0000.
If there is any update on the algorithm described in https://bugzilla.mozilla.org/show_bug.cgi?id=1059989, please let me know that.
I'll try to update the spec based on https://bugzilla.mozilla.org/show_bug.cgi?id=1059989#c10 in a few days.
comment: 130
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c130
Hayato Ito wrote on 2014-10-31 04:23:02 +0000.
Let me try to implement the new proposal locally (only in my machine :)) in blink before speccing that.
I'd like to make sure that it really works and breaks nothing in blink. I need an experiment to have a confidence.
comment: 131
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c131
Hayato Ito wrote on 2014-11-11 08:07:36 +0000.
Status update:
No progress so far. Please let me know if this issue is blocking something. I'll raise the priority of this task.
I'll use the algorithm described in https://bugzilla.mozilla.org/show_bug.cgi?id=1059989 as a reference. If there is any update on this, please let me know that.
comment: 132
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c132
Olli Pettay wrote on 2015-01-21 11:54:17 +0000.
(In reply to Hayato Ito from comment #131)
Status update:
No progress so far. Please let me know if this issue is blocking something.
I'll raise the priority of this task.
Well, this is one of the biggest issues open in shadow DOM, IMO
(this and is-in-document case)
comment: 133
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c133
Hayato Ito wrote on 2015-01-22 01:58:19 +0000.
(In reply to Olli Pettay from comment #132)
(In reply to Hayato Ito from comment #131)
Status update:
No progress so far. Please let me know if this issue is blocking something.
I'll raise the priority of this task.
Well, this is one of the biggest issues open in shadow DOM, IMO
(this and is-in-document case)
Thanks for letting me know that. I've marked this as P1. I'll work on this soon.
comment: 134
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c134
Hayato Ito wrote on 2015-02-04 22:05:28 +0000.
Status update:
I've asked koji (kojii@chromium-(migrated-from-bugzilla-to-avoid-pinnging-users-in-github).org) to work on this.
kojii@, could you tentatively implement the new proposal in blink and see how things work well?
comment: 135
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c135
Koji Ishii wrote on 2015-02-05 14:14:01 +0000.
I'm on it.
comment: 136
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c136
Koji Ishii wrote on 2015-02-06 08:42:30 +0000.
Olli, could you clarify one point I didn't get very well in the proposed algorithm?
https://bugzilla.mozilla.org/show_bug.cgi?id=1059989#c10
5.2.3.1. Let SHADOW-POOL-HOST be the shadow insertion point into which CURRENT shadow root is projected.
I don't understand what "projected" means, I'm guessing it's equivalent to "distributed" from comment 122. But the whole 5.2 is executed only when "the destination insertion points of CURRENT" is empty by the "if" in 5.1, so the CURRENT is not supposed to be distributed.
Do I misunderstand what "projected" means, or something else?
comment: 137
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c137
Hayato Ito wrote on 2015-02-06 09:23:04 +0000.
"reprojection" is the old terminology, which the recent Shadow DOM spec hasn't used.
It should be equivalent to "re-distribution".
Olli, if you are using 'reprojection' in other meaning, please let kojii@ know that.
comment: 138
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c138
Olli Pettay wrote on 2015-02-06 11:42:35 +0000.
Yeah, it is about distribution.
comment: 139
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c139
Koji Ishii wrote on 2015-02-07 10:45:39 +0000.
Thank you Olli and Hayato.
But then the
5.2.3.1. Let SHADOW-POOL-HOST be the shadow insertion point into which CURRENT shadow root is projected.
looks like a bug to me, because this is executed only when the destination insertion points of CURRENT is empty, so the CURRENT shadow root is not supposed to be distributed and SHADOW-POOL-HOST becomes NULL.
Did I miss something?
comment: 140
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c140
Hayato Ito wrote on 2015-02-09 06:32:32 +0000.
(In reply to Koji Ishii from comment #139)
Thank you Olli and Hayato.
But then the
5.2.3.1. Let SHADOW-POOL-HOST be the shadow insertion point into which
CURRENT shadow root is projected.
You are right. This looks wrong.
I guess the what this sentence is tying to say is:
Let SHADOW-POOL_HOST be the shadow insertion point in the younger shadow root (of the CURRENT shadow root) if such the shadow insertion points exits.
comment: 141
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c141
Koji Ishii wrote on 2015-02-12 06:28:18 +0000.
With so much thanks to Olli, wchen, and Hayato, I've experimentally implemented the proposed algorithm, and now I'm seeing two different results in our tests.
The two are minor, but since they are changes in the behavior not discussed in this thread yet, any opinions/insights from anyone is also appreciated.
- The proposed algorithm stops event path for orphaned nodes
When target nodes are not distributed, the event path in the current spec bubbles up to Document, but the proposed algorithm stops. Example:
A
SR
B
Note that SR does not have IP. When an event fires on B, the event path is:
Current: B, SR, A, Document, Window
Proposed: B
This also occurs when younger shadow roots do not have any shadow insertion points.
I think this is a subtle detail we can ignore, and this difference does not prevent the proposed algorithm to be accepted. If anyone think differently, appreciate comments.
- Events that are Always Stopped at...where?
The current spec of "Events that are Always Stopped" says "...stopped at the root node of the node tree"
http://w3c.github.io/webcomponents/spec/shadow/#h-events-that-are-always-stopped
Current our implementation allows older and younger shadow roots to receive the event and stops before shadow hosts. The behavior was changed by the experimental implementation, probably because the order was changed.
I can't determine if "the root node of the node tree" includes older/younger shadow roots, I'll discuss with Hayato for clarification.
comment: 142
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c142
Hayato Ito wrote on 2015-02-12 08:49:36 +0000.
We should treat this kind of unintentional change as a bug of the new proposal unless these are intentional ones.
Please don't change any behavior except for reordering insertion points in the event path because that's the only proposed change, AFAIK.
Please file a new bug if this is an intentional change.
comment: 143
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c143
Olli Pettay wrote on 2015-02-12 12:48:57 +0000.
(In reply to Koji Ishii from comment #141)
With so much thanks to Olli, wchen, and Hayato, I've experimentally
implemented the proposed algorithm, and now I'm seeing two different results
in our tests.
The two are minor, but since they are changes in the behavior not discussed
in this thread yet, any opinions/insights from anyone is also appreciated.
- The proposed algorithm stops event path for orphaned nodes
When target nodes are not distributed, the event path in the current spec
bubbles up to Document, but the proposed algorithm stops. Example:
A
SR
B
Note that SR does not have IP. When an event fires on B, the event path is:
Current: B, SR, A, Document, Window
Proposed: B
But shouldn't B's destination insertion points be empty and from B event propagate to A.
Or do I misunderstand your example?
(B is a child node of A)
This also occurs when younger shadow roots do not have any shadow insertion
points.
In which case exactly? If you dispatch event to a node in older shadow tree and younger
doesn't have insertion point, event should propagate only to the older shadow root.
- Events that are Always Stopped at...where?
The current spec of "Events that are Always Stopped" says "...stopped at the
root node of the node tree"
http://w3c.github.io/webcomponents/spec/shadow/#h-events-that-are-always-
stopped
Current our implementation allows older and younger shadow roots to receive
the event and stops before shadow hosts.
hosts? plural?
mousemove within older shadow dom shouldn't propagate to the younger shadow dom
comment: 144
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c144
Koji Ishii wrote on 2015-02-13 01:39:44 +0000.
Moved the second issue to bug 28008. It looks like a spec bug, and may need further discussions.
Allow me to look into the first issue again, I tried to minimize the case and it looks like I lost some required condition to reproduce, or maybe my impl problem.
comment: 145
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c145
Koji Ishii wrote on 2015-02-13 02:12:59 +0000.
The case where younger shadow roots do not have shadow insertion points:
A
SR1
B
SR2
When an event fired on B:
Current: B, SR1, A, Document, Window
Proposed: B, SR1
comment: 146
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c146
Hayato Ito wrote on 2015-02-13 03:08:49 +0000.
(In reply to Olli Pettay from comment #143)
- Events that are Always Stopped at...where?
The current spec of "Events that are Always Stopped" says "...stopped at the
root node of the node tree"
http://w3c.github.io/webcomponents/spec/shadow/#h-events-that-are-always-
stopped
Current our implementation allows older and younger shadow roots to receive
the event and stops before shadow hosts.
hosts? plural?
mousemove within older shadow dom shouldn't propagate to the younger
shadow dom
That's orthogonal problem, which should be handled by event-related-target algorithm and the following sentence in http://w3c.github.io/webcomponents/spec/shadow/#event-dispatch
If the relatedTarget and target are the same for a given node, its the event listeners must not be invoked. TouchEvent is not subject to this rule.
In addition to that, in this case, it's okay for the younger to see the mouse move within the older for the same reason that the shadow tree can see mouse move for distributed nodes in it's shadow host. I think the current algorithm is designed to achieve that.
comment: 147
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c147
Hayato Ito wrote on 2015-02-13 03:19:02 +0000.
(In reply to Koji Ishii from comment #145)
The case where younger shadow roots do not have shadow insertion points:
A
SR1
B
SR2
When an event fired on B:
Current: B, SR1, A, Document, Window
Proposed: B, SR1
I can't agree this proposal because this violates the design principle of Shadow DOM event dispatching of the following:
"Attaching a Shadow Root to an element shouldn't have any effect for an event dispatching in the node tree which the shadow host participates in, basically"
In the proposal, attaching the younger shadow root would affect the event dispatching in the document tree. That would violated the principle.
comment: 148
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c148
Koji Ishii wrote on 2015-02-13 04:45:29 +0000.
(In reply to Olli Pettay from comment #143)
(In reply to Koji Ishii from comment #141)
- The proposed algorithm stops event path for orphaned nodes
When target nodes are not distributed, the event path in the current spec
bubbles up to Document, but the proposed algorithm stops. Example:
A
SR
B
Note that SR does not have IP. When an event fires on B, the event path is:
Current: B, SR, A, Document, Window
Proposed: B
But shouldn't B's destination insertion points be empty and from B event
propagate to A.
Or do I misunderstand your example?
(B is a child node of A)
Sorry, I simplified the test case too much. The reproducible case is:
A
SR
SIP
B
In this tree, B's destination insertion point is not empty, but B is not distributed. When an event fired on B:
Current: B, SIP, SR, A, Document, Window
Proposed: B
So the two cases are similar but hit different causes:
1.1. When the shadow tree contains but no
1.2. When the younger shadow tree does not contain
When the shadow tree does not contain either nor , you're right that both algorithms produce the same results.
comment: 149
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c149
Olli Pettay wrote on 2015-02-13 22:34:21 +0000.
(In reply to Hayato Ito from comment #147)
(In reply to Koji Ishii from comment #145)
The case where younger shadow roots do not have shadow insertion points:
A
SR1
B
SR2
When an event fired on B:
Current: B, SR1, A, Document, Window
Proposed: B, SR1
I can't agree this proposal because this violates the design principle of
Shadow DOM event dispatching of the following:
No it doesn't
"Attaching a Shadow Root to an element shouldn't have any effect for an
event dispatching in the node tree which the shadow host participates in,
basically"
B is a child of SR1 in this case, not part of the node tree of A.
In the proposal, attaching the younger shadow root would affect the event
dispatching in the document tree. That would violated the principle.
No, B is a child of SR1.
(or I misunderstand where B is.)
comment: 150
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c150
Olli Pettay wrote on 2015-02-13 22:42:03 +0000.
(In reply to Koji Ishii from comment #148)
Sorry, I simplified the test case too much. The reproducible case is:
A
SR
SIP
B
In this tree, B's destination insertion point is not empty, but B is not
distributed. When an event fired on B:
Current: B, SIP, SR, A, Document, Window
Proposed: B
Wait, what is the destination insertion point for B in this case? And why?
The spec says
"Each node that is not an insertion point has an ordered list, called destination insertion points, which consists of insertion points to where the node is distributed"
So how can B's destination insertion point list be non-empty but B is not distributed.
That seems to contradict with the definition of 'destination insertion points'.
Or do I misunderstand something here?
comment: 151
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c151
Hayato Ito wrote on 2015-02-16 01:50:49 +0000.
(In reply to Olli Pettay from comment #149)
(In reply to Hayato Ito from comment #147)
(In reply to Koji Ishii from comment #145)
The case where younger shadow roots do not have shadow insertion points:
A
SR1
B
SR2
When an event fired on B:
Current: B, SR1, A, Document, Window
Proposed: B, SR1
I can't agree this proposal because this violates the design principle of
Shadow DOM event dispatching of the following:
No it doesn't
"Attaching a Shadow Root to an element shouldn't have any effect for an
event dispatching in the node tree which the shadow host participates in,
basically"
B is a child of SR1 in this case, not part of the node tree of A.
In the proposal, attaching the younger shadow root would affect the event
dispatching in the document tree. That would violated the principle.
No, B is a child of SR1.
(or I misunderstand where B is.)
Let me make it clear what I said as follows:
- Attaching a shadow root to any node in any node tree doesn't affect an event dispatching which will happen on any node, in the every node trees, except for the newly attached shadow tree, basically.
In the example case, we can interpret this as follows:
- Attaching a shadow root (SR1) to any node (A) in any node tree (document tree) doesn't affect an event dispatching which will happen on any node (B), in the every node trees (both document tree and SR1 tree), except for the newly attached shadow tree (SR2 tree), basically.
"Basically" here means "as long as we don't call event.stopPropagation() in the newly attached shadow tree".
comment: 152
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c152
Koji Ishii wrote on 2015-02-16 01:56:24 +0000.
(In reply to Olli Pettay from comment #150)
(In reply to Koji Ishii from comment #148)
Sorry, I simplified the test case too much. The reproducible case is:
A
SR
SIP
B
In this tree, B's destination insertion point is not empty, but B is not
distributed. When an event fired on B:
Current: B, SIP, SR, A, Document, Window
Proposed: B
Wait, what is the destination insertion point for B in this case? And why?
The spec says
"Each node that is not an insertion point has an ordered list, called
destination insertion points, which consists of insertion points to where
the node is distributed"
So how can B's destination insertion point list be non-empty but B is not
distributed.
That seems to contradict with the definition of 'destination insertion
points'.
Or do I misunderstand something here?
I agree it's weird. I had to re-read the spec when investigating this issue, but the SIP above meets the criteria defined in "3.3 Shadow Insertion Points"
http://w3c.github.io/webcomponents/spec/shadow/#h-shadow-insertion-points
I opened bug 28031 for this. I'm ok either way, just want it to be interoperable.
comment: 153
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c153
Hayato Ito wrote on 2015-02-16 02:12:05 +0000.
(In reply to Koji Ishii from comment #152)
(In reply to Olli Pettay from comment #150)
(In reply to Koji Ishii from comment #148)
Sorry, I simplified the test case too much. The reproducible case is:
A
SR
SIP
B
In this tree, B's destination insertion point is not empty, but B is not
distributed. When an event fired on B:
Current: B, SIP, SR, A, Document, Window
Proposed: B
Wait, what is the destination insertion point for B in this case? And why?
The spec says
"Each node that is not an insertion point has an ordered list, called
destination insertion points, which consists of insertion points to where
the node is distributed"
So how can B's destination insertion point list be non-empty but B is not
distributed.
That seems to contradict with the definition of 'destination insertion
points'.
Or do I misunderstand something here?
I agree it's weird. I had to re-read the spec when investigating this issue,
but the SIP above meets the criteria defined in "3.3 Shadow Insertion Points"
http://w3c.github.io/webcomponents/spec/shadow/#h-shadow-insertion-points
I opened bug 28031 for this. I'm ok either way, just want it to be
interoperable.
I'm afraid that you didn't understand what Olli pointed out.
In this tree, B's destination insertion point is not empty, but B is not
distributed. When an event fired on B:
I don't understand what this sentence mean...
"node is not distributed" should mean "node's destination insertion is empty". That's the spec's intention.
comment: 154
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c154
Koji Ishii wrote on 2015-02-16 05:30:51 +0000.
Understood what I misunderstood, so the SIP in the oldest shadow root acts as a content insertion point. Apologies for the confusion.
While my interpretation was incorrect, the result still matches.
Now run the algorithm against:
A
SR
SIP
B
There's a bit ambiguous definition in the proposed algorithm:
5.4.1. Let PROJECTED-SHADOW be the shadow root projected into CURRENT.
What is the PROJECTED-SHADOW when CURRENT is SIP?
My code gets nullptr, because no shadow roots are distributed to the SIP, and terminates the loop. Does Gecko implement this differently?
comment: 155
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c155
Koji Ishii wrote on 2015-02-16 06:32:50 +0000.
I put test cases on jsbin: http://jsbin.com/casoyi
Chrome Canary:
- B1,SIP,SR1,H1,BODY,HTML,[object HTMLDocument],[object Window]
- B2,SR2,H2,BODY,HTML,[object HTMLDocument],[object Window]
- C3,SR4
Chrome with experimental impl:
- B1
- B2,SR2
- C3,SR4
Could you tell me what the current Gecko gives? That'd help us to figure out if the bug is in algorithm, or in my impl.
The third case was added because I added an assertion to ensure the INSERTION-POINTS stack is empty at the end of the algorithm, and it fails on one of our tests for editing. The produced event paths are the same. Not sure if this is an non-issue, or issues in both algorithm. Hayato, do you have insights?
comment: 156
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c156
Koji Ishii wrote on 2015-02-16 06:41:38 +0000.
Correction: on test case 3, events should fire on B3. Updated the jsbin: http://jsbin.com/casoyi
Chrome Canary:
- B1,SIP,SR1,H1,BODY,HTML,[object HTMLDocument],[object Window]
- B2,SR2,H2,BODY,HTML,[object HTMLDocument],[object Window]
- B3,SHADOW,SHADOW,SR5,C3,SR4,H3,BODY,HTML,[object HTMLDocument],[object Window]
- C3,SR4
Chrome with experimental impl:
- B1
- B2,SR2
- B3
- C3,SR4
comment: 157
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c157
Olli Pettay wrote on 2015-02-16 11:37:39 +0000.
(In reply to Hayato Ito from comment #151)
Let me make it clear what I said as follows:
- Attaching a shadow root to any node in any node tree doesn't affect an
event dispatching which will happen on any node, in the every node trees,
except for the newly attached shadow tree, basically.
In the example case, we can interpret this as follows:
- Attaching a shadow root (SR1) to any node (A) in any node tree (document
tree) doesn't affect an event dispatching which will happen on any node (B),
in the every node trees (both document tree and SR1 tree), except for the
newly attached shadow tree (SR2 tree), basically.
Ah, you mean that. It is indeed then possible that the algorithm needs some tweaking, since it was written
during the time when older shadow trees not distributed to any shadow insertion point were thought to be not-in-document.
comment: 158
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c158
Olli Pettay wrote on 2015-02-16 11:50:26 +0000.
(In reply to Olli Pettay from comment #157)
Ah, you mean that. It is indeed then possible that the algorithm needs some
tweaking, since it was written
during the time when older shadow trees not distributed to any shadow
insertion point were thought to be not-in-document.
(and that is in fact still the behavior in Gecko, since it kind of makes sense that one can sort of replace the older shadow tree. Going through bug 26365 to see why the spec has different definition atm.)
comment: 159
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c159
Koji Ishii wrote on 2015-02-16 15:45:45 +0000.
Olli, can you also check case 1 and 3 of the http://jsbin.com/casoyi ?
Case 2 is about nodes not distributed to any shadow insertion point, but case 1 and 3 are different.
If you could come up with a tweaked algorithm that fixes all these issues, that'd be greatly appreciated.
comment: 160
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c160
Koji Ishii wrote on 2015-02-17 01:46:43 +0000.
Incorporated case 2 of comment 141 into http://jsbin.com/casoyi
Chrome Canary:
- click on B1: B1, SIP1, SR1, H1, BODY, HTML, [object HTMLDocument], [object Window]
- click on B2: B2, SR2, H2, BODY, HTML, [object HTMLDocument], [object Window]
- selectstart on B3: B3, SR4, SIP2, SR5
- selectstart on B4: B4, SIP2, SIP3, SR7, C3, SR6, H4, BODY, HTML, [object HTMLDocument], [object Window]
Experimental impl on Chrome:
- click on B1: B1
- click on B2: B2, SR2
- selectstart on B3: B3, SR4
- selectstart on B4: B4
Notes:
- Issue 2 is about the orphaned nodes.
- Issue 4 exits the algorithm with non-empty stack.
If you see differences in Gecko, it's possible that my interpretation of https://bugzilla.mozilla.org/show_bug.cgi?id=1059989#c10 is wrong.
comment: 161
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c161
Hayato Ito wrote on 2015-02-17 07:56:06 +0000.
(In reply to Olli Pettay from comment #158)
(In reply to Olli Pettay from comment #157)
Ah, you mean that. It is indeed then possible that the algorithm needs some
tweaking, since it was written
during the time when older shadow trees not distributed to any shadow
insertion point were thought to be not-in-document.
(and that is in fact still the behavior in Gecko, since it kind of makes
sense that one can sort of replace the older shadow tree. Going through bug
26365 to see why the spec has different definition atm.)
Yeah, that's a important (and confusing) point.
Recently, to makes things simple, I've tried to think as follows:
- Insertion points don't have any effect on DOM connectivity.
- Insertion points have an effect on how style inheritance is calculated. As a result, some nodes would became 'display: none', however, they are still considered as connected.
- From this point of view, we shouldn't call the older shadow tree orphaned even when the younger shadow tree doesn't have a shadow insertion point.
It's something like: 'display: none' is set to the pseudo parent element of (child nodes of the older shadow root).
I've tried to make the current spec consistent with this concept, recently. However, I agree we should say it more explicitly (and in a friendly way), maybe in a non-normative section.
comment: 162
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c162
Olli Pettay wrote on 2015-02-17 11:28:33 +0000.
(In reply to Hayato Ito from comment #161)
- Insertion points don't have any effect on DOM connectivity.
But they very much have DOM "connectivity", since they do need to affect to
the DOM event dispatch.
But as I said in bug 26365, I can probably live with the approach where
non-distributed older shadow doms are still in the document if the host is.
(That approach may makes certain things easier in implementations, but behavior and functionality a bit weirder.)
comment: 163
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c163
Olli Pettay wrote on 2015-02-23 19:03:29 +0000.
So we want to change
3. If CURRENT is not the youngest shadow root hosted by SHADOW-POOL-HOST:
to something like
- If CURRENT is not the youngest shadow root hosted by SHADOW-POOL-HOST and CURRENT is distributed to a shadow insertion point:
comment: 164
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c164
Koji Ishii wrote on 2015-02-25 19:07:04 +0000.
Hayato and I discussed today and came up with a new proposal, in the hope of achieving both-win.
The proposal is to add an exception to the spec:
CURRENT:
The event path calculation algorithm must be used to determine event path and must be equivalent to processing the following steps
PROPOSED:
The event path must be equivalent to processing the event path calculation algorithm described below, except that insertion points may appear in different order, as long as they’re after the content distributed to them (or fallback content in the case of fallback) and before their parents
In case you’re uncertain whether this is a good idea, please allow me to give some background of this proposal.
What made the most sense to us was Dimitri’s comment 125 saying web developers would never notice this difference.
Still we needed to try hard to come up with a better algorithm, one because mental models vary by people, and the other because all of us want this algorithm simple and fast, while the “simple and fast” was different by the underlying architecture.
The algorithm in the current spec works good for Blink architecture, while, as far as I understand from the discussion in Bugzilla, it adds insanely complex burden to Gecko.
On the other hand, we implemented the proposed new algorithm, but we figured out that it makes our code more complex, and we are still unable to fix 3 regressions.
This indicates that, in this case, we can’t make all implementations -- including ones in future -- happy with single algorithm.
We considered another option discussed earlier, which is not to include any insertion points in the event path. But Steve, as one of the representatives of web developers, said in comment 16 that he prefers “all insertion points in event path” than “no insertion points”. This option may still work, but it looks like a both-lose option.
Our conclusion is to allow implementation dependency in the manner of not to trouble web developers is the only way to make all of us happy.
We’re hoping that this option provides good enough interoperability for web developers, while allowing simple and fast implementations on any underlying architecture to implementers, and thus also provides faster event dispatch to web developers.
Does this look like a workable solution? Comments appreciated.
comment: 165
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c165
Olli Pettay wrote on 2015-02-26 15:14:31 +0000.
(In reply to Koji Ishii from comment #164)
Hayato and I discussed today and came up with a new proposal, in the hope of
achieving both-win.
Don't understand what "both-win" refers to.
The proposal is to add an exception to the spec:
CURRENT:
The event path calculation algorithm must be used to determine event path
and must be equivalent to processing the following steps
PROPOSED:
The event path must be equivalent to processing the event path calculation
algorithm described below, except that insertion points may appear in
different order, as long as they’re after the content distributed to them
(or fallback content in the case of fallback) and before their parents
I don't understand what this all means.
Still we needed to try hard to come up with a better algorithm, one because
mental models vary by people, and the other because all of us want this
algorithm simple and fast, while the “simple and fast” was different by the
underlying architecture.
The algorithm in the current spec works good for Blink architecture, while,
as far as I understand from the discussion in Bugzilla, it adds insanely
complex burden to Gecko.
The main issue with the current algorithm in the spec is its inconsistency and mentally really
hard to get right.
What is wrong with the proposed change + the fix similar to comment 163?
I very much thought we had agreement here and just had to tweak the proposed algorithm a bit.
On the other hand, we implemented the proposed new algorithm, but we figured
out that it makes our code more complex, and we are still unable to fix 3
regressions.
What regressions?
Our conclusion is to allow implementation dependency in the manner of not to
trouble web developers is the only way to make all of us happy.
I hope you don't mean we would have implementation specific event dispatch? That is something Gecko will not do.
Shadow DOM will not be enabled by default before we have a good enough spec for it and the implementation follows it.
comment: 166
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c166
Dimitri Glazkov wrote on 2015-02-26 15:42:34 +0000.
(In reply to Olli Pettay from comment #165)
I hope you don't mean we would have implementation specific event dispatch?
That is something Gecko will not do.
Shadow DOM will not be enabled by default before we have a good enough spec
for it and the implementation follows it.
Agreed. Leaving event dispatch open to implementer interpretation sounds like a lose-lose, not a win-win.
comment: 167
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c167
Koji Ishii wrote on 2015-02-27 03:08:11 +0000.
(In reply to Olli Pettay from comment #165)
(In reply to Koji Ishii from comment #164)
Hayato and I discussed today and came up with a new proposal, in the hope of
achieving both-win.
Don't understand what "both-win" refers to.
I meant to make all impls simple and fast regardless of underlying architecture, and make all insertion points appear in the event path.
What is wrong with the proposed change + the fix similar to comment 163?
I very much thought we had agreement here and just had to tweak the proposed
algorithm a bit.
On the other hand, we implemented the proposed new algorithm, but we figured
out that it makes our code more complex, and we are still unable to fix 3
regressions.
What regressions?
The Comment 160 reported failures of 4 test case at http://jsbin.com/casoyi. I saw your comment to work on not-distributed case, and I suppose comment 163 is the one, but I don't think it fixes other cases.
I can implement it to re-test if you believe it fixes all 4 issues, but can't figure out what the proposal in comment 163 is. Could you clarify:
...and CURRENT is distributed to a shadow insertion point:
Did you mean "...and at least one shadow insertion point is the destination insertion points" or "...and the final destination point is a shadow insertion point"?
Our conclusion is to allow implementation dependency in the manner of not to
trouble web developers is the only way to make all of us happy.
I hope you don't mean we would have implementation specific event dispatch?
That is something Gecko will not do.
Shadow DOM will not be enabled by default before we have a good enough spec
for it and the implementation follows it.
Hm, looks like we have some misunderstanding. Can you clarify what I miss?
- You'd like to change the algorithm in the current spec.
- The proposed algorithm showed 4 regressions.
- You kindly tweaked the proposed algorithm for one of them (not really sure "one" until I understand comment 163 and re-test), but nobody has raised hands to solve other issues.
From these understandings, options I can see are:
A. Someone fixes all other issues in the proposed algorithm
B. No insertion points
C. Go with the current spec
D. Stuck
If A is not happening, the new proposal looks still better than B-D, no?
comment: 168
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c168
Hayato Ito wrote on 2015-02-27 04:38:51 +0000.
I think the point is we still don't have a new concrete algorithm which achieve the new proposal.
If Koji's idea is not acceptable, someone must try to fix the current proposed algorithm so that it doesn't have any changes which we haven't agreed.
As I said in this thread, I don't have a strong opinion about the location of insertion points in the event path.
I can live with the world where the location of insertion points are changed. However, if a new algorithm becomes too complex than the current one, I'm afraid that I have to raise the concern because it might be a bad signal, which we can discuss later once we can get a working algorithm.
Olli, I appreciate if you could help Koji to fix the proposed algorithm.
comment: 169
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c169
Olli Pettay wrote on 2015-03-01 17:01:23 +0000.
(In reply to Koji Ishii from comment #167)
Hm, looks like we have some misunderstanding. Can you clarify what I miss?
- You'd like to change the algorithm in the current spec.
yes
(http://jsbin.com/casoyi/edit?html,js,output
Note, that uses the old syntax for . It was agreed long ago would need to have as a child.
But I can't now recall where that change was agreed. But that isn't really about this bug.)
Chrome Canary:
- click on B1: B1, SIP1, SR1, H1, BODY, HTML, [object HTMLDocument], [object Window]
- click on B2: B2, SR2, H2, BODY, HTML, [object HTMLDocument], [object Window]
- selectstart on B3: B3, SR4, SIP2, SR5
- selectstart on B4: B4, SIP2, SIP3, SR7, C3, SR6, H4, BODY, HTML, [object HTMLDocument], [object Window]
Experimental impl on Chrome:
- click on B1: B1
Ok, so this case would need another tweak
"Repeat while CURRENT is a shadow insertion point."
->
"Repeat while CURRENT is a shadow insertion point and there is a shadow root distributed to it".
- click on B2: B2, SR2
This is fixed by comment 163.
- selectstart on B3: B3, SR4
This is the expected behavior. selectstart shouldn't be exposed outside its initial shadow DOM.
The spec says "The following events must always be stopped at the root node of the node tree" and SR4 is a root.
- selectstart on B4: B4
and this is similar to (1)
comment: 170
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c170
Koji Ishii wrote on 2015-03-04 00:08:16 +0000.
(In reply to Olli Pettay from comment #169)
(In reply to Koji Ishii from comment #167)
Hm, looks like we have some misunderstanding. Can you clarify what I miss?
- You'd like to change the algorithm in the current spec.
yes
Great, thank you for your support.
(http://jsbin.com/casoyi/edit?html,js,output
Note, that uses the old syntax for . It was agreed long ago
would need to have as a child.
But I can't now recall where that change was agreed. But that isn't really
about this bug.)
I wasn't aware of that, will check.
Chrome Canary:
- click on B1: B1, SIP1, SR1, H1, BODY, HTML, [object HTMLDocument], [object Window]
- click on B2: B2, SR2, H2, BODY, HTML, [object HTMLDocument], [object Window]
- selectstart on B3: B3, SR4, SIP2, SR5
- selectstart on B4: B4, SIP2, SIP3, SR7, C3, SR6, H4, BODY, HTML, [object HTMLDocument], [object Window]
Experimental impl on Chrome:
- click on B1: B1
Ok, so this case would need another tweak
"Repeat while CURRENT is a shadow insertion point."
->
"Repeat while CURRENT is a shadow insertion point and there is a shadow root
distributed to it".
- click on B2: B2, SR2
This is fixed by comment 163.
Ok, I'll try.
- selectstart on B3: B3, SR4
This is the expected behavior. selectstart shouldn't be exposed outside its
initial shadow DOM.
The spec says "The following events must always be stopped at the root node
of the node tree" and SR4 is a root.
- selectstart on B4: B4
and this is similar to (1)
True that text and algorithm contradict to each other here. Hayato confirmed that the algorithm is correct. Please refer to bug 28008. IIUC, otherwise it will contradict with the policy of creating younger shadow roots should not change behavior of older shadow roots.
comment: 171
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c171
Koji Ishii wrote on 2015-03-04 06:11:06 +0000.
(In reply to Koji Ishii from comment #170)
(In reply to Olli Pettay from comment #169)
- click on B1: B1
Ok, so this case would need another tweak
"Repeat while CURRENT is a shadow insertion point."
->
"Repeat while CURRENT is a shadow insertion point and there is a shadow root
distributed to it".
- click on B2: B2, SR2
This is fixed by comment 163.
Ok, I'll try.
Confirmed that these two changes fixed the 2 issues, but new 7 tests start failing. Here's the Blink test results:
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/50623/layout-test-results/results.html
Olli, do you want to keep fixing the proposed algorithm? If so, does the above page provide good enough information for you to work?
comment: 172
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c172
Olli Pettay wrote on 2015-03-04 13:34:39 +0000.
That link gives 10 different pages, so I don't know which 7 failures
I'd be happy to fix the algorithm, if there is something to fix still, but,
fixing does mean changes to the expected results. The event path won't be the same, and doesn't make sense for me to fix blink's tests.
But could you perhaps email me links to the failing tests and
and tell me how the tests are failing and we can go through the
tests that way, and no need to spam this bug about blink test failures.
And if we found that there are still issues in the algorithm, then
propose changes here.
comment: 173
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c173
Koji Ishii wrote on 2015-04-09 06:25:15 +0000.
Status updates:
- Olli and I are working on fixing the algorithm. I forgot to update this bug for a while, sorry about that. I will try to keep the status updated here.
- The new algorithm proposed here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1059989#c10
was tweaked by comments in this bug and as Olli and I discuss. I'm keeping the updated algorithm in a branch of the spec here:
https://github.com/kojiishi/webcomponents/blob/event-path-23887/spec/shadow/index.html
- A WIP patch implementing the algorithm to Blink is here:
https://codereview.chromium.org/906123002/
Olli and I are using Blink tests to find/fix issues in the algorithm. We've narrowed down to 4 more test failures to attack.
Personally, I still can't be very clear that this additional complexity in the spec and in the implementation is worth the perceived benefits. But I think that comparison should be done once the algorithm is done without known flaws, and the final call would be up to the WG, so I'm putting that aside for now.
comment: 174
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c174
Koji Ishii wrote on 2015-04-15 01:53:38 +0000.
While I'm still unable to fix the 4 test failures in the proposed algorithm, I took another approach to re-start from the current algorithm and came up with "candidate 2" algorithm. This algorithm:
- Produces the same result as specified in comment 115
- Passes all tests in Blink
- Produces the same results as the currently proposed algorithm for all tests in Blink, except the 4 failing tests
Olli told me that he can review this tomorrow or so, and we can update you all here after Olli and I discuss, but comments are appreciated if any.
"Candidate 2" algorithm:
https://cdn.rawgit.com/kojiishi/webcomponents/125051d4ea5973b459fc291636ccef8d0508501f/spec/shadow/index.html#bug23887-koji
(If re-spec prevents your browser to jump to the point, please find "candidate 2")
Blink WIP patch:
https://codereview.chromium.org/906123002/
comment: 175
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c175
Hayato Ito wrote on 2015-04-15 03:07:17 +0000.
Thank you for updating the subject, however,
[Shadow] Update event retargeting algorithm to never violate the rule of parentNode always preceeding the node in event path
the new subject is not correct because the current algorithm doesn't violate the rule.
I think comment #124 is still a good summary of the new proposal.
https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c124
Let me update the subject so that it reflects the proposal correctly.
comment: 176
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c176
Olli Pettay wrote on 2015-04-15 20:43:52 +0000.
(In reply to Koji Ishii from comment #174)
"Candidate 2" algorithm:
https://cdn.rawgit.com/kojiishi/webcomponents/
125051d/spec/shadow/index.html#bug23887-koji
(If re-spec prevents your browser to jump to the point, please find
"candidate 2")
That is very similar to the older proposed algorithm, but seems to then
fix some of its issues. I didn't look at how it ended up fixing those, but
c2 looks indeed rather good to me, and gives the expected results at least in
the cases I evaluated on paper.
comment: 177
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c177
Koji Ishii wrote on 2015-04-17 06:02:40 +0000.
Thank you Olli for your review and confirmation. I don't understand how it ended up fixing the issues either ;) but the example tree in the spec and its expected result mentioned in comment #115 are in Blink tests and the WIP patch passes it.
I submitted the spec change PR at:
#42
comment: 178
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c178
Hayato Ito wrote on 2015-04-17 06:47:40 +0000.
Thank you. Now we have a finally working proposal, right? :)
Let me have a look carefully.
comment: 179
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c179
Koji Ishii wrote on 2015-04-17 07:06:07 +0000.
(In reply to Hayato Ito from comment #178)
Thank you. Now we have a finally working proposal, right? :)
Yes, finally ;)
Let me have a look carefully.
Thank you, look forward to hearing from you.
Blink patch is also updated in case it helps:
https://codereview.chromium.org/906123002/
comment: 180
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c180
Hayato Ito wrote on 2015-04-17 07:57:19 +0000.
Bad news for you :(
Exclude shadow insertion points whose containing shadow root has older shadow roots from INSERTION-POINTS
I'm afraid this this would take us to the lose situation.
For example:
(Let me allow to use the pseudo notation to describe a tree of trees)
Note that #a's destination insertion points is [#shadow, #content].
If an event happens on #a, the event path should be (in terms of the new proposal):
[#a => #oldest-shadow-root-of-shadow-host => #content => #shadow-root-of-shadow-host-b => #shadow => #shadow-host-b => #youngest-shadow-root-of-shadow-host => #shadow-host]
But according to the algorithm of candidate 2, the event path would be:
[#a => #content => #shadow-root-of-shadow-host-b => #shadow-host-b => #youngest-shadow-root-of-shadow-host => #shadow-host]
The point is the proposed algorithm can't handle the case where re-distribution happens through shadow insertion points and content insertion points.
I'll take another look later.
comment: 181
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c181
Hayato Ito wrote on 2015-04-17 08:05:03 +0000.
If my evaluation on the paper is correct, that means Blink doesn't have such a Layout test. My bad. We should have a Layout test for that.
comment: 182
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c182
Koji Ishii wrote on 2015-04-17 08:09:01 +0000.
(In reply to Hayato Ito from comment #180)
Bad news for you :(
Thank you for having a look, actually, it's a great news to find issues earlier. I'll add a test for this case and look into further.
comment: 183
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c183
Hayato Ito wrote on 2015-04-17 08:16:17 +0000.
(In reply to Koji Ishii from comment #182)
(In reply to Hayato Ito from comment #180)
Bad news for you :(
Thank you for having a look, actually, it's a great news to find issues
earlier. I'll add a test for this case and look into further.
Thanks! I appreciate that.
comment: 184
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c184
Koji Ishii wrote on 2015-04-17 09:52:23 +0000.
Confirmed the example in comment #180 produces:
Current: #A, #SR1, #SIP, #CIP, #SR3, #SH2, #SR2, #SH1
Candidate 1: #A, #CIP, #SR3, #SR1, #SIP, #SH2, #SR2, #SH1
Candidate 2: #A, #CIP, #SR3, #SR1, #SIP, #SH2, #SR2, #SH1
I'll work on fix.
comment: 185
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c185
Hayato Ito wrote on 2015-04-17 11:21:30 +0000.
I have an idea to resolve this. Let me tackle this issue later.
I'd like to have a simple clean algorithm.
comment: 186
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c186
Koji Ishii wrote on 2015-04-18 04:40:29 +0000.
To share what Hayato and I discussed with others (before comment #185):
- The c1/c2 results do NOT violate principals and thus acceptable
- It's probably better for SR1 after SR2, right before SH1.
- Hayato would try if fixing that could simplify the algorithm further, from the other principle that the simpler is the better.
comment: 187
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c187
Hayato Ito wrote on 2015-04-20 03:21:26 +0000.
Okay. Let me propose the new algorithm. Let's call this "hayato-2015-04" algorithm.
- Let PATH be the empty ordered list of nodes
- Let INSERTION-POINT-STACK be an empty stack of nodes
- Let CURRENT be NODE
- Repeat while CURRENT exists:
- Add CURRENT to PATH
- Let INSERTION-POINTS be the destination insertion points of CURRENT
- If INSERTION-POINTS is not empty:
- Push INSERTION-POINTS into INSERTION-POINT-STACK in order of first destination to final destination
- Pop INSERTION-POINT-STACK and set CURRENT to be the popped node.
- Otherwise if CURRENT is a shadow root:
- If CURRENT is the oldest shadow root:
- Let HOST be the shadow host which hosts CURRENT
- Otherwise:
- Let HOST be the older shadow root relative to CURRENT
- If INSERTION-POINT-STACK is not empty and the most recent node in the INSERTION-POINT-STACK is in the same node tree as HOST
- Pop INSERTION-POINT-STACK and set CURRENT to be the popped node.
- Otherwise if CURRENT and NODE participates in the same node tree and EVENT is one of the events which must be stopped:
- Stop this algorithm
- Otherwise:
- Let CURRENT be HOST
- Otherwise:
- Let CURRENT be the parent node of CURRENT
"hayato-2015-04" is based on my mental model - A concept of "Multiple shadow roots" are just a syntax sugar for "the older shadow root hosts the younger shadow root".
According to this concept, we can merge (4.4.1) and (4.4.2) further by the following one sentence:
"Let HOST be the shadow host (or the shadow root) which hosts CURRENT".
Note that this is the moment where we move from the child tree to the parent tree in a tree of trees. The new algorithm matches a concept of a tree of trees nicely.
As a result, "hayato-2015-04" doesn't have to use a term of {older shadow root, younger shadow root, oldest shadow root, youngest shadow root} nor {content insertion point, shadow insertion point} at all. The algorithm doesn't need to distinguish them. I'm feeling this is a good signal.
One side effect:
- Now the older shadow root receives an event after the younger shadow root. However, this behavior matches "A shadow host receives an event after the oldest shadow root".
I can say, in a uniform way, "The root node of the parent tree receives an event after the root node of the child tree" in a tree of trees.
Caution: The new algorithm is only evaluated in my head in a short time. I'll have another look later, however, I appreciate if you could give me early feedback since I'm flying to Mountain View / San Francisco soon.
comment: 188
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c188
Koji Ishii wrote on 2015-04-20 03:52:11 +0000.
Thank you Hayato, I'll implement and run the tests.
comment: 189
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c189
Koji Ishii wrote on 2015-04-20 08:51:50 +0000.
Hayato's algorithm and its test results are here:
https://codereview.chromium.org/906123002/
- It changes the result for the spec example to:
#D, #C, #M, #L, #R, #Q, #P, #O, #N, #K, #J, #I, #H, #G, #U, #T, #S, #F, #X, #W, #V, #E, #B, #A
- There's one case where the length of the path is longer by one additional node.
I'll examine these changes, but Hayato, if you could look at them from the view point that they match to what you're trying to accomplish, that'd be helpful too.
I'll post my analysis later on.
comment: 190
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c190
Olli Pettay wrote on 2015-04-20 17:58:14 +0000.
E ends up being in a really odd place. Somehow one of the shadow trees is being split and the younger tree is in the path before the that shadow tree's root.
I don't really see any reason for that.
comment: 191
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c191
Hayato Ito wrote on 2015-04-20 21:48:56 +0000.
(In reply to Koji Ishii from comment #189)
Hayato's algorithm and its test results are here:
https://codereview.chromium.org/906123002/
- It changes the result for the spec example to:
#D, #C, #M, #L, #R, #Q, #P, #O, #N, #K, #J, #I, #H, #G, #U, #T, #S, #F, #X,
#W, #V, #E, #B, #A
This result is expected one for me. I'm glad to see this result.
As for the position of #E, the short answer is:
If we remove an arrow from B (the shadow host) to V (the youngest shadow root), then add a new arrow from E (the older shadow root) to V (the younger shadow root) in Fig 5 in the spec, 1, this result might look natural.
The long answer is:
Let me prepare a document how we should view multiple shadow roots. I briefly explained it in comment #187, however, I need to explain further.
You might notice that the modified Fig 5 is similar to a Fig 1, [2], which explains a tree of trees. Ignore dotted arrows there. Just focus the non-dotted arrows.
There, we can view the fig as follows:
- "shadow root in C hosts D"
- "shadow root in D hosts E"
Yes, we can remove "Multiple" from our head.
If an element A hosts multiple shadow roots, [B, C, D], where B is the oldest shadow root and D is the youngest shadow root, we can view this "Multiple shadow roots" as follows:
- An element, A, hosts a shadow root, B
- The shadow root, B, hosts another shadow root, C.
- The shadow root, C, hosts another shadow root, D.
I found that if we can view multiple shadow roots in this way, we could make a lot of things much cleaner and simpler. That's the motivation why I defined a tree of trees as the current spec defines.
I'll update this thread when I write a document, which wouldn't be long one, I guess.
1 Shadow DOM spec Fig. 5: http://w3c.github.io/webcomponents/spec/shadow/index.htmlfig-an-example-tree-of-trees.-nodes-which-are-not-involved-in-the-example-event-path-which-is-explained-later-are-omitted.x
[2] Shadow DOM spec Fig. 1: http://w3c.github.io/webcomponents/spec/shadow/index.html#fig-a-tree-of-trees.x
comment: 192
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c192
Koji Ishii wrote on 2015-04-21 05:04:27 +0000.
Finished analyzing the test results, here are two differences from current/candidates algorithms.
- As Hayato explained in the comment #191, older shadow roots are moved from "before the IP they're distributed to" to "after younger shadow roots".
D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,E,X,W,V,B,A
D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,X,W,V,E,B,A
- Older shadow roots that were not in the path are now included. Two examples below, when event target is "div <-- target", SR-OLDEST and SR-OLDER are now included.
HOST
div <-- target
SR-OLDEST
div
SR-MIDDLE
content
SR-YOUNGEST
shadow
HOST
SR-OLDER
SR-YOUNGER
shadow
div <-- target
I'm fine with either algorithm; both makes each own logical sense, do not break the principal rules, and technically correct (2nd diff above needs confirmation from Hayato though.)
I think the most valuable thing at this point is to stabilize the algorithm and make implementations interoperable. Hope the WG resolves this issue this week.
comment: 193
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c193
Hayato Ito wrote on 2015-04-22 00:37:42 +0000.
Thank you for analyzing.
(In reply to Koji Ishii from comment #192)
Finished analyzing the test results, here are two differences from
current/candidates algorithms.
- As Hayato explained in the comment #191, older shadow roots are moved
from "before the IP they're distributed to" to "after younger shadow roots".
D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,E,X,W,V,B,A
D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,X,W,V,E,B,A
- Older shadow roots that were not in the path are now included. Two
examples below, when event target is "div <-- target", SR-OLDEST and
SR-OLDER are now included.
HOST
div <-- target
SR-OLDEST
div
SR-MIDDLE
content
SR-YOUNGEST
shadow
HOST
SR-OLDER
SR-YOUNGER
shadow
div <-- target
Both are expected to me. Unless there is a use case where this causes a trouble, I'm okay to have this behavior. This behavior is fine because I think we shouldn't skip an intermediate node tree in a tree of trees in dispatching an event.
A node trees which are involved in dispatching an event should be connected, I think.
comment: 194
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c194
Hayato Ito wrote on 2015-04-22 00:40:11 +0000.
As a follow-up for comment #191, I've written a draft doc about multiple shadow roots: https://github.com/w3c/webcomponents/wiki/Multiple-Shadow-Roots-as-%22a-Shadow-Root-hosts-another-Shadow-Root%22
comment: 195
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c195
Olli Pettay wrote on 2015-04-23 16:26:03 +0000.
(In reply to Koji Ishii from comment #192)
Finished analyzing the test results, here are two differences from
current/candidates algorithms.
- As Hayato explained in the comment #191, older shadow roots are moved
from "before the IP they're distributed to" to "after younger shadow roots".
D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,E,X,W,V,B,A
D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,X,W,V,E,B,A
And that is the issue. Now older shadow trees would have all the power to
affect whether event even enters to younger tree (by using capturing listeners),
but from rendering point of view the younger tree is still the parent of the older.
In my mind the older tree is always distributed to , which means that
older shadow root is the connecting link between the older shadow tree and the
in the younger one.
In other words "shadow hosts" the older tree.
comment: 196
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c196
Hayato Ito wrote on 2015-04-27 04:53:21 +0000.
Update: I and Olli chatted at Web Components f2f. We concluded that there was no remaining issue, given that Multiple Shadow Roots will be removed.
I think "hayato-2014-5" will work "as is" in a world without "Multiple Shadow Roots", though let me keep this open until I update the current event path for the world without Multiple Shadow Roots.
React
Typescript
Django
Laravel
Facebook
Microsoft
Google
Alibaba
D3
Tencent