whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.18k stars 2.69k forks source link

[images] Lazy loading and out of band loads. #10671

Open emilio opened 1 month ago

emilio commented 1 month ago

What is the issue with the HTML Standard?

Consider this test-case (live):

<!doctype html>
<img style="display: none" onload="this.style.display = ''" loading="lazy">
<script>
const SRC = "https://wpt.live/images/green.png?" + Math.random();
document.querySelector("img").src = SRC;
setTimeout(() => {
  new Image().src = SRC;
}, 3000);
</script>

WebKit and Blink somehow switch the lazy load to eager after the image loads (showing the green rectangle).

We found one website depending on this behavior in Mozilla bug 1922416.

AFAICT this just goes against the spec (and it's super bizarre)... Two questions:

cc @zcorpan @annevk @mfreed7

annevk commented 1 month ago

So to be clear, this does not happen without the new Image().src assignment? That does seem strangely side-effecting.

cc @rwlbuis @nt1m

rwlbuis commented 1 month ago

I have not debugged, but from a quick code inspection I suspect what happens is that the lazy load gets registered in the WebKit cached resources subsystem, and once the cached resource load finishes it causes notifications to all load observers, including the lazy load image.

So I think this is a side-effect of the WebKit cached resources subsystem. I do not know though if it is crucial to register lazy loads in the webkit cached resources subsystem, but I would not be surprised if things break without that.

I'd be curious to hear from Blink folks if they made a conscious decision about this and if they have same behaviour as WebKit because of a similar cached resources subsystem.

emilio commented 1 month ago

So to be clear, this does not happen without the new Image().src assignment? That does seem strangely side-effecting.

Correct, and agreed.

dholbert commented 1 month ago

Chrome and Safari actually do agree with Firefox on the testcase under one condition: if I open the browser's network-DevTools panel and check the "Disable Caches" checkbox. (Under that configuration, Chrome and Safari render the testcase as blank.)

The fact that this checkbox gives different behavior vs. the "cache-skipping reload" (Shift+Reload) in those browsers seems unexpected/unwanted.

emilio commented 1 month ago

Chrome and Safari have even worse bugs here, see: https://issues.chromium.org/issues/375209498

domenic commented 3 weeks ago

@domfarolino, back in the day you used to work on lazy-loading images---would you be able to help with a Chromium and/or spec perspective here?

domfarolino commented 3 weeks ago

For Chromium, this is an artifact of two things:

  1. The fact that the decision to lazy load an image in Chrome is made too late in the loading pipeline. Specifically, it is made after the underlying loading infrastructure creates and registers a not-yet-populated Resource object representing the deferred image
  2. Resource coalescion: that is, when two "matching" resources are requested, they are sort of coalesced, and their sinks are both registered as clients to the same underlying Resource object. Since the new Image() "matches" the lazyloaded image, but is not lazyloaded, its load is not deferred. When it's finished it counts both images as loaded, since they were coalesced.

If the images didn't "match" (for example, the crossorigin credentials attribute didn't match), then we would not see the quirk @emilio points out. For example:

<!doctype html>
<img style="display: none" onload="this.style.display = ''" loading="lazy">
<script>
const SRC = "https://wpt.live/images/green.png?" + Math.random();
document.querySelector("img").src = SRC;
function run() {
  const x = new Image();
  x.crossOrigin = 'use-credentials'; // This is important.
  x.src = SRC;
}
</script>
<button onclick="run()">Do the stuff</button>

Spec'ing the resource coalescing logic seems hard, and I'm not sure how beneficial it would be. That is, how web-observable is it? Besides lazyload, I don't know of other cases where one resource would not be expected to load, but would end up "force-loading" because of another "matching" one that got coalesced with it.

For Chromium, the shortest path to killing off this quirk might be to simply include lazyload status in the matching criteria, so that the resources in the original example would not "match", and therefore would not have side-effects on each other. I'd be curious to hear from @rwlbuis to see if this matches what could be done with WebKit.

emilio commented 2 weeks ago

FWIW I wrote a test for this: https://wpt.fyi/results/html/semantics/embedded-content/the-img-element/update-the-image-data/lazy-out-of-band-load.html

Extra Safari issue: https://wpt.fyi/results/html/semantics/embedded-content/the-img-element/update-the-image-data/src-then-lazy-load.html