whatwg / html

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

auto-sizes concrete object size ignoring natural dimensions is not implementable #9448

Closed zcorpan closed 10 months ago

zcorpan commented 1 year ago

Feedback from @progers in https://chromium-review.googlesource.com/c/chromium/src/+/4520266/comment/d6012d27_30cf4ea2/ about concrete object size ignoring natural dimensions introduced in #8008

If I understand the intended behavior, I think it may not be implementable.

The goal of the spec was to get stable, predictable, non-racy loading behavior of auto-sizes images. Therefore the natural dimensions of the loaded image shouldn't cause a different image to be selected from srcset.

Consider a page like this:

<img loading=lazy sizes=auto srcset="a 2w, b 4w, c 8w, d 16w, ..." style="min-width: 1px">

The layout width before loading anything is 1px, so maybe the browser would select a. When that has loaded, the layout width changes to 2px, which could cause the browser to instead select b. And so on until the largest image has loaded.

The spec checks for 0px concrete object size ignoring natural dimensions and translates auto to 100vw. But since min-width can be used, the image's natural dimensions can still affect the layout size as shown above.

@emilio pointed out that the natural dimensions can affect outer elements, so it's not easy to determine the concrete object size ignoring natural dimensions without doing full layout twice.

So I think we need a different approach here. Maybe checking if there's a specified 'width' (or 'height' + 'aspect-ratio'), and if not use 100vw. Any other ideas?

cc @tcaptan-cr @yoavweiss

bfgeek commented 1 year ago

There are cases even with the spec as-is where you could create a cyclic dependency. A path forward might be to add size containment for images with this behaviour.

[1] E.g. https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=11811 Here the width of the outer box is the natural-size of the image.(Say 200px). Then the image 50% resolves against this, and becomes 100px.

We select an image based off this with a natural-size of say 400px, and wash-rinse-repeat.

Ian

zcorpan commented 1 year ago

So require contain: size to enable auto-sizes, else resolve auto to 100vw?

yoavweiss commented 1 year ago

Requiring strict dimensions (width or height + aspect-ratio) makes sense to me, and seems like it'd be easier to apply to existing content compared to contain: size. Would strict dimensions not be enough to prevent cyclic dependency?

zcorpan commented 1 year ago

Percentage widths as in @bfgeek's example is still problematic. Do you mean disallow percentages with "strict dimensions"?

zcorpan commented 1 year ago

As for height + aspect-ratio we'd need to disallow auto aspect-ratio (used by default) since it again makes layout width different between no image loaded and an image loaded.

progers commented 1 year ago

When reading up on containment, I found (this link) which says size containment was used to solve a similar cycle issue for container queries, so there is some precedent with this approach. Is it possible to make auto sizes imply size containment so it doesn't need to be specified?

