This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 23887 - [Shadow] Change the order of insertion points which are involved in a re-distribution in event path
Summary: [Shadow] Change the order of insertion points which are involved in a re-dist...
Status: RESOLVED MOVED
Alias: None
Product: WebAppsWG
Classification: Unclassified
Component: HISTORICAL - Component Model (show other bugs)
Version: unspecified
Hardware: PC Linux
: P1 normal
Target Milestone: ---
Assignee: Hayato Ito
QA Contact: public-webapps-bugzilla
URL:
Whiteboard:
Keywords:
Depends on: 28008 28031
Blocks: 28552 28564
  Show dependency treegraph
 
Reported: 2013-11-22 10:34 UTC by Olli Pettay
Modified: 2015-05-27 03:01 UTC (History)
10 users (show)

See Also:


Attachments
Simple testcase (1.12 KB, text/html)
2014-03-21 21:39 UTC, Boris Zbarsky
Details
example (1.80 KB, text/html)
2014-03-22 16:26 UTC, Olli Pettay
Details

Description Olli Pettay 2013-11-22 10:34:35 UTC
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 Hayato Ito 2013-11-22 10:42:43 UTC
Sounds reasonable. Thank you for filing the bug!
Let me update the spec. I think there is no concern for that.
Comment 2 Hayato Ito 2013-11-25 04:56:47 UTC
+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 Steve Orvell 2013-11-25 18:11:56 UTC
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 Hayato Ito 2013-11-26 00:04:47 UTC
(In reply to Steve Orvell from comment #3)
> 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?

Both. Both uses the result of event path calculation algorithm. http://w3c.github.io/webcomponents/spec/shadow/#event-paths

> 
> 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.

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,
- Event listeners registered on insertion point 1 can't see the click event because child1's final destination is not insertion point 1.
- Event listeners registered on insertion point 3 can see the click event because child1's final destination is insertion point 3.


I supposed that insertion point 1 doesn't have to see events which happened on child1.
Comment 5 Steve Orvell 2013-11-26 01:47:00 UTC
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:

<host-1>
  SR
    <host-2><content onclick="handler()">
  <child-1>

I must be guaranteed that host-1 can see events on its <content> 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 Olli Pettay 2013-11-26 12:40:32 UTC
(In reply to Steve Orvell from comment #5)
> If an element host-1 like this:
> 
> <host-1>
>   SR
>     <host-2><content onclick="handler()">
>   <child-1>
> 
> I must be guaranteed that host-1 can see events on its <content> element
> independent of what host-2 does.
Why? You can add listener to host-2.
And it is very odd that if you have
listener both on host-2 and on content, both get called, per the current spec.
Comment 7 Steve Orvell 2013-11-26 17:09:50 UTC
> Why? You can add listener to host-2.

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.

> And it is very odd that if you have
> listener both on host-2 and on content, both get called, per the current spec.

It's a natural consequence of host-1's content being in the event path, which I don't find odd =).
Comment 8 Olli Pettay 2013-11-26 17:41:53 UTC
It is very odd from normal DOM event dispatch point of view.
Comment 9 Hayato Ito 2013-11-27 02:01:18 UTC
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 Dimitri Glazkov 2013-11-27 04:51:25 UTC
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 Olli Pettay 2013-11-27 11:54:19 UTC
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 Olli Pettay 2013-11-27 12:02:11 UTC
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 Olli Pettay 2013-11-27 17:54:48 UTC
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
  - SR1
    - IP1
      - SH2
        -SR2
          -IP2
  - Child

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 Steve Orvell 2013-11-27 21:47:11 UTC
> What is the real world use case for having non-final insertion points
> in the path?

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 <content> 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
    <!-- decorations -->
    <content select="li" onclick="handler()"></content>

It works and I'm happy. Then I decide to decorate my list with another element:

fancy-list
  SR
    <fancy-decoration>
      <content onclick="handler()"></content>
    </fancy-decoration>

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 Steve Orvell 2013-11-27 21:52:50 UTC
(In reply to Olli Pettay from comment #13)
> 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.

Handling the event twice would be bad, no argument there.

> I know that setup is odd, but nothing seem to prevent adding shadow host
> under an insertion point

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 Steve Orvell 2013-11-27 21:55:47 UTC
(In reply to Hayato Ito from comment #9)
> 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

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:

1. all insertion points in event path.
2. no insertion points in event path.
3. last insertion point in event path.
Comment 17 Hayato Ito 2013-11-28 01:54:04 UTC

(In reply to Steve Orvell from comment #15)
> (In reply to Olli Pettay from comment #13)
> > 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.
> 
> Handling the event twice would be bad, no argument there.

If this happens in any cases, that means there is a serious error in the spec.

> 
> > I know that setup is odd, but nothing seem to prevent adding shadow host
> > under an insertion point
> 
> 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.

Right. In this case, SH2 is not used at all.
Comment 18 Hayato Ito 2013-11-28 02:04:34 UTC
(In reply to Steve Orvell from comment #16)
> (In reply to Hayato Ito from comment #9)
> > 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
> 
> 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:
> 
> 1. all insertion points in event path.
> 2. no insertion points in event path.
> 3. last insertion point in event path.

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,

> Need to perhaps change getDistributedNodes() handling a bit to make it
> useful, since one should be able to observe changes to that list.

Adding an such event, like distributionChanged, would be useful to accept 2?
Comment 19 Olli Pettay 2013-11-28 16:05:00 UTC
(In reply to Steve Orvell from comment #14)

> fancy-list
>   SR
>     <fancy-decoration>
>       <content onclick="handler()"></content>
>     </fancy-decoration>
> 
> I know that fancy-decoration will show my list items and it looks nice so it
> makes fancy-list better. 
> 

You should add listener to SR or somewhere, higher up in the propagation path.


> 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.
It is really really odd if developer needs to put listener to <content> and putting the
listener to the parent wouldn't work.
Comment 20 Olli Pettay 2013-11-28 16:06:33 UTC
(In reply to Steve Orvell from comment #15)
> 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.

What has rendering to do with this all. We're talking about event dispatch ;)
Comment 21 Steve Orvell 2013-12-03 07:24:43 UTC
(In reply to Olli Pettay from comment #20)
> (In reply to Steve Orvell from comment #15)
> > 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.
> 
> What has rendering to do with this all. We're talking about event dispatch ;)

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 Olli Pettay 2013-12-03 07:42:34 UTC
(In reply to Steve Orvell from comment #21) 
> If the fallback content is not used, I think the event path is:
> 
> Child->IP1->SR1->SH1
I'd like to understand what in the spec says this.


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 Hayato Ito 2013-12-03 08:48:52 UTC
(In reply to Olli Pettay from comment #22)
> (In reply to Steve Orvell from comment #21) 
> > If the fallback content is not used, I think the event path is:
> > 
> > Child->IP1->SR1->SH1
> I'd like to understand what in the spec says this.
> 
> 
> 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.

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 Olli Pettay 2013-12-03 09:07:48 UTC
(In reply to Hayato Ito from comment #23)

> 
> A). In some shadow trees, an insertion point receives an event, but its
> parentNode doesn't receive an event. That's odd!
Yes. And that leads also to the odd behavior that sibling nodes may receive the same event.
Comment 25 Hayato Ito 2013-12-03 09:45:49 UTC
(In reply to Olli Pettay from comment #24)
> (In reply to Hayato Ito from comment #23)
> 
> > 
> > A). In some shadow trees, an insertion point receives an event, but its
> > parentNode doesn't receive an event. That's odd!
> Yes. And that leads also to the odd behavior that sibling nodes may receive
> the same event.

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 Olli Pettay 2013-12-03 10:07:56 UTC
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 Hayato Ito 2013-12-04 03:44:19 UTC
(In reply to Olli Pettay from comment #26)
> 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.

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 Hayato Ito 2013-12-04 04:14:26 UTC
I think there is a confusion. So let me explain the design principle of the current event path.

- An event can be dispatched across node trees.
- If we view each node tree as a separate tree, there should be a *certain level* of consistency for the existing event dispatching model within the same node tree.


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:
 - For the document tree, the event path would be [child1, 1st SH, document]
 - For 1st shadow tree, the event path would be [IP1, 2nd SH, 1st SR]
 - For 2nd shadow tree, the event path would be [IP3, IP3's parent, 2nd SR]
Comment 29 Olli Pettay 2013-12-04 06:31:16 UTC
(In reply to Hayato Ito from comment #27)
> (In reply to Olli Pettay from comment #26)
> > 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.
> 
> Okay. But in general, we don't have to treat a shadow root as a child of the
> shadow host.
That shadow host is a child of its parent node. And its parent node has an
insertion point as a child.


 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 30 Olli Pettay 2013-12-04 06:40:44 UTC
(In reply to Hayato Ito from comment #28)
> I think there is a confusion. 
I don't think so ;)


So let me explain the design principle of the
> current event path.
> 
> - An event can be dispatched across node trees.
> - If we view each node tree as a separate tree, there should be a *certain
> level* of consistency for the existing event dispatching model within the
> same node tree.
> 
> 
> 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:
>  - For the document tree, the event path would be [child1, 1st SH, document]
>  - For 1st shadow tree, the event path would be [IP1, 2nd SH, 1st SR]
>  - For 2nd shadow tree, the event path would be [IP3, IP3's parent, 2nd SR]
The example just happens to make that look nice.
But if 1st shadow tree for example has a non-SH-node in the place of 2nd SH, and
that node has 2nd SH as a child. Then for the 1st shadow tree the event path
would be [IP1, 2nd SH, the-parent-of-2nd-SH, 2ns SR]

Now, the-parent-of-2nd-SH is the parent of both IP1 and 2nd SH, and event goes through both those siblings.
Comment 31 Hayato Ito 2013-12-04 07:01:43 UTC
(In reply to Olli Pettay from comment #30)
> (In reply to Hayato Ito from comment #28)
> > I think there is a confusion. 
> I don't think so ;)
> 
> 
> So let me explain the design principle of the
> > current event path.
> > 
> > - An event can be dispatched across node trees.
> > - If we view each node tree as a separate tree, there should be a *certain
> > level* of consistency for the existing event dispatching model within the
> > same node tree.
> > 
> > 
> > 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:
> >  - For the document tree, the event path would be [child1, 1st SH, document]
> >  - For 1st shadow tree, the event path would be [IP1, 2nd SH, 1st SR]
> >  - For 2nd shadow tree, the event path would be [IP3, IP3's parent, 2nd SR]
> The example just happens to make that look nice.
> But if 1st shadow tree for example has a non-SH-node in the place of 2nd SH,
> and
> that node has 2nd SH as a child. Then for the 1st shadow tree the event path
> would be [IP1, 2nd SH, the-parent-of-2nd-SH, 2ns SR]
> 
> Now, the-parent-of-2nd-SH is the parent of both IP1 and 2nd SH, and event
> goes through both those siblings.

Thank you for the explanation! But I still think this is the root cause of the confusion. :)

> that node has 2nd SH as a child. Then for the 1st shadow tree the event path

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 Olli Pettay 2013-12-10 12:22:27 UTC
(In reply to Olli Pettay from comment #22)

> > that node has 2nd SH as a child. Then for the 1st shadow tree the event path
> 
> 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].

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 Hayato Ito 2013-12-10 13:25:17 UTC
(In reply to Olli Pettay from comment #32)
> (In reply to Olli Pettay from comment #22)
> 
> > > that node has 2nd SH as a child. Then for the 1st shadow tree the event path
> > 
> > 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].
> 
> Hmm, was my example wrong. Trying to find a better example.

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):

> A). In some shadow trees, an insertion point receives an event, but its parentNode doesn't receive an event. That's odd!

If we can create an example which causes A), I think it might be worth to fix the current spec.


> 
> 
> 
> 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 34 Olli Pettay 2013-12-10 13:51:31 UTC
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 Hayato Ito 2013-12-10 13:59:28 UTC
(In reply to Olli Pettay from comment #34)
> 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)

Thank you. That's (2) in the following list.

> 1. all insertion points in event path.
> 2. no insertion points in event path.
> 3. last insertion point in event path.

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 Olli Pettay 2013-12-10 14:06:16 UTC
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 Hayato Ito 2013-12-16 07:36:48 UTC
(In reply to Olli Pettay from comment #36)
> 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.

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?

> To not break consistency with normal DOM where if a node is in
event path,

I think we have to find a balance. (1) is complicated, but (1) gives developers more power.
Comment 38 Olli Pettay 2013-12-16 12:34:40 UTC
(In reply to Hayato Ito from comment #37)
> I think we have to find a balance. (1) is complicated, but (1) gives
> developers more power.

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 Steve Orvell 2013-12-16 17:27:02 UTC
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.

    <content select=".header"></content>
    <content select=".content"></content>

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 Olli Pettay 2013-12-16 17:34:05 UTC
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 Steve Orvell 2013-12-16 18:15:15 UTC
Sure, that's another option. In either case, the developer must do more work to get the desired behavior.
Comment 42 Hayato Ito 2013-12-17 03:26:50 UTC
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.

> 1. all insertion points in event path.
> 2. no insertion points in event path.
> 3. last insertion point in event path.

My understanding between (1) and (2) is:

By (1):
For each node tree:
 - For the document tree, the event path would be [child1, 1st SH, document]
 - For 1st shadow tree, the event path would be [IP1, 2nd SH, 1st SR]
 - For 2nd shadow tree, the event path would be [IP3, IP3's parent, 2nd SR]

By (2):
For each node tree:
 - For the document tree, the event path would be [child1, 1st SH, document]
 - For 1st shadow tree, the event path would be [2nd SH, 1st SR]
 - For 2nd shadow tree, the event path would be [IP3's parent, 2nd SR]


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 Steve Orvell 2014-02-26 03:20:46 UTC
Let's go back to this case in https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c39 and review the counter-argument.

    <content select=".header"></content>
    <content select=".content"></content>


> Why? Add a listener to the shadow host? You can easily filter out events in the 
> listener, for example by using the content.getDistributedNodes().
> Or you can add listeners to the distributed nodes themselves.

Neither option is acceptable.

If a listener is added to the shadowRoot, then each handler must start at event.target and walk up the tree checking if the element is in the specific content's list of distributed nodes. This has a non-trivial cost in code and performance.

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 Hayato Ito 2014-03-10 08:12:55 UTC
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 Hayato Ito 2014-03-10 08:15:22 UTC
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 Olli Pettay 2014-03-21 16:57:32 UTC
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 Olli Pettay 2014-03-21 17:08:16 UTC
(In reply to Steve Orvell from comment #43)
> Let's go back to this case in
> https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c39 and review the
> counter-argument.
> 
>     <content select=".header"></content>
>     <content select=".content"></content>
> 
> 
> > Why? Add a listener to the shadow host? You can easily filter out events in the 
> > listener, for example by using the content.getDistributedNodes().
> > Or you can add listeners to the distributed nodes themselves.
> 
> Neither option is acceptable.
> 
> If a listener is added to the shadowRoot, then each handler must start at
> event.target and walk up the tree checking if the element is in the specific
> content's list of distributed nodes. This has a non-trivial cost in code and
> performance.
Making propagation path more complicated has also performance cost, and
makes event propagation inconsistent.
Also, it is rather trivial cost in code, and makes code less unexpected, since
propagation path doesn't have inconsistencies.


> 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.
Which is why I suggested making getDistributedNodes more useful. As of now it is rather
dummy plain list, but one should be able to get notification when it has been changed.

> 
> 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.

XBL1 has the long tradition to put listener to the binding parent (shadowroot),
and that hasn't been a perf problem.
Comment 48 Olli Pettay 2014-03-21 17:22:48 UTC
(In reply to Steve Orvell from comment #43) 
> 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.
In other words, is there any real world use case when this would be a
performance issue enough to warrant to inconsistent propagation path.
(inconsistencies make code harder to read and understand, and error prone.)
Comment 49 Steve Orvell 2014-03-21 21:34:40 UTC
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 Boris Zbarsky 2014-03-21 21:39:10 UTC
I would like to clarify what the expected behavior is here, at least in the simple case.

Say the light DOM looks like this:

  <div id="1"> 
    <div id="2">
      <div id="3"></div>
    <div>
  </div>

The <div id="1"> has a shadow DOM like so (I'm going to give the shadow root an ID, though I realize it's not an element; that's just for ease of referring to it):

  <shadowroot id="4">
    <div id="5">
      <div id="6">
        <content id="7"></content>
      </div>
    </div>
  </shadowroot>

and the <div id="6"> has a shadow DOM like so:

  <shadowroot id="8">
    <div id="9">
      <content id="10"></content>
    </div>
  </shadowroot>

Now an event is fired at <div id="3">.  In the composed tree, the parent chain looks like 3, 2, 9, 6, 5, 1, I believe.

If I am following the algorithm at http://w3c.github.io/webcomponents/spec/shadow/#event-paths correctly, we start with the event path being [3, 2], then we see that the destination insertion points of <div id="3"> is not empty and that none of them are shadow insertion points, so we set the path to [3, 2, 7, 10] (note that the order of the insertion points here is "rootmost first", unlike normal event path behavior).

Now we start off at the <content id="10"> 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 Boris Zbarsky 2014-03-21 21:39:31 UTC
Created attachment 1457 [details]
Simple testcase
Comment 52 Dimitri Glazkov 2014-03-21 22:19:31 UTC
(In reply to Boris Zbarsky from comment #50)

Thanks for the test case -- I turned it into a jsbin:
http://jsbin.com/newir/1/edit


> What I don't quite understand, if we posit that we do want to put all the
> insertion points in the path, is why the path is not the somewhat more
> intuitive:
> 
>   3,2,10,9,8,7,6,5,4,1

Here's how the reasoning works. Since insertion points aren't in the composed tree, but are in event path, we have to come up with a mental model of how they are added.

Here's the mental model that the spec ascribes:

During distribution, first 2 is distributed into insertion point 7. So the path is now 3,2,7.

Then, the result of that distributed into 10, so the path is now 3,2,7,10. 

In other words, the spec follows the distribution sequence and builds the path accordingly.

FWIW, I am fine for insertion points to not receive any events. I'll be sad about it for a day, and then I'll get over it.
Comment 53 Dimitri Glazkov 2014-03-21 22:40:38 UTC
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 Dimitri Glazkov 2014-03-21 23:47:42 UTC
(In reply to Olli Pettay from comment #47)
> (In reply to Steve Orvell from comment #43)
> > Let's go back to this case in
> > https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c39 and review the
> > counter-argument.
> > 
> >     <content select=".header"></content>
> >     <content select=".content"></content>
> > 
> > 
> > > Why? Add a listener to the shadow host? You can easily filter out events in the 
> > > listener, for example by using the content.getDistributedNodes().
> > > Or you can add listeners to the distributed nodes themselves.
> > 
> > Neither option is acceptable.
> > 
> > If a listener is added to the shadowRoot, then each handler must start at
> > event.target and walk up the tree checking if the element is in the specific
> > content's list of distributed nodes. This has a non-trivial cost in code and
> > performance.
> Making propagation path more complicated has also performance cost, and
> makes event propagation inconsistent.

I agree on this point. The key here is deciding whether we should carry this burden as a platform or pass it on to developers.

> Also, it is rather trivial cost in code, and makes code less unexpected,
> since
> propagation path doesn't have inconsistencies.

A good general exercise here is to assume that each shadow root chunk is written by different people at different times. As a rule, these developers should not need to have any insight into how these chunks are composed. In other words, if I as a developer have to defensively anticipate what will happen outside of my shadow tree, we failed the main objective of this whole shadow DOM exercise -- composition.

Having all insertion points included in the event path satisfies this constraint very well. As a developer, I have a consistent, simple way to find out what events are being dispatched in the parts of the outer tree that are distributed to my insertion points.

Having no insertion points included in the event path also satisfies this constraint, and it's pretty clear that our guinea pig developer (Steve) already worked around this problem by simply putting a div around the insertion point.

Having only final insertion point included in event path is craziness, because now I, as a developer of my component may or may not receive an event depending on how some other dude/dudette composed it in their app. That's composition fail.

So, to keep on moving, we should just zap the insertion points altogether from the event path.
Comment 55 Boris Zbarsky 2014-03-22 00:43:49 UTC
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 <content id="7"> 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 Boris Zbarsky 2014-03-22 00:47:53 UTC
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 Jonas Sicking (Not reading bugmail) 2014-03-22 04:17:52 UTC
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 <shadowroot id="8"> come before the <content id="7">, 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 <a> element between the '6' and the '7' nodes, such that we get the following tree:

  <shadowroot id="4">
    <div id="5">
      <div id="6">
        <a>
          <content id="7"></content>
        <a>
      </div>
    </div>
  </shadowroot>


In this case no node is inserted into multiple insertion points, which makes the whole thing much simpler. The '2' node is inserted into the '7' insertion point, and the 'a' node is inserted into the '10' insertion point.

In this case it's pretty clear that an event fired at the '3' node would get a propagation path of 3,2,7,a,10,9,8,6,5,4,1

So it makes sense to me that if you simply remove the 'a' and insert the '7' directly into the '6', that you'd get the same propagation path but with the 'a' removed. I.e. 3,2,7,a,10,9,8,6,5,4,1


I've read through the initial comments in this bug but I don't see the inconsistencies that are being discussed there. I.e. all of the following appears true:

* No node ever receives the same event twice.
* Any time an event reaches a node, it also reaches the node's parent.
* During the capturing phase, a node always receives an event before its children.
* During the bubbling phase, a node always receives an event after its children.
* The only way to receive an event multiple times in the same shadow tree is by
  registering listeners on multiple nodes, where some of the nodes are ancestors of
  other nodes.

All of the above appears to be true both both with and without the shadow DOM spec.

The only way that I can see that you could receive the same event multiple times is if you register event listeners in multiple different shadow trees. In that case it doesn't seem surprising that you can receive an event multiple times. It's something that happens even in the very simple event model that XBL1 uses.

Is the above wrong? Or is there some other inconsistency that I'm missing? Or is the concern about performance at this point?

Examples would be very helpful.
Comment 58 Jonas Sicking (Not reading bugmail) 2014-03-22 04:31:38 UTC
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:

<document>
  <shadowhost1>
    <shadowroot id=sr1>
      <shadowhost2>
        <shadowroot id=sr2>
          <namelesschild>
            <content id="insertion point 3">
              <content id="insertion point 1">
                <child1/>
              </content>
              <child3/>
            </content>
            <namelesschild/>
          </namelesschild>
        </shadowroot>
      </shadowhost2>
      <content id="insertion point 2">
        <c2/>
      </content>
    </shadowroot>
  <shadowhost1>
</document>

[1] http://w3c.github.io/webcomponents/spec/shadow/#distribution-results
Comment 59 Olli Pettay 2014-03-22 13:34:25 UTC
(In reply to Jonas Sicking from comment #57)
> Is the above wrong? Or is there some other inconsistency that I'm missing?
> Or is the concern about performance at this point?


The inconsistency comes from the non-linear event propagation path in the
final flattened tree.
Comment 60 Olli Pettay 2014-03-22 14:18:27 UTC
(In reply to Dimitri Glazkov from comment #54)
> So, to keep on moving, we should just zap the insertion points altogether
> from the event path.

This would be fine to me. A lot less surprising propagation path.
Comment 61 Boris Zbarsky 2014-03-22 15:01:14 UTC
> non-linear event propagation path in the final flattened tree.

I'm still thinking about Jonas' example, but do you mind explaining clearly the issue here?  At least in my simple example, the nodes in the final flattened tree (so ignoring the insertion points and shadow roots, which are not present in the flattened tree) see the event in the order 3,2,9,6,5,1, which seems like the expected order...
Comment 62 Olli Pettay 2014-03-22 16:26:42 UTC
Created attachment 1458 [details]
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:
<div id="sh1">
  <span id="s1"> [click me 1] </span>
  <span id="s2"> [click me 2] </span>
</div>

attached to sh1:
<shadowroot>
  <span id='sh2'>
    <content id='ip1'></content>
   </span>
</shadowroot>

attached to sh2:
<shadowroot>
  <content id='ip2.1' select='#s1'></content>
  <content id='ip2.2' select='#s2'></content>
</shadowroot>

propagation paths are
- SPAN[id=s1], CONTENT[id=ip1], CONTENT[id=ip2.1], SPAN[id=sh2], DIV[id=sh1]
- SPAN[id=s2], CONTENT[id=ip1], CONTENT[id=ip2.2], SPAN[id=sh2], DIV[id=sh1]
Comment 63 Olli Pettay 2014-03-22 17:00:48 UTC
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 Jonas Sicking (Not reading bugmail) 2014-03-22 17:45:04 UTC
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):

<div id="sh1">
  <shadowroot id='sr1'>
    <span id='sh2'>
      <shadowroot id='sr2'>
        <content id='ip2.1' select='#s1'>
          <content id='ip1'>
            <span id="s1"> [click me 1] </span>
          </content>
        </content>
        <content id='ip2.2' select='#s2'>
          <content id='ip1'>
            <span id="s2"> [click me 2] </span>
          </content>
        </content>
      </shadowroot>
    </span>
  </shadowroot>
</div>

Note that the <content id='ip1'> 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 Olli Pettay 2014-03-22 18:11:26 UTC
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 Dimitri Glazkov 2014-03-23 16:02:54 UTC
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 Hayato Ito 2014-03-24 04:50:29 UTC
Thank you for all guys for comments for the event path.

I am feeling that I have to explain the background of the current event path calculation algorithms and the rationale of that.
I've designed the current event path algorithms so that we can achieve the following goal:

Objective A) In *each* node tree (forget other node trees), the event path should be a *linear* path.

That means:
  - If a node on the event path has the parent node in the node tree, the parent node should also be on the event path. Only exception is the root node of a node tree.
  - In each node tree, the child node must come before its parent node on the event path.

From the view of developers whose concern is only one node tree where they live, they see the behavior of the event path doesn't change at all as long as their concern is one node tree. They shouldn't care of other node trees.



The following is *not* the goal of the current algorithm:

Objective B) Define a reasonable total *order* between nodes which are in different node trees on the same event path.


I'd like to point out the following fact because I guess every one here doesn't notice this fact:

- Think about a tree of trees. Node trees that are involved in one even path can *form* a tree of node trees. Node trees can't be linear here.


I suggest you guys to think about this fact carefully. You can easily create this case by modifying the example in comment #50 as follows:

- Make <div id=5> a shadow host and let it host the following shadow tree.

  <shadowroot id="11">
    <div id="12">
      <content id="13"></content>
    </div>
  </shadowroot>

In this case:
- Event path will be [3, 2, 7, 10, 9, 8, 6, 13, 12, 11, 5, 4, 1].
- There are 4 node trees which are involved in this event path.
- A node tree whose root node is #4 have *two* child node trees, a node tree whose root node is #8 and a node tree whose root node is #11.

It'd be difficult to define a reasonable total order between nodes in this case. Can you define a reasonable total order in this case?
DFS pre-oder? BFS-post-order? Neither can be a reason strong enough to abandon the current simple *one-pass* algorithm.
That's the reason why objective B) is not the goal.

The current algorithm uses one-pass algorithm and can achieve the objective A) at the same time.
I think simple one-pass algorithm is important here for the efficiency.


I'm not saying that we should include insertion points in the event path. That's orthogonal.

However, comment #63 can't be a strong reason not to include insertion points because the claim of 'union of two event paths is a tree' is not correct according to the current spec.
The current spec doesn't define one *super* node tree, which are composed of nodes which participate in a node tree in the same tree of trees.

The spec doesn't define ancestor/descendant relationships between nodes if they are in different node trees. That's intentional.

That means a *tree* used in 'union of two event paths is a tree' can't have a clear definition. Nodes in different node trees can't be a tree.
They live in separated worlds and should be considered as disconnected.



One more thing:

If we would exclude all insertion points from event path, we must have exceptional rule for insertion points which have fallback elements.

Example:

  <shadowroot id="n1">
    <div id="n2">
      <content id="n3" select="#non-existent>  // doesn't select nodes. In this case, a fallback element, "n4", is distributed to this.
        <div id="n4">
        </div>
      </content>
    </div>
  </shadowroot>


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 Hayato Ito 2014-03-24 05:49:48 UTC
As I mentioned that in comment #9, I'd like to note the following again:

If we exclude all shadow roots and insertion points from the current event path, the event path will be equivalent with "inclusive ancestors of the target node in the composed tree" with the following exceptional cases:

-  The target node itself is an insertion point.
-  If fallback elements are distributed to a insertion point, we must include the insertion points to the event path so that we can achieve objective A).
Comment 69 Dimitri Glazkov 2014-03-25 16:16:52 UTC
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 Jonas Sicking (Not reading bugmail) 2014-03-26 22:34:05 UTC
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 <span> 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 Dimitri Glazkov 2014-03-27 16:18:13 UTC
(In reply to Jonas Sicking from comment #70)
> I don't see a way to accomplish that if we also want to have a simple total
> order between nodes?
> 
> 
> That said, it's still not clear to me what the right answer is on the topic
> of when/if to fire events on insertion points.

Maybe we can resolve this if we reduce the overall problem to this question: is introducing more complex total order in event path net positive or net negative? 

Cons:
- increased complexity
- (please add more)

Pros:
- more intuitive for the consumer localized to any given tree
- useful to listen for events coming out of elements, distributed to insertion points
- fallback content is handled consistently
Comment 72 Olli Pettay 2014-03-27 17:19:28 UTC
(In reply to Dimitri Glazkov from comment #71)

> Cons:
> - increased complexity
Slower event dispatch since event target chain creation becomes
more complicated because ... see the next point

> - (please add more)
Inconsistent targeting from target T to T' in the event target chain,
which makes event dispatch less intuitive when one needs to think about the
whole event dispatch, not only local shadow tree.
And because of .stop*Propagation() for example one needs to think about the whole
path in some cases, not only local parts. Same with default handling. If you use <a> in the
shadow trees and click somewhere, which link to follow?  The first shadow tree
can affect to the behavior of the nested shadow trees. 


> 
> Pros:
> - more intuitive for the consumer localized to any given tree
> - useful to listen for events coming out of elements, distributed to
> insertion points
> - fallback content is handled consistently


(I'm not saying pros aren't pros)





I guess XBL1 doesn't have this problem since it doesn't have explicit insertion points,
so event propagation follows always the natural path.
Comment 73 Jonas Sicking (Not reading bugmail) 2014-03-27 17:59:28 UTC
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

<content>         <-- xbl1 equivalent of shadowroot, sort of
  <div style="border:1px solid red">
    <div style="border:1px solid green">
      <children/>   <-- xbl1 equivalent of <content>
    </div>
  </div>
</content>

And content like:

<mybutton style="-moz-binding: url(#fancybutton)">
  <img src="icon.gif">
  <span>text here</span>
</mybutton>

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 Olli Pettay 2014-03-27 18:25:30 UTC
(In reply to Jonas Sicking from comment #73)
> XBL1 these days do have explicit insertion points.
Oh, right, the recent change to make anonymous content to look more like
shadow dom.
I'm talking about the classical xbl1 which Gecko had for 10+ years


> 
> then clicking on the icon or the text in the button does not fire any events
> in the XBL1 shadow content at all. 
Yes it does. Event propagates to the insertion point.
Comment 75 Boris Zbarsky 2014-03-27 19:07:16 UTC
In XBL1, the event path is just along the composed tree.
Comment 76 Jonas Sicking (Not reading bugmail) 2014-03-27 19:38:24 UTC
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 Olli Pettay 2014-03-27 20:25:44 UTC
(In reply to Jonas Sicking from comment #76)
> So then XBL1 also has exact same the complex weaving of nodes from different
> trees.
No. In XBL if you have target T, the next target in the chain is always T'
Comment 78 Hayato Ito 2014-03-28 04:25:50 UTC
(In reply to Jonas Sicking from comment #76)

> I guess we could also argue over if the shadow-root should be in the event
> path or not, but I don't see any disadvantages of having it in the event
> path, and don't think I've seen anyone in here argue that it shouldn't be in
> the event path. Or did I miss that?

Good point. I've thought it too. 

My comment,

> If we exclude all shadow roots and insertion points from the current event path, the event path will be equivalent with "inclusive ancestors of the target node in the composed tree.

implies that. :)

If we remove insertion points, we might want to remove shadow roots too from the event path.


I'm not saying that we should remove insertion points (and shadow roots) from the event path.
I still don't see a strong disadvantage of having these in the event path.

> So the question is, does the ability to listen to events from nodes inserted
> into an insertion point warrant that extra complexity?


I've seen 'extra complexity' of having insertion points in even path several times in this discussion.
However, I'd like to figure out a real issue what this *complexity* would cause.

From the experience of implementing the event path in blink, this didn't cause any extra burden for implementors.

Therefore, I'd like to know what is a real issue for web developers by having insertion points in the event path.
From the discussion, it looks the advantages outweigh the disadvantages, if exists, for web developers.

I'd like to note that the original motivation of including insertion points in the event path is we thought it's useful for web developers.
Comment 79 Hayato Ito 2014-03-28 04:32:46 UTC
(In reply to Hayato Ito from comment #78)
> From the experience of implementing the event path in blink, this didn't
> cause any extra burden for implementors.

I'm not saying that this will apply to all user-agent's implementations.
I'd like to share my experience in blink, hoping it might be useful for the discussion.
Comment 80 Jonas Sicking (Not reading bugmail) 2014-03-28 05:52:39 UTC
> > So then XBL1 also has exact same the complex weaving of nodes from different
> > trees.
> No. In XBL if you have target T, the next target in the chain is always T'

I was referring to "normal" nodes here. Both XBL1 and Shadow DOM alternates "normal" nodes from different shadow trees in the event path. I thought this was one of the sources of complexity that you were worried about, but it appears that I misunderstood?

So it indeed then seems like the only contentious part here is weather to include insertion points in the event path or not.
Comment 81 Olli Pettay 2014-03-28 11:12:42 UTC
(In reply to Hayato Ito from comment #79)
> I'm not saying that this will apply to all user-agent's implementations.
> I'd like to share my experience in blink, hoping it might be useful for the
> discussion.
(IIRC when shadow dom event dispatch was added to webkit/blink, its
event dispatch speed regressed significantly. At least there was a time
when doing perf testing I noticed chrome's event dispatch speed regressed quite a bit)
It wouldn't be an implementation issue for Gecko, but perf would regress a bit.
Comment 82 Olli Pettay 2014-03-28 11:27:22 UTC
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 <a> elements in shadow tree 1 and in shadow tree 2, default
handling goes via the deepest most <a> 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 Dimitri Glazkov 2014-03-28 18:13:01 UTC
(In reply to Olli Pettay from comment #81)
> (In reply to Hayato Ito from comment #79)
> > I'm not saying that this will apply to all user-agent's implementations.
> > I'd like to share my experience in blink, hoping it might be useful for the
> > discussion.
> (IIRC when shadow dom event dispatch was added to webkit/blink, its
> event dispatch speed regressed significantly. At least there was a time
> when doing perf testing I noticed chrome's event dispatch speed regressed
> quite a bit)

FWIW, this code has been extensively rewritten many times and we have not seen this particular codepath being a perf issue in real world.

I am not saying that we haven't done our share of poorly-written code when we first started exploring the problem, but the algorithm we settled on and what's in spec today is O(N). You shouldn't see any dramatic perf problems.
Comment 84 Boris Zbarsky 2014-03-28 18:57:18 UTC
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 Dimitri Glazkov 2014-03-28 19:00:15 UTC
(In reply to Boris Zbarsky from comment #84)
> Just to note the obvious, O(N) is a necessary but not sufficient condition
> for non-sucky event dispatch performance.  The constant matters a lot.

Yup :) I similarly don't want us to speculate about performance problems without any actual data to back it up.
Comment 86 Olli Pettay 2014-03-28 19:20:15 UTC
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 Olli Pettay 2014-03-28 19:33:07 UTC
(In reply to Olli Pettay from comment #86)
> In other words
> Replace the "If the destination insertion points of CURRENT is not empty: .."
> part with something like where you have the same list of insertion points as
> now, but if an insertion point foo has the same parent as what is a
> shadowroot's host, you put insertion point after that shadowroot in the
> event path.

Except that this can also lead to diamond shapes if you compare two paths.
(but otherwise this feels more logical than the current spec behavior)
Comment 88 Dimitri Glazkov 2014-03-28 19:41:52 UTC
(In reply to Olli Pettay from comment #86)
> In other words
> Replace the "If the destination insertion points of CURRENT is not empty: .."
> part with something like where you have the same list of insertion points as
> now, but if an insertion point foo has the same parent as what is a
> shadowroot's host, you put insertion point after that shadowroot in the
> event path.

I thought Hayato provided a pretty good rationale in comment 67 for why this idea is not good? Did you have a chance to read that comment?
Comment 89 Olli Pettay 2014-03-28 20:06:13 UTC
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 Boris Zbarsky 2014-03-28 20:37:17 UTC
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:

1. Let PATH be the empty ordered list of nodes
2. Let CURRENT be NODE
3. Add CURRENT to PATH
4. Set INSERTION_POINTS to be the destination insertion points of NODE.
5. If INSERTION_POINTS is not empty:
   1. Set INS to the last element of INSERTION_POINTS.
   2. Remove the last element from INSERTION_POINTS.
   3. Set CURRENT to INS.
   4. Add CURRENT to PATH.
4. Repeat while CURRENT exists:
   1. If CURRENT is a shadow root:
      1. If NODE and CURRENT are in the same node tree and EVENT is one of the
         events which must be stopped:
         1. Stop this algorithm
      2. If INSERTION_POINTS is not empty:
         1. Set INS to the last element of INSERTION_POINTS.
         2. While INS is a shadow insertion point:
            1. Remove the last element from INSERTION_POINTS.
            2. Add INS to PATH.
            3. Add the older shadow root relative to the root node of INS to
               PATH.
         3. Remove the last element from INSERTION_POINTS.
         4. Add INS to PATH.
      3. Let CURRENT be the shadow host which hosts CURRENT
         1. Add CURRENT to PATH
   2. Otherwise:
      1. Let CURRENT be the parent node of CURRENT
      2. If CURRENT exists:
         1. Add CURRENT to PATH

Take the shadow insertion point bit with a grain of salt.  For example, I don't understand why the spec has the "If SHADOW-ROOT is not the oldest shadow root" clause: it seems like if "destination insertion points" contains a shadow insertion point that means ipso facto that its root was not the oldest shadow root....  In any case, I modeled this above as basically the newer shadow tree being applied to the old shadow root, with distribution happening into shadow insertion points instead of normal insertion points.  Hopefully that understanding of how multiple shadow trees and shadow insertion points work is correct.

Basically, the upshot is that insertion points here are kept with their parent in the dispatch path as opposed to being kept with their distributed nodes.

It's not obvious that this is any simpler spec-wise than what's in the spec right now (and in fact it looks a bit more complicated, though mostly because of the shadow handling).  What do people think of the effects on the mental model for consumers of shadow DOM, though?
Comment 91 Jonas Sicking (Not reading bugmail) 2014-03-28 20:50:34 UTC
I would be fine with this approach assuming that it doesn't cause any perf issues.
Comment 92 Dimitri Glazkov 2014-03-28 21:10:56 UTC
(In reply to Boris Zbarsky from comment #90)
> Basically, the upshot is that insertion points here are kept with their
> parent in the dispatch path as opposed to being kept with their distributed
> nodes.

I am still walking through examples, and it looks like the modification preserves the invariant I care about -- that inside of a given shadow tree, the author sees a consistent even path -- is preserved. Both spec and this proposal result in a matryoshka nesting, which is good.
Comment 93 Olli Pettay 2014-03-28 21:16:06 UTC
(In reply to Jonas Sicking from comment #91)
> I would be fine with this approach assuming that it doesn't cause any perf
> issues.

It shouldn't be any slower than what is in the spec.
(removing insertion points from the path would be a bit faster).


This approach is fine to me. It has the diamond issue as I said, but
at least it would make event propagate in the final tree in somewhat expected order.
Comment 94 Dimitri Glazkov 2014-03-28 23:39:49 UTC
(In reply to Olli Pettay from comment #82)
> The setup the spec has oddly prioritizes top most shadow trees over the
> nested ones.
> Using https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c62 as an example.
> if p1.onclick = function(e) { e.stopPropagation(); }, then nothing
> in the shadow tree 2 will see the event during bubble phase.
> And since shadow tree 1 is there first also during capture phase, 
> p1.parentNode.addEventListener("click", function(e) { e.stopPropagation() },
> true); would also prevent shadow tree 2 to see the event.
> Why the shadow tree which has the final insertion has lower priority than
> the first shadow tree?

I tried to apply this thinking to both the spec'd and newly proposed event paths and I am stuck. Please help.

Let's take example 2 from https://etherpad.mozilla.org/RKukNtHHlO.

Suppose that I am an author of sh1 and sh2 is actually some component that I found on the internet. I have no idea how it works. The author of this component, however, calls e.stopPropagation() in ip2. Let's say I don't know this. That's what composition is about, right? :)

In spec'd event path, I see this the resulting event path as [x1, ip1]. Which sort of makes sense -- I clicked on x1, and I should at least hear something coming out of ip1, because _I know_ that x1 is distributed there.

In new proposal, I see [x1] only. That seems bad. Suddenly, even though x1 is distributed into my ip1, some force of nature grabbed the emergency brake and my event just disappeared.

Am I crazy? At least to me, the spec'd event path actually makes sense, whereas the new proposal results in mysterious silence.
Comment 95 Jonas Sicking (Not reading bugmail) 2014-03-28 23:43:31 UTC
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 Dimitri Glazkov 2014-03-28 23:44:39 UTC
(In reply to Jonas Sicking from comment #95)
> Dimitri, isn't the same thing true in the current spec if you are the author
> of sh2 and sh1 is something you found on the internet?

That doesn't work that way :) I can't find something that uses me. Only the other way round.
Comment 97 Boris Zbarsky 2014-03-29 01:36:44 UTC
> I can't find something that uses me.

You can, because of capturing listeners.  :(

If in example 2 a capturing listener on ip2 or sr2 calls stopPropagation, the author of sh1 will see the event hit capturing listeners on ip1 but not hit any listeners at all on x1.

In fact, even in the spec's setup calling stopPropagation in a capturing listener on ip2 will mean that sr1 gets the event in the capture phase but then ip1 and x1 never see it.

The only sane way to avoid composition issues with stopPropagation is to make stopPropagation tree-local or something...  or something.  Speaking of complexity.
Comment 98 Boris Zbarsky 2014-03-29 01:39:44 UTC
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 Hayato Ito 2014-03-31 08:02:48 UTC
The following has been on my radar for a long time:

- Whether calling event.stopPropagation() should affect the event dispatching in other node trees or not.


AFAIR, I think this is the first time when this topic is being discussed in the Shadow DOM spec bugzilla.

In the current spec, calling event.stopPropagation() in a node tree affects the event dispatching in other node trees, as you notice.

In the past, I thought that we shouldn't make event.stopPropagation() affect the behavior of event dispatching in node trees other than the current node tree. I thought that an event dispatching in each node tree must be completely *independent*. However, no one has taken care of this topic so far, except for me.

This is the typical situation where it's difficult to satisfy both worlds - the tree of trees world and the composed tree world.

I prefer the current rule. Calling an event.stopPropagation stops an entire event dispatching, including event dispatching in other node trees. It's simple and easy to implement and might be useful for some situations.

However it's worth discussing. I think we haven't have enough opinions from web developers about this topic.
Comment 100 Olli Pettay 2014-04-11 13:46:05 UTC
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 Hayato Ito 2014-04-11 14:26:41 UTC
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 Boris Zbarsky 2014-04-11 15:51:18 UTC
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 Boris Zbarsky 2014-04-11 15:53:26 UTC
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 Hayato Ito 2014-04-11 21:24:02 UTC
(In reply to comment #103)
> Though actually, maybe that's not the right ordering.  I'll need to sit down
> with some concrete cases to see what the right behavior here is.
> 
> I agree that it will make the algorithm more complicated. :(

Yeah, I guess it might be tough task to define the algorithm nicely so that the relative order of insertion points in the same destination insertion points gets *reversed* in the event path.

That requires a kind of a non-trivial technique and I'm afraid that makes the algorithm complicated one.
Comment 105 Hayato Ito 2014-04-21 09:10:24 UTC
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 Hayato Ito 2014-05-09 08:24:58 UTC
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.

[1]: http://w3c.github.io/webcomponents/spec/shadow/#event-paths-example
Comment 107 Olli Pettay 2014-05-09 09:14:17 UTC
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 Hayato Ito 2014-05-09 10:03:31 UTC
Thank you. I appreciate that.

(In reply to Olli Pettay from comment #107)
> Just a note that the general consensus during the f2f (if I managed to
> follow the discussion correctly remotely - I did miss some of it) seemed to
> be that  the setup in the spec isn't what is wanted, but the alternative more
> normal child-parent propagation.

Yep, I thought that. However once I tried to make it a concrete algorithm, I realized that there are a lot of unclear points. Therefore I need your help to understand the alternative.

Let's solve this long discussion together.
Comment 109 Hayato Ito 2014-08-25 03:28:47 UTC
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 Olli Pettay 2014-08-25 13:47:52 UTC
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 Hayato Ito 2014-08-26 04:05:09 UTC
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 Olli Pettay 2014-08-28 13:03:41 UTC
The example is hard to read. Showing the final flattened tree next to it would be useful.
Comment 113 Olli Pettay 2014-08-28 13:29:36 UTC
[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 Olli Pettay 2014-08-28 19:59:13 UTC
Looks like the wip algorithm needs still small tweak to cover event propagation
N->K-J-I-H
Comment 115 Olli Pettay 2014-08-28 21:37:20 UTC
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 Hayato Ito 2014-08-29 02:49:13 UTC
Olli, thank you for the explanation. 

Now I can say I understand the intention clearly. It's great to design the concrete algorithm. I've enjoyed reading and running the WIP algorithm in my head. It seems promising.

Aa far as I understand, the result between the current one and WIP one differ only if a re-distribution happens.
If re-distribution occurs, they put non-final destination insertion points in different places.

I don't have a strong opinions about which is better because both satisfy the main goal which I explained in '5.3 Event Paths Example':

http://w3c.github.io/webcomponents/spec/shadow/#event-paths-example

> That means if we focus on one node tree and forget all other node trees, the event path would be seen as if the event happened only on the node tree which we are focused on. This is an important aspect in a sense that hosting shadow trees doesn't have any effect to the event path within the node tree the shadow host participate in as long as the event is not stopped somewhere in the descendant trees.

> For example, from the view of the document tree 1, the event path would be seen as [D, C, B, A]. From the view of the shadow tree 2, the event path would be seen as [I, H, G, F, E]. The similar things also apply to other node trees.

> It is also worth pointing out that if we exclude all insertion points and shadow roots from an event path, the result would be equivalent to the inclusive ancestors of the node on which the event is dispatched, in the composed tree.


From the view of difficulty in implementing, I don't have any concerns because both algorithm seem simple *enough*. That's great.

I am not sure how many developers will notice the difference between them and how important the difference is.
I'd like to hear opinions from developers about what use cases would tell the difference between them and when the difference matters.
Comment 117 Jonas Sicking (Not reading bugmail) 2014-08-29 02:57:28 UTC
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 Olli Pettay 2014-09-19 11:17:45 UTC
Hayato, have you had chance to look at
the proposed algorithm in https://bugzilla.mozilla.org/show_bug.cgi?id=1059989
Comment 119 Hayato Ito 2014-09-22 08:46:31 UTC
No. It seems the algorithm has been updated after I checked last time.
Let me take a look. Thank you for letting me know that.

> Hayato, have you had chance to look at
> the proposed algorithm in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1059989
Comment 120 Hayato Ito 2014-09-30 06:23:10 UTC
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 Olli Pettay 2014-09-30 09:32:40 UTC
Sure. Could it be just OLDER-SHADOW-ROOT?
Comment 122 Hayato Ito 2014-10-01 06:36:50 UTC
(In reply to Olli Pettay from comment #121)
> Sure. Could it be just OLDER-SHADOW-ROOT?

I meant we might want to update the following sentence in the algorithm, as an example:

> Let SHADOW-POOL-HOST be the shadow insertion point into which CURRENT shadow root is projected.

Because the spec doesn't define what '*projected*' means for a shadow root, I think we cay say, instead, if we want to make it more explicit:

> Let SHADOW-POOL-HOST be the shadow insertion point in the younger shadow tree relative to the shadow tree whose root is CURRENT shadow root.

We can update other sentences where *projected* is used, in a similar way.

BTW, I don't have much worry about optimizing (and correcting?) the proposed algorithm as long as the result will be an equivalent because now I understand the intention of the proposal and it's feasibility.

Rather, I'd like to hear early feedbacks from developers, especially polymer developers and users, to know use cases where the difference of resulting event path between the current one and the proposed one matters. I have no idea when it matters.

I appreciate any feedbacks and opinions so that we can decide.
Comment 123 Olli Pettay 2014-10-07 19:39:03 UTC
(In reply to Hayato Ito from comment #122)
> Rather, I'd like to hear early feedbacks from developers, especially polymer
> developers and users, to know use cases where the difference of resulting
> event path between the current one and the proposed one matters. I have no
> idea when it matters.
Have you asked Polymer devs, or hopefully other devs too? (we should not focus too much on feedback
from one script library only.) Any feedback yet?


> I appreciate any feedbacks and opinions so that we can decide.
(I could also remind what was discussed during the last Webapps f2f ;) )
Comment 124 Hayato Ito 2014-10-08 05:56:09 UTC
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)


1. The current one:

See http://w3c.github.io/webcomponents/spec/shadow/#dfn-event-path-calculation-algorithm for details.
The result event path for the example will be (as the spec explains):
  [D,C,I,M,L,P,R,Q,O,N,K,J,H,G,U,T,S,F,E,X,W,V,B,A]


2. The new one

See https://bugzilla.mozilla.org/show_bug.cgi?id=1059989 for details.
The result event path for the example will be:
  [D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,E,X,W,V,B,A]


Some notes:

Both satisfy the original goal which the spec explained as follows:
> That means if we focus on one node tree and forget all other node trees, the event path would be seen as if the event happened only on the node tree which we are focused on. This is an important aspect in a sense that hosting shadow trees doesn't have any effect to the event path within the node tree the shadow host participate in as long as the event is not stopped somewhere in the descendant trees.
> For example, from the view of the document tree 1, the event path would be seen as [D, C, B, A]. From the view of the shadow tree 2, the event path would be seen as [I, H, G, F, E]. The similar things also apply to other node trees.
> It is also worth pointing out that if we exclude all insertion points and shadow roots from an event path, the result would be equivalent to the inclusive ancestors of the node on which the event is dispatched, in the composed tree.


I don't worry about the difficulty of implementation in both. I think there is no significant difference between them in a term of time complexity in a sense of big-O notation.

Please feel free to add additional information.
Comment 125 Dimitri Glazkov 2014-10-08 14:07:10 UTC
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 Olli Pettay 2014-10-08 14:25:55 UTC
(In reply to Dimitri Glazkov from comment #125)
> With my developer hat only, my experience is that this mostly does not
> matter. 
This would be my guess too

> This is one of those platform changes that platform developers
> agonize over and web developers never notice.
Well, web developers do notice the inconsistencies in API behavior eventually.
Inconsistencies in APIs tend to show up later, often when it is too late
to fix them.
("load" event propagation as an example - now we need to have a special event propagation path for one specific event type. It complicates the platform and
people tend to not remember the special case.
But it was too late to fix "load" already in 2006.)
Comment 127 Steve Orvell 2014-10-08 18:35:08 UTC
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 Hayato Ito 2014-10-09 01:59:02 UTC
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 Hayato Ito 2014-10-16 09:32:18 UTC
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 Hayato Ito 2014-10-31 04:23:02 UTC
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 Hayato Ito 2014-11-11 08:07:36 UTC
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 Olli Pettay 2015-01-21 11:54:17 UTC
(In reply to Hayato Ito from comment #131)
> Status update:
> 
> No progress so far. Please let me know if this issue is blocking something.
> I'll raise the priority of this task.

Well, this is one of the biggest issues open in shadow DOM, IMO
(this and is-in-document case)
Comment 133 Hayato Ito 2015-01-22 01:58:19 UTC
(In reply to Olli Pettay from comment #132)
> (In reply to Hayato Ito from comment #131)
> > Status update:
> > 
> > No progress so far. Please let me know if this issue is blocking something.
> > I'll raise the priority of this task.
> 
> Well, this is one of the biggest issues open in shadow DOM, IMO
> (this and is-in-document case)

Thanks for letting me know that. I've marked this as P1. I'll work on this soon.
Comment 134 Hayato Ito 2015-02-04 22:05:28 UTC
Status update:

I've asked koji (kojii@chromium.org) to work on this.

kojii@, could you tentatively implement the new proposal in blink and see how things work well?
Comment 135 Koji Ishii 2015-02-05 14:14:01 UTC
I'm on it.
Comment 136 Koji Ishii 2015-02-06 08:42:30 UTC
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 Hayato Ito 2015-02-06 09:23:04 UTC
"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 Olli Pettay 2015-02-06 11:42:35 UTC
Yeah, it is about distribution.
Comment 139 Koji Ishii 2015-02-07 10:45:39 UTC
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 Hayato Ito 2015-02-09 06:32:32 UTC
(In reply to Koji Ishii from comment #139)
> Thank you Olli and Hayato.
> 
> But then the
> 
> 5.2.3.1. Let SHADOW-POOL-HOST be the shadow insertion point into which
> CURRENT shadow root is projected.

You are right. This looks wrong.

I guess the what this sentence is tying to say is:

  Let SHADOW-POOL_HOST be the shadow insertion point in the younger shadow root (of the CURRENT shadow root) if such the shadow insertion points exits.
Comment 141 Koji Ishii 2015-02-12 06:28:18 UTC
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.

1. The proposed algorithm stops event path for orphaned nodes
When target nodes are not distributed, the event path in the current spec bubbles up to Document, but the proposed algorithm stops. Example:

A
  SR
  B

Note that SR does not have IP. When an event fires on B, the event path is:
Current: B, SR, A, Document, Window
Proposed: B

This also occurs when younger shadow roots do not have any shadow insertion points.

I think this is a subtle detail we can ignore, and this difference does not prevent the proposed algorithm to be accepted. If anyone think differently, appreciate comments.

2. Events that are Always Stopped at...where?
The current spec of "Events that are Always Stopped" says "...stopped at the root node of the node tree"
http://w3c.github.io/webcomponents/spec/shadow/#h-events-that-are-always-stopped

Current our implementation allows older and younger shadow roots to receive the event and stops before shadow hosts. The behavior was changed by the experimental implementation, probably because the order was changed.

I can't determine if "the root node of the node tree" includes older/younger shadow roots, I'll discuss with Hayato for clarification.
Comment 142 Hayato Ito 2015-02-12 08:49:36 UTC
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 Olli Pettay 2015-02-12 12:48:57 UTC
(In reply to Koji Ishii from comment #141)
> With so much thanks to Olli, wchen, and Hayato, I've experimentally
> implemented the proposed algorithm, and now I'm seeing two different results
> in our tests.
> 
> The two are minor, but since they are changes in the behavior not discussed
> in this thread yet, any opinions/insights from anyone is also appreciated.
> 
> 1. The proposed algorithm stops event path for orphaned nodes
> When target nodes are not distributed, the event path in the current spec
> bubbles up to Document, but the proposed algorithm stops. Example:
> 
> A
>   SR
>   B
> 
> Note that SR does not have IP. When an event fires on B, the event path is:
> Current: B, SR, A, Document, Window
> Proposed: B


But shouldn't B's destination insertion points be empty and from B event propagate to A.
Or do I misunderstand your example?
(B is a child node of A)


> 
> This also occurs when younger shadow roots do not have any shadow insertion
> points.
In which case exactly? If you dispatch event to a node in older shadow tree and younger
doesn't have insertion point, event should propagate only to the older shadow root.

 
> 2. Events that are Always Stopped at...where?
> The current spec of "Events that are Always Stopped" says "...stopped at the
> root node of the node tree"
> http://w3c.github.io/webcomponents/spec/shadow/#h-events-that-are-always-
> stopped
> 
> Current our implementation allows older and younger shadow roots to receive
> the event and stops before shadow hosts.
hosts? plural?

mousemove _within_ older shadow dom shouldn't propagate to the younger shadow dom
Comment 144 Koji Ishii 2015-02-13 01:39:44 UTC
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 Koji Ishii 2015-02-13 02:12:59 UTC
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 Hayato Ito 2015-02-13 03:08:49 UTC
(In reply to Olli Pettay from comment #143)
>  
> > 2. Events that are Always Stopped at...where?
> > The current spec of "Events that are Always Stopped" says "...stopped at the
> > root node of the node tree"
> > http://w3c.github.io/webcomponents/spec/shadow/#h-events-that-are-always-
> > stopped
> > 
> > Current our implementation allows older and younger shadow roots to receive
> > the event and stops before shadow hosts.
> hosts? plural?
> 
> mousemove _within_ older shadow dom shouldn't propagate to the younger
> shadow dom


That's orthogonal problem, which should be handled by event-related-target algorithm and the following sentence in http://w3c.github.io/webcomponents/spec/shadow/#event-dispatch

> If the relatedTarget and target are the same for a given node, its the event listeners must not be invoked. TouchEvent is not subject to this rule.


In addition to that, in this case, it's okay for the younger to see the mouse move within the older for the same reason that the shadow tree can see mouse move for distributed nodes in it's shadow host. I think the current algorithm is designed to achieve that.
Comment 147 Hayato Ito 2015-02-13 03:19:02 UTC
(In reply to Koji Ishii from comment #145)
> The case where younger shadow roots do not have shadow insertion points:
> 
> A
>   SR1
>     B
>   SR2
> 
> When an event fired on B:
> Current: B, SR1, A, Document, Window
> Proposed: B, SR1

I can't agree this proposal because this violates the design principle of Shadow DOM event dispatching of the following:

"Attaching a Shadow Root to an element shouldn't have any effect for an event dispatching in the node tree which the shadow host participates in, basically"

In the proposal, attaching the younger shadow root would affect the event dispatching in the document tree. That would violated the principle.
Comment 148 Koji Ishii 2015-02-13 04:45:29 UTC
(In reply to Olli Pettay from comment #143)
> (In reply to Koji Ishii from comment #141)
> > 
> > 1. The proposed algorithm stops event path for orphaned nodes
> > When target nodes are not distributed, the event path in the current spec
> > bubbles up to Document, but the proposed algorithm stops. Example:
> > 
> > A
> >   SR
> >   B
> > 
> > Note that SR does not have IP. When an event fires on B, the event path is:
> > Current: B, SR, A, Document, Window
> > Proposed: B
> 
> But shouldn't B's destination insertion points be empty and from B event
> propagate to A.
> Or do I misunderstand your example?
> (B is a child node of A)

Sorry, I simplified the test case too much. The reproducible case is:

A
  SR
    SIP
  B

In this tree, B's destination insertion point is not empty, but B is not distributed. When an event fired on B:
Current: B, SIP, SR, A, Document, Window
Proposed: B

So the two cases are similar but hit different causes:
1.1. When the shadow tree contains <shadow> but no <content>
1.2. When the younger shadow tree does not contain <shadow>

When the shadow tree does not contain either <shadow> nor <content>, you're right that both algorithms produce the same results.
Comment 149 Olli Pettay 2015-02-13 22:34:21 UTC
(In reply to Hayato Ito from comment #147)
> (In reply to Koji Ishii from comment #145)
> > The case where younger shadow roots do not have shadow insertion points:
> > 
> > A
> >   SR1
> >     B
> >   SR2
> > 
> > When an event fired on B:
> > Current: B, SR1, A, Document, Window
> > Proposed: B, SR1
> 
> I can't agree this proposal because this violates the design principle of
> Shadow DOM event dispatching of the following:
No it doesn't

> 
> "Attaching a Shadow Root to an element shouldn't have any effect for an
> event dispatching in the node tree which the shadow host participates in,
> basically"
B is a child of SR1 in this case, not part of the node tree of A.

> 
> In the proposal, attaching the younger shadow root would affect the event
> dispatching in the document tree. That would violated the principle.

No, B is a child of SR1.
(or I misunderstand where B is.)
Comment 150 Olli Pettay 2015-02-13 22:42:03 UTC
(In reply to Koji Ishii from comment #148)
> Sorry, I simplified the test case too much. The reproducible case is:
> 
> A
>   SR
>     SIP
>   B
> 
> In this tree, B's destination insertion point is not empty, but B is not
> distributed. When an event fired on B:
> Current: B, SIP, SR, A, Document, Window
> Proposed: B

Wait, what is the destination insertion point for B in this case? And why?


The spec says
"Each node that is not an insertion point has an ordered list, called destination insertion points, which consists of insertion points to where the node is distributed"
So how can B's destination insertion point list be non-empty but B is not distributed. 
That seems to contradict with the definition of 'destination insertion points'.

Or do I misunderstand something here?
Comment 151 Hayato Ito 2015-02-16 01:50:49 UTC
(In reply to Olli Pettay from comment #149)
> (In reply to Hayato Ito from comment #147)
> > (In reply to Koji Ishii from comment #145)
> > > The case where younger shadow roots do not have shadow insertion points:
> > > 
> > > A
> > >   SR1
> > >     B
> > >   SR2
> > > 
> > > When an event fired on B:
> > > Current: B, SR1, A, Document, Window
> > > Proposed: B, SR1
> > 
> > I can't agree this proposal because this violates the design principle of
> > Shadow DOM event dispatching of the following:
> No it doesn't
> 
> > 
> > "Attaching a Shadow Root to an element shouldn't have any effect for an
> > event dispatching in the node tree which the shadow host participates in,
> > basically"
> B is a child of SR1 in this case, not part of the node tree of A.
> 
> > 
> > In the proposal, attaching the younger shadow root would affect the event
> > dispatching in the document tree. That would violated the principle.
> 
> No, B is a child of SR1.
> (or I misunderstand where B is.)


Let me  make it clear what I said as follows:

- Attaching a shadow root to any node in any node tree doesn't affect an event dispatching which will happen on any node, in the every node trees, except for the newly attached shadow tree, basically.



In the example case, we can interpret this as follows:

- Attaching a shadow root (SR1) to any node (A) in any node tree (document tree) doesn't affect an event dispatching which will happen on any node (B), in the every node trees (both document tree and SR1 tree), except for the newly attached shadow tree (SR2 tree), basically.


"Basically" here means "as long as we don't call event.stopPropagation() in the newly attached shadow tree".
Comment 152 Koji Ishii 2015-02-16 01:56:24 UTC
(In reply to Olli Pettay from comment #150)
> (In reply to Koji Ishii from comment #148)
> > Sorry, I simplified the test case too much. The reproducible case is:
> > 
> > A
> >   SR
> >     SIP
> >   B
> > 
> > In this tree, B's destination insertion point is not empty, but B is not
> > distributed. When an event fired on B:
> > Current: B, SIP, SR, A, Document, Window
> > Proposed: B
> 
> Wait, what is the destination insertion point for B in this case? And why?
> 
> The spec says
> "Each node that is not an insertion point has an ordered list, called
> destination insertion points, which consists of insertion points to where
> the node is distributed"
> So how can B's destination insertion point list be non-empty but B is not
> distributed. 
> That seems to contradict with the definition of 'destination insertion
> points'.
> 
> Or do I misunderstand something here?

I agree it's weird. I had to re-read the spec when investigating this issue, but the SIP above meets the criteria defined in "3.3 Shadow Insertion Points"
http://w3c.github.io/webcomponents/spec/shadow/#h-shadow-insertion-points

I opened bug 28031 for this. I'm ok either way, just want it to be interoperable.
Comment 153 Hayato Ito 2015-02-16 02:12:05 UTC
(In reply to Koji Ishii from comment #152)
> (In reply to Olli Pettay from comment #150)
> > (In reply to Koji Ishii from comment #148)
> > > Sorry, I simplified the test case too much. The reproducible case is:
> > > 
> > > A
> > >   SR
> > >     SIP
> > >   B
> > > 
> > > In this tree, B's destination insertion point is not empty, but B is not
> > > distributed. When an event fired on B:
> > > Current: B, SIP, SR, A, Document, Window
> > > Proposed: B
> > 
> > Wait, what is the destination insertion point for B in this case? And why?
> > 
> > The spec says
> > "Each node that is not an insertion point has an ordered list, called
> > destination insertion points, which consists of insertion points to where
> > the node is distributed"
> > So how can B's destination insertion point list be non-empty but B is not
> > distributed. 
> > That seems to contradict with the definition of 'destination insertion
> > points'.
> > 
> > Or do I misunderstand something here?
> 
> I agree it's weird. I had to re-read the spec when investigating this issue,
> but the SIP above meets the criteria defined in "3.3 Shadow Insertion Points"
> http://w3c.github.io/webcomponents/spec/shadow/#h-shadow-insertion-points
> 
> I opened bug 28031 for this. I'm ok either way, just want it to be
> interoperable.

I'm afraid that you didn't understand what Olli pointed out.

> > > In this tree, B's destination insertion point is not empty, but B is not
> > > distributed. When an event fired on B:

I don't understand what this sentence mean...
"node is not distributed" should mean "node's destination insertion is empty". That's the spec's intention.
Comment 154 Koji Ishii 2015-02-16 05:30:51 UTC
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 Koji Ishii 2015-02-16 06:32:50 UTC
I put test cases on jsbin: http://jsbin.com/casoyi

Chrome Canary:
1. B1,SIP,SR1,H1,BODY,HTML,[object HTMLDocument],[object Window]
2. B2,SR2,H2,BODY,HTML,[object HTMLDocument],[object Window]
3. C3,SR4

Chrome with experimental impl:
1. B1
2. B2,SR2
3. C3,SR4

Could you tell me what the current Gecko gives? That'd help us to figure out if the bug is in algorithm, or in my impl.

The third case was added because I added an assertion to ensure the INSERTION-POINTS stack is empty at the end of the algorithm, and it fails on one of our tests for editing. The produced event paths are the same. Not sure if this is an non-issue, or issues in both algorithm. Hayato, do you have insights?
Comment 156 Koji Ishii 2015-02-16 06:41:38 UTC
Correction: on test case 3, events should fire on B3. Updated the jsbin: http://jsbin.com/casoyi

Chrome Canary:
1. B1,SIP,SR1,H1,BODY,HTML,[object HTMLDocument],[object Window]
2. B2,SR2,H2,BODY,HTML,[object HTMLDocument],[object Window]
3. B3,SHADOW,SHADOW,SR5,C3,SR4,H3,BODY,HTML,[object HTMLDocument],[object Window]
4. C3,SR4

Chrome with experimental impl:
1. B1
2. B2,SR2
3. B3
4. C3,SR4
Comment 157 Olli Pettay 2015-02-16 11:37:39 UTC
(In reply to Hayato Ito from comment #151)
> 
> Let me  make it clear what I said as follows:
> 
> - Attaching a shadow root to any node in any node tree doesn't affect an
> event dispatching which will happen on any node, in the every node trees,
> except for the newly attached shadow tree, basically.
> 
> 
> 
> In the example case, we can interpret this as follows:
> 
> - Attaching a shadow root (SR1) to any node (A) in any node tree (document
> tree) doesn't affect an event dispatching which will happen on any node (B),
> in the every node trees (both document tree and SR1 tree), except for the
> newly attached shadow tree (SR2 tree), basically.
> 
Ah, you mean that. It is indeed then possible that the algorithm needs some tweaking, since it was written
during the time when older shadow trees not distributed to any shadow insertion point were thought to be not-in-document.
Comment 158 Olli Pettay 2015-02-16 11:50:26 UTC
(In reply to Olli Pettay from comment #157)
> Ah, you mean that. It is indeed then possible that the algorithm needs some
> tweaking, since it was written
> during the time when older shadow trees not distributed to any shadow
> insertion point were thought to be not-in-document.
(and that is in fact still the behavior in Gecko, since it kind of makes sense that one can sort of replace the older shadow tree. Going through bug 26365 to see why the spec has different definition atm.)
Comment 159 Koji Ishii 2015-02-16 15:45:45 UTC
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 Koji Ishii 2015-02-17 01:46:43 UTC
Incorporated case 2 of comment 141 into http://jsbin.com/casoyi

Chrome Canary:
1. click on B1: B1, SIP1, SR1, H1, BODY, HTML, [object HTMLDocument], [object Window]
2. click on B2: B2, SR2, H2, BODY, HTML, [object HTMLDocument], [object Window]
3. selectstart on B3: B3, SR4, SIP2, SR5
4. selectstart on B4: B4, SIP2, SIP3, SR7, C3, SR6, H4, BODY, HTML, [object HTMLDocument], [object Window]

Experimental impl on Chrome:
1. click on B1: B1
2. click on B2: B2, SR2
3. selectstart on B3: B3, SR4
4. selectstart on B4: B4

Notes:
 * Issue 2 is about the orphaned nodes.
 * Issue 4 exits the algorithm with non-empty stack.

If you see differences in Gecko, it's possible that my interpretation of https://bugzilla.mozilla.org/show_bug.cgi?id=1059989#c10 is wrong.
Comment 161 Hayato Ito 2015-02-17 07:56:06 UTC
(In reply to Olli Pettay from comment #158)
> (In reply to Olli Pettay from comment #157)
> > Ah, you mean that. It is indeed then possible that the algorithm needs some
> > tweaking, since it was written
> > during the time when older shadow trees not distributed to any shadow
> > insertion point were thought to be not-in-document.
> (and that is in fact still the behavior in Gecko, since it kind of makes
> sense that one can sort of replace the older shadow tree. Going through bug
> 26365 to see why the spec has different definition atm.)


Yeah, that's a important (and confusing) point.

Recently, to makes things simple, I've tried to think as follows:

- Insertion points don't have any effect on *DOM connectivity*.
- Insertion points have an effect on how style inheritance is calculated. As a result, some nodes would became 'display: none', however, they are still considered as *connected*.
- From this point of view, we shouldn't call the older shadow tree *orphaned* even when the younger shadow tree doesn't have a shadow insertion point.
  It's something like: 'display: none' is set to the *pseudo parent element* of (child nodes of the older shadow root).


I've tried to make the current spec consistent with this concept, recently. However, I agree we should say it more explicitly (and in a friendly way), maybe in a non-normative section.
Comment 162 Olli Pettay 2015-02-17 11:28:33 UTC
(In reply to Hayato Ito from comment #161)
> - Insertion points don't have any effect on *DOM connectivity*.
But they very much have DOM "connectivity", since they do need to affect to
the DOM event dispatch.


But as I said in bug 26365, I can probably live with the approach where
non-distributed older shadow doms are still in the document if the host is.
(That approach may makes certain things easier in implementations, but behavior and functionality a bit weirder.)
Comment 163 Olli Pettay 2015-02-23 19:03:29 UTC
So we want to change
3. If CURRENT is not the youngest shadow root hosted by SHADOW-POOL-HOST:
to something like

3. If CURRENT is not the youngest shadow root hosted by SHADOW-POOL-HOST and CURRENT is distributed to a shadow insertion point:
Comment 164 Koji Ishii 2015-02-25 19:07:04 UTC
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 Olli Pettay 2015-02-26 15:14:31 UTC
(In reply to Koji Ishii from comment #164)
> Hayato and I discussed today and came up with a new proposal, in the hope of
> achieving both-win.
Don't understand what "both-win" refers to.

> 
> The proposal is to add an exception to the spec:
> 
> CURRENT:
> The event path calculation algorithm must be used to determine event path
> and must be equivalent to processing the following steps
> 
> PROPOSED:
> The event path must be equivalent to processing the event path calculation
> algorithm described below, except that insertion points may appear in
> different order, as long as they’re after the content distributed to them
> (or fallback content in the case of fallback) and before their parents
I don't understand what this all means.
 
> Still we needed to try hard to come up with a better algorithm, one because
> mental models vary by people, and the other because all of us want this
> algorithm simple and fast, while the “simple and fast” was different by the
> underlying architecture.
> 
> The algorithm in the current spec works good for Blink architecture, while,
> as far as I understand from the discussion in Bugzilla, it adds insanely
> complex burden to Gecko.
The main issue with the current algorithm in the spec is its inconsistency and mentally really
hard to get right. 
What is wrong with the proposed change + the fix similar to comment 163?

I very much thought we had agreement here and just had to tweak the proposed algorithm a bit.

> On the other hand, we implemented the proposed new algorithm, but we figured
> out that it makes our code more complex, and we are still unable to fix 3
> regressions.
> 
What regressions?



> Our conclusion is to allow implementation dependency in the manner of not to
> trouble web developers is the only way to make all of us happy.
I hope you don't mean we would have implementation specific event dispatch? That is something Gecko will not do.
Shadow DOM will not be enabled by default before we have a good enough spec for it and the implementation follows it.
Comment 166 Dimitri Glazkov 2015-02-26 15:42:34 UTC
(In reply to Olli Pettay from comment #165)

> I hope you don't mean we would have implementation specific event dispatch?
> That is something Gecko will not do.
> Shadow DOM will not be enabled by default before we have a good enough spec
> for it and the implementation follows it.

Agreed. Leaving event dispatch open to implementer interpretation sounds like a lose-lose, not a win-win.
Comment 167 Koji Ishii 2015-02-27 03:08:11 UTC
(In reply to Olli Pettay from comment #165)
> (In reply to Koji Ishii from comment #164)
> > Hayato and I discussed today and came up with a new proposal, in the hope of
> > achieving both-win.
> Don't understand what "both-win" refers to.

I meant to make all impls simple and fast regardless of underlying architecture, and make all insertion points appear in the event path.

> What is wrong with the proposed change + the fix similar to comment 163?
>
> I very much thought we had agreement here and just had to tweak the proposed
> algorithm a bit.
> 
> > On the other hand, we implemented the proposed new algorithm, but we figured
> > out that it makes our code more complex, and we are still unable to fix 3
> > regressions.
> > 
> What regressions?

The Comment 160 reported failures of 4 test case at <http://jsbin.com/casoyi>. I saw your comment to work on not-distributed case, and I suppose comment 163 is the one, but I don't think it fixes other cases.

I can implement it to re-test if you believe it fixes all 4 issues, but can't figure out what the proposal in comment 163 is. Could you clarify:

> ...and CURRENT is distributed to a shadow insertion point:

Did you mean "...and at least one shadow insertion point is the destination insertion points" or "...and the final destination point is a shadow insertion point"?

> > Our conclusion is to allow implementation dependency in the manner of not to
> > trouble web developers is the only way to make all of us happy.
>
> I hope you don't mean we would have implementation specific event dispatch?
> That is something Gecko will not do.
> Shadow DOM will not be enabled by default before we have a good enough spec
> for it and the implementation follows it.

Hm, looks like we have some misunderstanding. Can you clarify what I miss?

1. You'd like to change the algorithm in the current spec.
2. The proposed algorithm showed 4 regressions.
3. You kindly tweaked the proposed algorithm for one of them (not really sure "one" until I understand comment 163 and re-test), but nobody has raised hands to solve other issues.

From these understandings, options I can see are:
A. Someone fixes all other issues in the proposed algorithm
B. No insertion points
C. Go with the current spec
D. Stuck

If A is not happening, the new proposal looks still better than B-D, no?
Comment 168 Hayato Ito 2015-02-27 04:38:51 UTC
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 Olli Pettay 2015-03-01 17:01:23 UTC
(In reply to Koji Ishii from comment #167)
> Hm, looks like we have some misunderstanding. Can you clarify what I miss?
> 
> 1. You'd like to change the algorithm in the current spec.
yes




(http://jsbin.com/casoyi/edit?html,js,output
Note, that uses the old syntax for <shadow>. It was agreed long ago <shadow> would need to have <content> as a child.
But I can't now recall where that change was agreed. But that isn't really about this bug.)

>Chrome Canary:
>1. click on B1: B1, SIP1, SR1, H1, BODY, HTML, [object HTMLDocument], [object Window]
>2. click on B2: B2, SR2, H2, BODY, HTML, [object HTMLDocument], [object Window]
>3. selectstart on B3: B3, SR4, SIP2, SR5
>4. selectstart on B4: B4, SIP2, SIP3, SR7, C3, SR6, H4, BODY, HTML, [object HTMLDocument], [object Window]

>Experimental impl on Chrome:
>1. click on B1: B1
Ok, so this case would need another tweak
"Repeat while CURRENT is a shadow insertion point."
->
"Repeat while CURRENT is a shadow insertion point and there is a shadow root distributed to it".

2. click on B2: B2, SR2
This is fixed by comment 163.

3. selectstart on B3: B3, SR4
This is the expected behavior. selectstart shouldn't be exposed outside its initial shadow DOM.
The spec says "The following events must always be stopped at the root node of the node tree" and SR4 is a root.

4. selectstart on B4: B4
and this is similar to (1)
Comment 170 Koji Ishii 2015-03-04 00:08:16 UTC
(In reply to Olli Pettay from comment #169)
> (In reply to Koji Ishii from comment #167)
> > Hm, looks like we have some misunderstanding. Can you clarify what I miss?
> > 
> > 1. You'd like to change the algorithm in the current spec.
> yes

Great, thank you for your support.

> (http://jsbin.com/casoyi/edit?html,js,output
> Note, that uses the old syntax for <shadow>. It was agreed long ago <shadow>
> would need to have <content> as a child.
> But I can't now recall where that change was agreed. But that isn't really
> about this bug.)

I wasn't aware of that, will check.

> >Chrome Canary:
> >1. click on B1: B1, SIP1, SR1, H1, BODY, HTML, [object HTMLDocument], [object Window]
> >2. click on B2: B2, SR2, H2, BODY, HTML, [object HTMLDocument], [object Window]
> >3. selectstart on B3: B3, SR4, SIP2, SR5
> >4. selectstart on B4: B4, SIP2, SIP3, SR7, C3, SR6, H4, BODY, HTML, [object HTMLDocument], [object Window]
> 
> >Experimental impl on Chrome:
> >1. click on B1: B1
> Ok, so this case would need another tweak
> "Repeat while CURRENT is a shadow insertion point."
> ->
> "Repeat while CURRENT is a shadow insertion point and there is a shadow root
> distributed to it".
> 
> 2. click on B2: B2, SR2
> This is fixed by comment 163.

Ok, I'll try.

> 3. selectstart on B3: B3, SR4
> This is the expected behavior. selectstart shouldn't be exposed outside its
> initial shadow DOM.
> The spec says "The following events must always be stopped at the root node
> of the node tree" and SR4 is a root.
> 
> 4. selectstart on B4: B4
> and this is similar to (1)

True that text and algorithm contradict to each other here. Hayato confirmed that the algorithm is correct. Please refer to bug 28008. IIUC, otherwise it will contradict with the policy of creating younger shadow roots should not change behavior of older shadow roots.
Comment 171 Koji Ishii 2015-03-04 06:11:06 UTC
(In reply to Koji Ishii from comment #170)
> (In reply to Olli Pettay from comment #169)
> 
> > >1. click on B1: B1
> > Ok, so this case would need another tweak
> > "Repeat while CURRENT is a shadow insertion point."
> > ->
> > "Repeat while CURRENT is a shadow insertion point and there is a shadow root
> > distributed to it".
> > 
> > 2. click on B2: B2, SR2
> > This is fixed by comment 163.
> 
> Ok, I'll try.

Confirmed that these two changes fixed the 2 issues, but new 7 tests start failing. Here's the Blink test results:
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/50623/layout-test-results/results.html

Olli, do you want to keep fixing the proposed algorithm? If so, does the above page provide good enough information for you to work?
Comment 172 Olli Pettay 2015-03-04 13:34:39 UTC
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 Koji Ishii 2015-04-09 06:25:15 UTC
Status updates:

1. Olli and I are working on fixing the algorithm. I forgot to update this bug for a while, sorry about that. I will try to keep the status updated here.

2. The new algorithm proposed here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1059989#c10
was tweaked by comments in this bug and as Olli and I discuss. I'm keeping the updated algorithm in a branch of the spec here:
https://github.com/kojiishi/webcomponents/blob/event-path-23887/spec/shadow/index.html

3. A WIP patch implementing the algorithm to Blink is here:
https://codereview.chromium.org/906123002/
Olli and I are using Blink tests to find/fix issues in the algorithm. We've narrowed down to 4 more test failures to attack.

Personally, I still can't be very clear that this additional complexity in the spec and in the implementation is worth the perceived benefits. But I think that comparison should be done once the algorithm is done without known flaws, and the final call would be up to the WG, so I'm putting that aside for now.
Comment 174 Koji Ishii 2015-04-15 01:53:38 UTC
While I'm still unable to fix the 4 test failures in the proposed algorithm, I took another approach to re-start from the current algorithm and came up with "candidate 2" algorithm. This algorithm:
 * Produces the same result as specified in comment 115
 * Passes all tests in Blink
 * Produces the same results as the currently proposed algorithm for all tests in Blink, except the 4 failing tests

Olli told me that he can review this tomorrow or so, and we can update you all here after Olli and I discuss, but comments are appreciated if any.

"Candidate 2" algorithm:
https://cdn.rawgit.com/kojiishi/webcomponents/125051d4ea5973b459fc291636ccef8d0508501f/spec/shadow/index.html#bug23887-koji
(If re-spec prevents your browser to jump to the point, please find "candidate 2")

Blink WIP patch:
https://codereview.chromium.org/906123002/
Comment 175 Hayato Ito 2015-04-15 03:07:17 UTC
Thank you for updating the subject, however,


>  [Shadow] Update event retargeting algorithm to never violate the rule of parentNode always preceeding the node in event path 

the new subject is not correct because the current algorithm doesn't violate the rule.

I think comment #124 is still a good summary of the new proposal.
https://www.w3.org/Bugs/Public/show_bug.cgi?id=23887#c124

Let me update the subject so that it reflects the proposal correctly.
Comment 176 Olli Pettay 2015-04-15 20:43:52 UTC
(In reply to Koji Ishii from comment #174)
> "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")
That is very similar to the older proposed algorithm, but seems to then
fix some of its issues. I didn't look at how it ended up fixing those, but
c2 looks indeed rather good to me, and gives the expected results at least in
the cases I evaluated on paper.
Comment 177 Koji Ishii 2015-04-17 06:02:40 UTC
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:
https://github.com/w3c/webcomponents/pull/42
Comment 178 Hayato Ito 2015-04-17 06:47:40 UTC
Thank you. Now we have a finally *working* proposal, right? :)

Let me have a look carefully.
Comment 179 Koji Ishii 2015-04-17 07:06:07 UTC
(In reply to Hayato Ito from comment #178)
> Thank you. Now we have a finally *working* proposal, right? :)

Yes, finally ;)

> Let me have a look carefully.

Thank you, look forward to hearing from you.

Blink patch is also updated in case it helps:
https://codereview.chromium.org/906123002/
Comment 180 Hayato Ito 2015-04-17 07:57:19 UTC
Bad news for you :(


> Exclude shadow insertion points whose containing shadow root has older shadow roots from INSERTION-POINTS

I'm afraid this this would take us to the lose situation.


For example:

(Let me allow to use the pseudo notation to describe a tree of trees)


<div id="shadow-host">
  <shadow-root id="oldest-shadow-root-of-shadow-host">
    <div id="a">
  <shadow-root id="youngest-shadow-root-of-shadow-host">
    <div id="shadow-host-b">
      <shadow id="shadow">
      <shadow-root id="shadow-root-of-shadow-host-b">
        <content id="content">

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 Hayato Ito 2015-04-17 08:05:03 UTC
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 Koji Ishii 2015-04-17 08:09:01 UTC
(In reply to Hayato Ito from comment #180)
> Bad news for you :(

Thank you for having a look, actually, it's a great news to find issues earlier. I'll add a test for this case  and look into further.
Comment 183 Hayato Ito 2015-04-17 08:16:17 UTC
(In reply to Koji Ishii from comment #182)
> (In reply to Hayato Ito from comment #180)
> > Bad news for you :(
> 
> Thank you for having a look, actually, it's a great news to find issues
> earlier. I'll add a test for this case  and look into further.

Thanks! I appreciate that.
Comment 184 Koji Ishii 2015-04-17 09:52:23 UTC
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 Hayato Ito 2015-04-17 11:21:30 UTC
I have an idea to resolve this. Let me tackle this issue later.
I'd like to have a simple clean algorithm.
Comment 186 Koji Ishii 2015-04-18 04:40:29 UTC
To share what Hayato and I discussed with others (before comment #185):

 * The c1/c2 results do NOT violate principals and thus acceptable
 * It's probably better for SR1 after SR2, right before SH1.
 * Hayato would try if fixing that could simplify the algorithm further, from the other principle that the simpler is the better.
Comment 187 Hayato Ito 2015-04-20 03:21:26 UTC
Okay. Let me propose the new algorithm. Let's call this "hayato-2015-04" algorithm.

1. Let PATH be the empty ordered list of nodes
2. Let INSERTION-POINT-STACK be an empty stack of nodes
3. Let CURRENT be NODE
4. Repeat while CURRENT exists:
   1. Add CURRENT to PATH
   2. Let INSERTION-POINTS be the destination insertion points of CURRENT
   3. If INSERTION-POINTS is not empty:
      1. Push INSERTION-POINTS into INSERTION-POINT-STACK in order of first destination to final destination
      2. Pop INSERTION-POINT-STACK and set CURRENT to be the popped node.
   4. Otherwise if CURRENT is a shadow root:
      1. If CURRENT is the oldest shadow root:
         1. Let HOST be the shadow host which hosts CURRENT
      2. Otherwise:
         1. Let HOST be the older shadow root relative to CURRENT
      3. If INSERTION-POINT-STACK is not empty and the most recent node in the INSERTION-POINT-STACK is in the same node tree as HOST
         1. Pop INSERTION-POINT-STACK and set CURRENT to be the popped node.
      4. Otherwise if CURRENT and NODE participates in the same node tree and EVENT is one of the events which must be stopped:
         1. Stop this algorithm
      5. Otherwise:
         1. Let CURRENT be HOST
   5. Otherwise:
      1. Let CURRENT be the parent node of CURRENT



"hayato-2015-04" is based on my mental model - A concept of "Multiple shadow roots" are just a syntax sugar for "the older shadow root *hosts* the younger shadow root".
According to this concept, we can merge (4.4.1) and (4.4.2) further by the following one sentence:

  "Let HOST be the shadow host (or the shadow root) which *hosts* CURRENT".

Note that this is the moment where we move from the child tree to the parent tree in a tree of trees. The new algorithm matches a concept of a tree of trees nicely.

    
As a result, "hayato-2015-04" doesn't have to use a term of {older shadow root, younger shadow root, oldest shadow root, youngest shadow root} nor {content insertion point, shadow insertion point} at all. The algorithm doesn't need to distinguish them. I'm feeling this is a good signal.


One side effect:
- Now the older shadow root receives an event after the younger shadow root. However, this behavior matches "A shadow host receives an event after the oldest shadow root".
  
I can say, in a uniform way, "The root node of the parent tree receives an event after the root node of the child tree" in a tree of trees.

  
Caution: The new algorithm is only evaluated in my head in a short time. I'll have another look later, however, I appreciate if you could give me early feedback since I'm flying to Mountain View / San Francisco soon.
Comment 188 Koji Ishii 2015-04-20 03:52:11 UTC
Thank you Hayato, I'll implement and run the tests.
Comment 189 Koji Ishii 2015-04-20 08:51:50 UTC
Hayato's algorithm and its test results are here:
https://codereview.chromium.org/906123002/

* It changes the result for the spec example to:
#D, #C, #M, #L, #R, #Q, #P, #O, #N, #K, #J, #I, #H, #G, #U, #T, #S, #F, #X, #W, #V, #E, #B, #A
* There's one case where the length of the path is longer by one additional node.

I'll examine these changes, but Hayato, if you could look at them from the view point that they match to what you're trying to accomplish, that'd be helpful too.

I'll post my analysis later on.
Comment 190 Olli Pettay 2015-04-20 17:58:14 UTC
#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 Hayato Ito 2015-04-20 21:48:56 UTC
(In reply to Koji Ishii from comment #189)
> Hayato's algorithm and its test results are here:
> https://codereview.chromium.org/906123002/
> 
> * It changes the result for the spec example to:
> #D, #C, #M, #L, #R, #Q, #P, #O, #N, #K, #J, #I, #H, #G, #U, #T, #S, #F, #X,
> #W, #V, #E, #B, #A

This result is expected one for me. I'm glad to see this result.

As for the position of #E, the short answer is:

  If we remove an arrow from B (the shadow host) to V (the youngest shadow root), then add a new arrow from E (the older shadow root) to V (the younger shadow root) in Fig 5 in the spec, [1], this result might look natural.

The long answer is:
  Let me prepare a document how we should view multiple shadow roots. I briefly explained it in comment #187, however, I need to explain further.


You might notice that the modified Fig 5 is similar to a Fig 1, [2], which explains a tree of trees. Ignore dotted arrows there. Just focus the non-dotted arrows.
There, we can view the fig as follows:

- "shadow root in C *hosts* D"
- "shadow root in D *hosts* E"

Yes, we can remove "Multiple" from our head.

If an element A hosts multiple shadow roots, [B, C, D], where B is the oldest shadow root and D is the youngest shadow root, we can view this "Multiple shadow roots" as follows:

- An element, A,  hosts a shadow root, B
- The shadow root, B, *hosts* another shadow root, C.
- The shadow root, C, *hosts* another shadow root, D.

I found that if we can view multiple shadow roots in this way, we could make a lot of things much cleaner and simpler. That's the motivation why I defined a tree of trees as the current spec defines.

I'll update this thread when I write a document, which wouldn't be long one, I guess.


[1] Shadow DOM spec Fig. 5: http://w3c.github.io/webcomponents/spec/shadow/index.htmlfig-an-example-tree-of-trees.-nodes-which-are-not-involved-in-the-example-event-path-which-is-explained-later-are-omitted.x

[2] Shadow DOM spec Fig. 1: http://w3c.github.io/webcomponents/spec/shadow/index.html#fig-a-tree-of-trees.x
Comment 192 Koji Ishii 2015-04-21 05:04:27 UTC
Finished analyzing the test results, here are two differences from current/candidates algorithms.

1. As Hayato explained in the comment #191, older shadow roots are moved from "before the IP they're distributed to" to "after younger shadow roots".

D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,E,X,W,V,B,A
D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,X,W,V,E,B,A

2. Older shadow roots that were not in the path are now included. Two examples below, when event target is "div <-- target", SR-OLDEST and SR-OLDER are now included.

HOST
  div <-- target
  SR-OLDEST
    div
  SR-MIDDLE
    content
  SR-YOUNGEST
    shadow

HOST
  SR-OLDER
  SR-YOUNGER
    shadow
    div <-- target

I'm fine with either algorithm; both makes each own logical sense, do not break the principal rules, and technically correct (2nd diff above needs confirmation from Hayato though.)

I think the most valuable thing at this point is to stabilize the algorithm and make implementations interoperable. Hope the WG resolves this issue this week.
Comment 193 Hayato Ito 2015-04-22 00:37:42 UTC
Thank you for analyzing.

(In reply to Koji Ishii from comment #192)
> Finished analyzing the test results, here are two differences from
> current/candidates algorithms.
> 
> 1. As Hayato explained in the comment #191, older shadow roots are moved
> from "before the IP they're distributed to" to "after younger shadow roots".
> 
> D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,E,X,W,V,B,A
> D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,X,W,V,E,B,A
> 
> 2. Older shadow roots that were not in the path are now included. Two
> examples below, when event target is "div <-- target", SR-OLDEST and
> SR-OLDER are now included.
> 
> HOST
>   div <-- target
>   SR-OLDEST
>     div
>   SR-MIDDLE
>     content
>   SR-YOUNGEST
>     shadow
> 
> HOST
>   SR-OLDER
>   SR-YOUNGER
>     shadow
>     div <-- target

Both are expected to me. Unless there is a use case where this causes a trouble, I'm okay to have this behavior. This behavior is fine because I think we shouldn't skip an intermediate node tree in a tree of trees in dispatching an event.

A node trees which are involved in dispatching an event should be connected, I think.
Comment 194 Hayato Ito 2015-04-22 00:40:11 UTC
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 Olli Pettay 2015-04-23 16:26:03 UTC
(In reply to Koji Ishii from comment #192)
> Finished analyzing the test results, here are two differences from
> current/candidates algorithms.
> 
> 1. As Hayato explained in the comment #191, older shadow roots are moved
> from "before the IP they're distributed to" to "after younger shadow roots".
> 
> D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,E,X,W,V,B,A
> D,C,M,L,R,Q,P,O,N,K,J,I,H,G,U,T,S,F,X,W,V,E,B,A
> 


And that is the issue. Now older shadow trees would have all the power to
affect whether event even enters to younger tree (by using capturing listeners), 
but from rendering point of view the younger tree is still the parent of the older.
In my mind the older tree is always distributed to <shadow>, which means that
older shadow root is the connecting link between the older shadow tree and the
<shadow> in the younger one.
In other words <shadow> "shadow hosts" the older tree.
Comment 196 Hayato Ito 2015-04-27 04:53:21 UTC
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.
Comment 197 Hayato Ito 2015-05-27 03:01:48 UTC
Moved to https://github.com/w3c/webcomponents/issues/98