document.open() doesn't work after window.stop()
Categories
(Core :: DOM: Core & HTML, defect, P3)
Tracking
()
People
(Reporter: TimothyGu, Assigned: hsivonen)
References
Details
(Whiteboard: [webcompat])
Attachments
(2 files)
81 bytes,
text/html
|
Details | |
7.46 KB,
patch
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.99 Safari/537.36 Steps to reproduce: Load the following HTML file in Firefox. ------8<------ <p>This should get erased.</p> <script> window.stop(); document.open(); </script> ------>8------ Actual results: The `document.open()` call is ignored, and the page loads as usual. Expected results: The `document.open()` call is respected, and the page is cleared for a new document. This is the behavior in Chrome and Safari. When `window.open()` is called in the inline script, there should be an active parser associated with the document. window.stop() [1] is then defined to call the "abort a document" algorithm [2], which in its step 3 aborts [3] the active parser. At this point, the parser becomes no longer active, which is defined to be a parser "that has not yet been stopped or aborted" [4]. The crux of this issue lies in step 5 of "document open steps" in the HTML Standard [5], called by document.open(). > 5. If document has an active parser whose script nesting level is greater than 0, then return document. When document.open() is called the parser is no longer active, so the condition should not be true, and the rest of the document open steps should be executed. Yet, in Firefox's implementation, the aborted parser is still considered to be active, which is an incorrect interpretation of the spec. See [6]: > // Note that aborting a parser leaves the parser "active" with its > // insertion point "not undefined". (Note, the comments reference an older version of the HTML Standard [7] which uses "insertion point is not undefined" instead of the current "script nesting level is greater than 0" condition (this was changed in [8]/[9]). The "active parser" part remains the same from the old spec to the current one though.) Proposed WPT available at https://github.com/web-platform-tests/wpt/pull/11929. [1]: https://html.spec.whatwg.org/multipage/window-object.html#dom-window-stop [2]: https://html.spec.whatwg.org/multipage/browsing-the-web.html#abort-a-document [3]: https://html.spec.whatwg.org/multipage/parsing.html#abort-a-parser [4]: https://html.spec.whatwg.org/multipage/dom.html#active-parser [5]: https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#document-open-steps [6]: https://searchfox.org/mozilla-central/rev/c579ce13ca7864c5df9711eda730ceb00501aed3/dom/html/nsHTMLDocument.cpp#1249-1257 [7]: https://cdn.rawgit.com/whatwg/html/7b0112bbfb4923e8f64b0fd1c66d14fbd526fc33%5E/complete.html#dom-document-open [8]: https://www.w3.org/Bugs/Public/show_bug.cgi?id=17869 [9]: https://github.com/whatwg/html/commit/7b0112bbfb4923e8f64b0fd1c66d14fbd526fc33
Comment 1•6 years ago
|
||
Sounds like the spec changed after our implementation? Henri, can you take a look?
Comment 2•6 years ago
|
||
How do various browsers behave after the user hits the "stop" button in the UI?
Updated•6 years ago
|
Reporter | ||
Comment 3•6 years ago
|
||
> Sounds like the spec changed after our implementation?
Yes, but the "active parser" part never got changed (nor the definition of an active parser), which means that this has always been a valid bug.
Reporter | ||
Comment 4•6 years ago
|
||
> How do various browsers behave after the user hits the "stop" button in the UI? Given this HTML file: ------8<------ <p>This should get erased.</p> <button onclick="document.open();">document.open()</button> <script src="http://localhost:8888/slow.js"></script> <script> alert("You were too slow! Hit stop before this message."); </script> ------>8------ and this Node.js server: ------8<------ "use strict"; const http = require("http"); const server = http.createServer((req, res) => { if (req.url.includes("slow.js")) { setTimeout(() => { res.setHeader("Content-Type", "text/javascript"); res.end("void 0;"); }, 5000); } else { res.statusCode = 404; res.setHeader("Content-Type", "text/plain"); res.end("Not found"); } }); server.listen(8888); ------>8------ Start up the Node.js server, load test.html from anywhere, hit stop before the page finishes loading, and then hit the "document.open()" button. In Chrome and Safari the page gets cleared. In Edge, the page becomes unresponsive after the "document.open()" button is hit.
Reporter | ||
Comment 5•6 years ago
|
||
(Note: in Edge the page becomes unresponsive after the "document.open()" button is hit, regardless of if the stop button was hit in time or not.)
Comment 6•5 years ago
|
||
This is still an issue even after bug 1489308 is fixed.
Updated•5 years ago
|
Assignee | ||
Comment 7•5 years ago
|
||
Assignee | ||
Comment 8•5 years ago
|
||
The mParserAborted
check in document.open()
was added to make https://mylith.com/mozilla/753278/ work. Simply removing it regresses that test case, which does show "Hello" in Chrome.
Assignee | ||
Comment 9•5 years ago
|
||
Maybe we should make nsDocShell::InternalLoad
calling nsDocShell::Stop
a more forceful stop than another call to nsDocShell::Stop
. We could add STOP_EVER_MORE
here: https://searchfox.org/mozilla-central/source/__GENERATED__/dist/include/nsIWebNavigation.h#108
Is there a better explanation for Chrome's behavior than navigation away via location
stopping the document so forcefully as to prevent document.open
but stop()
stopping it less forcefully and allowing subsequent document.open
?
Comment 10•5 years ago
|
||
Per the HTML standard, navigation effectively terminates the event loop (tasks belong to a no longer active document). window.stop() terminates fetching, but the event loop will still spin and activity can happen afterwards in response to user interaction for instance.
Hope that helps.
Assignee | ||
Comment 11•5 years ago
|
||
(In reply to Anne (:annevk) from comment #10)
Per the HTML standard, navigation effectively terminates the event loop (tasks belong to a no longer active document). window.stop() terminates fetching, but the event loop will still spin and activity can happen afterwards in response to user interaction for instance.
Thanks. It seems we should make nsDocShell::InternalLoad
mark the current doc as "navigation away started" and disable document.open
if that flag has been set and not disable document.open
if the "parser aborted" flag has been set. (Unclear if we even need the "parser aborted" flag in that case anymore.)
Assignee | ||
Updated•5 years ago
|
Comment 12•5 years ago
•
|
||
InternalLoad is the start of a navigation. It does not terminate the event loop per spec; it just does https://html.spec.whatwg.org/multipage/browsing-the-web.html#abort-a-document which includes:
If document has an active parser, then abort that parser and set document's salvageable state to false.
which calls into https://html.spec.whatwg.org/multipage/parsing.html#abort-a-parser
Also per spec, https://html.spec.whatwg.org/multipage/window-object.html#dom-window-stop calls https://html.spec.whatwg.org/multipage/browsing-the-web.html#stop-document-loading which just calls https://html.spec.whatwg.org/multipage/browsing-the-web.html#abort-a-document
So per spec, these should have the same exact effect on the parser, as far as I can tell. And that effect, again as far as I can tell, should be that the parser should be considered done and open() and write() should work as if after DOMContentLoaded.
So in the testcase at https://mylith.com/mozilla/753278/ I would expect the sequence of events to be as follows per spec:
- The location set terminates the parser.
- The open() call cancels the location-set-started load, opens the doc.
- The write() call writes stuff.
So "Hello" should NOT appear per spec as currently written. Timothy, do you know what is Chrome actually doing here? Because it doesn't seem to match the spec afaict. The spec does talk about "active parser whose script nesting level is greater than 0" but https://html.spec.whatwg.org/multipage/parsing.html#abort-a-parser will make the parser not active.
Is this some situation where Chrome actually processes .location sets async instead of doing what the spec says, maybe? If the location set is doing all its work after the open()
call, that would explain the Chrome behavior in this testcase... :(
Reporter | ||
Comment 13•5 years ago
|
||
Is this some situation where Chrome actually processes .location sets async instead of doing what the spec says, maybe?
That’s correct. Blink’s Location.href setter calls into Frame::ScheduleNavigation
, which queues a task to navigate rather than navigating directly. More in general, I want to say that all “normal” navigations in Chrome are asynchronous.
Comment 14•5 years ago
|
||
OK. That's definitely not what the spec says. Is there a Chrome bug tracking this, do you know?
Reporter | ||
Comment 15•5 years ago
|
||
I asked Nate Chapin and he mentioned
https://bugs.chromium.org/p/chromium/issues/detail?id=914587 as the bug for that issue. Additionally he’s actively working on making Location-object navigation (and initial iframe load) synchronous as per spec.
Comment 16•5 years ago
|
||
Awesome, thank you!
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 17•5 years ago
|
||
See bug 1547409. Moving webcompat whiteboard tags to project flags.
Comment 18•5 years ago
|
||
OK, it looks like Chrome has changed its behavior to sync-navigate from location.href sets, so the testcase in comment 8, which has this basic form:
<script>
location.href = "data:text/html,<h1>Hello</h1>";
document.open();
</script>
Now shows a blank page in Chrome dev builds (when run inside a subframe, so the navigation to data: actually works in general). That's what the spec says should happen, and I think we should align with it.
Comment 19•5 years ago
|
||
That said, doing that without adding a "planned navigation" async thing for form submission would make us have the web compat problem described at https://bugs.chromium.org/p/chromium/issues/detail?id=955556.
Updated•5 years ago
|
Comment 20•5 years ago
|
||
Per discussion in https://github.com/whatwg/html/issues/4723 it looks like we're going to align on our current behavior.
Comment 21•5 years ago
|
||
I filed bug 1580381 to implement the test changes to account for that likely spec change.
Comment 22•5 years ago
|
||
Karl, we may need to get the webcompat issue fixed some other way, but Chrome aligning with us here should help, hopefully...
Comment 23•5 years ago
|
||
Comment 24•5 years ago
|
||
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #22)
Karl, we may need to get the webcompat issue fixed some other way, but Chrome aligning with us here should help, hopefully...
Understood. Thanks for the heads up.
Reopened https://webcompat.com/issues/28103#issuecomment-530205507 and switched to needscontact
Description
•