Bug 1628894 - Defer loading images with srcset attribute if the loading was set to "lazy" before we start loading the srcset images. r=emilio

As per the HTML standard spec[1], checking the document active state is the
first step of the "update the image data", so we just check the lazy loading
state just before we call LoadSelectedImage() in the ImageLoadTask instead
of using ShouldLoadImage().

NOTE: To be more compliant with the spec the lazy loading check should be done
in nsImageLoadingContent::LoadImage just after we initialized the referrer info
for the image element [2], but it involves some amount of work since we've
already done a bunch of stuff before we reach there (e.g. firing events).  Given
that Chrome handles srcset attribute changes in a sync way ([3] for example),
even if this change is not fully compliant with the spec, it will not introduce
new web compatibility issues.

[1] https://html.spec.whatwg.org/multipage/images.html#update-the-image-data
[2] https://searchfox.org/mozilla-central/rev/9120151ddb35f2d4c37bfe613a54a4f10a9a3dc5/dom/base/nsImageLoadingContent.cpp#1168
[3] https://bugs.chromium.org/p/chromium/issues/detail?id=774722

Differential Revision: https://phabricator.services.mozilla.com/D70479

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Hiroyuki Ikezoe 2020-04-10 23:21:03 +00:00
parent 9562ef262a
commit d70e0416b7
3 changed files with 49 additions and 1 deletions

View File

@ -86,7 +86,15 @@ class ImageLoadTask final : public MicroTaskRunnable {
if (mElement->mPendingImageLoadTask == this) {
mElement->mPendingImageLoadTask = nullptr;
mElement->mUseUrgentStartForChannel = mUseUrgentStartForChannel;
mElement->LoadSelectedImage(true, true, mAlwaysLoad);
// Defer loading this image if loading="lazy" was set after this microtask
// was queued.
// NOTE: Using ShouldLoadImage() will violate the HTML standard spec
// because ShouldLoadImage() checks the document active state which should
// have done just once before this queue is created as per the spec, so
// we just check the lazy loading state here.
if (!mElement->IsLazyLoading()) {
mElement->LoadSelectedImage(true, true, mAlwaysLoad);
}
}
mDocument->UnblockOnload(false);
}

View File

@ -197,6 +197,8 @@ class HTMLImageElement final : public nsGenericHTMLElement,
return mLazyLoading || mPendingImageLoadTask;
}
bool IsLazyLoading() const { return mLazyLoading; }
Loading LoadingState() const;
already_AddRefed<Promise> Decode(ErrorResult& aRv);

View File

@ -0,0 +1,38 @@
<!DOCTYPE html>
<head>
<title>loading='lazy' image with srcset</title>
<link rel="help" href="https://html.spec.whatwg.org/multipage/images.html#update-the-image-data">
<link rel="help" href="https://html.spec.whatwg.org/multipage/images.html#will-lazy-load-image-steps">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
</head>
<div style="height:1000vh;"></div>
<img srcset="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABQAAAAUCAIAAAAC64paAAAAG0lEQVR42mP8z0A%2BYKJA76jmUc2jmkc1U0EzACKcASfOgGoMAAAAAElFTkSuQmCC" loading="lazy">
<img loading="lazy" srcset="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABQAAAAUCAIAAAAC64paAAAAG0lEQVR42mP8z0A%2BYKJA76jmUc2jmkc1U0EzACKcASfOgGoMAAAAAElFTkSuQmCC">
<script>
promise_test(async t => {
let loaded_images = 0;
const imgs = document.querySelectorAll("img");
imgs.forEach(img => {
img.addEventListener("load", () => { loaded_images++; }, { once: true });
});
await new Promise(resolve => window.addEventListener("load", resolve));
assert_equals(loaded_images, 0,
"lazy-load images with srcset shouldn't be loaded yet");
const promises = [
new Promise(resolve => imgs[0].addEventListener("load", resolve)),
new Promise(resolve => imgs[1].addEventListener("load", resolve)),
];
imgs[1].scrollIntoView();
await Promise.all(promises);
imgs.forEach(img => {
assert_true(img.complete,
"Now the lazy-load image with srcset should be loaded");
});
});
</script>