Test moving <script>s between documents by domenic · Pull Request #5911 · web-platform-tests/wpt

domenic · 2017-05-12T21:09:02Z

This follows the changes in whatwg/html#2673, but also tests the issue at whatwg/html#2137 in favor of the current spec.

ghost · 2017-05-12T21:17:30Z

View the complete job log.

Firefox (nightly)

Testing web-platform-tests at revision 65891e2
Using browser at version BuildID 20170519133342; SourceStamp 8e98dab5054dd093a37ba20c62cf0523e484cfbd
Starting 10 test iterations
All results were stable

All results

1 test ran

/html/semantics/scripting-1/the-script-element/moving-between-documents.html

Subtest	Results	Messages
	OK
`text/javascript: moving to another document during parsing, inline script`	PASS
`text/javascript: moving to another document during parsing, external script`	PASS
`text/javascript: moving to another Window's document during fetching`	FAIL	`assert_unreached: onerror Reached unreachable code`
`text/javascript: moving to a document where scripting is disabled during fetching`	FAIL	`assert_unreached: onerror Reached unreachable code`
`module: moving to another document during parsing, inline script`	PASS
`module: moving to another document during parsing, external script`	PASS
`module: moving to another Window's document during fetching`	PASS
`module: moving to a document where scripting is disabled during fetching`	PASS

ghost · 2017-05-12T21:17:30Z

View the complete job log.

Chrome (unstable)

Testing web-platform-tests at revision 4a26457
Using browser at version 60.0.3100.0 dev
Starting 10 test iterations
All results were stable

All results

1 test ran

/html/semantics/scripting-1/the-script-element/moving-between-documents.html

Subtest	Results	Messages
	OK
`text/javascript: moving to another document during parsing, inline script`	FAIL	`assert_equals: The script must not have executed in this window expected (undefined) undefined but got (boolean) true`
`text/javascript: moving to another document during parsing, external script`	PASS
`text/javascript: moving to another Window's document during fetching`	FAIL	`assert_unreached: onload Reached unreachable code`
`text/javascript: moving to a document where scripting is disabled during fetching`	FAIL	`assert_unreached: onload Reached unreachable code`
`module: moving to another document during parsing, inline script`	PASS
`module: moving to another document during parsing, external script`	FAIL	`assert_unreached: onload Reached unreachable code`
`module: moving to another Window's document during fetching`	PASS
`module: moving to a document where scripting is disabled during fetching`	PASS

domenic · 2017-05-12T21:18:35Z

Test result summary for classic scripts, with blank cells meaning browsers match the proposed spec:

Test case	EdgeHTML	Blink + WebKit	Gecko
Moving to another document during parsing, inline script	executes	executes
Moving to another document during parsing, external script		onload + executes
Moving to another Window's document during fetching		onload + executes	onerror (but does not execute)
Moving to a document where scripting is disabled during fetching		onload + executes	onerror (but does not execute)

For module scripts, Edge interestingly starts failing the "Moving to a document where scripting is disabled during fetching" case (via onload + executes), but passing the first one.

hiroshige-g · 2017-05-19T22:10:22Z