An alternative is to modify source selection to only go up in srcset options. The spec states that source selection is implementation-defined (selecting an image source), and Chromium currently implements this only-go-up behavior as an optimization to use larger images if they are available in the cache. This would not work for art direction, but that is probably best solved with other APIs (https://github.com/w3c/csswg-drafts/issues/5889 has some info).

zcorpan commented 1 year ago

The timing for parsing sizes is not synced with layout, so making the former cause style changes (sometimes! sizes could resolve to something other than auto) also seems like a source of layout instability.

Size containment for images makes the height collapse to 0 if you only specify a width. It seems nice if we can avoid size containment as a requirement.

An alternative is to modify source selection to only go up in srcset options.

I think that still doesn't solve this, since the selection can go up to the largest image as noted in OP. Maybe you need width: 110% or zoom: 1.1 or similar (could be applied on :hover for example) in a fit-content parent.

bfgeek commented 1 year ago

One additional thing is that it's possible to create these cyclic dependencies with "auto" width/heights as well.

E.g. https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=11816 (the final width of the images are dependent on each others height).

eeeps commented 1 year ago

@zcorpan:

Size containment for images makes the height collapse to 0 if you only specify a width. It seems nice if we can avoid size containment as a requirement.

We have inline-size containment, which doesn't. I guess for this we would need width containment, which would be a different, new thing.

eeeps commented 1 year ago

Basically we need to ensure that image layout size doesn't rely on intrinsic width. We can check if intrinsic width will have any effect beforehand, and if so sub 100vw for auto (same behavior that happens today), or explicitly mandate that intrinsic width will not have any effect using containment.

Checking & falling back to 100vw feels harder, spec-wise. It feels like there are many edge cases (?). I like that it wouldn't break existing layouts that are relying on intrinsic widths, but don't like that when sizes=auto isn't doing what people think it's doing, the problem will often be invisible. Shrinking intrinsically-sized images to 0-width is more noticeable, for better or for worse.

bfgeek commented 1 year ago

We can check if intrinsic width will have any effect beforehand

Detecting if we've used an intrinsic size in layout gets into known hard layout problems very quickly. I suspect containment will be the best path forward.

An option (perhaps not a good one) is to add a UA-style similar to img[sizes=auto] { width: 100% } to provide a better default.

progers commented 1 year ago

Could the concrete object size be adjusted so the natural dimensions don't change as the result of auto sizes selection? For example, if natural dimensions are available when selecting the auto sizes source, adjust the selected image's natural dimensions to match. For cases where layout depends on intrinsic sizes, this would only perform two cycles.

For the original issue of selecting all srcset options:

<img loading=lazy sizes=auto style="min-width: 1px" srcset="3x3.png 2w, 5x5.png 4w, 9x9.png 8w, ...">

This approach would select "3x3.png 2w", resulting in a layout size of 3x3, then select "5x5.png 4w", which would still result in a layout size of 3x3 because the natural dimensions of 5x5.png would be adjusted to 3x3.

For the width: 200%, fit-content parent issue:

<div style="width: fit-content;">
  <img loading=lazy sizes=auto style="min-width: 1px; width: 200%;" srcset="2x2.png 2w, 4x4.png 4w, ...">
</div>

This approach would select "2x2.png 2w", resulting in a layout size of 4x4, then select "4x4.png 4w", which would still result in a layout size of 4x4 because the natural dimensions of 4x4.png would be adjusted to 2x2.

zcorpan commented 1 year ago

I wrote

Maybe checking if there's a specified 'width' (or 'height' + 'aspect-ratio'), and if not use 100vw.

This would be easy to write in the spec but maybe less easy to implement, since specified style isn't generally available per @emilio. Is this feasible in Chromium and WebKit?

@eeeps

We have inline-size containment, which doesn't. I guess for this we would need width containment, which would be a different, new thing.

Good point, we could allow contain: inline-size if writing mode is horizontal.

@bfgeek

Detecting if we've used an intrinsic size in layout gets into known hard layout problems very quickly. I suspect containment will be the best path forward.

OK.

An option (perhaps not a good one) is to add a UA-style similar to img[sizes=auto] { width: 100% } to provide a better default.

This doesn't work because sizes can be omitted (equivalent to sizes=auto) and auto keyword can be used anywhere in sizes e.g. sizes="(min-width: 500px) 500px, auto" or sizes="auto, 50vw" (provide fallback for older browsers).

@progers two requests per image which hasn't been acceptable in the past.

progers commented 1 year ago

Don't we still have the cycle issues with size containment approaches that don't constrain all axes?

emilio commented 1 year ago

I think so, yeah... If we require explicit sizes="auto" even for lazy images we can enforce size containment via attribute mapping / spec wording magic without too much problem.

emilio commented 1 year ago

@zcorpan checking if there's a width or height specified is not enough, you need to make sure that the width is absolute right?

zcorpan commented 1 year ago

auto and percentage are problematic (unless contain: size). I'm not sure which units for <length> are problematic, if any. cqw might be?

bfgeek commented 1 year ago

auto and percentage are problematic (unless contain: size). I'm not sure which units for are problematic, if any. cqw might be?

Keep in mind the min-width defaults to auto which can affect things even when width: 0px for example: https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=11820

eeeps commented 1 year ago

@progers:

Don't we still have the cycle issues with size containment approaches that don't constrain all axes?

Do you have an example where intrinsic height affects the concrete width of an <img> with contain: inline-size and writing-mode: horizontal? (Maybe something with columns/wrapping?)

If so... I wonder how container queries get around this (or how the problems are different).

bfgeek commented 1 year ago

If so... I wonder how container queries get around this (or how the problems are different).

Container queries don't persist "state" outside of layout (unless done by some web-developer script). The primary difference here (similar to content-visibility) is that "state" (what image you've selected). Put another way if you tear everything down, and run style+layout you'll end up with the same result each time.

