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-flex] Don't allow items' heights to affect intrinsic block sizes #27183

Open
wants to merge 1 commit into
base: master
from

Conversation

@chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Jan 14, 2021

Blink currently makes this flex item 200px tall. Firefox gives it the
correct height of 100px.

<div style="display: flex; flex-direction: column">
<div style="flex-basis: 100px; height: 200px">
<div style="height: 100%">

Blink was letting the inner div's height resolve against the flex item's
200px, giving the flex item an automatic minimum size of 200px, even
though the min-content size is supposed to be calculated with the item
having an 'auto' size. Meaning we should ignore the item's 200px height.

To keep the 200px from having an effect, this patch broadens the
definition of ConstraintSpace().IsFixedBlockSizeDefinite. It no longer
has to accompany a FixedBlockSize. It now means that the element is to
be laid out as if it has an auto height.

Flexbox now sets this constraint on items when the flexbox needs the
item's intrinsic block size.

Bug: 1033476
Change-Id: I74feb5a0dc0c4ae6e4a5eb2e01496992b23cc552
Reviewed-on: https://chromium-review.googlesource.com/2613485
WPT-Export-Revision: 74f225f0fdeb3a6a3d599806ca7d341bf690bd29

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

The review process for this patch is being conducted in the Chromium project.

Blink currently makes this flex item 200px tall. Firefox gives it the
correct height of 100px.

<div style="display: flex; flex-direction: column">
  <div style="flex-basis: 100px; height: 200px">
    <div style="height: 100%">

Blink was letting the inner div's height resolve against the flex item's
200px, giving the flex item an automatic minimum size of 200px, even
though the min-content size is supposed to be calculated with the item
having an 'auto' size. Meaning we should ignore the item's 200px height.

To keep the 200px from having an effect, this patch broadens the
definition of ConstraintSpace().IsFixedBlockSizeDefinite. It no longer
has to accompany a FixedBlockSize. It now means that the element is to
be laid out as if it has an auto height.

Flexbox now sets this constraint on items when the flexbox needs the
item's intrinsic block size.

Bug: 1033476
Change-Id: I74feb5a0dc0c4ae6e4a5eb2e01496992b23cc552
@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-2613485 branch from 7637334 to 4f35551 Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants