Closed Bug 1527954 Opened 5 years ago Closed 5 years ago

window.event on wrong window

Categories

(Core :: DOM: Events, defect, P3)

65 Branch
defect

Tracking

()

RESOLVED INVALID

People

(Reporter: cvazac, Unassigned)

Details

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:65.0) Gecko/20100101 Firefox/65.0

Steps to reproduce:

(1) go to about:config
(2) change dom.window.event.enabled to true
(3) go to https://cvazac.netlify.com/window-dot-event/
(4) open up devtools to check console.assert()'s

(I test this in Firefox 63, 64, and 65).

Actual results:

The event object is on the window of the <IFRAME>.

Expected results:

All asserts should pass. The event object should be retrievable via top.event.

Component: Untriaged → DOM: Events
Product: Firefox → Core

Henri might know what's up here.

Flags: needinfo?(hsivonen)

We found this because ours[1] is the code in iframe.html. And unfortunately, the code in the callback (from the site) relies on window.event instead of the first argument.

[1] https://github.com/akamai/boomerang

Am I reading the code right that console.assert(e === window.event, 'from top, e should equal window.event') tries to assert about the outer page's load event?

I observe that Chromium fires the same console.asserts as Firefox. Edge does something different, and the test case can't be run in IE, because it triggers the IE5 Quirks mode but uses APIs not available in that mode.

Flags: needinfo?(hsivonen) → needinfo?(annevk)

This is the behavior we decided upon when standardizing, as far as I can tell: https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke. The callback passed to addEventListener() (due to prototype munging) comes from the frame, so therefore that's the global on which we set the "current event".

cvazac, did you file a bug against Chrome? If so, could you paste the URL here?

Flags: needinfo?(annevk)
Flags: needinfo?(cvazac)

@hsivonen: that assert is intended to ensure that the event passed to the callback is that same as that on window/top.

(After making the test case more vanilla)
Edge 13-18, IE 11: all asserts pass
IE 10: same failing asserts as in Chrome 72 and Firefox 65
IE 9: didn't test :(

@annevk
I filed [1] which was merged into [2].
[1] https://bugs.chromium.org/p/chromium/issues/detail?id=932154
[2] https://bugs.chromium.org/p/chromium/issues/detail?id=664676

Let global be listener callback’s associated Realm’s global object.
It's true that the realm of the callback is the inner <IFRAME>. It is weird though, I'd have expected the realm to be the same as the object addEventListener() was called on (the same as the object that the event was dispatched to).

Flags: needinfo?(cvazac)

(fixing whitespace issue)

Let global be listener callback’s associated Realm’s global object.

It's true that the realm of the callback is the inner <IFRAME>. It is weird though, I'd have expected the realm to be the same as the object addEventListener() was called on (the same as the object that the event was dispatched to).

cvazac, https://github.com/whatwg/dom/issues/334#issuecomment-379707829 might help. Basically, there wasn't much consistency there either if you poke at the edge cases.

cvazac, is this something you could fix on your end?

I don't think it's possible, at least not in Firefox 65.

window.event is writeable, but once you set it, it stays that new thing forever. And if you delete window.event, it never again gets assigned to by dispatched events.

I take that[1] back, looks like we can save off (and reassign) the property descriptor.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1527954#c9

Is there a reason you cannot always inspect the event argument rather than grab it from the global by the way? window.event is not the greatest API and indeed isn't yet cross-browser (at least I think it's still disabled by default in shipping Firefox).

Priority: -- → P3

Yes, that is certainly preferred. But it's not our code that is relying on window.event. Our instrumentation of addEventListener is unfortunately tripping up brittle code. And usage is sadly high [1].

[1] https://github.com/search?l=JavaScript&q=%22window.event%22&type=Code

I see, I was wondering why you had overridden the prototype.

Given that Chrome has closed their bug I'm also going to close this, since it seems we first need to come to a new agreement standards-wise, if any, before proceeding. If you want to pursue that https://github.com/whatwg/dom/issues/new would be the place.

The main downside with using the global of the target is that this is not reliable between Chrome/Safari and Firefox, if the target moved between documents. We could solve that by using the target's node document's relevant global object, if the target is a node, but that would have to be acceptable to everyone involved.

Status: UNCONFIRMED → RESOLVED
Closed: 5 years ago
Resolution: --- → INVALID

Created a new issue[1]. FYI, Chrome closed 932154 as a duplicate of 664676, so the issue I reported will still (in theory) get fixed.

[1] https://github.com/whatwg/dom/issues/735

Thanks for raising that!

As for Chrome, as far as I can tell your issue was closed as a duplicate of an issue that's resolved as fixed, so I would not expect Chrome to make changes. (That would also go against the standard.)

You need to log in before you can comment on or make changes to this bug.