eeeps commented 1 year ago

@bfgeek that makes a high-level kind of sense. The takeaway is, authors generally wouldn't be able use auto-sizes on content they don't know the intrinsic aspect ratio of. That will severely limit use (69% of <img>s lack an extrinsic height in CSS and 72% of images lack height and width attributes in HTML).

progers commented 1 year ago

A nice aspect of requiring size containment is that it's conservative and could be relaxed in the future based on developer feedback.

@progers two requests per image which hasn't been acceptable in the past.

With auto sized natural dimensions, the typical behavior would just load one image. In cases where layout depends on the initial request, a second request is needed but it's progressive enhancement. The spec change for this would be to replace "concrete object size ignoring natural dimensions" with "auto sized natural dimensions" which is determined after some image in the srcset loads. Having a stateful srcset does seem strange though.

zcorpan commented 1 year ago

@emilio:

If we require explicit sizes="auto" even for lazy images we can enforce size containment via attribute mapping / spec wording magic without too much problem.

This seems feasible, at least if the auto keyword is required to be first in sizes without a media condition.

@eeeps:

Do you have an example where intrinsic height affects the concrete width of an <img> with contain: inline-size and writing-mode: horizontal?

I'm also curious about this! (Should be writing-mode: horizontal-tb btw.)

@progers:

In cases where layout depends on the initial request, a second request is needed but it's progressive enhancement.

I think performance-savvy developers at least will consider it a bug and will have to either not use auto-sizes or be mindful about how they use CSS to avoid double image requests. IMO it's not acceptable.

bfgeek commented 1 year ago

Do you have an example where intrinsic height affects the concrete width of an with contain: inline-size and writing-mode: horizontal?

It's relatively rare - but can happen. ("easiest" is likely with grid-layout - which has two passes, the row sizes from the first pass can affect the column sizes of the 2nd pass).

progers commented 1 year ago

Scrollbars are another example:

<div style="width: 125px; height: 100px; overflow-y: auto;">
  <img loading=lazy
      sizes=auto
      srcset="120x90.png 120w, 125x125.png 125w"
      style="width: 100%; contain: inline-size;">
</div>
zcorpan commented 1 year ago

Thanks!

Based on the comments so far, I suggest we do this:

This means it's not possible to use sizes="auto" without size containment, which is unfortunate, but anything else seems like it can result in unstable resource selection and double downloads in some cases.

progers commented 1 year ago

I am worried that size containment may not work for the common usecase where the image's height depends on the image. Is there data I could gather to determine if this is a requirement? For example, would it be useful to categorize usecases using lazysizes in httparchive data?

I looked at example pages using srcset's w descriptor (link) and a lot of these could not be implemented with size containment. https://arabic.cnn.com is one of the first examples and the srcset images shrink in height as the browser window is dynamically sized smaller. I also looked into lazysizes.js and found it has double-downloading behavior for the scrollbar example above, and seems to prevent infinite loops by not aggressively tracking layout changes.

eeeps commented 1 year ago

