[Shadow] Change the order of insertion points which are involved in a re-distribution in event path (bugzilla: 23887) #98
Assignees
Labels
Comments
|
Let me close this issue tentatively. Event path calculation algorithm was updated so that it doesn't consider multiple shadow roots. I'm afraid that the current situation is different than the situation when the this bug was filed. If this issue is still blocking Mozilla's implementation, please feel free to re-open this issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Title: [Shadow] Change the order of insertion points which are involved in a re-distribution in event path (bugzilla: 23887)
Migrated from: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887
comment: 0
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c0
Olli Pettay wrote on 2013-11-22 10:34:35 +0000.
To not break consistency with normal DOM where if a node is in
event path, also its parent node is in the path, Shadow DOM shouldn't put
non-final destination insertion points to the path.
comment: 1
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c1
Hayato Ito wrote on 2013-11-22 10:42:43 +0000.
Sounds reasonable. Thank you for filing the bug!
Let me update the spec. I think there is no concern for that.
comment: 2
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c2
Hayato Ito wrote on 2013-11-25 04:56:47 +0000.
+sorvell
Do you have any opinion for this proposal?
My concern is whether Polymer depends on the behavior of the current event path or not.
I don't think we want to include intermeidiate insertion points in the event path, but I'd like to make sure we really don't need that.
comment: 3
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c3
Steve Orvell wrote on 2013-11-25 18:11:56 +0000.
Can we have an example to work with to see the problem more clearly?
Is the proposal to change just the information exposed in event.path or to change the set of elements to which events bubble?
My concern is that this will interact poorly with composition. It seems desirable to maintain the invariant that event handlers on insertion points can see events which bubble from elements distributed to them.
comment: 4
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c4
Hayato Ito wrote on 2013-11-26 00:04:47 +0000.
(In reply to Steve Orvell from comment #3)
Both. Both uses the result of event path calculation algorithm. http://w3c.github.io/webcomponents/spec/shadow/#event-paths
Let's use the example. See Fig.4
http://w3c.github.io/webcomponents/spec/shadow/#distribution-results
When a click event happens on child1 node,
I supposed that insertion point 1 doesn't have to see events which happened on child1.
comment: 5
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c5
Steve Orvell wrote on 2013-11-26 01:47:00 +0000.
Thanks for the example, yes, I am not in favor of this change for the reason stated in #3: it does not interact well with composition.
If an element host-1 like this:
SRI must be guaranteed that host-1 can see events on its element independent of what host-2 does. This is necessary to preserve the integrity of the subtree from the perspective of host-1's shadowRoot.
Note that even in this example: http://jsbin.com/IhIziHo/1/edit where host-2 entirely replaces the rendered subtree, the basic flow of events is maintained. If child-1 fires a bubbling event, then host-1's content must see the event.
comment: 6
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c6
Olli Pettay wrote on 2013-11-26 12:40:32 +0000.
(In reply to Steve Orvell from comment #5)
comment: 7
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c7
Steve Orvell wrote on 2013-11-26 17:09:50 +0000.
Whether host-2 has a shadowRoot and what it does to it is not a detail I should need to be aware of when I'm setting up listeners inside host-1's shadowRoot.
It's a natural consequence of host-1's content being in the event path, which I don't find odd =).
comment: 8
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c8
Olli Pettay wrote on 2013-11-26 17:41:53 +0000.
It is very odd from normal DOM event dispatch point of view.
comment: 9
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c9
Hayato Ito wrote on 2013-11-27 02:01:18 +0000.
This might be a good opportunity to discuss how event path should be.
In the most strictest world, I don't think we should add either insertion points, whether it's final destination or not, or shadow roots to an event path. If we don't add these to the event path, the event path would become equivalent to just "the ancestors in the composed tree". That would make the spec simpler. :)
But I think that's a kind of ego of a spec editor. :) I guess that is very handy and convenience for developers if we add insertion points or shadow roots to the event path. That will make it easy for developers to listen events for distributed nodes, I think.
If some developers don't like this dirty world, I think they can simply ignore all insertion points or shadow roots in the event path. That shouldn't interfere outer world essentially, should it? If we can close our eyes on events which happen on insertion points or shadow roots, the current event dispatching model might be close to the normal DOM event dispatch point of view.
I don't have a strong opinion for this issue. Let's discuss and make the spec better one.
comment: 10
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c10
Dimitri Glazkov wrote on 2013-11-27 04:51:25 +0000.
I totally agree that these insertion points in an event path are odd. They stink. I also think Steve is right -- you need this oddity to preserve composition properties.
Otherwise, it would be very hard for UI controls to reason about what's happening to elements, distributed into their insertion points without also being aware of all other shadow trees. Which is a bad thing.
comment: 11
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c11
Olli Pettay wrote on 2013-11-27 11:54:19 +0000.
We should aim for clean and consistent APIs. The current event propagation in
shadow DOM isn't such.
What is the real world use case for having non-final insertion points
in the path?
Since an insertion point may not be the final, it is anyway impossible
to know what is happening to the element distributed to insertion points.
The element may be in some totally different place in the final tree than
what the initial shadow tree expects. So I don't quite understand the "preserve composition properties" argument.
Also, one can always add event listeners to the elements in the
insertionPoint.getDistributedNodes() list.
comment: 12
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c12
Olli Pettay wrote on 2013-11-27 12:02:11 +0000.
Need to perhaps change getDistributedNodes() handling a bit to make it
useful, since one should be able to observe changes to that list.
comment: 13
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c13
Olli Pettay wrote on 2013-11-27 17:54:48 +0000.
Unless I'm missing something in the spec,
the current setup let's also
the same event to be handled twice in an insertion point.
-SH1
-SR2
-IP2
I know that setup is odd, but nothing seem to prevent adding shadow host
under an insertion point and if there is then an insertion point, it will be
the final, yet the other insertion point is in the event path too.
So event would be dispatched from child:
Child->IP1->IP2->SR2->SH2->IP1(again)->SR1->SH1
comment: 14
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c14
Steve Orvell wrote on 2013-11-27 21:47:11 +0000.
I'll take a shot at this. Be kind, it's a little contrived. I start by making a fancy-list element that has a shadowRoot with a in it and I want to see the events on my list items so I put a listener on the content element:
fancy-list
SR
It works and I'm happy. Then I decide to decorate my list with another element:
fancy-list
SR
I know that fancy-decoration will show my list items and it looks nice so it makes fancy-list better.
Now I'm stuck because fancy-list unexpectedly stopped working. If I move the event listener, it'll work, but do we really expect developers to understand that they need to do this? This seems pretty arcane.
comment: 15
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c15
Steve Orvell wrote on 2013-11-27 21:52:50 +0000.
(In reply to Olli Pettay from comment #13)
Handling the event twice would be bad, no argument there.
I'm not quite sure I follow this. Is SH2 a child of IP1? If so, it's fallback content in the insertion point and would only render if Child was not selected by IP1 so I'm not quite sure how the resulting event flow could occur.
comment: 16
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c16
Steve Orvell wrote on 2013-11-27 21:55:47 +0000.
(In reply to Hayato Ito from comment #9)
I would prefer this to the proposal here (only the last insertion point in the path).
If insertion points are sometimes in the path, I worry that it will be too confusing to know when they can be used and it will become an anti-pattern to attach listeners there.
So my preferences in order are:
comment: 17
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c17
Hayato Ito wrote on 2013-11-28 01:54:04 +0000.
(In reply to Steve Orvell from comment #15)
If this happens in any cases, that means there is a serious error in the spec.
Right. In this case, SH2 is not used at all.
comment: 18
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c18
Hayato Ito wrote on 2013-11-28 02:04:34 +0000.
(In reply to Steve Orvell from comment #16)
Thank you. That means 2 is an acceptable proposal?
I wonder whether there is a missing piece of APIs or not in order to accept 2.
For example,
Adding an such event, like distributionChanged, would be useful to accept 2?
comment: 19
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c19
Olli Pettay wrote on 2013-11-28 16:05:00 +0000.
(In reply to Steve Orvell from comment #14)
You should add listener to SR or somewhere, higher up in the propagation path.
comment: 20
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c20
Olli Pettay wrote on 2013-11-28 16:06:33 +0000.
(In reply to Steve Orvell from comment #15)
What has rendering to do with this all. We're talking about event dispatch ;)
comment: 21
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c21
Steve Orvell wrote on 2013-12-03 07:24:43 +0000.
(In reply to Olli Pettay from comment #20)
Yeah, that's fair. The key bit is that SH2 and Child1 cannot co-exist in the event path.
If the fallback content is not used, I think the event path is:
Child->IP1->SR1->SH1
If an event is fired on the fallback content:
(some distributed child of SH2)->IP2->SR2->SH2->IP1->SR1->SH1
Given this, I believe the event should never be able to bubble to the same insertion point more than once.
comment: 22
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c22
Olli Pettay wrote on 2013-12-03 07:42:34 +0000.
(In reply to Steve Orvell from comment #21)
And whether or not an insertion point can be twice in the event path, event propagation
in event path is inconsistent with the rest of the DOM, and should be changed.
So far I haven't seen any real world use case which requires the current odd behavior.
comment: 23
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c23
Hayato Ito wrote on 2013-12-03 08:48:52 +0000.
(In reply to Olli Pettay from comment #22)
I'd like to know which current behavior is odd in order to make the discussion clear.
In my understanding:
A). In some shadow trees, an insertion point receives an event, but its parentNode doesn't receive an event. That's odd!
Is my understanding correct? Have you found any odd behavior other than A)?
comment: 24
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c24
Olli Pettay wrote on 2013-12-03 09:07:48 +0000.
(In reply to Hayato Ito from comment #23)
comment: 25
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c25
Hayato Ito wrote on 2013-12-03 09:45:49 +0000.
(In reply to Olli Pettay from comment #24)
Thank you.
Could you provide a reproduce case for sibling nodes? That's not what I expected.
I am afraid that I might miss something.
comment: 26
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c26
Olli Pettay wrote on 2013-12-03 10:07:56 +0000.
http://w3c.github.io/webcomponents/spec/shadow/#distribution-results
In the composed tree the second shadow host has insertion point as a child, but
also a shadow root. If event is dispatched to child1, both the
insertion point and shadow root get the event.
comment: 27
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c27
Hayato Ito wrote on 2013-12-04 03:44:19 +0000.
(In reply to Olli Pettay from comment #26)
Okay. But in general, we don't have to treat a shadow root as a child of the shadow host. In event path, they might be adjacent to each other. But we should view them in separate trees. What I worry about is the odd behavior in the same node tree.
comment: 28
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c28
Hayato Ito wrote on 2013-12-04 04:14:26 +0000.
I think there is a confusion. So let me explain the design principle of the current event path.
For example, given a fig4 (http://w3c.github.io/webcomponents/spec/shadow/index.html#distribution-results):
If child 1 is clicked, the event path would be:
[child1, IP1, IP3, IP3's parent, 2nd SR, 2nd SH, 1st SR, 1st SH, document]
For each node tree:
comment: 29
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c29
Olli Pettay wrote on 2013-12-04 06:31:16 +0000.
(In reply to Hayato Ito from comment #27)
In event path, they might be adjacent to each other. But we
comment: 30
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c30
Olli Pettay wrote on 2013-12-04 06:40:44 +0000.
(In reply to Hayato Ito from comment #28)
So let me explain the design principle of the
Now, the-parent-of-2nd-SH is the parent of both IP1 and 2nd SH, and event goes through both those siblings.
comment: 31
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c31
Hayato Ito wrote on 2013-12-04 07:01:43 +0000.
(In reply to Olli Pettay from comment #30)
Thank you for the explanation! But I still think this is the root cause of the confusion. :)
In this case, IP3 cannot select nodes which are distributed into IP1.
Because IP1 is not a child node of 2nd SH.
That means CHILD1's destination insertion point is [IP1], rather than [IP1, IP3].
comment: 32
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c32
Olli Pettay wrote on 2013-12-10 12:22:27 +0000.
(In reply to Olli Pettay from comment #22)
Hmm, was my example wrong. Trying to find a better example.
Anyhow, there is still no example where non-final-destination insertion
points are needed in the event path and since this makes event propagation
inconsistent with the rest to the platform, there is no need to make this
special case.
comment: 33
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c33
Hayato Ito wrote on 2013-12-10 13:25:17 +0000.
(In reply to Olli Pettay from comment #32)
Yeah, it might be better to have a better example.
Especially, I am curious whether we can create an example which hits the following case A):
If we can create an example which causes A), I think it might be worth to fix the current spec.
comment: 34
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c34
Olli Pettay wrote on 2013-12-10 13:51:31 +0000.
Actually, the more I think this issue, the less I understand why we need
any insertion points in the event path.
All we need is the parent of the final destination insertion point
then propagate event there through shadow boundaries up to the non-shadow dom.
One can always add listeners to shadow host or to the distributed node.
(XBL1 adds listeners by default to the bound element == shadow host)
comment: 35
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c35
Hayato Ito wrote on 2013-12-10 13:59:28 +0000.
(In reply to Olli Pettay from comment #34)
Thank you. That's (2) in the following list.
If we can get rid of (3) from this list, we must choose (1) or (2). The current spec is (1).
My opinion for (2) is comment #18.
https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c18
comment: 36
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c36
Olli Pettay wrote on 2013-12-10 14:06:16 +0000.
yeah, and I don't understand why the complicated (1) is chosen.
distributionChanged event might be useful, but probably not needed.
One can always just add listeners to the host.
And distributionChanged could be added later if needed.
comment: 37
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c37
Hayato Ito wrote on 2013-12-16 07:36:48 +0000.
(In reply to Olli Pettay from comment #36)
I'd like to hear a opinion from Steve.
I agree that (1) is complicated than (2), but (1) doesn't break the original claim in comment #1, does that?
I think we have to find a balance. (1) is complicated, but (1) gives developers more power.
comment: 38
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c38
Olli Pettay wrote on 2013-12-16 12:34:40 +0000.
(In reply to Hayato Ito from comment #37)
What more power? Event listener can always be added to the distributed nodes
or to their parent.
We should not add complicated stuff without good use cases.
comment: 39
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c39
Steve Orvell wrote on 2013-12-16 17:27:02 +0000.
Removing insertion points from the event path is a significant change. I think it's important to try to gather more feedback before moving forward.
One obvious issue I see is that to capture the same behavior there are some cases where additional 'wrapper' elements will be required, e.g.
With the current rules, I can listen to events that bubble from elements selected into each insertion point via listeners on the content element. If we remove insertion points from the event path, one would need wrapper elements around the insertion points with the listeners on them.
comment: 40
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c40
Olli Pettay wrote on 2013-12-16 17:34:05 +0000.
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.
comment: 41
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c41
Steve Orvell wrote on 2013-12-16 18:15:15 +0000.
Sure, that's another option. In either case, the developer must do more work to get the desired behavior.
comment: 42
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c42
Hayato Ito wrote on 2013-12-17 03:26:50 +0000.
My biggest concern about removing insertion point from the event path is whether it would cause developers a kind of burden or not.
We need more feedbacks.
My understanding between (1) and (2) is:
By (1):
For each node tree:
By (2):
For each node tree:
Although I don't have a strong opinion whether which, (1) or (2) is better, I am still not convinced that (2) is much better than (1). So far, I think (1) is more useful for developers than (2).
comment: 43
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c43
Steve Orvell wrote on 2014-02-26 03:20:46 +0000.
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.
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.
Alternatively, if handlers are attached to the distributed nodes, there's no good way today for the developer to respond to nodes that are added/removed. You can watch for mutations on the host's childList, but you will not see elements that are re-projected this way.
At this point, I don't see an alternative to requiring all insertion points to be in the event path. If we don't include insertion points, we run into the problems listed above. If we include only the last insertion point, we have composition fail.
comment: 44
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c44
Hayato Ito wrote on 2014-03-10 08:12:55 +0000.
Thank you for the explanation, Steve.
Is there any comments about this?
If we don't have any further objection to the current spec, let me close this bug in a week.
Remember that we can re-open this bug anytime if we feel to discuss this topic again.
comment: 45
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c45
Hayato Ito wrote on 2014-03-10 08:15:22 +0000.
Because I am cleaning up the bugs, I might close this bug in a few days. Please feel free to re-open this if you need further discussion.
comment: 46
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c46
Olli Pettay wrote on 2014-03-21 16:57:32 +0000.
Uh, I didn't get bugmail about this.
Reading comments and possibly reopening...
I'm not happy shadow dom adding bizarre event propagation paths.
comment: 47
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c47
Olli Pettay wrote on 2014-03-21 17:08:16 +0000.
(In reply to Steve Orvell from comment #43)
XBL1 has the long tradition to put listener to the binding parent (shadowroot),
and that hasn't been a perf problem.
comment: 48
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c48
Olli Pettay wrote on 2014-03-21 17:22:48 +0000.
(In reply to Steve Orvell from comment #43)
comment: 49
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c49
Steve Orvell wrote on 2014-03-21 21:34:40 +0000.
I'm ok with eliminating insertion points from the event path.
We shouldn't include just the final one because, as discussed, this doesn't support composition.
I still do think it would be better to have all the insertion points in the event path to support the cases mentioned previously. However, these cases feel uncommon to me. Most of the time, there is going to be a convenient node around the insertion point on which to hang the listener.
comment: 50
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c50
Boris Zbarsky wrote on 2014-03-21 21:39:10 +0000.
I would like to clarify what the expected behavior is here, at least in the simple case.
Say the light DOM looks like this:
The
and the
Now an event is fired at
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
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
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)
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.
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:
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)
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)
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.
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:
attached to sh1:
attached to sh2:
propagation paths are
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):
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:
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:
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
In this case:
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:
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)
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:
Pros:
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)
(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:
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)
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)
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)
Good point. I've thought it too.
My comment,
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.
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)
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.
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)
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)
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)
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)
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)
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:
events which must be stopped:
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)
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)
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)
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)
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.
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:
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)
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)
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
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.
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)
I meant we might want to update the following sentence in the algorithm, as an example:
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:
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)
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)
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]
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:
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)
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)
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)
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)
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.
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.
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)
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)
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)
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
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)
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)
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)
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)
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)
Let me make it clear what I said as follows:
In the example case, we can interpret this as follows:
"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)
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)
I'm afraid that you didn't understand what Olli pointed out.
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:
Chrome with experimental impl:
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:
Chrome with experimental impl:
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)
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)
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:
Experimental impl on Chrome:
Notes:
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)
Yeah, that's a important (and confusing) point.
Recently, to makes things simple, I've tried to think as follows:
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)
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
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)
I very much thought we had agreement here and just had to tweak the proposed algorithm a bit.
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)
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)
I meant to make all impls simple and fast regardless of underlying architecture, and make all insertion points appear in the event path.
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:
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"?
Hm, looks like we have some misunderstanding. Can you clarify what I miss?
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)
(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.)
This is fixed by comment 163.
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.
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)
Great, thank you for your support.
I wasn't aware of that, will check.
Ok, I'll try.
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)
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:
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
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:
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,
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)
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)
Yes, finally ;)
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 :(
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)
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)
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):
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.
"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:
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/
#D, #C, #M, #L, #R, #Q, #P, #O, #N, #K, #J, #I, #H, #G, #U, #T, #S, #F, #X, #W, #V, #E, #B, #A
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)
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:
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:
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.
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
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)
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)
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.
The text was updated successfully, but these errors were encountered: