Spec for iframe removal from the document doesn't match browser behavior

[bzbarsky]

Consider this testcase:

<iframe></iframe>
<script>
  frames[0].onunload = () => console.log("removed");
  document.querySelector("iframe").remove();
</script>

Should anything get logged? Per spec, no, because:

When an iframe element is removed from a document, the user agent must discard the element's nested browsing context, if it is not null, and then set the element's nested browsing context to null.

followed by a note that

This happens without any unload events firing (the nested browsing context and its Document are discarded, not unloaded).

But Firefox, Chrome, and Safari all fire an unload event in this situation.

Of particular interest, that means that in those browsers https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#document-open-steps step 11 can re-enter document.open, as far as I can tell. It's not clear to me how that's supposed to work. There's a web platform test at html/browsers/browsing-the-web/unloading-documents/004.html that sort of ends up exercising this, but it has various issues (like html/browsers/browsing-the-web/unloading-documents/support/004b.html opening parent.document but then writing and closing document which mean that no one passes it right now...

@annevk @timothygu @domenic

[bzbarsky]

One simple solution for the document.open bits might be to just bump the "ignore-opens-during-unload counter" value while doing step there...

Another solution would be to align all UAs with the spec on the unload behavior of removing <iframe> elements from the DOM, but I would be a little worried about web compat there.

[bzbarsky]

opening parent.document but then writing and closing document

This might actually be correct... In any case, this test ended up triggering Gecko assertions with some other unrelated changes I was making, which is how I ended up looking into this.

[annevk]

When we fix this, timing of unload relative to clearing of browsing contexts needs to be made clear: https://software.hixie.ch/utilities/js/live-dom-viewer/saved/6910. I suspect we want to basically have the removal steps equivalent of #4354 for this.

[annevk]

cc @nox

[rniwa]

@cdumez

[domenic]

I'd be curious to hear from browsers what they do here. It seems like there are a few things in play:

• Probably the spec should say "unload" instead of "dispose" when removing an iframe from the document.
• This causes document.open(), which can remove iframes, to potentially fire unload events, which can cause document.open() to happen reentrantly, which is probably very bad? Upthread @bzbarsky proposes a fix here to prevent reentrancy, but I'm curious what browsers do today.
• @annevk points out that, if we make such a change (dispose -> unload), then we need to be careful about the ordering of unload vs. nulling out the browsing context. If we did a simple substitution of dispose -> unload, then nulling out occurs afterward, which matches Chrome and Safari in https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=6910 but mismatches Firefox. I think this calls for a web platform test and Firefox bug, probably?

[bzbarsky]

This causes document.open(), which can remove iframes, to potentially fire unload events, which can cause document.open() to happen reentrantly, which is probably very bad?

Hmm. I was going to say that https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#document-open-steps step 5 would no-op that, but of course the script nesting level would still be 0 here. I'm not sure what browsers actually do in practice here; I'll try to write a testcase if no one beats me to it.

[annevk]

As I noted elsewhere, what Chrome/Safari do is the equivalent of mutation events, so if the plan is still to get rid of those, that behavior would be best avoided.

[domfarolino]

So I came to this issue after looking at whatwg/dom#808 and thinking more about how to resolve it from the perspective of invoking a node's removing steps hook during removal.

This issue seemed to originally be about:

1. Whether an iframe could synchronously invoke script during removal (via unload)
   • Answer: all browsers seem to allow the unload event to fire synchronously during removal, however at least Chromium (and I think others at this point too?) are experimenting with deprecating this[^1].
2. And if so (YES), whether the browsing context (now "navigable" in today's language) is nulled before or after script runs.

It was said earlier in this thread that Chrome/Safari agree on clearing the browsing context after the unload event, but that Firefox appeared to clear the browsing context before. Now, however, I'm observing that all browsers are fully aligned on this: https://iframe.glitch.me/remove.html (Glitch site since loading that site's source in Live DOM Viewer seemed to break the site in Firefox... sorta? I think Glitch is closer to the ground truth here, since there are less iframe rendering shenanigans).

It seems that in an iframe's unload event handler, all browsers agree that JavaScript cannot observe any part of the DOM removal having taken place yet. This makes me worried about the following: https://github.com/whatwg/dom/pull/732#issuecomment-562235210:

The other piece of relevant feedback here is whatwg/html#4611. We need these deferred steps for removal too. Hurray!

Assuming "deferred removal steps" means "executing unload after the DOM removal mutation", then I feel like we definitely don't want "deferred removal steps," because in the iframe case that means we'd be saving all of the script execution (unload handler) to run in a document that is freshly detached from the DOM. That would be bad — in fact, not running script when the DOM is in a weird state is exactly what the "deferred insertion steps" idea in https://github.com/whatwg/dom/issues/808 was supposed to solve. So I'm not really sure we should change from what all browsers currently do: run script-executing side effects before DOM insertion is complete.

Related: another example I can think of is removing a focused <input> element. Should that fire a blur event? And if so, should the blur event be fired before or after the node is disconnected from the DOM? https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=12403 shows me that Chrome fires a blur event before the node is disconnected/removed, whereas Firefox and Safari don't fire anything at all... 🤯

document.open() specifically

This issue quickly went into a discussion about whether document.open() specifically can or cannot be called from within an unload event handler, and what that means. Maybe things have changed a lot in the intervening period, buttttt it seems that https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#document-open-steps:~:text=Similarly%2C%20if%20document%27s%20unload%20counter%20is%20greater%20than%200%2C%20then%20return%20document gives us a clear answer, no? If so, then I think that specific aspect of this issue can be put to rest.

/cc @mfreed7 @noamr

[^1]: For more info, see https://chromestatus.com/feature/5579556305502208, https://github.com/mozilla/standards-positions/issues/691#issuecomment-1513046517, and https://github.com/fergald/docs/blob/master/explainers/permissions-policy-deprecate-unload.md; this has gone at least as far as making Chromium not fire unload events in blank same-origin iframes in WPTs, making the OP sorta untestable!

[smaug----]

• all browsers seem to allow the unload event to fire synchronously during removal

What about pagehide? I don't think anyone is trying to deprecated that and it fires around the same time as unload.

[domfarolino]

Good call on pagehide. I've amended https://iframe.glitch.me/remove.html to also listen for pagehide and have found that in all implementations, it is synchronously fired during the removal of iframes[^1]. This means that the "unload a document" algorithm is run upon iframe removal, despite HTML saying it is not: https://html.spec.whatwg.org/C#the-iframe-element:unload-a-document.

I'll amend the WPTs in https://github.com/web-platform-tests/wpt/pull/44308 to show this more explicitly.

Then spec-wise to close this issue out I think we just need to make HTML fire the "unload" algorithm upon child navigable destruction. At least that's the gist. It's not quite that simple because some of the unload algorithms seem to unconditionally queue tasks, which wouldn't match implementations right now.

[^1]: This is at least true for same-origin iframes because they share an event loop with their parent (remover). For cross-origin ones with a separate event loop, I imagine pagehide/unload is fired asyncly with respect to the parent.