HTML: test <img sizes=auto> by zcorpan · Pull Request #34427 · web-platform-tests/wpt

zcorpan · 2022-06-14T13:36:24Z

See whatwg/html#8008

zcorpan · 2022-07-06T09:49:37Z

zcorpan · 2022-07-08T09:38:19Z

I'll be off work between now and August 8. I can continue working on this then.

yoavweiss · 2022-07-08T09:40:12Z

I'll be off work between now and August 8. I can continue working on this then.

Enjoy! Feel free to ping me once you need me to review changes, etc

zcorpan · 2022-08-11T10:50:32Z

OK everything I was planning to test is now done. @yoavweiss PTAL

yoavweiss · 2022-08-31T12:30:59Z

+
+  t('width attribute changes', function(img) {
+    img.width = '4';
+  }, 'load');


So update the image data re-fires the load event, even if the image hasn't changed?

Yes. I'm not a huge fan, but it's what browsers do.

I just did a test using only srcset with several options and the same url, and when I resized the browser window, I don't see the on-load event fire. Is this test expectation for auto-sizes correct? I would expect the on-load event to fire if a different image was selected, as it does for srcset.

Browsers fire a 'load' event for relevant mutations, see https://software.hixie.ch/utilities/js/live-dom-viewer/saved/11615

Since the spec makes layout size changes a relevant mutation, the implication is that layout changes for the image are expected to fire load even if currentSrc didn't change. See whatwg/html#8492 about changing this.

Changed pass conditions per whatwg/html#8008 (comment)

yoavweiss

Tests LGTM assuming we can land this spec change from a compat perspective.

Spec PR: whatwg/html#8008 WPT tests PR: #34427 R=pdr Bug: 1359051 Change-Id: I8e9bc836e9582a9209db53d55e0edc96f90499f2

tcaptan-cr · 2023-06-01T06:03:58Z

