Skip to content
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

[css-sizing-4] Should aspect-ratio apply for width: auto; height: auto;? #5060

Open
cbiesinger opened this issue May 11, 2020 · 26 comments
Open
Labels
Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered. Commenter Response Pending css-sizing-4

Comments

@cbiesinger
Copy link

https://drafts.csswg.org/css-sizing-4/#aspect-ratio

With width: auto; height: auto; aspect-ratio: 1/1; (in a horizontal writing mode), we still have a definite width, should we use that to calculate a height using the aspect ratio? Or should we require width: 100%;?

/cc @bfgeek @fantasai @tabatkins

@cbiesinger
Copy link
Author

Similar question for height: auto on a stretched row flex item, where this still gets a definite height.

@AmeliaBR
Copy link
Contributor

I would expect all sizing for elements with a fixed aspect-ratio to behave the same as for a replaced element with a fixed aspect ratio.

For a block element in flow layout, that means that yes, the block size would be maximized to fit (after adjusting for padding/border, same as normal), and then the aspect ratio would determine the height. In other layout situations, different calculations would apply, but I think we now have defined algorithms for all these cases, including the flexbox one (if I recall correctly: it stretches to fit the row height, then the width gets calculated from the aspect ratio & becomes the flex-basis, but then flex grow or shrink applies and can distort the aspect ratio from its ideal).

The main difference between a CSS box with aspect ratio, versus an image or SVG with an aspect ratio, would be in how min-content and max-content sizes are calculated, based on the normal rules for text/child CSS content instead of based on the intrinsic sizes of the replaced content.

@fantasai
Copy link
Collaborator

Yes, in the auto/auto case we do calculate the block size against the inline size using the aspect ratio. As @AmeliaBR points out, this is already defined because we had to define it for SVGs with an intrinsic aspect ratio and no intrinsic size, and aspect-ratio defines itself as applying those rules.

The box’s automatic sizes are calculated the same as for a replaced element with an intrinsic aspect ratio and no intrinsic dimension in that axis, see e.g. CSS2 § 10 and CSS Flexible Box Model Level 1 § 9.2.

(If there's particular layout algorithms which don't handle this case, it needs a separate issue, since the problem applies to both such images as well as boxes with aspect-ratio.)

@fantasai
Copy link
Collaborator

Closing the issue, since I think the spec already defines this and there's nothing to do here.

@fantasai fantasai added the Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered. label May 12, 2020
@cbiesinger
Copy link
Author

@AmeliaBR So, I want to follow up on the flex item case. First I thought you were right, now I'm not sure. See:
https://drafts.csswg.org/css-flexbox/#definite-sizes step 3
"Once the cross size of a flex line has been determined, the cross sizes of items in auto-sized flex containers are also considered definite" (emphasis mine)

But https://drafts.csswg.org/css-flexbox/#algo-main-item happens before the cross-size determination.

As such, I think the flex item case does require a height: 100% to trigger aspect-ratio in this case.

@AmeliaBR
Copy link
Contributor

@cbiesinger Are you only looking in Chrome? Because there is still cross browser inconsistency here. Chrome collapses elements to zero, Firefox does not, see modified version of your test case with SVG elements as well.

@fantasai would probably remember better than I, but I'm pretty sure the WG consensus was that the “elements with aspect ratio but no defined size collapse to zero in flexbox” was a bad thing. 😏 If you want to work on tracking down the necessary tests & bug fixes, that would be greatly appreciated!

Either way, the CSS box with aspect-ratio should behave the same as the SVG.

@cbiesinger
Copy link
Author

cbiesinger commented May 14, 2020

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 14, 2020
…uto;

See discussion in w3c/csswg-drafts#5060

R=ikilpatrick@chromium.org

Fixed: 1082479
Change-Id: Ic1f47a5ba92cdbff96ce0b1d60e9741c1051c291
@AmeliaBR
Copy link
Contributor

Some relevant past issues & discussion:

Resolution in #951 (comment) says “Replaced elements with only an intrinsic aspect ratio has a min-content size of 0” but see also #694 about clarifying the flexbox spec.

The current relevant spec text is in the Flex Layout Algorithm:

If the flex item has ...

  • an intrinsic aspect ratio,
  • a used flex basis of content, and
  • a definite cross size,

