Closed Bug 1180525 Opened 4 years ago Closed 4 years ago
A function from a window which got blown away by document
.open is not invoked when used as an event listener
Pushlog https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fba44ae0de48&tochange=872d79fd1982 Regressed by: Bug 862627 - Switch EventListener to WebIDL codegen
I don't understand "regressed by". You agree this is a bug? Is there a way to fix it in my current browser? More likely I have to wait for an update? $ yum list installed firefox reports firefox.x86_64 37.0.2-1.fc21 @updates
It means that Alice has confirmed that the behaviour you see is still present in the latest Firefox, and found at what point it was introduced. The next step is that the person who introduced the behaviour investigates whether this is the expected behaviour, and if it isn't, corrects it.
"regressed by" means that this used to work, and it stopped working when bug 862627 was fixed. What's going on here is that the implicit document.open() from the post-onload document.write() replaces the old Window with a new one. And CallbackObject has a check to make sure we're not invoking a callback that comes from a non-current Window. We _could_ probably change that check to be a HasActiveDocument() check instead of what is effectively an IsCurrentInnerWindow() check. That seems ok to me: it wouldn't affect the entry document situation (which is what the current check is documented as addressing), since it's in fact the same document. Bobby, thoughts?
Summary: addeventlistener after document write → A function from a window which got blown away by document.open is not invoked when used as an event listener
(In reply to Boris Zbarsky [:bz] from comment #4) > "regressed by" means that this used to work, and it stopped working when bug > 862627 was fixed. > > What's going on here is that the implicit document.open() from the > post-onload document.write() replaces the old Window with a new one. And > CallbackObject has a check to make sure we're not invoking a callback that > comes from a non-current Window. > > We _could_ probably change that check to be a HasActiveDocument() check > instead of what is effectively an IsCurrentInnerWindow() check. That seems > ok to me: it wouldn't affect the entry document situation (which is what the > current check is documented as addressing), since it's in fact the same > document. > > Bobby, thoughts? That seems good to me. My primary concern on stuff like this is aligning with other browsers. I checked and Blink+WebKit seem to run the listener, so I think we should too.
Blink+webkit run listeners in totally unloaded windows too. See <http://web.mit.edu/bzbarsky//www/testcases/events/event-listener-from-unloaded-document-1.html>. I tried IE with the attached testcase, but it throws an exception because the getElementById("x") call returns null. Anyway, I guess I'll do the HasActiveDocument() thing for now, until a spec actually defines this stuff.
The distinction only matters when document.open() makes a different window current without changing the active document.
Attachment #8630329 - Flags: review?(bobbyholley)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #8630329 - Flags: review?(bobbyholley) → review+
OK, so we have a few try failures: 1) dom/base/test/test_bug372964-2.html | Shouldn't have handled an event because the context has changed - got 2, expected 1. This test is using open()/close() to change out the window and then fire an event at an XHR instance. That's precisely what this patch is trying to allow. Talked to Olli and will change this test to actually navigate the browsing context. 2) Unexpected passes in WPT html/dom/dynamic-markup-insertion/opening-the-input-stream/016.html. Looking at the structure of this test, I agree it should pass after this change and that it doing so is OK. 3) Unexpected fail and unexpected OK on WPT old-tests/submission/Opera/script_scheduling/094.html due to reaching the assert_unreached. I know why we're doing that now (because the setTimeout fires, when it didn't use to), but why exactly does this test expect the setTimeout to not fire or if it does for the assert_unreached to not fail the test? James, is this test using the test harness properly? 4) Unexpected pass on WPT old-tests/submission/Opera/script_scheduling/101.html -- this behavior change is expected.
Per irc discussion, the test is making bogus assumptions; it doesn't consider the fact that the timeout is created after the new document is created, or the fact that the document.open() call overwrites the original test so we can't collect the result.
You need to log in before you can comment on or make changes to this bug.