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] min/max size transfers across aspect-ratio #6071

Open
fantasai opened this issue Mar 3, 2021 · 8 comments
Open

[css-sizing-4] min/max size transfers across aspect-ratio #6071

fantasai opened this issue Mar 3, 2021 · 8 comments

Comments

@fantasai
Copy link
Collaborator

fantasai commented Mar 3, 2021

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

Sizing constraints in either axis (the origin axis) are transferred through the preferred aspect ratio to the other axis (the destination axis) as follows:

First, any definite minimum size is converted and transferred from the origin to destination axis. This transferred minimum is capped by any definite preferred or maximum size in the destination axis.

Then, any definite maximum size is converted and transferred from the origin to destination. This transferred maximum is floored by any definite preferred or minimum size in the destination axis as well as by the transferred minimum, if any.

I think these should be defined as only impacting automatic or content-based sizes, rather than being defined as being an independent constraint clamped by the preferred size, because e.g. Flexbox makes a distinction between a size constraint applied to the "minimum size" vs a size constraint applied to the "preferred size" and it's otherwise not clear how this independent constraint would apply.

@fantasai
Copy link
Collaborator Author

We need to go through https://drafts.csswg.org/css-flexbox/#min-size-auto and make sure that the new text is consistent with that; replaced elements at least should not have new behavior because of the new css-sizing-4 text.

@fantasai
Copy link
Collaborator Author

Key point: a flex item that has min-width: 0 has no minimum for flex shrinkage. Any constraints transferred through the aspect ratio have an effect on the minimum size only if they are applied via min-width: auto or min-width: min-content etc. They can still affect the flex base size also, if that is an automatic or content-based size, but they're not an independent minimum in the same way that min-width is.

So the css-sizing-4 text is wrong here.

@cbiesinger
Copy link