+  // lazy-loaded images should have fired their first 'load' event at this point.
+
+  t('width attribute changes', function(img) {
+    img.width = '4';


Should this test also expect timeout, like the test bellow which changes img.style.width?

The width attribute is listed as a relevant mutation: https://html.spec.whatwg.org/#relevant-mutations

However, it might be a leftover that should have been removed in whatwg/html#5900 . I can file an issue.

whatwg/html#9371

tcaptan-cr · 2023-06-01T07:03:39Z

+
+  t('loading attribute state changes', function(img) {
+    img.loading = 'eager';
+  }, 'load');


If the image is already loaded, why would changing the loading attribute trigger a load event?

State changes to the loading attribute is a relevant mutation per https://github.com/whatwg/html/pull/8008/files#diff-41cf6794ba4200b839c53531555f0f3998df4cbb01a4d5cb0b94e3ca5e23947dR30115-R30116

The "auto" keyword is calculated differently depending on the state of loading. But I'm now uncertain if it should be a relevant mutation. https://html.spec.whatwg.org/#the-img-element:lazy-loading-attribute already handles the state changing to Eager, so also making it a relevant mutation is questionable...

@yoavweiss @domfarolino any opinions?

Yeah, it seems that that specific algorithm replaces the relevant mutation logic (and is more specific). So I'm favorable of removing it from the relevant mutations.

Thanks. Thinking about it again, it's invalid to use auto for eager images, so if you want to switch to eager you should also change sizes (which is a relevant mutation). OK I'll fix the spec and tests.

What would happen if authors don't change sizes in that case? Would that modify the image's intrinsic dimensions?

Yes, auto would compute to 100vw, but not until another relevant mutation happens I think.

Hmm, that runs a risk of being confusing.
I wonder if there's room to define auto + eager as taking the known dimensions in case they are indeed known (and only fallback to 100vw when they aren't)

whatwg/html#8008 (comment)

I prefer the invalid mutation case being somewhat confusing to introducing non-deterministic loading of eager images.

Fair! (and apologies for rehashing a past discussion :D)

tcaptan-cr

Overall the change looks good to me. I only have 2 questions on relevant-mutations-lazy.html.

Auto Sizes allows developers to omit the sizes attribute (or set it to sizes=auto) for lazy loaded image elements with srcset. For this use case the image's width should be provided. The browser will use the layout width of the image in order to select the source url from the srcset. Spec PR: whatwg/html#8008 WPT tests PR: #34427 Chrome Platform Status link: https://chromestatus.com/feature/5191555708616704 Intent to Prototype link: https://groups.google.com/a/chromium.org/g/blink-dev/c/AYoqvNluyeA R=pdr Bug: 1359051 Change-Id: I8e9bc836e9582a9209db53d55e0edc96f90499f2

Auto Sizes allows developers to omit the sizes attribute (or set it to sizes=auto) for lazy loaded image elements with srcset. For this use case the image's width should be provided. The browser will use the layout width of the image in order to select the source url from the srcset. Spec PR: whatwg/html#8008 WPT tests PR: #34427 Chrome Platform Status link: https://chromestatus.com/feature/5191555708616704 Intent to Prototype link: https://groups.google.com/a/chromium.org/g/blink-dev/c/AYoqvNluyeA R=pdr Bug: 1359051 Low-Coverage-Reason: For html_srcset_parser.cc, it should be covered by `image-srcset-cache.html` but there seems to be cov reporing problem. Change-Id: I8e9bc836e9582a9209db53d55e0edc96f90499f2

wpt-pr-bot added the html label Jun 14, 2022

wpt-pr-bot assigned domenic Jun 14, 2022

wpt-pr-bot requested review from annevk, domenic, foolip, jdm, jgraham and zqzhang June 14, 2022 13:36

zcorpan mentioned this pull request Jun 14, 2022

Add sizes=auto to lazy-loaded <img> whatwg/html#8008

Merged

3 tasks

zcorpan requested a review from yoavweiss July 6, 2022 09:46

yoavweiss reviewed Jul 6, 2022

View reviewed changes

Comment thread html/semantics/embedded-content/the-img-element/sizes/sizes-auto.html Outdated

Comment thread html/semantics/embedded-content/the-img-element/sizes/sizes-auto.html Outdated

zcorpan commented Jul 6, 2022

View reviewed changes

Comment thread html/semantics/embedded-content/the-img-element/sizes/sizes-auto.html Outdated

yoavweiss reviewed Jul 8, 2022

View reviewed changes

Comment thread html/semantics/embedded-content/the-img-element/sizes/sizes-auto.html Outdated

Comment thread html/semantics/embedded-content/the-img-element/sizes/sizes-auto.html Outdated

yoavweiss reviewed Aug 31, 2022

View reviewed changes

yoavweiss approved these changes Sep 5, 2022

View reviewed changes

chromium-wpt-export-bot mentioned this pull request May 12, 2023

Implement auto sizes for lazy-loaded img #39993

Closed

chromium-wpt-export-bot pushed a commit that referenced this pull request May 12, 2023

Prototype auto sizes for lazy-loaded img

f70f511

Spec PR: whatwg/html#8008 WPT tests PR: #34427 R=pdr Bug: 1359051 Change-Id: I8e9bc836e9582a9209db53d55e0edc96f90499f2

chromium-wpt-export-bot pushed a commit that referenced this pull request May 18, 2023

WIP - Prototype auto sizes for lazy-loaded img

3b9a4f2

Spec PR: whatwg/html#8008 WPT tests PR: #34427 R=pdr Bug: 1359051 Change-Id: I8e9bc836e9582a9209db53d55e0edc96f90499f2

chromium-wpt-export-bot pushed a commit that referenced this pull request May 18, 2023

WIP - Prototype auto sizes for lazy-loaded img

4584108

Spec PR: whatwg/html#8008 WPT tests PR: #34427 R=pdr Bug: 1359051 Change-Id: I8e9bc836e9582a9209db53d55e0edc96f90499f2

chromium-wpt-export-bot pushed a commit that referenced this pull request May 19, 2023

WIP - Prototype auto sizes for lazy-loaded img

fee3385

Spec PR: whatwg/html#8008 WPT tests PR: #34427 R=pdr Bug: 1359051 Change-Id: I8e9bc836e9582a9209db53d55e0edc96f90499f2

chromium-wpt-export-bot pushed a commit that referenced this pull request May 20, 2023

WIP - Prototype auto sizes for lazy-loaded img

6c90be6

Spec PR: whatwg/html#8008 WPT tests PR: #34427 R=pdr Bug: 1359051 Change-Id: I8e9bc836e9582a9209db53d55e0edc96f90499f2

chromium-wpt-export-bot pushed a commit that referenced this pull request May 20, 2023

WIP - Prototype auto sizes for lazy-loaded img

67e90ff

Spec PR: whatwg/html#8008 WPT tests PR: #34427 R=pdr Bug: 1359051 Change-Id: I8e9bc836e9582a9209db53d55e0edc96f90499f2

zcorpan requested a review from tcaptan-cr May 31, 2023 17:05

tcaptan-cr reviewed Jun 1, 2023

View reviewed changes

tcaptan-cr approved these changes Jun 1, 2023

View reviewed changes

zcorpan added 10 commits June 2, 2023 15:39

Apply suggestions from code review

a4e9a23

Test percentage widths

c45f692

Test aspect-ratio and logical properties

f2ec6ea

Test var() and calc()

d9917a2

Test no specified dimensions

c0fb531

Test abs pos with left/right and no width

8d68cf0

Test changes to relevant mutations

b8a07ac

Fix max-width test and add a new width-constrained test

e036888

Omit events for layout changes (spec change)

fea6762

loading attribute not a relevant mutation

a442c1c

zcorpan force-pushed the bocoup/img-sizes-auto branch from 1c7b50e to a442c1c Compare June 2, 2023 13:40

zcorpan merged commit 1d96e07 into master Jun 2, 2023

zcorpan deleted the bocoup/img-sizes-auto branch June 2, 2023 14:23

moz-wptsync-bot added mozilla:gecko-blocked and removed mozilla:gecko-blocked labels Jun 2, 2023

eeeps mentioned this pull request Sep 30, 2024

Auto-sizes for lazy-loaded images web-platform-tests/interop#761

Closed

Conversation

zcorpan commented Jun 14, 2022

Uh oh!

zcorpan commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zcorpan commented Jul 8, 2022

Uh oh!

yoavweiss commented Jul 8, 2022

Uh oh!

zcorpan commented Aug 11, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yoavweiss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tcaptan-cr left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

zcorpan commented Jul 6, 2022 •

edited

Loading