Bug 282609
| Summary: | [Navigation] navigation-history-entry/entries-after-blob-navigation.html is failing | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Rob Buis <rbuis> |
| Component: | Page Loading | Assignee: | Chris Dumez <cdumez> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | beidson, cdumez, webkit-bug-importer |
| Priority: | P2 | Keywords: | InRadar |
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| See Also: | https://github.com/web-platform-tests/wpt/pull/49553 | ||
| Bug Depends on: | |||
| Bug Blocks: | 258384 | ||
Rob Buis
Fix blob: navigation related timeout.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Rob Buis
navigation-history-entry/entries-after-blob-navigation.html times out.
The support for blob: seems to be good in WebKit, so it may be a problem similar to webkit.org/b/282601.
Radar WebKit Bug Importer
<rdar://problem/139725684>
Chris Dumez
The test times out because the Blob created by the test is not recognized as html so we try to download it instead of rendering it.
Updating the test to use:
```
i.src = URL.createObjectURL(new Blob(["<body></body>"], { type: "text/html" }));
```
instead of
```
i.src = URL.createObjectURL(new Blob(["<body></body>"]));
```
makes it run.
However, the test still fails later:
```
--- /Volumes/Work/WebKit/OpenSource/WebKitBuild/Debug/layout-test-results/imported/w3c/web-platform-tests/navigation-api/navigation-history-entry/entries-after-blob-navigation-expected.txt
+++ /Volumes/Work/WebKit/OpenSource/WebKitBuild/Debug/layout-test-results/imported/w3c/web-platform-tests/navigation-api/navigation-history-entry/entries-after-blob-navigation-actual.txt
@@ -1,6 +1,4 @@
-Harness Error (TIMEOUT), message = null
+FAIL entries() after navigation to a blob: URL assert_not_equals: got disallowed value "a971b867-1f08-47e2-ab20-932e9615c1ea"
-TIMEOUT entries() after navigation to a blob: URL Test timed out
-
```
Chris Dumez
Fixing the WPT test in https://github.com/web-platform-tests/wpt/pull/49553 but we still need to investigate the TEXT failure once the timeout is addressed.
Chris Dumez
I did some debugging and the remaining issue is not related to blobs. The issue is that `i.src = URL.createObjectURL(new Blob(["<body></body>"], { type: "text/html" }));` is called synchronously inside the top frame's load event. As a result, we re-use the same HistoryItem for the navigation. If this was called inside a setTimeout(0), the test would pass.
As far as I know, it is pretty standard behavior to lock the back/forward list if the navigation is triggered from inside the load event. I have to look more into it to see if and why we differ with other browsers here.
Chris Dumez
I think this is the relevant spec:
https://html.spec.whatwg.org/#navigate-an-iframe-or-frame
Step 2:
> If element's content navigable's active document is not completely loaded, then set historyHandling to "replace".
Since we're inside the load event handler, WebKit treats the document as not completely loaded and decides to replace the HistoryItem, whereas the test expects 2 separate history items.
Chris Dumez
Interestingly, I think we don't match the HTML spec here.
It says that the document is "completely loaded" if its "completely loaded time" is set.
As per https://html.spec.whatwg.org/#completely-finish-loading, the "completely loaded time" should get set *right before* firing the load event. I guess ideally we'd match the spec. This is a large change though.
Chris Dumez
It is this check in `NavigationScheduler::scheduleLocationChange()` that is causing this:
```
if (lockBackForwardList == LockBackForwardList::No)
lockBackForwardList = mustLockBackForwardList(m_frame);
```
The caller here says NOT to lock the back/forward list. However, lockBackForwardList gets overwritten here.
Chris Dumez
Pull request: https://github.com/WebKit/WebKit/pull/37514
EWS
Committed 287549@main (f183cbdda713): <https://commits.webkit.org/287549@main>
Reviewed commits have been landed. Closing PR #37514 and removing active labels.