then the flex base size is calculated from its used inner cross size and the flex item’s intrinsic aspect ratio.

I think if you follow all the linked definitions for terms, that says the Firefox behavior is correct. Note that the stretch value for align-items isn't relevant: because the cross-size is definite (a single-row flexbox with defined height), that gets used to determine the cross size of the box with an aspect ratio and no other size constraints.

@cbiesinger
Copy link
Author

You convinced me for the align-self: stretch case, but I don't see why that's the case for non-stretched items? dholbert/fantasai, any thoughts?

@AmeliaBR
Copy link
Contributor

that the stretch value for align-items isn't relevant

or maybe not…

  1. If a single-line flex container has a definite cross size, the outer cross size of any stretched flex items is the flex container’s inner cross size (clamped to the flex item’s min and max cross size) and is considered definite.

So I'm not sure what's supposed to happen if it isn't stretched.

I'm going to have to defer to the experts on this one (with a plea for maybe a few more editorial improvements or examples? If Chris and I can't figure out the spec, pity the poor mortals who don't do this for a living!)

@cbiesinger
Copy link
Author

cbiesinger commented May 15, 2020

Yes, I was just going to point to that link :) (https://drafts.csswg.org/css-flexbox/#definite-sizes item 1)

I filed https://crbug.com/1083020 in the meantime.

@cbiesinger
Copy link
Author

Let me reopen this issue for now.

@cbiesinger cbiesinger reopened this May 15, 2020
@cbiesinger
Copy link
Author

cbiesinger commented May 15, 2020

Intriguingly, unlike the SVG, Firefox does not apply the right aspect ratio for <img>:
https://jsfiddle.net/dgrogan/djh5wu0x/1/

@dholbert , what's up with that? :)

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 15, 2020
…uto;

See discussion in w3c/csswg-drafts#5060

R=ikilpatrick@chromium.org

Fixed: 1082479
Change-Id: Ic1f47a5ba92cdbff96ce0b1d60e9741c1051c291
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 15, 2020
…uto;

See discussion in w3c/csswg-drafts#5060

R=ikilpatrick@chromium.org

Fixed: 1082479
Change-Id: Ic1f47a5ba92cdbff96ce0b1d60e9741c1051c291
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2202691
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769360}
@AmeliaBR
Copy link
Contributor

does not apply the right aspect ratio for

The distinction isn't that it is an <img>, it's that the PNG image has intrinsic size (16px square) as well as intrinsic aspect ratio. (But I'm not sure where in the algorithm that is causing the transferred size from the stretched height to be ignored when calculating the auto width.)

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 15, 2020
…uto;

See discussion in w3c/csswg-drafts#5060

R=ikilpatrick@chromium.org

Fixed: 1082479
Change-Id: Ic1f47a5ba92cdbff96ce0b1d60e9741c1051c291
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2202691
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769360}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue May 15, 2020
…uto;

See discussion in w3c/csswg-drafts#5060

R=ikilpatrick@chromium.org

Fixed: 1082479
Change-Id: Ic1f47a5ba92cdbff96ce0b1d60e9741c1051c291
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2202691
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769360}
@dholbert
Copy link
Member

In AmeliaBR's testcase with two SVG elements that she posted about above, I believe what's actually happening in Firefox is:

  • the first SVG element has a resolved flex-basis of 100px (from its specified height:100% height (cross-size) resolving against the container height, and transferring through the aspect ratio to produce a definite main-size)
  • the second SVG element has a resolved flex-basis of 220px (which I think comes from resolving its implicit width=100% attribute against the size of its container...?) It shrinks to something smaller, but if you use Firefox flex devtools or add flex-shrink:0, then you can see (in Firefox at least) that its flex-basis is 220px. (This might be a Firefox bug, not sure.) It seems like Chrome instead is treating its flex-basis as 0px here, I guess?
  • This is true for the SVG elements in the testcase-variant in biesi's response, too.

Here's a variant of Amelia's testcase with flex-shrink:0 added, to demonstrate this.

@dholbert
Copy link
Member

Intriguingly, unlike the SVG, Firefox does not apply the right aspect ratio for <img>:
https://jsfiddle.net/dgrogan/djh5wu0x/1/

@dholbert , what's up with that? :)

