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 25458 - [Shadow]: The return type of Event.path should leverage WebIDL sequences
Summary: [Shadow]: The return type of Event.path should leverage WebIDL sequences
Status: RESOLVED MOVED
Alias: None
Product: WebAppsWG
Classification: Unclassified
Component: HISTORICAL - Component Model (show other bugs)
Version: unspecified
Hardware: PC All
: P2 minor
Target Milestone: ---
Assignee: Hayato Ito
QA Contact: public-webapps-bugzilla
URL:
Whiteboard:
Keywords:
Depends on: 23682
Blocks: 28552 28564
  Show dependency treegraph
 
Reported: 2014-04-25 08:14 UTC by Hayato Ito
Modified: 2015-05-27 02:59 UTC (History)
9 users (show)

See Also:


Attachments

Description Hayato Ito 2014-04-25 08:14:01 UTC
The subject explains well.
Comment 2 Anne 2014-04-25 08:47:22 UTC
Note that [] is an IDL array which are going away. I guess we can still fix that down the road?
Comment 3 Hayato Ito 2014-04-25 08:50:29 UTC
Thanks.

Do you have a pointer about what we should use instead of T[]?
Comment 4 Hayato Ito 2014-04-25 08:55:00 UTC
I guess we should use `Sequence<EventTarget>`. Is that correct?
Comment 5 Anne 2014-04-25 08:58:30 UTC
I don't think it exists yet because nobody is maintaining IDL :-(

sequence<> does not work with attributes.
Comment 6 Domenic Denicola 2014-04-25 13:38:28 UTC
Can they be real arrays with the exact semantics specified in prose?

This seems related to https://github.com/w3c/screen-orientation/issues/13
Comment 7 Boris Zbarsky 2014-04-25 16:17:40 UTC
> Can they be real arrays with the exact semantics specified in prose?

Yes.  You can put "object" in the IDL and then specify in prose.

That specification could even leverage WebIDL sequences in the sense of creating one, converting to a JS value, and caching the result.

The only open question is whether the result should be frozen or whether we should just let the consumer munge it.  Does anything internal look at .path on events?  If so, we may want to either freeze it or specify an internal accessor that returns the actual path.
Comment 8 Hayato Ito 2014-04-28 07:11:57 UTC
(In reply to Boris Zbarsky from comment #7)
> > Can they be real arrays with the exact semantics specified in prose?
> 
> Yes.  You can put "object" in the IDL and then specify in prose.
> 
> That specification could even leverage WebIDL sequences in the sense of
> creating one, converting to a JS value, and caching the result.
> 
> The only open question is whether the result should be frozen or whether we
> should just let the consumer munge it.  Does anything internal look at .path
> on events?  If so, we may want to either freeze it or specify an internal
> accessor that returns the actual path.

My expected behavior is 'frozen'.

If the following event listener is given,

button.addEventListener("click", function(event) {
  console.log(event.path.length);
  var p = event.path;
  p.pop();
  console.log(event.path.length);
  console.log(p.length);
  event.path = [];
  console.log(event.path);
});


this event listener should print the following as example:
  4
  4
  3
  4

As far as I tested, the current WebIDL implementation of blink for `readonly attribute T[]` behaves like that.
Comment 9 Boris Zbarsky 2014-04-28 07:25:05 UTC
> this event listener should print the following as example:

If event.path is a frozen array, this event listener will throw an exception on the "p.pop()" line.

> the current WebIDL implementation of blink for `readonly attribute T[]` behaves
> like that.

I'm not aware of Blink having any support for "attribute T[]" in IDL, readonly or not.  What it does instead is fake it with interfaces with indexed getters/setters.  For example, event.path in Blink is a NodeList.

That said Array.prototype.pop.call(someNonemptyNodeList), as currently specced, would land us in http://heycam.github.io/webidl/#delete which would return false, since there is no deleter defined.  And then http://people.mozilla.org/~jorendorff/es6-draft.html#sec-deletepropertyorthrow would throw.  This is in fact what happens in Firefox, though Blink seems to get this wrong.

And if Blink _did_ implement IDL arrays as currently specced for some reason, then you'd land in http://heycam.github.io/webidl/#platform-array-object-delete and once again throw, if the IDL array is not empty.

I have no idea where the "3" in your suggested log output came from, by the way, unless you think event.path should return a new object on every get (so that event.path == event.path always tests false)....
Comment 10 Hayato Ito 2014-04-28 07:50:36 UTC
Thank you.

WebIDL looks too complicated to me :)

I don't have any intention to say the current WebIDL implementation of blink is correct. I just assumed that WebIDL is *mature* and there is no significant difference between every user agents.

I am now feeling that we should make event.path into a method rather than an attribute unless we have a well-defined way to represent a *frozen* array.
I always get uncomfortable when an attribute returns something array-like.

We might want to fix the blink's implementation at first.
Let me investigate further.
Comment 11 Boris Zbarsky 2014-04-28 14:55:04 UTC
> I just assumed that WebIDL is *mature* and there is no significant difference
> between every user agents.