@progers Shrinking in height as the browser window is dynamically resized is not a problem, as long as both dimensions are extrinsic. In this case, they are. We have one dimension (width: 100%) and an aspect ratio (computed aspect-ratio: auto 780 / 439 -- those numbers come from the height and width attributes on <img>). So when I add img { contain: size; } to the page, nothing changes.

I've been thinking about the clearest way to teach this proposal and "if you're using auto-sizes, use width and height attributes on img" seems very clear. I think that covers almost all cases (even the extremely common one where max-width: 100% is used instead of width: 100% to achieve flexible image layout size), and is simple enough to implement, as long as you know the aspect ratio of the embedded resource.

I do however worry that there are many contexts in which HTML is being authored by a person or system that doesn't know that aspect ratio. Templates, user-generated content...

I'll spend some time tomorrow querying the HTTP Archive to see how many images that use srcset and w descriptors rely on intrinsic height.

bfgeek commented 1 year ago

One thing that could be considered is setting the aspect-ratio property as a presentational hint after the first image has been selected - it'd trigger another layout - but might be ok? Unclear.

zcorpan commented 1 year ago

Fixing https://github.com/whatwg/html/issues/3981 might help here, in that images with a different width than what was declared in srcset will still render at the width specified by sizes. So for sizes=auto, the width shouldn't change after loading the image. We basically ignore the image's natural width, but we can still apply the natural ratio from the image. No need for size containment??

eeeps commented 1 year ago

So I did some querying, and tied myself into quite a few knots. As discussed, determining whether a given <img>’s layout size is dependent on its intrinsic dimensions using only its DOM and CSSOM objects is hard, and I now see that my HTTP Almanac analysis was pretty naive. Anyways, here's what I've got:

Here are my takeaways:

