-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add sizes=auto to lazy-loaded <img> #8008
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! :) A few nits + a substantive question on the processing model
source
Outdated
<p>The keyword <dfn data-x="valdef-sizes-auto">auto</dfn> means the rendered width of the <span | ||
data-x="concept-sizes-associated-img">associated</span> <code>img</code> element, if any, in <span | ||
data-x="'px'">CSS pixels</span>, if that is not <code data-x="">0px</code>, and that | ||
<code>img</code> element is <span>being rendered</span>, is being rendered to a visual medium, and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worthwhile to integrate the concept of being rendered to a visual medium into the "being rendered" definition? As is, this is a bit confusing to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think the "is being rendered to a visual medium" is actually bogus. The width
and height
attributes used that, but I believe the branching should actually happen for "being rendered". Changed in f1d5577.
source
Outdated
data-x="'px'">CSS pixels</span>, if that is not <code data-x="">0px</code>, and that | ||
<code>img</code> element is <span>being rendered</span>, is being rendered to a visual medium, and | ||
the <code>img</code> element's <code data-x="attr-img-loading">loading</code> attribute is in the | ||
<span data-x="attr-loading-lazy-state">Lazy</span> state; otherwise, <code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we have a situation where the loading=lazy
attribute was removed after the initial parsing of the <img>
element? It's not a relevant mutation, but I would still expect sizes=auto
to give us the layout width of the image, assuming that layout already happened.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be better to ignore "auto" if layout hasn't happened yet, but take it into account if it has, regardless of the loading
attribute
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taking "auto" into account if layout has happened makes it non-deterministic for eager images, as it's racy whether layout has happened before the image fetch starts. I wrote this PR with the assumption that it's better to have deterministic and predictable behavior. Which means that for eager images, auto = 100vw always.
Do we want non-deterministic here?
Or should we add a load()
method for images, to explicitly start loading an image (like for media elements)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right that deterministic behavior would be better, and there's no simple way to enforce that without the loading attribute. I'm not sure there's a strong use-case for "this image was initially lazy, but now we need to load it eagerly", so maybe we can skip this for now, and look into explicit ways to eagerly load images in case it's needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think loading
needs to be a relevant mutation attribute, too, since it can change what sizes=auto
means.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added more conditions to relevant mutations.
With the rendered width being a possible relevant mutation, it could trigger on every frame (e.g. if the image dimensions are being animated). Is that a problem?
It sort of interferes with "Reacting to environment changes", since that aborts in 15.1 if there were relevant mutations (but maybe that's what we want for auto
).
"update the image data" step 6.3.7.3 will fire a load
event even if it's the same url, which doesn't seem great. If the image is broken, it will try to refetch it every time (#4480), which also doesn't seem great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! One question
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I wonder if the feature should more explicitly check for the case of no dimensions being specified, since the behavior is a bit weird in that case.
So, how to fix this? Use 100vw if the computed value (not used value) of the 'width' and 'min-width' properties are both |
Using 100vw when we don't have any better alternatives (since we don't know what the layout size would be), SGTM. The "computed-value width is |
0908d7e
to
fc651c8
Compare
Done in fc651c8. I suppose since the 'aspect-ratio' property now exists, it's possible to specify a width by setting a height and an aspect-ratio. So if width and min-width are both auto, we should check if 'aspect-ratio' and 'height'/'min-height' are also auto. Are there any other ways to specify a width? cc @whatwg/css |
To answer my own question, I think we have these:
So, an
Does that seem correct? Edit: I now also found these properties: https://drafts.csswg.org/css-sizing-4/#intrinsic-size-override |
Maybe a more general approach would be: An element's concrete object size ignoring natural dimensions is its concrete object size but acting as if there are no natural dimensions, thus producing a rectangle with an absolute width and height. This will match the actual concrete object size before the image has loaded. Then, we can check that the "concrete object size ignoring natural dimensions" width is > 0, and any changes to this width is a relevant mutation. |
I went with the approach described in the comment above. I believe it solves the problem discussed in #8008 (comment) and, at least in spec terms, doesn't introduce a lot of complexity. |
(Need to rebase this after #8175 is merged.) |
74854c8
to
51d94ff
Compare
@yoavweiss PTAL :) |
I'm a bit worried about compat for making more things "relevant mutations". Should we gate the new additions on usage of |
source
Outdated
descriptor</span>, the <code data-x="attr-img-sizes">sizes</code> attribute may also be present. | ||
If, additionally, the <code data-x="attr-img-loading">loading</code> attribute is in the <span | ||
data-x="attr-loading-eager-state">Eager</span> state, the <code | ||
data-x="attr-img-sizes">sizes</code> attribute must be present. The <dfn element-attr for="img" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also say that when the loading attribute is in the Eager state, the value of the sizes attribute must not be "auto"? Or are we cool with that meaning "100vw"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
source
Outdated
<li><p>If the element's <code data-x="attr-img-loading">loading</code> attribute is in the <span | ||
data-x="attr-loading-lazy-state">Lazy</span> state: the element starts or stops <span>being | ||
rendered</span>, or its <span>concrete object size ignoring natural dimensions</span> width | ||
changes.</p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does that mean for a lazy-loaded image that was already loaded? If its dimensions change, it may get reloaded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remind me of the motivation for this relevant mutations change? I found this, but it's not clear from that discussion what are the use cases this addresses. Apologies if we already went through this! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment below.
I share that concern, and not sure that gating it would help significantly. I understand the motivations for |
The size observer is to keep the image "responsive" to changes in rendered size. For example, consider a gallery with thumbnails and clicking a thumbnail shows the same |
OK, I understand the reasoning to include the concrete size in the relevant mutations, and I think it makes sense to restrict those to only cases where they matter (so when srcset has 'w' descriptors). IIUC, such content currently would be loaded once with a size of 100vw, and once we switch the the proposed behavior, multiple images may be loaded, if the image size is significantly increased or decreased. Is that understanding correct? |
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 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 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 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
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
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
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
Automatic update from web-platform-tests HTML: test <img sizes=auto> See whatwg/html#8008 -- wpt-commits: 1d96e07ecd4dc9597036afd773f2aa25015df5fd wpt-pr: 34427
Automatic update from web-platform-tests HTML: test <img sizes=auto> See whatwg/html#8008 -- wpt-commits: 1d96e07ecd4dc9597036afd773f2aa25015df5fd wpt-pr: 34427
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
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
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
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
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
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
Automatic update from web-platform-tests HTML: test <img sizes=auto> See whatwg/html#8008 -- wpt-commits: 1d96e07ecd4dc9597036afd773f2aa25015df5fd wpt-pr: 34427 UltraBlame original commit: eed1029e905a955343124475089f6b881fe123f5
Automatic update from web-platform-tests HTML: test <img sizes=auto> See whatwg/html#8008 -- wpt-commits: 1d96e07ecd4dc9597036afd773f2aa25015df5fd wpt-pr: 34427 UltraBlame original commit: eed1029e905a955343124475089f6b881fe123f5
Automatic update from web-platform-tests HTML: test <img sizes=auto> See whatwg/html#8008 -- wpt-commits: 1d96e07ecd4dc9597036afd773f2aa25015df5fd wpt-pr: 34427 UltraBlame original commit: eed1029e905a955343124475089f6b881fe123f5
This implements the HTML spec for applying auto sizes to lazy-loaded images by prepending `auto` to the `sizes` attribute generated by WordPress if the image has a `loading` attribute set to `lazy`. For browser that support this HTML spec, the image's size value will be set to the concrete object size of the image. For browsers that don't support the spec, the word "auto" will be ignored when parsing the sizes value. References: - https://html.spec.whatwg.org/multipage/images.html#sizes-attributes - whatwg/html#8008 Props mukesh27, flixos90, joemcgill, westonruter, peterwilsoncc. Fixes #61847. git-svn-id: https://develop.svn.wordpress.org/trunk@59008 602fd350-edb4-49c9-b593-d223f7449a82
This implements the HTML spec for applying auto sizes to lazy-loaded images by prepending `auto` to the `sizes` attribute generated by WordPress if the image has a `loading` attribute set to `lazy`. For browser that support this HTML spec, the image's size value will be set to the concrete object size of the image. For browsers that don't support the spec, the word "auto" will be ignored when parsing the sizes value. References: - https://html.spec.whatwg.org/multipage/images.html#sizes-attributes - whatwg/html#8008 Props mukesh27, flixos90, joemcgill, westonruter, peterwilsoncc. Fixes #61847. Built from https://develop.svn.wordpress.org/trunk@59008 git-svn-id: http://core.svn.wordpress.org/trunk@58404 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This implements the HTML spec for applying auto sizes to lazy-loaded images by prepending `auto` to the `sizes` attribute generated by WordPress if the image has a `loading` attribute set to `lazy`. For browser that support this HTML spec, the image's size value will be set to the concrete object size of the image. For browsers that don't support the spec, the word "auto" will be ignored when parsing the sizes value. References: - https://html.spec.whatwg.org/multipage/images.html#sizes-attributes - whatwg/html#8008 Props mukesh27, flixos90, joemcgill, westonruter, peterwilsoncc. Fixes #61847. Built from https://develop.svn.wordpress.org/trunk@59008 git-svn-id: https://core.svn.wordpress.org/trunk@58404 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This implements the HTML spec for applying auto sizes to lazy-loaded images by prepending `auto` to the `sizes` attribute generated by WordPress if the image has a `loading` attribute set to `lazy`. For browser that support this HTML spec, the image's size value will be set to the concrete object size of the image. For browsers that don't support the spec, the word "auto" will be ignored when parsing the sizes value. References: - https://html.spec.whatwg.org/multipage/images.html#sizes-attributes - whatwg/html#8008 Props mukesh27, flixos90, joemcgill, westonruter, peterwilsoncc. Fixes #61847. git-svn-id: https://develop.svn.wordpress.org/trunk@59008 602fd350-edb4-49c9-b593-d223f7449a82
This allows web developers to omit the
sizes
attribute, or explicitly specifysizes=auto
, for<img>
elements that haveloading=lazy
. The browser will use the layout width of the image as input to thesrcset
resource selection (lazy images don't start loading until layout is known, at the earliest). For eager-loading images, it's invalid and equivalent to100vw
.Fixes #4654.
(See WHATWG Working Mode: Changes for more details.)
/embedded-content.html ( diff )
/images.html ( diff )
/infrastructure.html ( diff )
/input.html ( diff )
/semantics.html ( diff )