Bug 301239
| Summary: | [LCP] largest-contentful-paint/image-upscaling.html fails due to window.open("") | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> |
| Component: | DOM | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW | ||
| Severity: | Normal | CC: | achristensen, annevk, basuke, cdumez, simon.fraser, webkit-bug-importer |
| Priority: | P2 | Keywords: | InRadar |
| Version: | Other | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Simon Fraser (smfr)
largest-contentful-paint/image-upscaling.html fails in wpt.fyi and when I run locally with web driver:
https://wpt.fyi/results/largest-contentful-paint/image-upscaling.html?label=master&label=experimental&aligned&view=interop&q=label%3Ainterop-2025-core-web-vitals
The test passes in WTR.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Radar WebKit Bug Importer
<rdar://problem/163154850>
Simon Fraser (smfr)
The test is trying to load the image from `'http://web-platform.test:8000/images/blue.png'` and this seems to fail (but I can load the image from that URL in a separate window).
Simon Fraser (smfr)
Ah, the test is doing:
const popup = window.open();
t.add_cleanup(() => popup.close());
const image = popup.document.createElement('img');
image.src = imageURL;
Simon Fraser (smfr)
The test passes if Safari is set to allow popups on all sites
Sam Sneddon [:gsnedders]
(In reply to Simon Fraser (smfr) from comment #4)
> The test passes if Safari is set to allow popups on all sites
I can’t reproduce that, and even after you changed the test to call `window.bless`, I still see this failing. The window definitely opens, it just never moves forward.
What’s _more strange_ is I see it consistently passing if I change `window.open()` to `window.open("about:blank")` (the default is `""`, and that should behave identically as it gets rewritten to "about:blank" per-spec as far as I can tell).
Sam Sneddon [:gsnedders]
So with `window.open("")`, we end up passing through an empty invalid URL all the way to WebCore::DocumentLoader::maybeLoadEmpty where we end up changing the URL to about:blank in https://github.com/WebKit/WebKit/blob/7657ab7302f0dc973220ea5b984e1caee50f106d/Source/WebCore/loader/DocumentLoader.cpp#L2069.
However, that leads http://wpt.live/largest-contentful-paint/image-upscaling.html to timing out in both Safari and MiniBrowser (but not in WebKitTestRunner), seemingly never calling the PerformanceObserver callback.
If we change the test to `window.open("about:blank")` (or, presumably, change WebCore::LocalDOMWindow::createWindow to more closely follow the spec (https://html.spec.whatwg.org/multipage/nav-history-apis.html#window-open-steps) and change it to about:blank early enough), then the test *does* pass.
This suggests that something is notably different based on when we set the URL to about:blank, especially when it comes to PerformanceObserver.
Beyond just changing it to about:blank earlier in the `window.open` case, I did wonder about adding some asserts that we have a valid URL, but bug 136010 makes it clear that passing invalid URLs all the way through was a deliberate choice. Nevertheless, I do wonder if it is worthwhile even locally adding some asserts and seeing how many tests fail, in case there's other similar situations (iframes most obviously) where there might be similar bugs lurking.
Sam Sneddon [:gsnedders]
Note that https://github.com/web-platform-tests/wpt/pull/56018 is moving the original test to /largest-contentful-paint/image-upscaling-empty-url.html.
Sam Sneddon [:gsnedders]
rdar://166556605