Things keep being added as needed, so it's not stable in the sense of frozen feature set.  But IE and Firefox are pretty close in following WebIDL in the parts they implement.

Blink and WebKit are off in their own world in all sorts of ways, as far as I can tell, WebKit more so than Blink.

> unless we have a well-defined way to represent a *frozen* array

See comment 7.
Comment 12 Domenic Denicola 2014-04-28 15:07:11 UTC
It is the TAG's opinion that in cases like these, the arrays should be mutable (non-frozen). It is un-JavaScript-ey to attempt to protect consumers from themselves.
Comment 13 Boris Zbarsky 2014-04-28 15:54:40 UTC
This case is a bit weird because the "consumers" of an event, esp with web components (which I assume is a given for event.path), are all sorts of pieces of unrelated code.  So it's more like protecting one consumer from another one that the first doesn't even know exists.
Comment 14 Domenic Denicola 2014-04-28 15:57:26 UTC
Sure, but the reasoning stands. Nobody does that in JS. The DOM sometimes tries to, but the TAG's opinion is that doing so is not good API design and should be discontinued if possible in new DOM APIs. (Historically, it seems like this was sometimes done because implementations did not distinguish between the integrity of the model layer and the integrity of the user-exposed data layer. This distinction is important and something the TAG highlighted recently.)
Comment 15 Boris Zbarsky 2014-04-28 15:59:37 UTC
> Nobody does that in JS.

In all fairness, until recently there weren't any tools in the language for people to do that, no?

I don't actually have a strong opinion on this stuff, as long as we make it clear that the internal implementation is not using the modified data.  I just want to make sure we're considering all the issues here.
Comment 16 Hayato Ito 2014-04-30 06:42:04 UTC
(In reply to Boris Zbarsky from comment #15)
> > Nobody does that in JS.
> 
> In all fairness, until recently there weren't any tools in the language for
> people to do that, no?
> 
> I don't actually have a strong opinion on this stuff, as long as we make it
> clear that the internal implementation is not using the modified data.  I
> just want to make sure we're considering all the issues here.

In the current blink's implementation, the internal implementation is not using the modified data.
An even.path always creates a new array instance and returns it. Modifying the returned array doesn't have any affect to the internal implementation.

Anyway, I'd like to know what is a recommended way for event.path.
Is there any suggestion?

1). Use 'readonly attribute EventTarget[] event.path' and fix the blink's implementation of T[].

2). Because T[] is going away in IDL, find the other reasonable way, though I'm not sure what is an available option here.

3). Make event.path an operation: 'sequence[EventTarget] getPath()'
Comment 17 Anne 2014-04-30 11:12:59 UTC
We want 2. The way you could define this is by saying it returns an "object" in IDL and define in prose that a JavaScript Array Object is returned which represents a copy of the underlying path concept (which is a list of EventTarget objects).

You should also keep an XXX somewhere in the specification to update this once IDL is fixed with proper support for JavaScript Array Objects.
Comment 18 Boris Zbarsky 2014-04-30 16:55:02 UTC
And define when that object is created and when a new one is created (presumably as part of event dispatch).
Comment 19 Hayato Ito 2014-05-01 04:03:08 UTC
Thank you. 2 looks a reasonable option here.

I updated the spec at
https://github.com/w3c/webcomponents/commit/6f296e4bb73238a15baab805a0079c16f9dfac7f

I appreciate any feedbacks for that.
Comment 20 Domenic Denicola 2014-05-01 04:10:19 UTC
It should not be a new array on each get; it should be the same array. Otherwise e.path !== e.path.

You should create the array object upon construction of the event, and return the same one each time.

I also think the issues you linked to are not really related to the problem at hand.
Comment 21 Boris Zbarsky 2014-05-01 04:42:35 UTC
> You should create the array object upon construction of the event

Construction or dispatch?  Dispatch would make more sense, since that's when the path is determined, and events can be redispatched.

Apart from that, I totally agree with comment 20.

