Lazy loading images fails if loading attribute is specified after srcset
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
People
(Reporter: svoisen, Assigned: hiro)
References
Details
Attachments
(1 file)
|
47 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
|
Details | Review |
The order of attributes on an <img> element that uses srcset determines if loading=lazy works as intended. This shouldn't be the case.
For instance, the following works as expected and the image is loaded lazily (as indicated by network panel in devtools):
<!DOCTYPE html>
<div style="height:10000px;"></div>
<img loading="lazy" srcset="image.png">
However, this does not work:
<!DOCTYPE html>
<div style="height:10000px;"></div>
<img srcset="image.png" loading="lazy">
This appears to depend on bug 1076583.
|
Assignee | |
Comment 1•4 years ago
|
||
Thank you for filing this bug!
(In reply to Sean Voisen (:svoisen) from comment #0)
<!DOCTYPE html> <div style="height:10000px;"></div> <img srcset="image.png" loading="lazy">
Ugh. I didn't suppose this case.. This is totally my fault. This can be easily fixed without bug 1076583.
|
|
Assignee | |
Comment 2•4 years ago
|
||
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
|
|
Assignee | |
Comment 3•4 years ago
|
||
Removing bug 1076583 from the dependency list, I was misunderstanding this issue.
Pushed by hikezoe.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/12e4f86d563d Defer loading images with srcset attribute if the loading was set to "lazy" before we start loading the srcset images. r=emilio
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/22862 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
|
||
Comment 7•4 years ago
|
||
| bugherder | ||
Upstream PR merged by moz-wptsync-bot
|
|
Assignee | |
Comment 9•4 years ago
|
||
Comment on attachment 9139677 [details]
Bug 1628894 - Defer loading images with srcset attribute if the loading was set to "lazy" before we start loading the srcset images. r?emilio
Beta/Release Uplift Approval Request
- User impact if declined: Images specified by srcset are loaded even if loading="lazy" is specified, i.e. lazy loading doesn't work at all in such cases
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): This change is pretty simple, just tries to defer loading images if loading="lazy" is specified
- String changes made/needed: None
|
||
Comment 10•4 years ago
|
||
Comment on attachment 9139677 [details]
Bug 1628894 - Defer loading images with srcset attribute if the loading was set to "lazy" before we start loading the srcset images. r?emilio
Approved for 76.0b5.
|
||
Comment 11•4 years ago
|
||
| bugherder uplift | ||
Description
•