Closed rakina closed 3 years ago
I love this. The consistency with window.open() is great, and unifying the missing attribute/empty string/invalid URL/explicit about:blank cases makes so much more sense than the current situation where those diverge.
I think we should update the spec in this direction, and I agree with your assessment that it should match Chrome and Safari observable behavior---so we already have multi-implementer interest!
Regarding window object reuse, I think that's a separable interop problem; the tests at https://wpt.fyi/results/html/browsers/the-window-object/window-reuse-in-nested-browsing-contexts.tentative.html?label=master&label=experimental&aligned show we have some work to do there. I think it's best to just not touch that aspect of the spec as part of this change, although I agree it has some impact.
/cc @smaug---- as well.
I will try to work on a spec change for this. The load event might be tricky so that'll need careful review.
@cdumez
How would this be consistent with window.open() ? (or perhaps browsers behave differently there too).
load event firing on iframe needs to be clarified. Is that happening synchronously or asynchronously?
And reusing the Window could be breaking change.
How would this be consistent with window.open() ? (or perhaps browsers behave differently there too).
Yes, it is consistent with how browsers behave on window.open(); see https://github.com/whatwg/html/pull/6869 for the proposed change to both of them.
load event firing on iframe needs to be clarified. Is that happening synchronously or asynchronously?
It fires asynchronously. https://html.spec.whatwg.org/#completely-finish-loading remains unchanged for now.
@rakina, could you expand the tests at https://github.com/web-platform-tests/wpt/pull/29745 to cover the iframe load event as well?
And reusing the Window could be breaking change.
Window reuse is a separate interop problem. It seems unlikely that pages are depending on any particular semantics given how non-interoperable that is. But I am happy to tackle that next if you're excited about aligning Gecko with the majority there.
However, in both Chrome and Safari, the non-initial about:blank is actually treated the same way as the initial empty document.
I don't think this is completely true. It definitely affects things like reuse of the Window object.
PS: I think some Chrome folks were thinking of following Firefox's behavior previously to remove all cross-document synchronous commits in Chrome's renderer, but I recently found some tests in Chrome that do not expect the document to be changed asynchronously, and pretty worried that this exists in real websites, so I think it's probably best to just avoid this special navigation entirely, and it's pretty consistent with the window.open() case too!
I wonder if the opposite problem might exist. Also see below.
Keeping the "initial empty document" status means the next same-origin document will reuse the Window (see Rationalize behavior of Window object reuse #3267). I think this is already what's happening in Chrome and maybe Safari, I haven't checked though. Not really sure if this is bad, but since browsers already have support for this maybe it's not really a big deal?
This is actually not the case in Chrome today: we will not reuse the Window object, since the synchronous (re-)navigation to about:blank commits something that is no longer considered the initial empty document. See https://crbug.com/778318
To be clear, I am not against trying to simplify things, but it will definitely have web-visible effects.
@rakina, could you expand the tests at web-platform-tests/wpt#29745 to cover the iframe load event as well?
I think this is already covered by this test, let me know if I need to add more!
PS: I think some Chrome folks were thinking of following Firefox's behavior previously to remove all cross-document synchronous commits in Chrome's renderer, but I recently found some tests in Chrome that do not expect the document to be changed asynchronously, and pretty worried that this exists in real websites, so I think it's probably best to just avoid this special navigation entirely, and it's pretty consistent with the window.open() case too!
I wonder if the opposite problem might exist. Also see below.
For the asynchronous -> synchronous / "completely remove" change, I think it's less of a problem, see this comment.
Keeping the "initial empty document" status means the next same-origin document will reuse the Window (see Rationalize behavior of Window object reuse #3267). I think this is already what's happening in Chrome and maybe Safari, I haven't checked though. Not really sure if this is bad, but since browsers already have support for this maybe it's not really a big deal?
This is actually not the case in Chrome today: we will not reuse the Window object, since the synchronous (re-)navigation to about:blank commits something that is no longer considered the initial empty document. See https://crbug.com/778318
To be clear, I am not against trying to simplify things, but it will definitely have web-visible effects.
Chatted a bit more with @zetafunction today, and looks like this is true at least in Chrome - the "is initial empty document" used for the Window reuse decision is set to false after the new about:blank
committed, so future navigations won't reuse the Window. However the "is initial empty document" is still true for history-related decisions (and other Chrome decisions in the browser process), so history replacement will still happen. So we kinda have two "is initial empty document" states that don't agree with each other for the new about:blank
case :(
However, from the test that @domenic linked, it looks like Firefox (the only browser passing the window sharing tests with the src='about:blank'
case) agrees more with the "is initial empty document for history" state in Chrome (so, it still treats the new about:blank
as the initial empty document for Window reuse), which is interesting.
Anyways, although I agree that the window reuse behavior is pretty weird and not interoperable, I guess we need to make sure it's not impossible to spec later after https://github.com/whatwg/html/pull/6869. I wonder if splitting the value used for window reuse decision vs history replacement decision in the spec too makes sense?
I think this is already covered by this test, let me know if I need to add more!
I think that test will pass whether the iframe load event is sync or async. Could we expand it to test that the event is not fired sync?
Anyways, although I agree that the window reuse behavior is pretty weird and not interoperable, I guess we need to make sure it's not impossible to spec later after #6869. I wonder if splitting the value used for window reuse decision vs history replacement decision in the spec too makes sense?
Yeah, I am pretty convinced it will still be possible to spec after #6869. And indeed, one path would be splitting the "should reuse Window
" boolean from the "is initial about:blank
" boolean.
(In general I am pulled on two directions on Window
object reuse. On the one hand, it's super-weird and annoying, so if we can minimize the number of cases it is visible to web developers, like Chrome and Safari seem to do, that seems good. On the other hand, unifying Window object reuse and history replacement like Firefox and the current spec does seems simpler, and simplicity is nice.)
I think that test will pass whether the iframe load event is sync or async. Could we expand it to test that the event is not fired sync?
Oh right! Updated, looks like only firefox fires it asynchronously, Chrome and Safari fires it synchronously :o (I guess because the commit is done synchronously as well)
Yeah, I am pretty convinced it will still be possible to spec after #6869. And indeed, one path would be splitting the "should reuse
Window
" boolean from the "is initialabout:blank
" boolean.(In general I am pulled on two directions on
Window
object reuse. On the one hand, it's super-weird and annoying, so if we can minimize the number of cases it is visible to web developers, like Chrome and Safari seem to do, that seems good. On the other hand, unifying Window object reuse and history replacement like Firefox and the current spec does seems simpler, and simplicity is nice.)
Thanks, sounds good then. Hope we can end up with something sensible there!
Alright, I've updated https://github.com/whatwg/html/issues/6863 to reflect the Safari/Chrome behavior for the iframe load event too. (And, it turns out, my reading of the spec was incorrect; before my latest push no load event would get fired for the initial insertion, for subtle reasons which are now explained in a note.)
I think that's probably the right move, since in general we're trying to align on 2/3 behavior here by explicitly acknowledging the initial insertion case as special.
So what remains is more re-reviews of https://github.com/whatwg/html/pull/6869, and I believe we'd want to flip the test expectations to expect the event to be sync, instead of expecting it to be async.
(This was discussed a bit in https://github.com/whatwg/html/issues/6491#issuecomment-801155357 but I'm making a separate issue since it's quite orthogonal, and since I just thought about a potential way forward on this)
Background When an iframe is created/inserted it will navigate to the URL from its
src
attribute (if it's set and thesrcdoc
attribute is unset), replacing the initial empty document. If this URL isabout:blank
, in the current spec, it should still replace the initial empty document with a newabout:blank
document. I think this navigation is triggered in all browsers, but how it runs (synchronously/asynchronously) and its effect (removing the "initial about:blank document"-ness or not) differs (see next section). Also, when thesrc
attribute is not set, according to the spec the iframe should stay on the initial empty document. However, at least in Chrome this actually triggers anabout:blank
navigation, as if thesrc
is set toabout:blank
.Behavior of the
about:blank
navigation on initial insertionabout:blank
document. Since it's asynchronous, the navigation can be canceled (e.g. by document.open(), as mentioned in this comment). After the navigation finished, the iframe will not show the initial empty document anymore.Web-exposed behavior differences
The "initial empty document"-ness differs across browsers as mentioned above. In Chrome & Safari, the new
about:blank
document will still be considered as the initial empty document, while in Firefox it's not. This has an impact on whether future navigations will replace that document or not. For more details see https://github.com/web-platform-tests/wpt/pull/28541 and https://github.com/whatwg/html/issues/6491.The document will be replaced asynchronously in Firefox, while it's replaced synchronously/immediately in Chrome and Safari. I think this is actually a very big difference, because in Chrome & Safari you can do this and the text will stay, while in Firefox it will be cleared when the new document committed:
Tester pages to compare in various browsers: iframe with no src, iframe with src="about:blank"
Proposal: stop triggering navigations to about:blank on iframe insertion I propose that we stop triggering navigations to
about:blank
on iframe insertion, so that when an iframe withsrc
set toabout:blank
is inserted, it will just stay on the initial empty document, similar to whensrc
is not set (at least the spec version). My reasoning is as follows:about:blank
document committed synchronously, it's virtually indistinguishable from the initial empty document: the origin and other document properties should be the same, and it's not possible to get a reference to the initial empty document before insertion so nobody knows that it was ever there to begin with.window.open()
when the URL is not set, empty or isabout:blank
: instead of triggering a navigation, it will just fire theload
event on the window (see test page).Potential problems:
load
event listener to thecontentWindow
before insertion, and theload
event would've already fired if we try to add it after insertion. I'm not sure if this has any effect on theload
event on the<iframe>
itself, but I think we can just fire theload
event here too like thewindow.open()
case above. CMIIW!Window
(see https://github.com/whatwg/html/issues/3267). I think this is already what's happening in Chrome and maybe Safari, I haven't checked though. Not really sure if this is bad, but since browsers already have support for this maybe it's not really a big deal?cc @domenic @annevk @zetafunction @ArthurSonzogni @csreis who might be interested.
PS: I think some Chrome folks were thinking of following Firefox's behavior previously to remove all cross-document synchronous commits in Chrome's renderer, but I recently found some tests in Chrome that do not expect the document to be changed asynchronously, and pretty worried that this exists in real websites, so I think it's probably best to just avoid this special navigation entirely, and it's pretty consistent with the
window.open()
case too!