HTML: document.open() and tasks #10818

annevk · 2018-05-03T11:24:01Z

This is not consistently implemented and given that Firefox does not appear to do it at all I hope we can get away without it. Tests: web-platform-tests/wpt#10818.

domenic · 2018-05-03T15:59:49Z

Some other potential tests:

An image being created and its load event being fired
A marquee element's start event being fired
A fetch completing

annevk · 2018-05-03T16:41:22Z

fetch() is interesting, I test that in #10789 and only Firefox fires an error event there. I guess I should add an onload with unreached_func to see if some trigger that.

TimothyGu · 2018-07-13T19:25:02Z

w3c-test:mirror

foolip · 2018-08-03T14:25:44Z

html/webappapis/dynamic-markup-insertion/opening-the-input-stream/tasks.window.js

+// second actually calls document.open() to test if the method call removes
+// that specific task from the queue.
+
+setup({


Can you point to the reason this is needed? (the promise rejection test below)

Reason being that this can also mask other problems with the test, so usage should be well motivated. (and is)

foolip · 2018-08-03T14:27:21Z

html/webappapis/dynamic-markup-insertion/opening-the-input-stream/tasks.window.js

+    t.add_cleanup(() => frame.remove());
+    frame.onload = t.step_func(() => {
+      // Make sure there is no parser. Firefox seems to have an additional
+      // non-spec-compliant readiness state "uninitialized", so test for the


Can you file/link to a bug for this?

foolip · 2018-08-03T14:28:21Z

html/webappapis/dynamic-markup-insertion/opening-the-input-stream/tasks.window.js

+
+  async_test(t => {
+    const frame = document.body.appendChild(document.createElement("iframe"));
+    // The empty HTML seems to be necessary to cajole Chrome into firing a load


Is there a bug that this could be linked to?

foolip · 2018-08-03T14:29:11Z

html/webappapis/dynamic-markup-insertion/opening-the-input-stream/tasks.window.js

+
+taskTest("timeout", (t, frame, open) => {
+  let happened = false;
+  // Work around the lint script, since we can't use step_timeout here.


I would probably add this to lint.whitelist, even if that means it's possible to accidentally add setTimeout elsewhere in this file. It's a small file, and that this fools the linter is arguably just a bug. (For more granular opt-outs some special comment syntax would be an option.)

foolip · 2018-08-03T15:34:30Z

html/webappapis/dynamic-markup-insertion/opening-the-input-stream/tasks.window.js

+taskTest("timeout", (t, frame, open) => {
+  let happened = false;
+  // Work around the lint script, since we can't use step_timeout here.
+  frame.contentWindow['setTimeout'](() => happened = true, 100);


Rather than setting happened here, how about just t.step_func_done() as the callback to pass the test ASAP?

The following test just times out if there isn't a 2nd message event, so I'd let this test also time out. If that's no good, is there any timeout one could pick that doesn't make the test more likely to be flaky on slow runners?

foolip · 2018-08-03T15:42:47Z

html/webappapis/dynamic-markup-insertion/opening-the-input-stream/tasks.window.js

+    counter++;
+    assert_less_than_equal(counter, 2);
+    if (counter == 2) {
+      t.done();


(This is the test that will time out after 30s or so.)

This didn't seem to timeout in the latest Travis run…

Travis unfortunately doesn't fail unless the tests are flaky, it doesn't help catch any other problem, even totally broken tests. Did you read the logs to see that it didn't time out, or how could you tell?

Oh right, I guess I meant rather that it wasn't flaky.

foolip · 2018-08-03T15:47:06Z

html/webappapis/dynamic-markup-insertion/opening-the-input-stream/tasks.window.js

+  });
+});
+
+taskTest("canvas.toBlob()", (t, frame, open) => {


The "queue a task" of toBlob happens inside "in parallel" so might not have happened when document.open() is called, is that a problem for this test? Does any browser currently fail this?

Good point. No browser actually seems to fail this, but I should note that setTimeout is defined the same way (wait in parallel, and then queue a task), and browsers have very different behavior with regards to that. That's why I think this is a good test.

Interesting, I didn't know setTimeout did that internally in the spec. I tend to think of "in parallel" as possibly happening on another thread or process (thus requiring "queue a task" to rejoin main thread) and that's certainly not how setTimeout is implemented. Test LGTM in any case, at worst it'll pass everywhere always :)

foolip · 2018-08-03T15:50:19Z

html/webappapis/dynamic-markup-insertion/opening-the-input-stream/tasks.window.js

+  const doc = frame.contentDocument;
+  const marquee = doc.body.appendChild(doc.createElement("marquee"));
+  open(frame.contentDocument);
+  marquee.addEventListener("start", t.step_func_done());


I like this one. If you want one which is a bit special in practice try a new audio element and changing the volume attribute to 0.5. That should queue a task to fire volumechange. But implementations do something special to emulate that per-instance task source that media elements are supposed to have, and I wouldn't be surprised if it's different from other things tested here.

Based on the work by Anne van Kesteren in whatwg#3651, but without the parts concerning the session history. Changes to that area will come later (see whatwg#3818). Tests: web-platform-tests/wpt#10773 Tests: web-platform-tests/wpt#10778 Tests: web-platform-tests/wpt#10789 Tests: web-platform-tests/wpt#10815 Tests: web-platform-tests/wpt#10818 Fixes whatwg#1698. Fixes whatwg#3286. Fixes whatwg#3306. Closes whatwg#3665. Co-authored-by: Anne van Kesteren <[email protected]>

Based on the work by Anne van Kesteren in whatwg#3651, but without the parts concerning the session history. Changes to that area will come later (see whatwg#3818). Tests: web-platform-tests/wpt#10773 Tests: web-platform-tests/wpt#10778 Tests: web-platform-tests/wpt#10815 Tests: web-platform-tests/wpt#10818 Fixes whatwg#1698. Fixes whatwg#3286. Fixes whatwg#3306. Closes whatwg#3665. Co-authored-by: Anne van Kesteren <[email protected]>

In particular, removes the realm creation, document unloading, and tasks removal steps. Based on the work by Anne van Kesteren in #3651, but without the parts concerning the session history. Changes to that area will come later (see #3818). These changes allow us to remove several auxiliary concepts that only existed to support document.open(): - The recycle parameter to the "unload a Document" algorithm - The window parameter to the "set the active document" algorithm Tests: web-platform-tests/wpt#10773 Tests: web-platform-tests/wpt#10778 Tests: web-platform-tests/wpt#10815 Tests: web-platform-tests/wpt#10818 Fixes #1698. Fixes #3286. Fixes #3306. Closes #3665.

foolip

Only remaining comments requiring change are the "link to bug" ones, if you like.

For whatwg/html#3651.

For whatwg/html#3918.

In particular, removes the realm creation, document unloading, and tasks removal steps. Based on the work by Anne van Kesteren in whatwg#3651, but without the parts concerning the session history. Changes to that area will come later (see whatwg#3818). These changes allow us to remove several auxiliary concepts that only existed to support document.open(): - The recycle parameter to the "unload a Document" algorithm - The window parameter to the "set the active document" algorithm Tests: web-platform-tests/wpt#10773 Tests: web-platform-tests/wpt#10778 Tests: web-platform-tests/wpt#10815 Tests: web-platform-tests/wpt#10818 Fixes whatwg#1698. Fixes whatwg#3286. Fixes whatwg#3306. Closes whatwg#3665.

wpt-pr-bot added the html label May 3, 2018

wpt-pr-bot requested review from ayg, jdm, jgraham and zqzhang May 3, 2018 11:24

annevk mentioned this pull request May 3, 2018

No longer remove tasks for document.open() whatwg/html#3665

Closed

annevk added the do not merge yet label May 3, 2018

TimothyGu mentioned this pull request Jul 12, 2018

Tracking issue for document.open() overhaul whatwg/html#3818

Closed

19 tasks

TimothyGu force-pushed the annevk/document-open-steps-and-tasks branch from 8cc338c to 39aa1e6 Compare July 13, 2018 19:24

wpt-pr-bot requested review from domenic, foolip, sideshowbarker and zcorpan and removed request for ayg July 13, 2018 19:25

TimothyGu self-assigned this Jul 31, 2018

foolip reviewed Aug 3, 2018

View reviewed changes

TimothyGu mentioned this pull request Aug 14, 2018

document.open() simplifications: realm creation, unloading, tasks removal whatwg/html#3918

Merged

TimothyGu force-pushed the annevk/document-open-steps-and-tasks branch from 1c519a7 to f07c4e2 Compare August 17, 2018 18:22

wpt-pr-bot added the infra label Aug 17, 2018

foolip approved these changes Aug 19, 2018

View reviewed changes

TimothyGu force-pushed the annevk/document-open-steps-and-tasks branch from c90163d to 176e9c0 Compare August 20, 2018 15:28

annevk added 2 commits August 20, 2018 14:30

HTML: document.open() and tasks

1680ac5

For whatwg/html#3651.

add a postMessage test

88a49f2

TimothyGu and others added 9 commits August 20, 2018 14:30

More tests

d376ffd

Pass linting

04cd24d

Use /common/blank.html

cc73057

experiment with promise rejection: ignore uncaught exceptions

a8c861e

Fix Safari

89a8677

Better fix

e9d76e4

Remove 005

1f8ecd0

address some comments

8b7bd3c

More comments

fcf210e

TimothyGu force-pushed the annevk/document-open-steps-and-tasks branch from 176e9c0 to fcf210e Compare August 20, 2018 18:33

TimothyGu merged commit 87329a1 into master Aug 20, 2018

TimothyGu deleted the annevk/document-open-steps-and-tasks branch August 20, 2018 18:53

zcorpan pushed a commit that referenced this pull request Aug 27, 2018

HTML: document.open() and tasks (#10818)

2a6959a

For whatwg/html#3918.

TimothyGu removed the do not merge yet label Sep 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HTML: document.open() and tasks #10818

HTML: document.open() and tasks #10818

annevk commented May 3, 2018 •

edited by wpt-pr-bot

Loading

domenic commented May 3, 2018

annevk commented May 3, 2018

TimothyGu commented Jul 13, 2018

foolip Aug 3, 2018

foolip Aug 3, 2018

foolip Aug 3, 2018

foolip Aug 3, 2018

foolip Aug 3, 2018

foolip Aug 3, 2018

TimothyGu Aug 17, 2018

foolip Aug 17, 2018

TimothyGu Aug 17, 2018

foolip Aug 3, 2018

TimothyGu Aug 17, 2018

foolip Aug 19, 2018

foolip Aug 3, 2018

foolip left a comment

HTML: document.open() and tasks #10818

HTML: document.open() and tasks #10818

Conversation

annevk commented May 3, 2018 • edited by wpt-pr-bot Loading

domenic commented May 3, 2018

annevk commented May 3, 2018

TimothyGu commented Jul 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

foolip left a comment

Choose a reason for hiding this comment

annevk commented May 3, 2018 •

edited by wpt-pr-bot

Loading