HTML: navigate cancels iframe lazy-load #45650

zcorpan · 2024-04-10T15:17:34Z

noamr · 2024-04-10T15:25:42Z

would be interesting to test also what happens when the iframe receives lazy steps after a navigation is started but before it's committed. e.g. you have an about:blank with lazy and without a src, you navigate it with a link, then set a src, and the new document is ready to commit before the lazy steps are run
I think what chromium does here would differ from your spec PR (not saying here what the "right" solution is)

zcorpan · 2024-04-11T09:07:08Z

@noamr per spec, when the iframe is first inserted, process the iframe attributes is run but returns at 2.2. Then something navigates. Then setting src will again call process the iframe attributes, which sets the lazy load resumption steps in 2.6.1.

zcorpan · 2024-04-11T09:11:12Z

html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-history-pushState.html

+<script src="/resources/testharnessreport.js"></script>
+<script>
+setup({single_test: true});
+onload = () => {


This doesn't work (reliably) because of https://html.spec.whatwg.org/multipage/iframe-embed-object.html#the-iframe-element:current-navigation-was-lazy-loaded-5

Can you clarify? To say onload overall "doesn't work" indicates that the lazyloaded support/blank.html iframe ends up loading ASAP (before this onload handler is set), causing the outer document's load event to fire before our handler is assigned to catch the load event.

But I can't imagine that's the case, right? Certainly the main document's load event does not fire before this script runs?

I mean the load event might fire before the iframe is loaded, since a lazy-loading iframe doesn't block the load event. Therefore it's racy. But the step_timeout should address this sufficiently.

zcorpan · 2024-04-11T12:28:17Z

These are unstable in Chrome:

/html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-nav-location-replace-set-src.html
/html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-nav-meta-refresh.optional.html

The first one I think should be stable per spec, since the lazy navigation happens after location.replace and it sets the ongoing navigation which should abort the previous navigation.

The meta refresh could arguably be unstable per spec if the UA decides to delay the navigation by the same amount of time as the test's step_timeout. But changing the timeout to 5s doesn't help, so it doesn't explain the instability in Chrome.

domfarolino · 2024-04-11T15:38:28Z

html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-history-pushState.html

+setup({single_test: true});
+onload = () => {
+  step_timeout(() => {
+    assert_equals(iframe.contentWindow.location.href, new URL("support/blank.htm?src", location.href).href);


Should we assert above this, that location.href equals support/blank.htm?foo (at least I think it will at this time?)

Right after calling pushState yes. When this script runs, it's racy whether the lazy load has happened.

The pushState and replaceState tests were incorrect since the iframe's doc's URL is about:blank, and so the URL components (except fragment) need to match about:blank. Fixed now.

When this script runs, it's racy whether the lazy load has happened.

What do you mean by "happened"? Do you mean https://html.spec.whatwg.org/multipage/iframe-embed-object.html#the-iframe-element:current-navigation-was-lazy-loaded-4 just being set to true? Or do you mean that plus the resumption steps executed, and the support/blank.htm?src navigation has completed?

In any case, I'm not sure I follow. By this point in the script, we've done nothing asynchronous, right? I think what has happened up to this point is:

https://html.spec.whatwg.org/multipage/iframe-embed-object.html#the-iframe-element:current-navigation-was-lazy-loaded-4 gets set to true, for the lazyload navigation to support/blank.html?src. But the iframe's document is still about:blank, since the lazy load resumption steps were not run immediately

Synchronously after, pushState() comes along and changes the about:blank Document's URL to about:blank#foo, which is allowed per https://html.spec.whatwg.org/C#can-have-its-url-rewritten

So is there any room in there for the lazy loaded navigation to have already completed by then?

The navigation might have completed. The event loop is allowed to spin between each script, and the pushState() happens in the first script element.

html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-history-pushState.html

.../semantics/embedded-content/the-iframe-element/iframe-loading-lazy-history-replaceState.html

html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-nav-form-submit.html

domfarolino · 2024-04-11T15:42:12Z

html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-history-pushState.html

+// Should have no effect on lazy-loading
+iframe.contentWindow.history.pushState(null, "", iframe.dataset.src);
+</script>
+<!-- Loading testharness.js here is intentional. -->


WebKit passes the tests if testharness.js is loaded in head, but fails these tests (or a simple demo without external scripts).

html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-nav-form-submit.html

domfarolino · 2024-04-24T20:37:48Z

Sorry for the delay, I took some time off last week and early this week. Getting through my backlog so I will take a look at this as soon as I can

domfarolino · 2024-04-24T20:49:57Z

html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-history-pushState.html

+let pushStateSuccess = true;
+try {
+  // Should have no effect on lazy-loading.
+  // Per https://html.spec.whatwg.org/#can-have-its-url-rewritten


Suggested change

// Per https://html.spec.whatwg.org/#can-have-its-url-rewritten

// Per https://html.spec.whatwg.org/C#can-have-its-url-rewritten

nit: Just so the average reader has an easier time with this link.

domfarolino · 2024-04-24T20:54:25Z

html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-history-pushState.html

+<script src="/resources/testharnessreport.js"></script>
+<script>
+setup({single_test: true});
+assert_true(pushStateSuccess);


Maybe we can just avoid this check altogether since pushState() will throw an exception in the error case and we don't have to handle that explicitly? That is, I assume the error thrown to the harness in the error case is enough to fail the test reasonable.

The harness isn't loaded yet when that exception is thrown, hence this assertion.

domfarolino · 2024-04-24T20:57:11Z

html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-history-pushState.html

+<script src="/resources/testharnessreport.js"></script>
+<script>
+setup({single_test: true});
+onload = () => {


Can you clarify? To say onload overall "doesn't work" indicates that the lazyloaded support/blank.html iframe ends up loading ASAP (before this onload handler is set), causing the outer document's load event to fire before our handler is assigned to catch the load event.

But I can't imagine that's the case, right? Certainly the main document's load event does not fire before this script runs?

domfarolino · 2024-04-24T21:10:50Z

html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-history-pushState.html

@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<title>History state change for iframe loading='lazy' before it is loaded: history.pushState</title>
+<iframe data-src="about:blank#foo" src="support/blank.htm?src" loading="lazy"></iframe>


Suggested change

<iframe data-src="about:blank#foo" src="support/blank.htm?src" loading="lazy"></iframe>

<iframe data-src="about:blank#push" src="support/blank.htm?src" loading="lazy"></iframe>

nit: more descriptive

domfarolino · 2024-04-24T21:20:20Z

html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-history-pushState.html

+setup({single_test: true});
+onload = () => {
+  step_timeout(() => {
+    assert_equals(iframe.contentWindow.location.href, new URL("support/blank.htm?src", location.href).href);


When this script runs, it's racy whether the lazy load has happened.

What do you mean by "happened"? Do you mean https://html.spec.whatwg.org/multipage/iframe-embed-object.html#the-iframe-element:current-navigation-was-lazy-loaded-4 just being set to true? Or do you mean that plus the resumption steps executed, and the support/blank.htm?src navigation has completed?

In any case, I'm not sure I follow. By this point in the script, we've done nothing asynchronous, right? I think what has happened up to this point is:

https://html.spec.whatwg.org/multipage/iframe-embed-object.html#the-iframe-element:current-navigation-was-lazy-loaded-4 gets set to true, for the lazyload navigation to support/blank.html?src. But the iframe's document is still about:blank, since the lazy load resumption steps were not run immediately

Synchronously after, pushState() comes along and changes the about:blank Document's URL to about:blank#foo, which is allowed per https://html.spec.whatwg.org/C#can-have-its-url-rewritten

So is there any room in there for the lazy loaded navigation to have already completed by then?

domfarolino · 2024-04-24T21:21:25Z

html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-history-pushState.html

+  step_timeout(() => {
+    assert_equals(iframe.contentWindow.location.href, new URL("support/blank.htm?src", location.href).href);
+    done();
+  }, 1000);


Does this have to be done on a timeout? Or can it be done on the lazyloaded iframe's load event, since it will be fired asynchronously (with respect to this code currently running) once support/blank.html loads?

Since some tests do potentially two navigations, a timeout is still needed. But maybe I can use the iframe load event instead of the window load event...

zcorpan · 2024-05-30T21:24:11Z

html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-history-pushState.html

+  // only the fragment can be changed for about: URLs.
+  iframe.contentWindow.history.pushState(null, "", iframe.dataset.src);
+} catch(ex) {
+  replaceStateSuccess = false;


Should be pushStateSuccess, oops

HTML: navigate cancels iframe lazy-load

cd96ee6

See whatwg/html#10226

wpt-pr-bot added the html label Apr 10, 2024

wpt-pr-bot assigned jgraham Apr 10, 2024

wpt-pr-bot requested review from annevk, domenic, foolip, jdm and jgraham April 10, 2024 15:17

zcorpan mentioned this pull request Apr 10, 2024

Set iframe lazy load resumption steps to null on navigation whatwg/html#10226

Open

5 tasks

zcorpan commented Apr 11, 2024

View reviewed changes

zcorpan added 4 commits April 11, 2024 12:05

Add a step_timeout since lazy iframes don't delay the load event

6f78950

Try to address instability

65a5310

Add a test for setting src after navigating

35ce3ab

Try again to address instability

ec14116

zcorpan requested a review from domfarolino April 11, 2024 12:29

domfarolino reviewed Apr 11, 2024

View reviewed changes

Address review comments + fix bug in pushState/replaceState

1c2ef91

zcorpan requested a review from domfarolino April 17, 2024 12:53

domfarolino reviewed Apr 24, 2024

View reviewed changes

Use the iframe load event instead

af5cbbf

zcorpan commented May 30, 2024

View reviewed changes

Nits

99b8481

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HTML: navigate cancels iframe lazy-load #45650

HTML: navigate cancels iframe lazy-load #45650

zcorpan commented Apr 10, 2024

noamr commented Apr 10, 2024

zcorpan commented Apr 11, 2024

zcorpan Apr 11, 2024

domfarolino Apr 24, 2024

zcorpan May 30, 2024

zcorpan commented Apr 11, 2024

domfarolino Apr 11, 2024

zcorpan Apr 15, 2024

zcorpan Apr 15, 2024

domfarolino Apr 24, 2024

zcorpan May 30, 2024

domfarolino Apr 11, 2024

zcorpan Apr 15, 2024

domfarolino commented Apr 24, 2024

domfarolino Apr 24, 2024

domfarolino Apr 24, 2024

zcorpan May 30, 2024

domfarolino Apr 24, 2024

domfarolino Apr 24, 2024

domfarolino Apr 24, 2024

domfarolino Apr 24, 2024

zcorpan May 30, 2024

zcorpan May 30, 2024

	// Per https://html.spec.whatwg.org/#can-have-its-url-rewritten
	// Per https://html.spec.whatwg.org/C#can-have-its-url-rewritten

	<iframe data-src="about:blank#foo" src="support/blank.htm?src" loading="lazy"></iframe>
	<iframe data-src="about:blank#push" src="support/blank.htm?src" loading="lazy"></iframe>

HTML: navigate cancels iframe lazy-load #45650

Are you sure you want to change the base?

HTML: navigate cancels iframe lazy-load #45650

Conversation

zcorpan commented Apr 10, 2024

noamr commented Apr 10, 2024

zcorpan commented Apr 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zcorpan commented Apr 11, 2024

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

domfarolino commented Apr 24, 2024

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