+      iframe.contentDocument.close();
+
+      assert_false(window.didExecute, "The script must not have executed in this window");
+      assert_equals(iframe.contentWindow.didExecute, undefined,


Even if the script tested ("s1") is not executed, the test fails on Chromium, because didExecute is false (not undefined).
iframe.contentWindow.didExecute is set to false at Line 32 and is reset to undefined when document.write() is called on Firefox (but not on Chromium) at Line 33.

Minimized case:

<body> <script> const iframe = document.createElement("iframe"); iframe.onload = () => { iframe.contentWindow.didExecute = false; iframe.contentDocument.write("<div>"); console.log(iframe.contentWindow.didExecute); }; document.body.appendChild(iframe); </script> </body>

console log: undefined on Firefox and false on Chromium.

Ah, because of the implicit document.open() in document.write(), OK. I've uploaded a patch that just uses undefined vs. true instead of false vs. true which should fix this. Please take a look :)

ghost · 2017-05-20T00:11:53Z

View the complete job log.

Sauce (safari)

Testing web-platform-tests at revision 65891e2
Using browser at version 10.0
Starting 10 test iterations
All results were stable

All results

1 test ran

/html/semantics/scripting-1/the-script-element/moving-between-documents.html

Subtest	Results	Messages
	TIMEOUT
`text/javascript: moving to another document during parsing, inline script`	FAIL	`assert_equals: The script must not have executed in this window expected (undefined) undefined but got (boolean) true`
`text/javascript: moving to another document during parsing, external script`	FAIL	`assert_equals: The script must not have executed in this window expected (undefined) undefined but got (boolean) true`
`text/javascript: moving to another Window's document during fetching`	PASS
`text/javascript: moving to a document where scripting is disabled during fetching`	PASS
`module: moving to another document during parsing, inline script`	PASS
`module: moving to another document during parsing, external script`	FAIL	`assert_unreached: onload Reached unreachable code`
`module: moving to another Window's document during fetching`	PASS
`module: moving to a document where scripting is disabled during fetching`	PASS

ghost · 2017-05-20T00:11:54Z

View the complete job log.

Sauce (MicrosoftEdge)

Testing web-platform-tests at revision 4a26457
Using browser at version 14.14393
Starting 10 test iterations

Unstable results

Test	Subtest	Results	Messages
`/html/semantics/scripting-1/the-script-element/moving-between-documents.html`		OK: 9/10, TIMEOUT: 1/10
`/html/semantics/scripting-1/the-script-element/moving-between-documents.html`	`text/javascript: moving to another document during parsing, inline script`	FAIL: 9/10, MISSING: 1/10	`assert_equals: The script must not have executed in this window expected (undefined) undefined but got (boolean) true`
`/html/semantics/scripting-1/the-script-element/moving-between-documents.html`	`text/javascript: moving to another document during parsing, external script`	PASS: 9/10, MISSING: 1/10
`/html/semantics/scripting-1/the-script-element/moving-between-documents.html`	`text/javascript: moving to another Window's document during fetching`	PASS: 9/10, MISSING: 1/10
`/html/semantics/scripting-1/the-script-element/moving-between-documents.html`	`text/javascript: moving to a document where scripting is disabled during fetching`	PASS: 9/10, MISSING: 1/10
`/html/semantics/scripting-1/the-script-element/moving-between-documents.html`	`module: moving to another document during parsing, inline script`	PASS: 9/10, MISSING: 1/10
`/html/semantics/scripting-1/the-script-element/moving-between-documents.html`	`module: moving to another document during parsing, external script`	PASS: 9/10, MISSING: 1/10
`/html/semantics/scripting-1/the-script-element/moving-between-documents.html`	`module: moving to another Window's document during fetching`	PASS: 9/10, MISSING: 1/10
`/html/semantics/scripting-1/the-script-element/moving-between-documents.html`	`module: moving to a document where scripting is disabled during fetching`	FAIL: 9/10, MISSING: 1/10	`assert_unreached: onload Reached unreachable code`

All results

1 test ran

/html/semantics/scripting-1/the-script-element/moving-between-documents.html

Subtest	Results	Messages
	OK: 9/10, TIMEOUT: 1/10
`text/javascript: moving to another document during parsing, inline script`	FAIL: 9/10, MISSING: 1/10	`assert_equals: The script must not have executed in this window expected (undefined) undefined but got (boolean) true`
`text/javascript: moving to another document during parsing, external script`	PASS: 9/10, MISSING: 1/10
`text/javascript: moving to another Window's document during fetching`	PASS: 9/10, MISSING: 1/10
`text/javascript: moving to a document where scripting is disabled during fetching`	PASS: 9/10, MISSING: 1/10
`module: moving to another document during parsing, inline script`	PASS: 9/10, MISSING: 1/10
`module: moving to another document during parsing, external script`	PASS: 9/10, MISSING: 1/10
`module: moving to another Window's document during fetching`	PASS: 9/10, MISSING: 1/10
`module: moving to a document where scripting is disabled during fetching`	FAIL: 9/10, MISSING: 1/10	`assert_unreached: onload Reached unreachable code`

annevk · 2018-02-20T15:15:16Z

+for (const type of ["text/javascript", "module"]) {
+  async_test(t => {
+    t.add_cleanup(() => {
+      window.didExecute = undefined;


Maybe also make this remove the iframe? (Same below.)

Happy to do so, but why?

I guess it's fine either way. Just seems a little cleaner to only have the results.

annevk · 2018-02-20T15:17:10Z

+        assert_equals(iframe.contentWindow.didExecute, undefined,
+          "The script must not have executed in the iframe window");
+        t.done();
+      }, 3000);


Why a timeout here? Since the parser inserts the script element, wouldn't it simply block on it loading if a user agent doesn't implement the spec?

That's also why the belated onload/onerror handlers seem problematic. They probably need to be in the markup, no?

They should probably be before setting s.src, it's true.

annevk · 2018-02-20T15:20:12Z

+      assert_equals(window.didExecute, undefined, "The script must not have executed in this window");
+      t.done();
+    }, 3000);
+  }, `${type}: moving to a document where scripting is disabled during fetching`);


I think using srcdoc with sandbox might be better or good as an alternative test, since it seems likely browsers would check "browsing-context connected" and not just "connected" (and in fact this is what Chrome, Edge, and Safari seem to do).

The definition of scripting-is-disabled means scripting is disabled for browsing-context-less nodes.

There's a separate question as to whether the prepare-a-script algorithm should be using connected vs. BC connected for fetching, but I'd rather keep that separate...

Anyway, happy to also test sandboxed iframes.

Hmm, on second thought, I'm unsure why we'd have such a test since it would always involve creating a new window (and thus settings object). srcdoc="" does not avoid that, as far as I can tell. So it'd be the same as other iframe tests.

If you disagree I'm happy for your to push an additional test showing me what you mean.

This follows the changes in whatwg/html#2673, but also tests the issue at whatwg/html#2137 in favor of the current spec.

…id Firefox vs. others issue

ghost · 2018-02-20T22:39:15Z

Build PASSED

Started: 2018-02-21 20:54:30
Finished: 2018-02-21 21:03:12

Failing Jobs

safari:11.0

View more information about this build on:

Turns out the timeout is needed at least a bit

Intent to deprecate and remove: https://groups.google.com/a/chromium.org/d/topic/blink-dev/KyB2mwOmjrk/discussion This CL also removes crash tests that require executing scripts moved between documents. Code paths related to scripts moved between documents will be covered by e.g. web-platform-tests/wpt#5911 Bug: 721914 Change-Id: I6acb7182c4a62accd5e5285adb6a77a2cd891465 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1843508 Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Cr-Commit-Position: refs/heads/master@{#704503}

domenic · 2020-04-22T03:17:38Z

Superceded by #19632

domenic added the status:needs-spec-decision label May 12, 2017

wpt-pr-bot added the html label May 12, 2017

wpt-pr-bot requested review from ayg, jdm, jgraham, zcorpan and zqzhang May 12, 2017 21:09

domenic mentioned this pull request May 12, 2017

Prevent execution of (some) scripts moving between documents whatwg/html#2673

Merged

3 tasks

hiroshige-g reviewed May 19, 2017

View reviewed changes

domenic force-pushed the scripts-between-documents branch from 861e061 to 0e5ba75 Compare May 20, 2017 00:00

nox mentioned this pull request Oct 9, 2017

Should execute script on the global of the document of a script element at time it is prepared. servo/servo#14390

Open

annevk reviewed Feb 20, 2018

View reviewed changes

domenic added 4 commits February 20, 2018 16:43

Test moving <script>s between documents

1829594

This follows the changes in whatwg/html#2673, but also tests the issue at whatwg/html#2137 in favor of the current spec.

Also test module scripts

e42a9f6

Make window.didExecute undefined, not false, for no execution, to avo…

ab2e964

…id Firefox vs. others issue

No need for timeout per annevk's review

e70aaf6

domenic force-pushed the scripts-between-documents branch from 0e5ba75 to e70aaf6 Compare February 20, 2018 22:30

Further fixes per review and my own discoveries

edb6add

Turns out the timeout is needed at least a bit

hiroshige-g mentioned this pull request Dec 12, 2019

Scripts moved between documents #19632

Merged

gsnedders closed this Jan 24, 2020

gsnedders deleted the scripts-between-documents branch January 24, 2020 18:07

gsnedders restored the scripts-between-documents branch January 24, 2020 18:40

Hexcles reopened this Jan 24, 2020

domenic closed this Apr 22, 2020

domenic deleted the scripts-between-documents branch April 22, 2020 03:17

Uh oh!

Conversation

domenic commented May 12, 2017 • edited by wpt-pr-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented May 12, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Firefox (nightly)

All results

Uh oh!

ghost commented May 12, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Chrome (unstable)

All results

Uh oh!

domenic commented May 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented May 20, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sauce (safari)

All results

Uh oh!

ghost commented May 20, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sauce (MicrosoftEdge)

Unstable results

All results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Feb 20, 2018 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build PASSED

Failing Jobs

Uh oh!

domenic commented Apr 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

domenic commented May 12, 2017 •

edited by wpt-pr-bot

Loading

ghost commented May 12, 2017 •

edited by ghost

Loading

ghost commented May 12, 2017 •

edited by ghost

Loading

domenic commented May 12, 2017 •

edited

Loading

ghost commented May 20, 2017 •

edited by ghost

Loading

ghost commented May 20, 2017 •

edited by ghost

Loading

ghost commented Feb 20, 2018 •

edited by ghost

Loading