[Shadow]: The return type of Event.path should leverage WebIDL sequences (bugzilla: 25458) #101
Comments
|
If you mean to make it a |
|
Yes, I'd like to use |
|
I think this issue shouldn't block v1. Let me defer this to v2. |
|
I think for v1 either you should use the FrozenArray sequence that @heycam says he will add to Web IDL, or you should at least add a warning that this will probably change in the future. |
|
I agree. I just thought ForzenArray is nice to have for v1. Let me bring this issue back to v1. We can decide what we can do about this issue for v1 later. |
|
Either we fix this for v1 or we remove the feature and reconsider it in v2... Don't think there's a middle ground here. |
|
Okay. Let me leave the current event.path as is with a warning. |
|
Recently checking out this spec. Can someone confirm: Has the |
|
@EisenbergEffect, you are correct in both questions:
|
|
@hayatoito Thank you for the confirmation. Can I assume that there's no way to access the original event target with a closed shadow tree then? (I'm working on some code and docs around this, so just want to make sure before I make that statement.) |
|
Yeah, there is no way to access. That's exactly one of the motivations we've introduced a closed shadow tree. I've designed a closed shadow tree so that there is no way to access an internal node in any case. That's the bottom line by design. |
|
There was an intent to implement However, it wasn't clear if changing |
|
No, but no implementations of |
|
I believe that |
|
We've added a RAPPORT metric to measure Let me check the usage later. |
|
Now Event.deepPath uses FrozenArray. See 6d79c89 |
Title: [Shadow]: The return type of Event.path should leverage WebIDL sequences (bugzilla: 25458)
Migrated from: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458
comment: 0
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c0
Hayato Ito wrote on 2014-04-25 08:14:01 +0000.
The subject explains well.
comment: 1
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c1
Hayato Ito wrote on 2014-04-25 08:18:18 +0000.
Committed at
4e9b329
comment: 2
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c2
Anne wrote on 2014-04-25 08:47:22 +0000.
Note that [] is an IDL array which are going away. I guess we can still fix that down the road?
comment: 3
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c3
Hayato Ito wrote on 2014-04-25 08:50:29 +0000.
Thanks.
Do you have a pointer about what we should use instead of T[]?
comment: 4
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c4
Hayato Ito wrote on 2014-04-25 08:55:00 +0000.
I guess we should use
Sequence<EventTarget>. Is that correct?comment: 5
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c5
Anne wrote on 2014-04-25 08:58:30 +0000.
I don't think it exists yet because nobody is maintaining IDL :-(
sequence<> does not work with attributes.
comment: 6
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c6
Domenic Denicola wrote on 2014-04-25 13:38:28 +0000.
Can they be real arrays with the exact semantics specified in prose?
This seems related to w3c/screen-orientation#13
comment: 7
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c7
Boris Zbarsky wrote on 2014-04-25 16:17:40 +0000.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c8
Hayato Ito wrote on 2014-04-28 07:11:57 +0000.
(In reply to Boris Zbarsky from comment #7)
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c9
Boris Zbarsky wrote on 2014-04-28 07:25:05 +0000.
If event.path is a frozen array, this event listener will throw an exception on the "p.pop()" line.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c10
Hayato Ito wrote on 2014-04-28 07:50:36 +0000.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c11
Boris Zbarsky wrote on 2014-04-28 14:55:04 +0000.
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.
See comment 7.
comment: 12
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c12
Domenic Denicola wrote on 2014-04-28 15:07:11 +0000.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c13
Boris Zbarsky wrote on 2014-04-28 15:54:40 +0000.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c14
Domenic Denicola wrote on 2014-04-28 15:57:26 +0000.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c15
Boris Zbarsky wrote on 2014-04-28 15:59:37 +0000.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c16
Hayato Ito wrote on 2014-04-30 06:42:04 +0000.
(In reply to Boris Zbarsky from comment #15)
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c17
Anne wrote on 2014-04-30 11:12:59 +0000.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c18
Boris Zbarsky wrote on 2014-04-30 16:55:02 +0000.
And define when that object is created and when a new one is created (presumably as part of event dispatch).
comment: 19
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c19
Hayato Ito wrote on 2014-05-01 04:03:08 +0000.
Thank you. 2 looks a reasonable option here.
I updated the spec at
6f296e4
I appreciate any feedbacks for that.
comment: 20
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c20
Domenic Denicola wrote on 2014-05-01 04:10:19 +0000.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c21
Boris Zbarsky wrote on 2014-05-01 04:42:35 +0000.
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 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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c22
Hayato Ito wrote on 2014-05-01 05:53:42 +0000.
(In reply to Domenic Denicola from comment #20)
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,
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c23
Hayato Ito wrote on 2014-05-01 06:01:20 +0000.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c24
Boris Zbarsky wrote on 2014-05-01 06:04:59 +0000.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c25
Hayato Ito wrote on 2014-05-01 06:47:56 +0000.
+Dimitri to CC List.
(In reply to Boris Zbarsky from comment #24)
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c26
Boris Zbarsky wrote on 2014-05-01 07:15:53 +0000.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c27
Anne wrote on 2014-05-01 09:36:58 +0000.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c28
Dimitri Glazkov wrote on 2014-05-01 16:40:15 +0000.
(In reply to Anne from comment #27)
Love it.
comment: 29
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c29
Hayato Ito wrote on 2014-05-27 11:26:03 +0000.
Let me resume this work after I resolve this issue:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458
comment: 30
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c30
Hayato Ito wrote on 2014-05-27 11:27:01 +0000.
Correction:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=25423
comment: 31
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c31
Erik Arvidsson wrote on 2014-05-29 23:17:26 +0000.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c32
Hayato Ito wrote on 2014-05-30 03:57:14 +0000.
(In reply to Erik Arvidsson from comment #31)
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:
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c33
Anne wrote on 2014-05-30 06:46:15 +0000.
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
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c34
Olli Pettay wrote on 2014-05-30 22:08:11 +0000.
Note, load event dispatched somewhere in the dom (or shadow dom) doesn't
propagate to Window.
comment: 35
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c35
Anne wrote on 2014-05-31 07:25:08 +0000.
Yeah, that is noted by HTML. We should probably formalize that a bit better somehow.
comment: 36
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25458#c36
Hayato Ito wrote on 2015-04-15 05:31:22 +0000.
|Window| was already addressed in the spec tentatively.
73b64ed
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.
The text was updated successfully, but these errors were encountered: