Skip to content

Should event dispatch check that the event target is in a fully active realm? #1084

@shaseley

Description

@shaseley

I can't find in the DOM or WebIDL specs where such a check exists; apologies if I missed it.

The scenario is (jsfiddle):

  • A page has an iframe
  • The iframe creates a new AbortController (controller) in its realm and hangs it off of its parent
  • The parent page adds an event listener (added and defined in its realm) to controller.signal
  • The iframe gets removed
  • controller.abort() is called in the parent page's context

Should the event handler run?

This is similar to whatwg/webidl#993, but it's the event target that is in a "detached realm", not the event listener or call to controller.abort().

All the engines are consistent though — no one runs the event listener:

  • Blink and WebKit both early out in ~2.10 of inner invoke.
    • For Blink specifically, we can't create the JS wrapper for the event because the context is gone.
  • Gecko skips handling the event during EventTargetChainItem::HandleEvent() because the EventListenerManager is null (IIUC it got cleared during detach). I think this is during targeting, just prior to starting inner invoke.

I was a bit surprised by the behavior, but given that no engine is running these, would it make sense to add an explicit check (assuming I didn't miss it) so the spec is in sync, and if so, thoughts on where?

Activity

domenic

domenic commented on Jun 3, 2022

@domenic
Member

Note that this appears to be enforced at the EventTarget level, not at the AbortController/AbortSignal level or at the Event level: https://jsfiddle.net/wL5evuh3/3/.

Given that this is about the EventTarget itself, and thus isn't covered by the Web IDL issue you list, probably the check would have to be at https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke . You might think we could factor it out to surround all of https://dom.spec.whatwg.org/#concept-event-dispatch but that would probably not do the right thing if the first listener removed the frame. (In that case I assume subsequent listeners should not run... but someone should test.)

shaseley

shaseley commented on Jun 3, 2022

@shaseley
ContributorAuthor

Note that this appears to be enforced at the EventTarget level, not at the AbortController/AbortSignal level or at the Event level: https://jsfiddle.net/wL5evuh3/3/.

Nice, right, this isn't specific to AbortSignal --- just using that as an example because that's just how I discovered this.

Given that this is about the EventTarget itself, and thus isn't covered by the Web IDL issue you list, probably the check would have to be at https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke . You might think we could factor it out to surround all of https://dom.spec.whatwg.org/#concept-event-dispatch but that would probably not do the right thing if the first listener removed the frame. (In that case I assume subsequent listeners should not run... but someone should test.)

Re: WebIDL, I was curious what should happen if constructing an object in a realm whose global's document is detached. This happens earlier (step 2 of fire an event), but Blink doesn't create the wrapper until much later.

I'll test the "detached during an event with subsequent handlers" case. I was thinking about that as well and was planning to write a WPT for that assuming we make a spec change. I agree that the check can't happen too early because if this case.

shaseley

shaseley commented on Jun 4, 2022

@shaseley
ContributorAuthor

It looks like all the engines don't run subsequent listeners after the event target's context gets detached. For Blink and WebKit, this follows from where we're checking this; for Gecko, I think the listeners get cleared on detach, so iteration over the listeners stops after detach.

And yeah, there would need to be a check in https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke to account for detaching from a listener. I'll try to send a PR next week along with some tests.

annevk

annevk commented on Jun 7, 2022

@annevk
Member
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Should event dispatch check that the event target is in a fully active realm? · Issue #1084 · whatwg/dom