Open bzbarsky opened 5 years ago
This test came out of when I was implementing the aborting behavior in Chrome. In Chrome, when the document.open call happens the refresh has already been queued, which would make document.open abort it as the note explicitly says that tasks queued to navigate should be aborted as well. (This does not seem to be the case in Firefox, as step 4 occurs after step 3.) I then decided to cancel all those refreshes queued that have a timeout of more than 0, as a 0 timeout is basically “going to navigate now.”
Now the question is whether the assumption that declarative refreshes are queued before load event is fair. As much as the spec is concerned, I think so, as Refresh-header refreshes are queued in https://html.spec.whatwg.org/multipage/browsing-the-web.html#initialise-the-document-object, which is before the load event gets fired. But if that assumption turns out to not match implementations, let’s just remove this test.
as Refresh-header refreshes are queued in https://html.spec.whatwg.org/multipage/browsing-the-web.html#initialise-the-document-object, which is before the load event gets fired
Sort of. Again, what's queued in there is waiting until some number of seconds (possibly 0) until after the document is "completely loaded", which happens after the load event is fired. That's certainly what Firefox does; I haven't dug into the code in other browsers yet.
As far as I can tell, the test has the following structure (focusing on the HTTP header part for now, and just the fetch case):
Refresh: 0
header.document.open
.The test then asserts that the fetch is canceled before it completes, presumably by the
document.open
call seeing a navigation in progress (from the Refresh) and hence canceling the fetch.In terms of the spec, this doesn't seem right. https://html.spec.whatwg.org/multipage/semantics.html#shared-declarative-refresh-steps step 13 says to navigate when the refresh has "come due" which is defined as the given number of seconds (0, in this case) elapsing after the document is "completely loaded". This last is set in the task queued at https://html.spec.whatwg.org/multipage/parsing.html#completely-loaded which is step 12 of https://html.spec.whatwg.org/multipage/parsing.html#the-end
But the load event fires from a task queued in step 7 of that same algorithm. Both tasks uses the same task source, so the load event should fire before the navigation caused by the Refresh header is started, and indeed before the refresh timer is started. So per spec as currently written, this test should consistently fail. There's the question of what should happen with "tasks queued to navigate" tracked in https://github.com/whatwg/html/issues/3447 but in this case there is no "task queued to navigate": there's a task queued to mark the document "completely loaded", which will then start a timer, which when it expires will queue a task to navigate....
OK, so what happens in browsers? In at least Firefox when this test passes the sequence of events is as follows:
document.open
call happens, doesn't cancel the fetch because there's no navigation ongoing.t.step_func_done()
before we actually load the new document from the refresh.The test will fail via hitting the
t.unreached_func
if the fetch ends up winning the race against the refresh timer in step 5 above.The test will fail via timeout (because the load handler gets removed during the first load event, so nothing ever ends the test) if the fetch cancellation loses the race against the refresh load in step 7 above.
I'm not really sure what the right thing is here in terms of the spec. The 1s/4s tests in abort-refresh-multisecond-header.window.js and abort-refresh-multisecond-meta.window.js seem to imply that
open()
should not do any aborting just because we're expecting to queue a task that will start a timer that when it fires will navigate... So arguably the same should be true for a 0s timer.Now how to test that, given the race betwen the timer-triggered navigation and its aborting of the document and the fetch... I don't know.
@TimothyGu @domenic @annevk