-
Notifications
You must be signed in to change notification settings - Fork 297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a fully active check for EventTarget in event listener inner invoke #1085
base: main
Are you sure you want to change the base?
Conversation
This adds tests for dispatching events to an EventTarget created in an iframe that gets detached --- both prior to and during event dispatch. The behavior of engines for this case is to not run event listeners after the EventTarget's associated global is detached, which is what we check for in the tests. Spec: https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke PR: whatwg/dom#1085 Bug: 1323391 Change-Id: I57aac7d444d3532ad0940a228452d206b5c1be07
This adds tests for dispatching events to an EventTarget created in an iframe that gets detached --- both prior to and during event dispatch. The behavior of engines for this case is to not run event listeners after the EventTarget's associated global is detached, which is what we check for in the tests. Spec: https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke PR: whatwg/dom#1085 Bug: 1323391 Change-Id: I57aac7d444d3532ad0940a228452d206b5c1be07
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@smaug---- what do you think of this?
Was this tested for various kind of events, including synthetic events?
Co-authored-by: Anne van Kesteren <[email protected]>
I manually tested with browser-initiated events via AbortController/AbortSignal and with synthetic events, for both detaching a frame prior to and during dispatch. I observed the behavior to be consistent across browsers. I have WPT tests for synthetic events here (not yet landed), and happy to add/run more tests if it makes sense. |
Firefox has checks in the webidl callbacks layer to prevent calling listener in the globals which are "gone". |
That would suggest it needs to be added to https://webidl.spec.whatwg.org/#call-a-user-objects-operation although that would also affect other callers of that algorithm. (None come to mind at the moment.) |
The global of the When I filed the issue, I did some debugging in FF code to see if I could figure out where the check was happening. I observed the following:
I don't know this code nearly as well as y'all, but this is what I observed with some hacky printf debugging; hopefully that's helpful. |
In Gecko if the global of the WebIDL callback itself isn't the current one anymore, the bindings layer prevents the call. |
This adds a fully active check in https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke that skips running event listeners if the EventTarget's realm's global's document is not fully active. Blink, Gecko, and WebKit were all skipping these event listeners, but the spec does not currently reflect this behavior.
I put the check where Blink and WebKit have it — just after the event listener was removed if it is a one-off. My understanding of Gecko's implementation is that the listeners are cleared on frame detach, so this should be compatable. The position of the check could be web observable if the document becomes fully active again, i.e. with bfcache, but Chromium at least doesn't cache pages with opener references.
Closes #1084.
(See WHATWG Working Mode: Changes for more details.)
Preview | Diff