I'm not sure offhand why there's an img/svg difference, but Chrome & Firefox do agree on that being a difference. Here's an expanded version of your fiddle, comparing img vs. svg-with-intrinsic-size, for both stretch and flex-start -- Firefox and Chrome agree on the rendering https://jsfiddle.net/dholbert/wxr4jsg1/

@cbiesinger
Copy link
Author

Thanks @dholbert ; two questions:

@dholbert
Copy link
Member

Thanks @dholbert ; two questions:

  • You mention an implicit width: 100%, but I didn't think SVGs did that?

I'm talking about how the lacuna value of the <svg width=...> attribute is 100%. Spec references:

If the [width/height] attribute is not specified, the effect is as if a value of '100%' were specified.
https://www.w3.org/TR/SVG11/struct.html#SVGElementWidthAttribute

The value auto for width and height on the ‘svg’ element is treated as 100%.
https://svgwg.org/svg2-draft/geometry.html#Sizing

(I suspect this isn't meant to actually behave as 100% in this context, though, so this may be a Firefox bug.)

Your testcase here is comparing width and height attributes against viewBox attribute.

These attributes do different things, intrinsic-sizing-wise.

The viewBox attribute only sets the intrinsic aspect ratio of a SVG element -- it does not set the intrinsic size. For an apples-to-apples comparison between <svg> and <img src="some-raster-image.png">, you need to provide pixel-valued width & height attribute-values on the SVG element (even if these attributes are not present on the img element).

The width and height attributes (if given a definite value) are what provide the intrinsic size, for SVG. This is easiest to reason about in an example where we have <img src="foo.svg"> with foo.svg containing <svg height="16" width="16" viewBox="0 0 100 100">, vs. another bar.svg which has <svg viewBox="0 0 100 100">. In both cases, the intrinsic aspect ratio is 1:1. The former case has an intrinsic size of 16x16. The latter case has no intrinsic size (but still has an intrinsic aspect ratio of 16x16).

Anyway -- the Chrome/Firefox difference on your second fiddle is merely an example of us (probably-mistakenly) using the lacuna "width=100%" value on the SVG element when establishing its flex-basis, as you can see if you add flex-shrink:0:
https://jsfiddle.net/jgcnm5sL/

@cbiesinger
Copy link
Author

Ah... thanks for the spec reference.

Anyway, my point with the width/height attribute was that it does not just set the intrinsic width, but it also makes the width specified. So we have a definite used flex basis and won't fall into the aspect ratio case here: https://drafts.csswg.org/css-flexbox/#algo-main-item

Do you want me to file a bug on Gecko to not use 100% here?

@dholbert
Copy link
Member

I see, that makes sense. So I think that means <svg> elements are in the special position where there's no way to give them an intrinsic BSize without also implicitly giving them a non-auto BSize and opting them out of the cross-axis stretching, perhaps.

I'm already in the process filing a gecko bug on the 100% thing here (prepping a semi-comprehensive testcase) and I'll post it here when I've filed it. Thanks!

@dholbert
Copy link
Member

dholbert commented May 18, 2020

(I'm not entirely sure the 100%-svg-flex-basis thing is a Firefox bug, actually. Firefox renders these two testcases the same, which makes sense to me; whereas Chrome renders them differently [and agrees with us on the first but not the second]. The only difference between the two is flex vs. block. The interoperable behavior on the first is basically the 100% behavior that I was talking about.)

data:text/html,<!DOCTYPE html><div style="display:block;width:500px"><svg style="border:3px solid black;" viewBox="0 0 1 1">

data:text/html,<!DOCTYPE html><div style="display:flex;width:500px"><svg style="border:3px solid black;" viewBox="0 0 1 1">

@dholbert
Copy link
Member

dholbert commented May 18, 2020

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1638937 on the SVG sizing-to-100% thing, BTW. The last piece of testcase 1 there ("Just viewBox") is the relevant bit where Chrome & Firefox differ.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 26, 2020
…even with width:auto;height:auto;, a=testonly

Automatic update from web-platform-tests
[AspectRatio] aspect-ratio should apply even with width:auto;height:auto;

See discussion in w3c/csswg-drafts#5060

R=ikilpatrick@chromium.org

Fixed: 1082479
Change-Id: Ic1f47a5ba92cdbff96ce0b1d60e9741c1051c291
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2202691
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769360}

--

wpt-commits: bbf1f57cc2539e68e1a8d56685d2cae2eda9cc81
wpt-pr: 23625
xeonchen pushed a commit to xeonchen/gecko that referenced this issue May 26, 2020
…even with width:auto;height:auto;, a=testonly

Automatic update from web-platform-tests
[AspectRatio] aspect-ratio should apply even with width:auto;height:auto;

See discussion in w3c/csswg-drafts#5060

R=ikilpatrick@chromium.org

Fixed: 1082479
Change-Id: Ic1f47a5ba92cdbff96ce0b1d60e9741c1051c291
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2202691
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769360}