So after discussing with fantasai, the css-sizing-4 transferred min/max size text should not apply in flexbox/grid. Instead, only the sizing inside those specs should apply. For flex:

  • Minimum sizes already applied per spec as part of content size suggestion in min-size: auto; should not apply for an explicit min size for compat with replaced elements
  • Maximum size only taken into account for flex base size (spec text does not mention this explicitly if cross size is indefinite but is part of max-content computation - [css-flexbox-1] be clearer about aspect ratio without definite cross size case #6109). If flexing, will distort image, but you asked for it (flex-grow defaults to 0)

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 19, 2021
This reverts commit 174c8fdaa25c49ca36b702598f3fc08f72cc4f29.

Reason for revert: see w3c/csswg-drafts#6071 (comment)

Original change's description:
> [AspectRatio] Fix flex-aspect-ratio-026.html in legacy
>
> Implement min/max size transfers for replaced elements:
> https://drafts.csswg.org/css-sizing-4/#aspect-ratio-size-transfers
>
> Mark both flex-aspect-ratio-025.html and flex-aspect-ratio-026.html as
> passing for legacy and update these tests to take both main and cross
> size padding into account.
>
> Add flex-aspect-ratio-027.html to verify that min/max-size transfer does not always happen just because aspect-ratio is present.
>
> Bug: 1164135
> Change-Id: I20a26cbe256ae5d6a66c5763c7b12f814a3d7ef6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2721723
> Commit-Queue: Rob Buis <rbuis@igalia.com>
> Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#859812}

Bug: 1164135
Change-Id: I6a36d40c7e74eed6ab333a5bebcf6d1471d7d1b1
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 24, 2021
This reverts commit 174c8fdaa25c49ca36b702598f3fc08f72cc4f29.

Reason for revert: see w3c/csswg-drafts#6071 (comment)

Original change's description:
> [AspectRatio] Fix flex-aspect-ratio-026.html in legacy
>
> Implement min/max size transfers for replaced elements:
> https://drafts.csswg.org/css-sizing-4/#aspect-ratio-size-transfers
>
> Mark both flex-aspect-ratio-025.html and flex-aspect-ratio-026.html as
> passing for legacy and update these tests to take both main and cross
> size padding into account.
>
> Add flex-aspect-ratio-027.html to verify that min/max-size transfer does not always happen just because aspect-ratio is present.
>
> Bug: 1164135
> Change-Id: I20a26cbe256ae5d6a66c5763c7b12f814a3d7ef6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2721723
> Commit-Queue: Rob Buis <rbuis@igalia.com>
> Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#859812}

Bug: 1164135
Change-Id: I6a36d40c7e74eed6ab333a5bebcf6d1471d7d1b1
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 24, 2021
This reverts commit 174c8fdaa25c49ca36b702598f3fc08f72cc4f29.

Reason for revert: see w3c/csswg-drafts#6071 (comment)

Original change's description:
> [AspectRatio] Fix flex-aspect-ratio-026.html in legacy
>
> Implement min/max size transfers for replaced elements:
> https://drafts.csswg.org/css-sizing-4/#aspect-ratio-size-transfers
>
> Mark both flex-aspect-ratio-025.html and flex-aspect-ratio-026.html as
> passing for legacy and update these tests to take both main and cross
> size padding into account.
>
> Add flex-aspect-ratio-027.html to verify that min/max-size transfer does not always happen just because aspect-ratio is present.
>
> Bug: 1164135
> Change-Id: I20a26cbe256ae5d6a66c5763c7b12f814a3d7ef6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2721723
> Commit-Queue: Rob Buis <rbuis@igalia.com>
> Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#859812}

Bug: 1164135
Change-Id: I6a36d40c7e74eed6ab333a5bebcf6d1471d7d1b1
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 24, 2021
This reverts commit 174c8fdaa25c49ca36b702598f3fc08f72cc4f29.

Reason for revert: see w3c/csswg-drafts#6071 (comment)

Original change's description:
> [AspectRatio] Fix flex-aspect-ratio-026.html in legacy
>
> Implement min/max size transfers for replaced elements:
> https://drafts.csswg.org/css-sizing-4/#aspect-ratio-size-transfers
>
> Mark both flex-aspect-ratio-025.html and flex-aspect-ratio-026.html as
> passing for legacy and update these tests to take both main and cross
> size padding into account.
>
> Add flex-aspect-ratio-027.html to verify that min/max-size transfer does not always happen just because aspect-ratio is present.
>
> Bug: 1164135
> Change-Id: I20a26cbe256ae5d6a66c5763c7b12f814a3d7ef6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2721723
> Commit-Queue: Rob Buis <rbuis@igalia.com>
> Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#859812}

Bug: 1164135
Change-Id: I6a36d40c7e74eed6ab333a5bebcf6d1471d7d1b1
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 24, 2021
This reverts commit 174c8fdaa25c49ca36b702598f3fc08f72cc4f29 and 674680c9a1a7f1e34ace0e050ea0276006252671.

Reason for revert: see w3c/csswg-drafts#6071 (comment)

This CL keeps the useful WPT test changes for flex-aspect-ratio-025.html and flex-aspect-ratio-026.html.

Bug: 1164135
Change-Id: I6a36d40c7e74eed6ab333a5bebcf6d1471d7d1b1
@bfgeek
Copy link

bfgeek commented Mar 25, 2021

What happens with grid minimum sizes here? E.g:

<!DOCTYPE html>
<div style="display: grid; width: 0; height: 0; position: relative;">
  <div style="overflow: hidden; min-width: 100px; min-height: 50px; aspect-ratio: 1/1; background: red;">
  </div>
  <div style="position: absolute; width: 100%; height: 100%; background: green; grid-area: 1/1/1/1;"></div>
</div>
<!DOCTYPE html>
<div style="display: grid; width: 0; height: 0; position: relative;">
  <div style="overflow: hidden; min-width: 50px; min-height: 100px; aspect-ratio: 1/1; background: red;">
  </div>
  <div style="position: absolute; width: 100%; height: 100%; background: green; grid-area: 1/1/1/1;"></div>
</div>

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 30, 2021
This reverts commit 174c8fdaa25c49ca36b702598f3fc08f72cc4f29 and 674680c9a1a7f1e34ace0e050ea0276006252671.

Reason for revert: see w3c/csswg-drafts#6071 (comment)

This CL keeps the useful WPT test changes for flex-aspect-ratio-025.html and flex-aspect-ratio-026.html.

Bug: 1164135
Change-Id: I6a36d40c7e74eed6ab333a5bebcf6d1471d7d1b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2773267
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Rob Buis <rbuis@igalia.com>
Cr-Commit-Position: refs/heads/master@{#867495}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 30, 2021
This reverts commit 174c8fdaa25c49ca36b702598f3fc08f72cc4f29 and 674680c9a1a7f1e34ace0e050ea0276006252671.

Reason for revert: see w3c/csswg-drafts#6071 (comment)

This CL keeps the useful WPT test changes for flex-aspect-ratio-025.html and flex-aspect-ratio-026.html.

Bug: 1164135
Change-Id: I6a36d40c7e74eed6ab333a5bebcf6d1471d7d1b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2773267
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Rob Buis <rbuis@igalia.com>
Cr-Commit-Position: refs/heads/master@{#867495}
pull bot pushed a commit to luojiguicai/chromium that referenced this issue Mar 30, 2021
This reverts commit 174c8fd and 674680c.

Reason for revert: see w3c/csswg-drafts#6071 (comment)

This CL keeps the useful WPT test changes for flex-aspect-ratio-025.html and flex-aspect-ratio-026.html.

Bug: 1164135
Change-Id: I6a36d40c7e74eed6ab333a5bebcf6d1471d7d1b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2773267
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Rob Buis <rbuis@igalia.com>
Cr-Commit-Position: refs/heads/master@{#867495}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 2, 2021
Automatic update from web-platform-tests
Revert r859812 and r860306

This reverts commit 174c8fdaa25c49ca36b702598f3fc08f72cc4f29 and 674680c9a1a7f1e34ace0e050ea0276006252671.

Reason for revert: see w3c/csswg-drafts#6071 (comment)

This CL keeps the useful WPT test changes for flex-aspect-ratio-025.html and flex-aspect-ratio-026.html.

Bug: 1164135
Change-Id: I6a36d40c7e74eed6ab333a5bebcf6d1471d7d1b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2773267
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Rob Buis <rbuis@igalia.com>
Cr-Commit-Position: refs/heads/master@{#867495}

--

wpt-commits: 52c2635ca8e3e94e18d3da6c409001d25abfba63
wpt-pr: 28149
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 2, 2021
Automatic update from web-platform-tests
Revert r859812 and r860306

This reverts commit 174c8fdaa25c49ca36b702598f3fc08f72cc4f29 and 674680c9a1a7f1e34ace0e050ea0276006252671.

Reason for revert: see w3c/csswg-drafts#6071 (comment)

This CL keeps the useful WPT test changes for flex-aspect-ratio-025.html and flex-aspect-ratio-026.html.

Bug: 1164135
Change-Id: I6a36d40c7e74eed6ab333a5bebcf6d1471d7d1b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2773267
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Rob Buis <rbuis@igalia.com>
Cr-Commit-Position: refs/heads/master@{#867495}

--

wpt-commits: 52c2635ca8e3e94e18d3da6c409001d25abfba63
wpt-pr: 28149
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 7, 2021
Automatic update from web-platform-tests
Revert r859812 and r860306

This reverts commit 174c8fdaa25c49ca36b702598f3fc08f72cc4f29 and 674680c9a1a7f1e34ace0e050ea0276006252671.

Reason for revert: see w3c/csswg-drafts#6071 (comment)

This CL keeps the useful WPT test changes for flex-aspect-ratio-025.html and flex-aspect-ratio-026.html.

Bug: 1164135
Change-Id: I6a36d40c7e74eed6ab333a5bebcf6d1471d7d1b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2773267
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Rob Buis <rbuis@igalia.com>
Cr-Commit-Position: refs/heads/master@{#867495}

--

wpt-commits: 52c2635ca8e3e94e18d3da6c409001d25abfba63
wpt-pr: 28149
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 8, 2021
Automatic update from web-platform-tests
Revert r859812 and r860306

This reverts commit 174c8fdaa25c49ca36b702598f3fc08f72cc4f29 and 674680c9a1a7f1e34ace0e050ea0276006252671.

Reason for revert: see w3c/csswg-drafts#6071 (comment)

This CL keeps the useful WPT test changes for flex-aspect-ratio-025.html and flex-aspect-ratio-026.html.

Bug: 1164135
Change-Id: I6a36d40c7e74eed6ab333a5bebcf6d1471d7d1b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2773267
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Rob Buis <rbuis@igalia.com>
Cr-Commit-Position: refs/heads/master@{#867495}

--

wpt-commits: 52c2635ca8e3e94e18d3da6c409001d25abfba63
wpt-pr: 28149
@rwlbuis
Copy link

rwlbuis commented May 17, 2021

So after discussing with fantasai, the css-sizing-4 transferred min/max size text should not apply in flexbox/grid. Instead, only the sizing inside those specs should apply. For flex:

  • Minimum sizes already applied per spec as part of content size suggestion in min-size: auto; should not apply for an explicit min size for compat with replaced elements
  • Maximum size only taken into account for flex base size (spec text does not mention this explicitly if cross size is indefinite but is part of max-content computation - [css-flexbox-1] be clearer about aspect ratio without definite cross size case #6109). If flexing, will distort image, but you asked for it (flex-grow defaults to 0)

Can somebody check if this makes existing aspect-ratio + flex test expectations invalid? For example:
https://wpt.fyi/results/css/css-sizing/aspect-ratio/flex-aspect-ratio-025.html
https://wpt.fyi/results/css/css-sizing/aspect-ratio/flex-aspect-ratio-026.html

@rwlbuis
Copy link

rwlbuis commented May 18, 2021

Ah, never mind, I see those tests got corrected.

@fantasai
Copy link
Collaborator Author

fantasai commented Sep 7, 2021

I think this was fixed in a5044be @cbiesinger let me know if the spec seems to be correct now?

@JaneOri
Copy link

JaneOri commented Aug 21, 2022

This codepen (created Dec 31st 2021) https://codepen.io/propjockey/pen/VwMPYNw?editors=1100
Used to render this:
image

But now aspect-ratio is not respecting max-height

Is it because of these changes?

It was really useful to emulate a conceptual element-fit: contain;... could we please consider allowing it again, or having a path to enable similar behavior?

mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
This reverts commit 174c8fdaa25c49ca36b702598f3fc08f72cc4f29 and 674680c9a1a7f1e34ace0e050ea0276006252671.

Reason for revert: see w3c/csswg-drafts#6071 (comment)

This CL keeps the useful WPT test changes for flex-aspect-ratio-025.html and flex-aspect-ratio-026.html.

Bug: 1164135
Change-Id: I6a36d40c7e74eed6ab333a5bebcf6d1471d7d1b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2773267
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Rob Buis <rbuis@igalia.com>
Cr-Commit-Position: refs/heads/master@{#867495}
GitOrigin-RevId: cb947d5c1bf9a3bf12f62a6e153eeb49b7533198
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants