WPT for initial empty document history replacement behaviors #28541

chromium-wpt-export-bot · 2021-04-16T12:25:46Z

Adds WPTs for various ways navigation can happen on initial empty
documents, and their impact on session history.

Bug: 1215096
Change-Id: If8754e600e5feb6860ebf9aa4df481330b746eaf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2831564
Reviewed-by: Domenic Denicola <[email protected]>
Commit-Queue: Rakina Zata Amni <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1039711}

wpt-pr-bot

The review process for this patch is being conducted in the Chromium project.

domenic · 2021-04-20T15:35:03Z

...sing-the-web/navigating-across-documents/initial-empty-document/iframe-src-204-fragment.html

+
+  // Do a fragment navigation within the initial empty document through iframe.src.
+  iframe.src = url1;
+  await new Promise(resolve => setTimeout(setTimeout(resolve, 100), 100));


setTimeout() takes a function, so passing it the result of setTimeout() (an integer) doesn't really work. I think this should be just await new Promise(resolve => setTimeout(resolve, 100))

Ah, of course. Thank you!

domenic · 2021-04-20T15:35:15Z

...ting-across-documents/initial-empty-document/iframe-src-aboutblank-navigate-immediately.html

+
+promise_test(async t => {
+  const startingHistoryLength = history.length;
+  // Create an ifame with src set to about:blank but navigate away from it immediately below.


"ifame" -> "iframe" here and elsewhere

domenic · 2021-04-20T15:37:18Z

...ers/browsing-the-web/navigating-across-documents/initial-empty-document/resources/helpers.js

+    window.addEventListener("message", t.step_func((event) => {
+      if (event.data == message)
+        resolve();
+    }), { once: true });


You could try removing { once: true } here to see if that helps the tests pass, e.g. if multiple messages are being sent somehow. If it doesn't help though then keeping it is best.

Thanks, I think this helped some tests

domenic · 2021-04-20T15:38:11Z

...ers/browsing-the-web/navigating-across-documents/initial-empty-document/resources/helpers.js

+    if (iframe.contentDocument.readyState === 'complete') {
+      // Wait a bit longer in case the about:blank navigation after the
+      // initial empty document is asynchronous.
+      setTimeout(resolve, 100);


In general all uses of setTimeout should instead be t.step_timeout, which gives them extra time on slow test runners.

Thanks, changed!

domenic · 2021-04-20T15:40:18Z

...ing-the-web/navigating-across-documents/initial-empty-document/window-open-204-fragment.html

+  const openedWindow = windowOpen204(t);
+
+  // Do a fragment navigation within the initial empty document through setting location.href.
+  openedWindow.location.href = url1;


I wonder if this test is breaking because it's parsing #foo relative to this document, and not relative to 204-url.

Try openedWindow.location.hash = "foo" instead?

I tried that and it seems to fail a lot more (then again I changed a lot of other parts as well). Do you think we should use location.hash now?

I'm just worried that this is trying to load window-open-204-fragment.html#foo instead of /common/blank.html?pipe=status(204)#foo. Maybe just explicitly do openedWindow.location.href = "/common/blank.html?pipe=status(204)#foo"?

Oh, I think that's one of the source of flakiness. I changed it to navigate to about:blank#foo explicitly now!

domenic · 2021-04-20T15:42:20Z

...ing-across-documents/initial-empty-document/window-open-aboutblank-navigate-immediately.html

+  openedWindow.location.href = url1;
+  await waitForMessage(t, "loaded");
+  assert_equals(openedWindow.history.length, 1,
+    "history.length must be 1 after normal navigation on initial empty document");


I'm surprised Chrome is failing this?

Yeah this is failing because of https://bugs.chromium.org/p/chromium/issues/detail?id=524208#c22, which is being fixed in crrev.com/c/2794069

domenic · 2021-04-21T15:36:27Z

It's unfortunate that this fails the stability checker, but it seems like this is just an inherently flaky area and we might need to ask for a stability checker override. Still, maybe you could debug at least on Chromium; see the command in https://github.com/web-platform-tests/wpt/pull/28541/checks?check_run_id=2395119277 . Maybe the timeouts just need to be longer or something.

rakina · 2021-04-22T02:58:30Z

It's unfortunate that this fails the stability checker, but it seems like this is just an inherently flaky area and we might need to ask for a stability checker override.

It passes the stability checker now! Looks like we need to wait for the window.open()-ed document to load first before continuing the navigation. Here's the latest Chrome, Firefox, Safari results. Firefox times out in most cases though, maybe they don't fire load for initial empty document loads? Or the load here is actually caused by Chrome and potentially Safari's non-initial empty document about:blank commit, hmm...

Also, unfortunately it looks like the behavior for iframe vs window.open() on is different for navigations after fragment navigations. For iframes, even after fragment navigations, we will still do replacement. For window.open(), we won't do a replace after the initial empty document. Not sure which behavior is nicer here :/

domenic · 2021-04-22T17:51:34Z

Oh, I'm so glad we were able to get a non-flaky version!

I'll discuss next steps in the spec issue based on these results.

hsivonen · 2022-01-12T10:00:33Z

Is this PR expected to be the full extent of WPT changes for this spec change? That is, is it expected that there aren't modifications to existing tests?

rakina · 2022-01-12T22:42:57Z

Is this PR expected to be the full extent of WPT changes for this spec change? That is, is it expected that there aren't modifications to existing tests?

I think so, the "always replace" behavior landed in Chromium and no existing WPTs needed an update

hsivonen · 2022-01-13T09:38:38Z

Is this PR expected to be the full extent of WPT changes for this spec change? That is, is it expected that there aren't modifications to existing tests?

I think so, the "always replace" behavior landed in Chromium and no existing WPTs needed an update

That surprises me. Thanks!

rakina · 2022-08-25T12:11:16Z

Looks like I completely forgot to land this, even after the relevant spec PRs had merged. Before we land, @hsivonen - do you have any comments?

hsivonen · 2022-08-25T12:50:44Z

I think the easiest way forward is to land these and let them propagate to mozilla-central. I'm currently working on about:blank changes, so it's good to have these land, and I'll file follow-ups if I find concerns. Thanks.

hsivonen · 2022-08-25T12:53:41Z

Previously, I wrote:

Is this PR expected to be the full extent of WPT changes for this spec change? That is, is it expected that there aren't modifications to existing tests?

Today, I discovered that https://github.com/web-platform-tests/wpt/blob/master/html/browsers/the-window-object/garbage-collection-and-browsing-contexts/discard_iframe_history_1-1.html#L11 and https://github.com/web-platform-tests/wpt/blob/master/html/browsers/the-window-object/garbage-collection-and-browsing-contexts/discard_iframe_history_2-1.html#L12 need changing.

(I intend to eventually change these two lines myself; I already have a pending patch, but it's going to take some time to get landed due to what else is in the same patch.)

rakina · 2022-08-26T06:01:48Z

Thanks @hsivonen! Regarding the tests you mentioned, looks like Chrome is already failing those since long ago (see expectations 1 and 2, we don't increase the history.length when the iframe loads), so looks like we do need to change the expectations in the WPT itself if Firefox starts behaving similarly.

Also, it looks like some cases are flakily failing only on Firefox: https://github.com/web-platform-tests/wpt/pull/28541/checks?check_run_id=8030346986. Not really sure why only some types of navigations are hitting that... are we OK landing the WPTs as is with the flakiness, or do you have suggestions to handle those (maybe disable the tests on Firefox somehow?)

hsivonen · 2022-08-26T10:00:21Z

Also, it looks like some cases are flakily failing only on Firefox: https://github.com/web-platform-tests/wpt/pull/28541/checks?check_run_id=8030346986. Not really sure why only some types of navigations are hitting that... are we OK landing the WPTs as is with the flakiness, or do you have suggestions to handle those (maybe disable the tests on Firefox somehow?)

My preference is landing these into WPT as flaky, letting them sync to mozilla-central, and then annotating them there. I'm actively working on a change that I expect to fix these on the Firefox side.

Adds WPTs for various ways navigation can happen on initial empty documents, and their impact on session history. Bug: 1215096 Change-Id: If8754e600e5feb6860ebf9aa4df481330b746eaf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2831564 Reviewed-by: Domenic Denicola <[email protected]> Commit-Queue: Rakina Zata Amni <[email protected]> Cr-Commit-Position: refs/heads/main@{#1039711}

rakina · 2022-08-26T13:53:57Z

My preference is landing these into WPT as flaky, letting them sync to mozilla-central, and then annotating them there. I'm actively working on a change that I expect to fix these on the Firefox side.

Thanks! I've landed the WPT on Chromium, lmk if I need to do anything else. Looking forward to see Firefox pass the tests!

WeizhongX · 2022-08-26T21:31:12Z

WPT Command: python3 ./wpt run --channel=nightly --verify --verify-no-chaos-mode --verify-repeat-loop=0 --verify-repeat-restart=10 --github-checks-text-file=/home/test/artifacts/checkrun.md --affected base_head --log-mach-level=info --log-mach=- -y --no-pause --no-restart-on-unexpected --install-fonts --no-headless --verify-log-full --binary=/home/test/build/firefox/firefox firefox

Some affected tests had inconsistent (flaky) results:

Unstable results

Test	Subtest	Results	Messages
/html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-src-204-fragment.html	src	FAIL: 5/10, PASS: 5/10	assert_equals: history.length must not change after normal navigation from initial empty document expected 1 but got 2
/html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-src-204-fragment.html	location.href	FAIL: 6/10, PASS: 4/10	assert_equals: history.length must not change after fragment navigation on initial empty document expected 2 but got 1;assert_equals: history.length must not change after normal navigation from initial empty document expected 1 but got 2
/html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/window-open-aboutblank.html	location.href	FAIL: 5/10, PASS: 5/10	assert_equals: history.length should not increase after normal navigation away from initial empty document expected 1 but got 2
/html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/window-open-aboutblank.html	location.assign	FAIL: 5/10, PASS: 5/10	assert_equals: history.length should not increase after normal navigation away from initial empty document expected 1 but got 2

These may be pre-existing or new flakes. Please try to reproduce (see the above WPT command, though some flags may not be needed when running locally) and determine if your change introduced the flake. If you are unable to reproduce the problem, please tag @web-platform-tests/wpt-core-team in a comment for help.

WeizhongX · 2022-08-26T21:34:40Z

@sj0602, this one is blocked because some tests are flaky on firefox. Below is some instructions to handle such case:

A common problem is the TaskCluster stability check failing, which indicates that a test is flaky. This is very often a problem with the test itself and will require help from the author, so deflaking these can take some time. To avoid these PRs from piling up we aim to clear them from the queue and apply a fix in the background.

Follow these steps (see #23617 for a recent example):

copy the "Unstable Results" from the TaskCluster log into the PR for easier viewing.
create a crbug to record the flake, which should point to the PR and be assigned to the CL author.
reference the new bug in the PR.
force merge the PR(ask @KyleJu or @DanielRyanSmith for help).

sj0602 · 2022-08-26T23:10:09Z

Files crbug.com/1357195.

WeizhongX · 2022-08-26T23:30:56Z

@KyleJu @DanielRyanSmith can you help admin merge? Thanks!

chromium-wpt-export-bot added chromium-export do not merge yet labels Apr 16, 2021

wpt-pr-bot added the html label Apr 16, 2021

wpt-pr-bot approved these changes Apr 16, 2021

View reviewed changes

rakina mentioned this pull request Apr 16, 2021

Which navigations should add/replace new history entry after initial empty document? whatwg/html#6491

Closed

chromium-wpt-export-bot force-pushed the chromium-export-cl-2831564 branch 2 times, most recently from b946725 to f2f3358 Compare April 19, 2021 09:31

domenic mentioned this pull request Apr 19, 2021

Always replace the initial about:blank Document whatwg/html#6595

Merged

3 tasks

chromium-wpt-export-bot force-pushed the chromium-export-cl-2831564 branch 4 times, most recently from 9c04a64 to 4ba08c1 Compare April 20, 2021 08:59

domenic reviewed Apr 20, 2021

View reviewed changes

chromium-wpt-export-bot force-pushed the chromium-export-cl-2831564 branch 2 times, most recently from 34be15c to 89abc91 Compare April 20, 2021 21:54

chromium-wpt-export-bot force-pushed the chromium-export-cl-2831564 branch from 89abc91 to 6548bf2 Compare April 22, 2021 01:57

chromium-wpt-export-bot force-pushed the chromium-export-cl-2831564 branch 7 times, most recently from 109c21e to a1bff1e Compare July 7, 2021 14:16

rakina mentioned this pull request Jul 8, 2021

Newly created browsing contexts will almost always reject history.pushState/replaceState whatwg/html#6836

Closed

rakina mentioned this pull request Jul 15, 2021

Proposal: stop triggering navigations to about:blank on iframe insertion whatwg/html#6863

Closed

domenic mentioned this pull request Jul 16, 2021

Change initial about:blank navigation behavior for iframes and popups whatwg/html#6869

Merged

3 tasks

chromium-wpt-export-bot force-pushed the chromium-export-cl-2831564 branch from a1bff1e to e75ccfb Compare August 11, 2021 11:16

chromium-wpt-export-bot force-pushed the chromium-export-cl-2831564 branch from 5b57154 to 4afced7 Compare August 25, 2022 09:38

chromium-wpt-export-bot force-pushed the chromium-export-cl-2831564 branch from 4afced7 to 4a261f9 Compare August 26, 2022 05:28

chromium-wpt-export-bot changed the title ~~WPT for initial empty document on iframes~~ WPT for initial empty document history replacement behaviors Aug 26, 2022

chromium-wpt-export-bot force-pushed the chromium-export-cl-2831564 branch from 4a261f9 to 2202cdb Compare August 26, 2022 12:03

chromium-wpt-export-bot removed the do not merge yet label Aug 26, 2022

chromium-wpt-export-bot force-pushed the chromium-export-cl-2831564 branch from 2202cdb to f9b746b Compare August 26, 2022 12:12

sj0602 closed this Aug 26, 2022

sj0602 reopened this Aug 26, 2022

sj0602 closed this Aug 26, 2022

sj0602 reopened this Aug 26, 2022

KyleJu approved these changes Aug 27, 2022

View reviewed changes

KyleJu merged commit 91c3778 into master Aug 27, 2022

KyleJu deleted the chromium-export-cl-2831564 branch August 27, 2022 00:33

moz-wptsync-bot added mozilla:gecko-blocked and removed mozilla:gecko-blocked labels Aug 31, 2022

domenic mentioned this pull request Mar 16, 2023

History length/index are not synchronously initialized in new navigables whatwg/html#9030

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WPT for initial empty document history replacement behaviors #28541

WPT for initial empty document history replacement behaviors #28541

chromium-wpt-export-bot commented Apr 16, 2021 •

edited

Loading

wpt-pr-bot left a comment

domenic Apr 20, 2021

rakina Apr 21, 2021

domenic Apr 20, 2021

domenic Apr 20, 2021

rakina Apr 21, 2021

domenic Apr 20, 2021

rakina Apr 21, 2021

domenic Apr 20, 2021

rakina Apr 21, 2021

domenic Apr 21, 2021

rakina Apr 22, 2021

domenic Apr 20, 2021

rakina Apr 21, 2021

domenic commented Apr 21, 2021

rakina commented Apr 22, 2021 •

edited

Loading

domenic commented Apr 22, 2021

hsivonen commented Jan 12, 2022

rakina commented Jan 12, 2022

hsivonen commented Jan 13, 2022

rakina commented Aug 25, 2022

hsivonen commented Aug 25, 2022

hsivonen commented Aug 25, 2022

rakina commented Aug 26, 2022

hsivonen commented Aug 26, 2022

rakina commented Aug 26, 2022

WeizhongX commented Aug 26, 2022

WeizhongX commented Aug 26, 2022

sj0602 commented Aug 26, 2022

WeizhongX commented Aug 26, 2022

WPT for initial empty document history replacement behaviors #28541

WPT for initial empty document history replacement behaviors #28541

Conversation

chromium-wpt-export-bot commented Apr 16, 2021 • edited Loading

wpt-pr-bot left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic commented Apr 21, 2021

rakina commented Apr 22, 2021 • edited Loading

domenic commented Apr 22, 2021

hsivonen commented Jan 12, 2022

rakina commented Jan 12, 2022

hsivonen commented Jan 13, 2022

rakina commented Aug 25, 2022

hsivonen commented Aug 25, 2022

hsivonen commented Aug 25, 2022

rakina commented Aug 26, 2022

hsivonen commented Aug 26, 2022

rakina commented Aug 26, 2022

WeizhongX commented Aug 26, 2022

Unstable results

WeizhongX commented Aug 26, 2022

sj0602 commented Aug 26, 2022

WeizhongX commented Aug 26, 2022

chromium-wpt-export-bot commented Apr 16, 2021 •

edited

Loading

rakina commented Apr 22, 2021 •

edited

Loading