--

wpt-commits: bbf1f57cc2539e68e1a8d56685d2cae2eda9cc81
wpt-pr: 23625
@fantasai fantasai removed the Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered. label Jun 5, 2020
@fantasai
Copy link
Collaborator

@cbiesinger

So, I want to follow up on the flex item case. First I thought you were right, now I'm not sure. See:
drafts.csswg.org/css-flexbox/#definite-sizes step 3
"Once the cross size of a flex line has been determined, the cross sizes of items in auto-sized flex containers are also considered definite" (emphasis mine)

But drafts.csswg.org/css-flexbox/#algo-main-item happens before the cross-size determination.

As such, I think the flex item case does require a height: 100% to trigger aspect-ratio in this case.

Funnily enough, this comment was posted just a few days after we committed the (mistaken, it turns out) simplification to the definiteness rules you asked for in #3531, which made this exact case no longer have a definite size. While reviewing this thread we realized it wasn't redundant at all, exactly for cases like this, and restored that text in ba8c7af, so now the spec is back to the expected behavior of the flex item having a definite cross size immediately (because it's stretched, and the flex container is single-line and has a definite cross size), so the aspect-ratio should activate and give the item a width of 100px.

You convinced me for the align-self: stretch case, but I don't see why that's the case for non-stretched items?

A non-stretched item won't gain a definite height like the above; it'll fall through to Flexbox algo step 3 case E, in which its width becomes max-content, which per css-sizing-3 depends on its available inline size, which is defined in Flexbox step 2 as the inner size of the flex container. At this point its width becomes definite, so the a-r should then give it a height. (Note that this doesn't require it to flex; this happens prior to that step.)

Firefox appears to get this case right, currently; Chrome for some reason keeps it at the naturalHeight regardless (testcase). So, this appears to be well-defined and in line with what we'd expect.

We think there's nothing left to do for this issue, if that seems right shall we close it?

~@tabatkins & @fantasai

@fantasai fantasai added Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered. Commenter Response Pending labels Jul 20, 2020
@cbiesinger
Copy link
Author

cbiesinger commented Sep 17, 2020

Hold on, in that testcase, why does the max-content size depend on the available inline size? Are you suggesting this is due to the implied 100% width? (I would argue that this falls really into clause A), but I guess that doesn't really make a difference)

I find it unintuitive that an <svg> inside a flexbox sizes to 100%, is that really the behavior we want?

@fantasai
Copy link
Collaborator

fantasai commented Oct 5, 2020

Hold on, in that testcase, why does the max-content size depend on the available inline size?

Because when a replaced element has no intrinsic size, we have to come up with one, and that falls under the “For boxes with an intrinsic aspect ratio, but no intrinsic size” clause in https://drafts.csswg.org/css-sizing-3/#intrinsic-sizes (which is itself extrapolated from the suggestion in https://www.w3.org/TR/CSS2/visudet.html#inline-replaced-width which leaves it otherwise undefined).

I find it unintuitive that an inside a flexbox sizes to 100%, is that really the behavior we want?

What other behavior would you suggest? Seems more useful to make sure it fits its container than to size it to zero or to arbitrarily pick 300px or something, imho!

mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
…uto;

See discussion in w3c/csswg-drafts#5060

R=ikilpatrick@chromium.org

Fixed: 1082479
Change-Id: Ic1f47a5ba92cdbff96ce0b1d60e9741c1051c291
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2202691
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#769360}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 4af37311f5b3bc70d29896dd6fc58c32a3b319a1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered. Commenter Response Pending css-sizing-4
Projects
None yet
Development

No branches or pull requests

4 participants