But we want to hide the shadow dom from the rest of the page as much as 
possible. Otherwise shadow DOM becomes rather meaningless.
If the page outside the shadow DOM needs to know about the shadow DOM,
it feels like shadow DOM shouldn't be used at all for that use case.



(In Gecko there is privileged JS code only
EventTarget[] getEventTargetChain(), but that is for devtools and such)

This is an exciting proposal, but I think it belongs in DOM core. We should expose event path as Event.path.

To alleviate Olli's concerns, for Shadow DOM, the Event.path should be adjusted to show only the chunk of the path that's visible in the context of a given tree.

If you expose just that though, you can find out the path yourself I think. We could consider adding that though.

(In reply to comment #3)
> If you expose just that though, you can find out the path yourself I think.
> We could consider adding that though.

You can only find the even path by building it, right? Since the tree could be mutated by event listeners, the path is unknowable until traveled.

That is a good point, and although comment 0 did not describe that particular situation, it does make a slightly more convincing case.

As explained in https://www.w3.org/Bugs/Public/show_bug.cgi?id=21067, it looks to me that it is good enough to have getDistributedParent() API rather than event.path to address the use case of #c0.

Created attachment 1343 [details]
event path use case

(In reply to comment #6)
> As explained in https://www.w3.org/Bugs/Public/show_bug.cgi?id=21067, it
> looks to me that it is good enough to have getDistributedParent() API rather
> than event.path to address the use case of #c0.

insertionParent is not sufficient because it does not account for re-distribution.

This is noted in https://www.w3.org/Bugs/Public/show_bug.cgi?id=21067#c11

Oops, I meant that insertionParent is not sufficient because it does not account for *reprojection*.

(In reply to comment #8)
> insertionParent is not sufficient because it does not account for
> re-distribution.
> 
> This is noted in https://www.w3.org/Bugs/Public/show_bug.cgi?id=21067#c11

Not sure I understand that comment. insertionParent should return the same
object which is the parent in the event target chain.

Given:
<a>
  <d>
SR
  <b>
    <content id="a">
  SR
  <c>  
    <content id="b">

The unflattened tree is:

<a>
  <b>
    <c>
      <content id="b">
        <content id="a">
          <d>

In the current implementation of insertionParent in blink, the insertionParent of <d> is content#b. Thus, one cannot see content#a in the event path. Perhaps we need a more precise definition of insertionParent.

Intuitively, insertionParent should return a value that would allow its consumer to reconstruct the composed tree. Otherwise, the consumer will be sad.

At one point Erik and I proved that there was no way to define an insertionParent that could reconstruct the event path. Unfortunately we did that in chat, and I don't remember the details. I will talk to him and try to rebuild that argument.

(In reply to comment #11)
> Given:
> <a>
>   <d>
> SR
>   <b>
>     <content id="a">
>   SR
>   <c>  
>     <content id="b">
> 
> The unflattened tree is:
> 
> <a>
>   <b>
>     <c>
>       <content id="b">
>         <content id="a">
>           <d>
> 
> In the current implementation of insertionParent in blink, the
> insertionParent of <d> is content#b. Thus, one cannot see content#a in the
> event path. Perhaps we need a more precise definition of insertionParent.

The implementation in Blink seems wrong to me. I think <d>.insertionParent should be #a; #a.insertionParent should be #b; and all other .insertionParent null.

(In reply to comment #14)

> The implementation in Blink seems wrong to me. I think <d>.insertionParent
> should be #a; #a.insertionParent should be #b; and all other
> .insertionParent null.

+1. I think .insertionParent should be sufficient to reconstruct the event path. Everything else seems like a bug. But Hayato-san spent a lot more time thinking on this. I want him to chime in with buckets of wisdom.

InsertionParent cannot be used to generate the event path. Here's an example:

<a>
	<span id=”one” />
	<span id=”two” />
SR-a
	<b>
		<content id=”contentA”></content>
	SR-b
		<content id=”contentB” select=”#one”></content>
		<content id=”contentB2” select=”#two”></content>


click -> #one:
#one
[contentA]
[contentB]
SR-b
b
SR-a
a

click -> #two:
#two
[contentA]
[contentB2]
SR-b
b
SR-a
a

#contentA's insertionParent cannot be both #contentB and #contentB2.

(In reply to comment #16)
> InsertionParent cannot be used to generate the event path. Here's an example:
> 
> <a>
> 	<span id=”one” />
> 	<span id=”two” />
> SR-a
> 	<b>
> 		<content id=”contentA”></content>
> 	SR-b
> 		<content id=”contentB” select=”#one”></content>
> 		<content id=”contentB2” select=”#two”></content>
> 
> 
> click -> #one:
> #one
> [contentA]
> [contentB]
> SR-b
> b
> SR-a
> a
> 
> click -> #two:
> #two
> [contentA]
> [contentB2]
> SR-b
> b
> SR-a
> a
> 
> #contentA's insertionParent cannot be both #contentB and #contentB2.

That's right. That's the reason we cannot use the current implementation of element.insertionParent() to emulate event.path as I noted in https://www.w3.org/Bugs/Public/show_bug.cgi?id=21067#c11

- event.path should contain insertion points, which does not appear in the flattened tree.
  The current element.insertionParent() cannot be used to traverse all insertion points in case of reprojection.
- We need a *context* to determine insertionPoint.insertionParent().
  e.g. To determine #contentA's insertionParent, we need an additional context (from where we are starting a path).
  In the above case, the context is #one or #two.

One idea is to make insertionParent() take additional parameter.

For example:
   contentA.insertionParent(#one) -> contentB
   contentA.insertionParent(#two) -> contentB2

Now, event.path can be emulated:

  var originalEventTarget = ....; // #one or #two
  var element = originalEventTarget;
  var eventPath = [element];
  while (element.insertionParent(originalEventTarget)) {
     element = element.insertionParent(originalEventTarget);
     // element might be an 'insertion point' in this case.
     eventPath.push(element);
  }

WDYT? Is this a reasonable API?

(In reply to comment #17)
> One idea is to make insertionParent() take additional parameter.
> 
> For example:
>    contentA.insertionParent(#one) -> contentB
>    contentA.insertionParent(#two) -> contentB2
> 
> Now, event.path can be emulated:
> 
>   var originalEventTarget = ....; // #one or #two
>   var element = originalEventTarget;
>   var eventPath = [element];
>   while (element.insertionParent(originalEventTarget)) {
>      element = element.insertionParent(originalEventTarget);
>      // element might be an 'insertion point' in this case.
>      eventPath.push(element);
>   }
> 
> WDYT? Is this a reasonable API?

My initial feeling is that this is not good API since it is likely to be misused.
Just providing event.path API is much better.

Metapoint: It sounds like when we have time we should start looking into what parts of Shadow DOM should move into DOM proper and what parts need to be described elsewhere. Though that depends on how stable the proposed Shadow DOM API is in general of course.

Fwiw, I agree with Hayato-san.

I worry that the 'event.path' emulation code will become boilerplate that many components will include.

(In reply to comment #20)
> Fwiw, I agree with Hayato-san.
> 
> I worry that the 'event.path' emulation code will become boilerplate that
> many components will include.

I am convinced.

(In reply to comment #19)
> Metapoint: It sounds like when we have time we should start looking into
> what parts of Shadow DOM should move into DOM proper and what parts need to
> be described elsewhere. Though that depends on how stable the proposed
> Shadow DOM API is in general of course.

Anne, I totally agree. Can we maybe discuss this at TPAC tomorrow/Friday?

(In reply to comment #21)
> 
> (In reply to comment #19)
> > Metapoint: It sounds like when we have time we should start looking into
> > what parts of Shadow DOM should move into DOM proper and what parts need to
> > be described elsewhere. Though that depends on how stable the proposed
> > Shadow DOM API is in general of course.

BTW, I think the easy steps here are:

1) in DOM spec, add event.path and  simply say that it returns a copy of the event path
2) in shadow DOM spec, tweak event handling section to explicitly mention that it modifies how an event path is built.

If many components need event path, there is something wrong with components spec. Components should be able to work without too much knowledge about the
things outside them, and normal DOM shouldn't need to care about Shadow DOM.

(In reply to comment #22)
> 1) in DOM spec, add event.path and  simply say that it returns a copy of the
> event path
> 2) in shadow DOM spec, tweak event handling section to explicitly mention
> that it modifies how an event path is built.
Sounds doable, though I don't know whether we want event.path
or someEventTarget.path("eventType") or some such.

Components need to be able to work with elements that are supplied to them from the outside and projected into shadow-dom (aka distributed elements). Those elements have different positions in the tree depending on your point of reference. 

In general, components (internally) need event.path in order to process events that have bubbled up from distributed elements. I don't believe there is any encapsulation problem here.

The alternative is attaching listeners explicitly to all possible useful targets, which can be extremely inefficient (see first note in this thread).

FYI.

I've landed an experimental Event Path API in blink.

- https://codereview.chromium.org/14508005/
- https://chromiumcodereview.appspot.com/15063004/

The name of this is bad. Methods should not be nouns. This should be a getter named `path` or the method should be renamed to something else.

(In reply to comment #26)
> The name of this is bad. Methods should not be nouns. This should be a
> getter named `path` or the method should be renamed to something else.

That's a mistake on my part. I should've caught this in review. I totally assumed Hayato-san was just using a property, since that's what I meant it to be in the first place.

I've updated the spec.

https://dvcs.w3.org/hg/webcomponents/diff/614b6b7050c0/spec/shadow/index.html

If you don't want this in DOM after all, please do fix the component first. If you do, don't mark it fixed ;-)

(In reply to comment #29)
> If you don't want this in DOM after all, please do fix the component first.
> If you do, don't mark it fixed ;-)

hurdur!

So why does this return a NodeList? Should this not return a JavaScript frozen (static) Array?

Uh, NodeList is very wrong. It should be an array of EventTargets.

Could you help me to understand that?
I guess each item in the *list* is not always a Node, right?
That might be something which extends EventTarget.

(In reply to comment #32)
> Uh, NodeList is very wrong. It should be an array of EventTargets.

Right, XMLHttpRequest is not a Node, Indexed DB has a tree of non-Node EventTarget objects, etc.

Also, should Window be included in the path for an event generetated on a Node in the document? Internally for the shadow dom polyfill I keep it in the list but I had to remove from it before returning the event.path to match Blink.

Right, Window is another good point as to why NodeList is a bad idea. It should be there.

(In reply to comment #34)
> Right, XMLHttpRequest is not a Node, Indexed DB has a tree of non-Node
> EventTarget objects, etc.

Thank you.

I've checked the spec here.
http://dom.spec.whatwg.org/#dispatching-events
> 5.7.4 - If event's target attribute value is participating in a tree, let event path be a static ordered list of all its ancestors in tree order, and let event path be the empty list otherwise.

An event path only includes *something* which can participate in a tree. I assumed that it's always a Node since whatever participates in a *DOM tree* is always a Node.

If index-db has a tree of EventTargets, I think we cannot use NodeList here.
That makes sense. Let's change that to an array of EventTargets if there is no objection.

(In reply to comment #36)
> Right, Window is another good point as to why NodeList is a bad idea. It
> should be there.

I don't think Window should be included since it doesn't participate in a tree.

window is part of the event target chain, so it definitely should be there.

Indeed, HTML says it participates in that tree and indeed events fired on nodes reach Window so it *has* to participate.

(there is one special case when events dispatched to a document which has .defaultView don't reach the window, and that is 'load'. Load event doesn't propagate from document to window.)

This got lost. Discussion ends abruptly nearly a year ago.

Let's get the patch in the Shadow DOM specification fixed first before considering how to integrate this into DOM.

(In reply to Anne from comment #43)
> Let's get the patch in the Shadow DOM specification fixed first before
> considering how to integrate this into DOM.

The patch has already gotten in the Shadow DOM spec.
Is it a time to integrate this into DOM?

(In reply to Hayato Ito from comment #44)
> The patch has already gotten in the Shadow DOM spec.
> Is it a time to integrate this into DOM?

Comment 40 does not seem fixed yet?

Also, to better set expectations. I don't have a good plan for integrating shadow DOM throughout the existing ecosystem yet. Some of it is extensions to HTML and some of it is new DOM bits, not quite sure how to deal with that.

(In reply to Anne from comment #45)
> (In reply to Hayato Ito from comment #44)
> > The patch has already gotten in the Shadow DOM spec.
> > Is it a time to integrate this into DOM?
> 
> Comment 40 does not seem fixed yet?

Agreed. Reopening this issue loos reasonable.

I am afraid that I need your help to understand how Window object participates in the tree. I appreciate where I can know how Window object participates in the tree.

I've read the following part:
http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.html#the-window-object

1. I am assuming that Document is the root of the node tree until now. Can I assume that?
2. Should I consider Window object is the parent of the Document?
3. Is there any better interpretation? I'd like to avoid 'Windows is the parent of Document' if we could.

I think the way to understand this is that you need to understand that the event path != node tree. Since you edit the shadow tree specification, that should be somewhat obvious ;-)

Now with the exception of events whose type is load, in the event path the Window object follows document.

Another way of saying this is that for the purposes of events, Window is the root of the node tree (unless event.type is "load").

Thank you. I am triaging issues in the Shadow DOM spec.

I think this issue can be actionable and can be fixed.

Of course, as we know, in Blink, a Window object has been receiving events so far. The current Shadow DOM spec and the implementation differs. I'd like to fix this. The blink's implementation won't change.

However, event.path will become to includes Window object at the beginning of that as a result. This is the only user visible change, I think.

We might have a plan B:
   event.path API return EventTarget[] where Window object is removed from the event path.

If someone have a strong concern about the compatibility and prefer plan B, please let me know that.

Partially fixed.
https://github.com/w3c/webcomponents/commit/73b64edd930a623616923c9cfc0f118fc8cc6abc

Note that the case of "event.type is 'load'"  is not yet considered.

(In reply to Hayato Ito from comment #49)
> 
> However, event.path will become to includes Window object at the beginning
> of that as a result. This is the only user visible change, I think.

and change the type to an array of EventTarget, right?

> We might have a plan B:
>    event.path API return EventTarget[] where Window object is removed from
> the event path.
> 
> If someone have a strong concern about the compatibility and prefer plan B,
> please let me know that.

I actually prefer this, not from the compatibility, but because I cannot see any useful cases of having window in event.path API.

I think the Shadow DOM spec is fine either way, since it's talking about how the actual event path is, but when we add the API to the DOM spec, we could simply say "window object is not included in the returned array from the event.path property."

Thoughts?

I don't understand why we'd want to exclude the Window object. When it's part of the event path, it would make sense to include it.

(In reply to Anne from comment #52)
> I don't understand why we'd want to exclude the Window object. When it's
> part of the event path, it would make sense to include it.

I'm ok with that. My thought is that, if the API creates an array everytime it's read, not including unuseful staff may be better, but it's just mild preference.

Anne, now that comment 40 is done, can you update the DOM spec? Should Hayato or I propose a patch to the whatwg/html-mirror?

(In reply to Koji Ishii from comment #53)
> I'm ok with that. My thought is that, if the API creates an array everytime
> it's read, not including unuseful staff may be better, but it's just mild
> preference.

How is Window object less useful than, say Document object or document root or body element for example?
Note, usually event handlers (onfoo DOM attributes) in body element are actually added to the Window object.

Thanks Olli, understood.

Since a patch might be too much for this case, here's the suggested text for all to review:

| readonly attribute sequence<EventTarget> path;
|
| Returns a list of EventTarget this event goes through in the order it bubbles.

Since Window object should be included unless specified not to, I didn't put in, but we could add a note if desired.

Does this definition and text look good? If so, I can go and fix Blink to match to this.

FYI, a PR for web-platform-tests is on its way:
https://github.com/w3c/web-platform-tests/pull/1524/files

A fix for DOM would be somewhat more complex I think. We need to flush out the definition of path so it's crystal clear.

Also, this makes it return an array, correct? And mutating that array won't have any effect on anything? I think that's fine... (There's some talk over in IDL land about being able to return frozen array objects for cases like this...)

(In reply to Anne from comment #57)
> A fix for DOM would be somewhat more complex I think. We need to flush out
> the definition of path so it's crystal clear.
> 
> Also, this makes it return an array, correct? And mutating that array won't
> have any effect on anything? I think that's fine... (There's some talk over
> in IDL land about being able to return frozen array objects for cases like
> this...)

Yeah, mutation shouldn't have any effect. The returned *array-like* should not be a   live array. That's my intention.

Does a frozen array mean the array itself can't be modified?

e.g

var path = event.path;
path[0] = 'a'; // => This throws an exception.
path.push('a'); // This also throws an exception.

Must be very clear to people here, but what I learnt yesterday from our IDL guy
was that sequence<T> is always passed by value, while T[] is always passed by
reference as defined in http://www.w3.org/TR/WebIDL/#idl-sequence

So making the type "sequence<EventTarget>" rather than "EventTarget[]"
should imply that mutating that array won't have any effect on anything in my
understanding.

Do you prefer to be more verbose on that point rather than implying it from the
type? Or am I incorrect to assume that sequence<T> implies that?

Also anything else you think the suggested text above is missing?

I looked at Event IDL: https://dom.spec.whatwg.org/#interface-event
and most methods/attributes are not very verbose, but your advice for how to
make crystal clearer would be appreciated. Oftentimes what's not clear is not
clear for who wrote the text.

(In reply to Koji Ishii from comment #59)
> Must be very clear to people here, but what I learnt yesterday from our IDL
> guy
> was that sequence<T> is always passed by value, while T[] is always passed by
> reference as defined in http://www.w3.org/TR/WebIDL/#idl-sequence
> 
> So making the type "sequence<EventTarget>" rather than "EventTarget[]"
> should imply that mutating that array won't have any effect on anything in my
> understanding.
> 
> Do you prefer to be more verbose on that point rather than implying it from
> the
> type? Or am I incorrect to assume that sequence<T> implies that?
> 
> Also anything else you think the suggested text above is missing?
> 
> I looked at Event IDL: https://dom.spec.whatwg.org/#interface-event
> and most methods/attributes are not very verbose, but your advice for how to
> make crystal clearer would be appreciated. Oftentimes what's not clear is not
> clear for who wrote the text.

According to the comment, https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c5, Sequence<T> doesn't work for attributes.

(In reply to Hayato Ito from comment #60)
> 
> According to the comment,
> https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c5, Sequence<T> doesn't
> work for attributes.

Ah! Thank you, I missed that. By re-reading the spec, I found the source:
http://www.w3.org/TR/WebIDL/#idl-sequence

]]]
Sequences must not be used as the type of an attribute, constant or exception
field.

Note
This restriction exists so that it is clear to specification writers and API
users that sequences are copied rather than having references to them passed
around. Instead of a writable attribute of a sequence type, it is suggested
that a pair of operations to get and set the sequence is used. Another option
is to use an array type, which can be used as the type of an attribute.
[[[

Earlier in this thread preferred an attribute than an operation, so we use
array, but clarify in the spec that it returns a copy everytime. Understood.

Should have read though bug 25458 before posting, so you want a type for event.path.

From curiosity, why is T[] going away? Because of lack of interoperablity?

(In reply to Koji Ishii from comment #62)
> From curiosity, why is T[] going away? Because of lack of interoperablity?

I recommend reading bug 23682. We'll have to wait for that to be sorted in some manner. I believe heycam plans to tackle that next.

Got it, thank you for the pointer. That's very clear.

It looks like we're heading to B), correct?
  readonly attribute FrozenArray<Gamepad> gamepads;

Look forward to it being resolved, but in the meantime, what do you prefer?

We're:
- event.path is nowhere in the spec. Events in shadow is in the shadow spec, but this is DOM API that could work without shadow DOM involved (though not useful), and we're stuck.
- Blink still implements NodeList without window included.
- Gecko is trying to follow the Blink behavior by explicitly removing window from the list.

Options for the spec updates:
A) Keep this undocumented until bug 23682 is resolved.
B) Put a tentative one once in Shadow DOM spec with the issue marked, then move to DOM when bug 23682 is resolved.
C) Put a tentative one to DOM spec and update when bug 23682 is resolved.

Since T[] is much closer to FrozenArray<T> than NodeList is, and it looks like not having window is troubling Gecko, I think impl could update to T[] to include window then update again when bug 23682 is done.

(In reply to Koji Ishii from comment #64)
> Got it, thank you for the pointer. That's very clear.
> 
> It looks like we're heading to B), correct?
>   readonly attribute FrozenArray<Gamepad> gamepads;
> 
> Look forward to it being resolved, but in the meantime, what do you prefer?
> 
> We're:
> - event.path is nowhere in the spec. Events in shadow is in the shadow spec,
> but this is DOM API that could work without shadow DOM involved (though not
> useful), and we're stuck.

I'm afraid I don't understand what is a real issue here. UA can provide event.path API without any Shadow DOM implementation, can't we?

> - Blink still implements NodeList without window included.
> - Gecko is trying to follow the Blink behavior by explicitly removing window
> from the list.

Gecko doesn't have to follow the current Blink behavior. The current spec already includes Window in event path.

> 
> Options for the spec updates:
> A) Keep this undocumented until bug 23682 is resolved.
> B) Put a tentative one once in Shadow DOM spec with the issue marked, then
> move to DOM when bug 23682 is resolved.

Shadow DOM spec mentions this issue of IDL as 'ISSUE 1'. Is it different from plan B?

> C) Put a tentative one to DOM spec and update when bug 23682 is resolved.
> 
> Since T[] is much closer to FrozenArray<T> than NodeList is, and it looks
> like not having window is troubling Gecko, I think impl could update to T[]
> to include window then update again when bug 23682 is done.

Never mind, didn't find event.path in the Shadow DOM spec and thought it's not in, sorry about the noise. So we're in B. Thanks.

Let me close this bug because all issues were resolved or filed as a separate bug. Let's focus each bug separately and close this bug.