Also, one more issue: I suggest you create the array by creating a WebIDL sequence<EventTarget> in your prose and then explicitly invoking the "convert to an ECMAScript value" concept from WebIDL.  That will actually rigorously define how the array is set up and avoid different implementations disagreeing about whether to use [[Set]] or [[DefineOwnProperty]] for the array entries.
Comment 22 Hayato Ito 2014-05-01 05:53:42 UTC
(In reply to Domenic Denicola from comment #20)
> It should not be a new array on each get; it should be the same array.
> Otherwise e.path !== e.path.
> 
> You should create the array object upon construction of the event, and
> return the same one each time.
> 
> I also think the issues you linked to are not really related to the problem
> at hand.

The situation might be complex here in event.path.
The return value of event.path can differ even in the lifecycle of the same event object.

For example,

- an event listener, A, is added to a node, NODE_A.
- an event listener, B, is added to a node, NODE_B.
- Suppose NODE_A and NODE_B are in the different node trees.
- Suppose NODE_A and NODE_B are in the same event path for an event. (Don't confuse "event path" with event.path API).

Suppose, an event listener A is invoked at first. Then an event listener B is invoked.

var pathInA;

function eventListenerA(event) {
  pathInA = event.path;
  // pathInA is something like [D, C, A] here.
}

function eventListenerB(event) {
  var pathInB = event.path;
  // pathInB is something like [D, C, B] here, which is different what we saw in eventListenerA.
  // pathInA and pathInB should be the same JavaScript Array object?
}
Comment 23 Hayato Ito 2014-05-01 06:01:20 UTC
FYI.
This is a related on-going discussion: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25423

If we could give away an *encapsulation* from "event.path", event.path could return the same JavaScript Array object.
Comment 24 Boris Zbarsky 2014-05-01 06:04:59 UTC
If this API is not exposing the full, immutable-during-the-dispatch event path from the target to the root, then it should be a method, not a getter, imo.  And it needs a better name.
Comment 25 Hayato Ito 2014-05-01 06:47:56 UTC
+Dimitri to CC List.

(In reply to Boris Zbarsky from comment #24)
> If this API is not exposing the full, immutable-during-the-dispatch event
> path from the target to the root, then it should be a method, not a getter,
> imo.  And it needs a better name.

That reminds me of the old discussion here.
See the comment https://code.google.com/p/chromium/issues/detail?id=234030#c22

Dimitri, WDYT?


One more note:

As I recall, the same event object can be *re-used* between event dispatching if we dispatch an event explicitly by calling EventTarget.dispatchEvent(event), re-using the same event object.
So the result of event.path could be *different* anyway for the same event object.
Comment 26 Boris Zbarsky 2014-05-01 07:15:53 UTC
> As I recall, the same event object can be *re-used* between event dispatching

Yes, see comment 21.  Creating a new array on explicit action like array dispatch is probably fine.  Creating one seemingly-randomly during event propagation seems a lot weirder.
Comment 27 Anne 2014-05-01 09:36:58 UTC
The way you want to define this is that events have an associated path and an associated path object. Then at various points in the lifetime of the event, you change the associated concepts as appropriate. E.g. initially, when you create an event, its associated path is the empty list and its associated path object is a new Array representing its associated path.

Then during dispatch when the path is calculated you set a new path and a new Array representing it.

Event.prototype.path meanwhile returns the path object. The specification algorithms use the path.
Comment 28 Dimitri Glazkov 2014-05-01 16:40:15 UTC
(In reply to Anne from comment #27)
> The way you want to define this is that events have an associated path and
> an associated path object. Then at various points in the lifetime of the
> event, you change the associated concepts as appropriate. E.g. initially,
> when you create an event, its associated path is the empty list and its
> associated path object is a new Array representing its associated path.
> 
> Then during dispatch when the path is calculated you set a new path and a
> new Array representing it.
> 
> Event.prototype.path meanwhile returns the path object. The specification
> algorithms use the path.

Love it.
Comment 29 Hayato Ito 2014-05-27 11:26:03 UTC
Let me resume this work after I resolve this issue:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458
Comment 30 Hayato Ito 2014-05-27 11:27:01 UTC
Correction:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=25423
Comment 31 Erik Arvidsson 2014-05-29 23:17:26 UTC
This discussion has mostly been about the IDL. I assume this bug is also about including the Window object in the event path?
Comment 32 Hayato Ito 2014-05-30 03:57:14 UTC
(In reply to Erik Arvidsson from comment #31)
> This discussion has mostly been about the IDL. I assume this bug is also
> about including the Window object in the event path?

Yep, I remember that we had a discussion about Window object, however, I am lost and couldn't find the discussion.

Could someone give me the link to the discussion?

I am feeling that we should file a bug for that as another bug.


I couldn't find any mention about Window object "in 4.7 Dispatching events" of DOM standard.

http://dom.spec.whatwg.org/#dispatching-events says:

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

I am not sure what is the best way to deal with Window object in event path. I am assuming that Window object is not the parent of Document. Please correct me if I'm wrong.
Comment 33 Anne 2014-05-30 06:46:15 UTC
Window is bug 21066. And yes, HTML defines Window and defines it as a parent of Document for the purposes of event dispatch. Ideally we make that a bit more explicit down the road. I still haven't figured out a good way to integrate shadow trees into DOM/HTML.
Comment 34 Olli Pettay 2014-05-30 22:08:11 UTC
Note, load event dispatched somewhere in the dom (or shadow dom) doesn't
propagate to Window.
Comment 35 Anne 2014-05-31 07:25:08 UTC
Yeah, that is noted by HTML. We should probably formalize that a bit better somehow.
Comment 36 Hayato Ito 2015-04-15 05:31:22 UTC
|Window| was already addressed in the spec tentatively.
https://github.com/w3c/webcomponents/commit/73b64edd930a623616923c9cfc0f118fc8cc6abc

So I think we can't resolve this issue until we can leverage WebIDL sequences.

Let me move this into "15480: [Shadow]: Things to Consider in the Future" category.
Comment 37 Hayato Ito 2015-05-27 02:59:29 UTC
Moved to https://github.com/w3c/webcomponents/issues/101