Checking |item-in-shadow-tree| does not perform correcrly |
|||
Issue description|item-in-shadow-tree| should be saved before calling listener, but current chrome implementation does not support it. (See 2: https://dom.spec.whatwg.org/#concept-event-path-append) Instead, it checks if tuple is in shadow tree at inner invoke with Node::IsInV1ShadowTree(), which calls Node::ContainingShadowRoot() inside, but this does not behave correctly with node moving inside listener. (See 2-8: https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke) We should create structs or tuples before calling listener and pass it to the method for invoking listener. (See: https://infra.spec.whatwg.org/#struct) There is a WPT test for this issue: https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/wpt/dom/events/event-global-extra.window.js?type=cs&q=+third_party/WebKit/LayoutTests/external/wpt/dom/events/event-global-extra.window.js&sq=package:chromium&g=0&l=48
,
Aug 16
I don't quite understand what comment #1 means. :( Could you have a chance to elaborate that? Note that we don't implement the each step of DOM Standard as is. As long as the output of the both UA-implemented and the specified algorithm to be exactly the same for all inputs, it is okay. I guess this is an issue of the implementation of window.event, so re-routing to binding-team.
,
Aug 16
We can observe a behavioral difference (as WPT demonstrates). In short, when dispatching an event, almost everything (if not really everything) must be planned beforehand so that invoked event listeners must not affect event dispatching. I've not deeply dived into the issue, but it's probably EventTarget::DispatchEvent's duty to remember all the necessary information for the plan. Such a plan includes window.event-related things, but not limited to them. I supposed that "Blink>DOM" team is interested in event dispatch (because it's part of DOM). If not, sorry for the noise.
,
Aug 16
Anne kindly added me as a reviewer to https://chromium-review.googlesource.com/c/chromium/src/+/1177406. Now I understood the context well. Thank you for the explanation! > I supposed that "Blink>DOM" team is interested in event dispatch (because it's part of DOM). If not, sorry for the noise. No problem. window.event has been owned by binding-team because it is implemented out of core/dom, as far as I remember. yukishiino@ would know the reason well. :)
,
Aug 16
(EventTarget::DispatchEvent is implemented in core/dom/events/ ... and I vaguely think that we'll need fix somewhere around there ... just FYI)
,
Aug 16
core/dom/events doesn't set window.event at this point, I think. If you need to update EventTarget::DispatchEvent or something to handle window.event issues, please feel free to add Blink>DOM. I'll look. |
|||
►
Sign in to add a comment |
|||
Comment 1 by yukishiino@chromium.org
, Aug 16Components: Blink>DOM