(also I hope @zcorpan’s right about #3981 and all of this is moot!)

zcorpan commented 1 year ago

Checking the cases in this thread assuming we fix #3981:

progers commented 1 year ago

@zcorpan, could you expand on what you are thinking a little bit? I think you are saying that, with #3981 fixed, the width won't change with auto sizes. Would auto sizes need to require an aspect ratio (possibly via width+height attributes) to ensure that the height also does not change?

zcorpan commented 1 year ago

In my mind, we would set the natural width of the element to whatever the sizes attribute resolves to, and then ignore the natural width of the loaded image. For sizes=auto, for the first layout the image shouldn't be loaded yet, so you can get the layout width and set the natural width to the layout width after parsing sizes. When the image loads, the width stays the same.

For the height, I thought we could maybe honor the image's aspect ratio. If it's still an issue (e.g. for grid layout) to use the image's height or aspect ratio, we can require width/height attributes, but then we're back to similar behavior as size containment and developers need to know the aspect ratio beforehand. Also width/height attributes is not a guarantee that height does not change, since CSS can set width/height/aspect-ratio to something else.

It's a bit hard to reason about this without being able to test. 🙂

eeeps commented 1 year ago

Trying to wrap my head around what's acceptable/not acceptable here.

In the scrollbar example with the "sizes sets the intrinsic width" proposal, we are running srcset selection twice, but I guess it's not a problem because (as long as we only load larger resources):

  1. srcset selection is guaranteed to only run twice, and not again and again &c
  2. srcset selection is guaranteed not to pick a different resource on the second run

Those two are related... but I guess my question is, what exactly are we trying to prevent? Loading multiple resources AND running srcset selection an unbounded number of times both seem like blockers to me, but I want to confirm that understanding (and confirm that running srcset selection twice and possibly triggering one re-layout after load, is acceptable).

eeeps commented 1 year ago

If the scrollbar example is ok in this proposal, I guess the question becomes, can intrinsic height increasing ever make an <img>'s clientWidth LARGER?

zcorpan commented 1 year ago

IMO the requirements are:

Nice to have:

It seems we can't get all of the nice to haves. If we remove "unknown aspect ratio" and say that authors need to specify correct aspect ratio with width and height attributes (or with CSS), we can get most of the others.

Given @eeeps research in httparchive, the vast majority of images already have a specified aspect ratio, so easy adoption should be possible in those cases, even if we use size containment.

To avoid duplicate loads and avoid magic, I think using size containment is best. In which case, #3981 is not a blocker, and we can do https://github.com/whatwg/html/issues/9448#issuecomment-1609960596

emilio commented 1 year ago
  • in the sizes parsing algorithm: if the computed value of contain is not size, then map auto to 100vw. Otherwise use the concrete object size (even if it's 0).

How does that work? Does sizes parsing update style sync? That seems wrong. Or is it using an existing style (whatever we were laid out with?).

I think we should probably enforce that sizes="auto" forces (not only maps to) contain: size. I don't think we have precedents for !important attribute mappings, but they should be implementable. Otherwise a magic adjustment seems not-hard-to-do either.

eeeps commented 1 year ago

Another research note:

I was worried that even though ~80% of <img>s with srcset+w have height and width HTML attributes, not all of those attributes would be accurate representations of the aspect ratio of the eventually-loaded resource. This worry was unfounded; on mobile (where accuracy is worse), 75% are as right as they can be, and 98% are accurate to within a couple of percentage points.

zcorpan commented 1 year ago
  • in the sizes parsing algorithm: if the computed value of contain is not size, then map auto to 100vw. Otherwise use the concrete object size (even if it's 0).

How does that work? Does sizes parsing update style sync? That seems wrong. Or is it using an existing style (whatever we were laid out with?).

The latter. Should we have a note in the spec?

I think we should probably enforce that sizes="auto" forces (not only maps to) contain: size. I don't think we have precedents for !important attribute mappings, but they should be implementable. Otherwise a magic adjustment seems not-hard-to-do either.

Hmm interesting, yeah that would be an alternative and then we don't need to check computed value.

Is it helpful to have stricter syntax constraints around auto so that we can use a selector in the UA stylesheet like:

img:is([sizes="auto" i], [sizes^="auto," i]) { contain: size !important; }

edit: should be :is()

zcorpan commented 1 year ago

@eeeps thank you!

johannesodland commented 1 year ago

Is it helpful to have stricter syntax constraints around auto so that we can use a selector in the UA stylesheet like:

img:has([sizes="auto" i], [sizes^="auto," i]) { contain: size !important; }

Remember that sizes can be defined on any of the <source> elements in the <picture> element. I don't think the selected source is exposed to CSS selectors, so it might be hard to write a UA rule?

<picture>
  <source srcset="..." sizes=2400px>
  <img sizes=auto loading=lazy>
</picture>
zcorpan commented 1 year ago

@johannesodland indeed! Good call.

Ideally the applied style rules don't change from initial layout. Resource selection for lazy images happens later, so having that affect UA style isn't good.

We can require sizes=auto on the img for auto-sizes to work on source (and then sizes can be optional on source).

zcorpan commented 1 year ago

PR: https://github.com/whatwg/html/pull/9493

eeeps commented 1 year ago

My own feelings and a tiny, informal poll lead me to believe that disappearing images without width + height will be worrying and confusing to authors who apply sizes=auto without understanding the contain: size mechanism, or the reasoning behind it.

One thing we might be able to do to help here is add a contain-intrinsic-size to the UA stylesheet rule set. I guess this is equivalent to changing the default object size for these elements. I would suggest either 300px 150px to be consistent with <video> or 200px 200px to capture the most common image aspect ratio (1:1) at the (rounded) most common pixel count.

zcorpan commented 1 year ago

@eeeps thanks, yeah agreed it's a bit surprising. Adding contain-intrinsic-size seems ok, what do others think? 300x150 seems better to match the default size of most other elements (canvas, video, iframe...). That it's not most common is ok, we want authors to set the width/height.