There is no attached test case, did you forget to add it?

Looks like it.  Attaching now.

Deleted: baz.html 162 bytes

baz.html 162 bytes View Download

As best as I can tell from the spec and the blink parser, the behavior in the test is racy.

Specifically, the parser hasn't actually exited parsing mode before the event fires and is processed.

Specifically, the parser hasn't processed the EOF and exited parsing mode before the onerror handler runs.  Since the parser is still in parsing mode when the onerror handler fires the document.write() is appended to the current position. (document.open specifically no-ops by spec if the document is still being parsed).

If you need to guarantee that the handler fires after the parser exits parsing it needs to look like the attached modified test case (though that may be racy for browsers that have already exited parsing).

Deleted: baz_dcl.html 226 bytes

baz_dcl.html 226 bytes View Download

> Specifically, the parser hasn't actually exited parsing mode before the event fires and is processed.

That doesn't matter, per spec.

> Since the parser is still in parsing mode when the onerror handler fires the document.write() is appended to the current position.

That's not what the spec says to do.  The spec says that write() calls open() if the insertion point is not defined.  In this case the insertion point is not defined, per spec.  It sounds to me like Blink doesn't implement the insertion point semantics correctly, which is exactly what this bug is about.

Just to step you through my testcase as regards the spec... When parsing starts the insertion point is undefined per <https://html.spec.whatwg.org/multipage/syntax.html#insertion-point>.  The places that change the insertion point are:

1) The document.open() algorithm.
2) https://html.spec.whatwg.org/multipage/syntax.html#scriptEndTag
3) https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inforeign:current-node-4
4) https://html.spec.whatwg.org/multipage/syntax.html#the-end

That's all of them.  In this testcase, #3 is not relevant because there are no <svg:script> tags.

OK, so stepping through.  Parsing starts, insertion point is undefined.  The </script> is reached.  This calls https://html.spec.whatwg.org/multipage/syntax.html#scriptEndTag which defines the insertion point and prepares the script.  That calls into https://html.spec.whatwg.org/multipage/scripting.html#prepare-a-script which gets to step 18 substep 5 and queues a task to fire an error event, because step 18 substep 4 failed for this particular src string.  Now we unwind out of prepare-a-script and set the insertion point back to the old value, which was undefined.

Now back in https://html.spec.whatwg.org/multipage/syntax.html#scriptEndTag there is no "pending parser blocking script" (because we never reached step 20 of prepare-a-script, which is what sets that).  So we continue parsing.  Maybe we reach EOF, maybe we don't; it doesn't matter.  The insertion point is undefined and remains so until the task that was posted to fire the error event runs.  When that event runs, it calls https://html.spec.whatwg.org/multipage/webappapis.html#document.write() which in step 5 is supposed to call open() because the insertion point is undefined.  The fact that this does not happen in Chrome is precisely the bug at issue here.

Except the spec for open() says: "The method has no effect if the Document is still being parsed."

https://html.spec.whatwg.org/multipage/webappapis.html#dom-document-open

That's why it matters if the parser is still parsing or not.  (added myself as a cc so I'll see any updates).

> "The method has no effect if the Document is still being parsed."

That's in an informative note, not "the spec for open()".  The actual normative spec for open() is the numbered set of steps, and never once refers to the concept of "still being parsed".  Let's walk through those steps one by one:

1. The Document is not an XML document, so nothing happens.

2. The "throw-on-dynamic-markup-insertion counter" is 0, because there are no custom elements involved so nothing happens.

3. The document is active, so nothing happens.

4. The origins match, so nothing happens.

5. The document does have an active parser.  However the script nesting level is zero, so nothing happens.

6. The ignore-opens-during-unload counter is 0, since we're not in unload, so nothing happens.

7. type is set to "text/html", because that's the default value.

8. The second argument is not "replace", so replace gets set to false.

9. The salvageable state is set to false.

10. There is no beforeunload listener, so prompting to unload has no effect and nothing happens.

After that point, there are no more early returns from this algorithm.  In particular, step 15 will run and remove all child nodes, which in particular will remove an ancestor of the "This text should not be visible" text.

I agree that the wording in the informative note is somewhat misleading, but that's normal in informative notes; they often reflect a simplified view of reality for ease of understanding the broad idea of the API being defined.  A more precise statement would have been something like: "The method has no effect if called during initial execution of the script of a parser-inserted <script> tag."  Or something along those lines.

One correction: I guess the "active parser" bit does refer to "still being parsed".  But note that step 5 of the document.open steps is also conditioned on the script nesting level, which is why it's a no-op in this case.

Thanks for the correction.  Opening back up and I'll take a look to see if I can track down specifically where it is going wrong.

Looks like the core issue is that Blink doesn't follow the spec for how it treats the insertion point.  In blink it is always the current point in the input stream and doesn't follow the spec'd behavior of going in and out of an undefined state (and has been that way for a LONG time):

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/parser/HTMLInputStream.h?sq=package:chromium&dr=CSs&rcl=1473247219&l=71

Kouhei@ or haraken@, any thoughts on how to proceed with this?  We could potentially track the state of "hasInsertionPoint" separately from the input stream and have it follow the transitions in the spec.  Not sure how much that would break though.

Separately, it looks like there is probably also a bug around the "ignoreDescructiveWriteCount" counter where it is incremented for the script event handlers.  Technically, the bug is probably that the script load/error handlers are dispatched directly and not queued as an async task:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/parser/HTMLScriptRunner.cpp?q=IgnoreDestructiveWriteCountIncrementer&sq=package:chromium&l=215&dr=C

> Technically, the bug is probably that the script load/error handlers are
> dispatched directly and not queued as an async task

Per spec, load is always dispatched directly, and error may be dispatched directly or may be an async task depending on the exact error condition.

I guess the question is if it is dispatched directly while in the script execution (before unwinding).  If so then it would not unwind the insertion point to undefined (assuming Chrome's parser had that concept).

Fired synchronously would bypass "Now we unwind out of prepare-a-script and set the insertion point back to the old value, which was undefined." because it is firing from within the script preparation.

> I guess the question is if it is dispatched directly while in the script execution (before unwinding)

https://html.spec.whatwg.org/multipage/scripting.html#execute-the-script-block step 2 and step 8.  Note that this last does sometimes queue a task to fire load, but I think that only applies to inline scripts and modules.

The async error firing is https://html.spec.whatwg.org/multipage/scripting.html#prepare-a-script step 18 substeps 2 and 5.

Note that while the spec seems to fire load events on inline scripts afaict browsers don't actually do that, except IE/Edge.  I filed https://github.com/whatwg/html/issues/1757 just to make sure that the load event for inline scripts is intended.

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

kouhei@, could you triage this issue?

any update?

Thanks for the ping. No. This is still on the backlog.
tkent, hayato: Feel free to take this bug if you are interested.