Skip to content

[css-flexbox] Percentage height resolution of children of flex items with indefinite flex basis #6822

Closed

Description

@svillar

For reference, this was filled after @davidsgrogan and me having different opinions in this chrome bug.

This is the scenario

<div style="display: flex; flex-direction: column;">
  <div style="flex: 1 1 content; height: 100px; min-height: 0px;">
    <div style="height: 30px; width: 100px; background: blue;"></div>
    <div style="height: 100%; width: 100px; background: red;"></div>
  </div>
</div>

ToT Gecko and Blink size the red <div> with a percentage height as 0px while WebKit computes a 30px height. That computation happens in two steps

  1. When computing the flex basis of the flex item we have to compute the content size. We have 30px for the first child and 0px for the second (as it's a percentage of an indefinite size). Summing up 30px. So far so good, all the engines agree with this
  2. Then after computing the flex basis and resolving lines we proceed to layout. At this point there are two different interpretations of the current specs.

Option 1 (Blink and Gecko)

@davidsgrogan cites the following text from the CSS2 specs

If the height of the containing block is not specified explicitly (i.e., it depends on content height) [...] the value computes to auto.

and from that concludes that red's percent height computes to auto because the height of its containing block (i.e. the flex item) depends on content height. So because red's percent height computes to auto, the final height is its content height, which is 0px.

Option 2 (WebKit)

In this case the engine considers that as the flex item is 30px height, then the div with the percentage height should be 30px as well.

I think the second option is the correct one for some reasons:

  1. That CSS2 text was written way before flexbox, that's why I think the spirit of that paragraph was mainly to resolve the typical case of a box with an auto height having a child with a percentage height (and similar ones).
  2. It's unclear to me that we could consider that the height of the containing block depends on the content height for the purpouse of resolving the percentage. It does indeed depend on the content height to compute the flex basis, but I don't think we could conclude that after computing the flex basis we should still consider the size as indefinite.

Anyway it's indeed controversial so I'll let more informed people to give their opinions.

PS: this affects 3 WPT tests

  • css/css-flexbox/percentage-heights-016.html
  • css/css-flexbox/percentage-heights-017.html
  • css/css-flexbox/percentage-heights-018.html

Activity

  1. changed the title [-][css-flexbox] Percentage height resolution of flex item children with indefinite flex basis[/-] [+][css-flexbox] Percentage height resolution of children of flex items with indefinite flex basis[/+] on Nov 18, 2021
  2. Loirooriol commented on Nov 18, 2021

    @Loirooriol
    Contributor

    Option 2 (WebKit): In this case the engine considers that as the flex item is 30px height, then the div with the percentage height should be 30px as well.

    That's not what I get in my local webkit, I get 100px instead, it's resolving against the computed height of the flex item (minus paddings and borders for box-sizing: border-box).

    This seems wrong to me, the percentage should either behave as auto (0px) or resolve against the actual size of the containing block (30px).

  3. svillar commented on Nov 19, 2021

    @svillar
    Author

    Option 2 (WebKit): In this case the engine considers that as the flex item is 30px height, then the div with the percentage height should be 30px as well.

    That's not what I get in my local webkit, I get 100px instead, it's resolving against the computed height of the flex item (minus paddings and borders for box-sizing: border-box).

    This seems wrong to me, the percentage should either behave as auto (0px) or resolve against the actual size of the containing block (30px).

    That's very likely because I've landed the flex-basis: content support just a few days ago so it was still not released in any WebKit browser.

  4. Loirooriol commented on Nov 19, 2021

    @Loirooriol
    Contributor

    I have updated my repo to latest https://trac.webkit.org/changeset/286061/webkit, and I still get a red 100x100 square. Anyways, I'm curious what happens if you use height: auto instead of height: 100px. Does the percentage still resolve against 30px?

  5. svillar commented on Nov 19, 2021

    @svillar
    Author

    I have updated my repo to latest https://trac.webkit.org/changeset/286061/webkit, and I still get a red 100x100 square.

    That's another bug then. Anyway the dilema still stands.

  6. Loirooriol commented on Nov 19, 2021

    @Loirooriol
    Contributor

    https://drafts.csswg.org/css-sizing-3/#cyclic-percentage-contribution

    Then, unless otherwise specified, when calculating the used sizes and positions of the containing block’s contents:

    • If the cyclic dependency was introduced due to a block-size or max-block-size on the containing block that causes it to depend on the size of its contents, the box’s percentage is not resolved and instead behaves as auto.
      Note: Grid items and flex items do allow percentages to resolve in this case.
    • Otherwise, the percentage is resolved against the containing block’s size. (The containing block’s size is not re-resolved based on the resulting size of the box; the contents might thus overflow or underflow the containing block).

    In this case the dependency comes from flex-basis and not (max-)block-size, so 30px I guess? But what would happen with flex-basis: content; height: auto?

  7. svillar commented on Nov 20, 2021

    @svillar
    Author

    https://drafts.csswg.org/css-sizing-3/#cyclic-percentage-contribution

    Then, unless otherwise specified, when calculating the used sizes and positions of the containing block’s contents:

    • If the cyclic dependency was introduced due to a block-size or max-block-size on the containing block that causes it to depend on the size of its contents, the box’s percentage is not resolved and instead behaves as auto.
      Note: Grid items and flex items do allow percentages to resolve in this case.
    • Otherwise, the percentage is resolved against the containing block’s size. (The containing block’s size is not re-resolved based on the resulting size of the box; the contents might thus overflow or underflow the containing block).

    In this case the dependency comes from flex-basis and not (max-)block-size, so 30px I guess? But what would happen with flex-basis: content; height: auto?

    My understanding is that no matter what height says in this case.

  8. davidsgrogan commented on Nov 21, 2021

    @davidsgrogan
    Member

    https://drafts.csswg.org/css-sizing-3/#cyclic-percentage-contribution

    This section only applies when calculating min-content and max-content contributions, though, right? And min-content and max-content contributions don't affect this issue, right?

  9. Loirooriol commented on Nov 22, 2021

    @Loirooriol
    Contributor

    @davidsgrogan The percentage is cyclic since the height of the containing block comes from flex-basis: content which is intrinsic. That CSS Sizing section is mostly about finding the intrinsic contribution, and there is interoperability that it should be 0px. But then the section also specifies if the percentage should be resolved with the final size of the containing block, or behave as auto. That's where we don't have interoperability.

  10. davidsgrogan commented on Dec 22, 2021

    @davidsgrogan
    Member

    I don't have strong feelings about this. I think it is in the spirit of CSS2 for the percentage height to resolve against auto to zero in this case, but I also think that authors are better served when percents resolve against something real.

    I'm interested in hearing from @dholbert @fantasai @aethanyc @tabatkins. If y'all think Webkit's behavior here is better, I'll change Blink to match.

  11. aethanyc commented on Dec 24, 2021

    @aethanyc
    Contributor

    I feel the spirit of CSS2 still applies here because flexbox's main size property (height in the testcase) has no control over things. A flex item's main size is controlled by the flex-basis. Here is the quote from main size property.

    In flex layout, the main size is controlled by the flex property rather than directly by the main size property.

    Also, flex-basis:content; height:100px is equivalent to height: max-content; per the definition of flex-basis:content.

    Re Option 2 (WebKit) in the first comment:

    It's unclear to me that we could consider that the height of the containing block depends on the content height for the purpouse of resolving the percentage. It does indeed depend on the content height to compute the flex basis, but I don't think we could conclude that after computing the flex basis we should still consider the size as indefinite.

    flex-basis determines the main size after flex algo 9.2.3. I don't feel it is reasonable to resolve the flex item's percentage height child again after the item's main size becomes definite, because it is just a cyclic percentage.

    Similarly, we don't want to resolve the inner height:100% after the outer block's height becomes definite, do we?

      <div style="display: block; height: max-content;">
        <div style="height: 30px; width: 100px; background: blue;"></div>
        <div style="height: 100%; width: 100px; background: red;"></div>
      </div>
    
  12. fantasai commented on Jan 26, 2022

    @fantasai
    Collaborator

    Fwiw, I agree with @davidsgrogan and @aethanyc's positions in the two comments above. I think it shouldn't resolve per CSS2; I can also see why we might want it to, and could be convinced otherwise if we agreed it helped authors. But generally cyclic percentage resolution is weird, and often results in overflow, so I'm not sure how helpful it is.
    Wrt

    If the cyclic dependency was introduced due to a block-size or max-block-size on the containing block

    I think the fact that it doesn't mention flex-basis is an oversight: we didn't consider such a case hitting this clause.

  13. css-meeting-bot commented on Jan 26, 2022

    @css-meeting-bot
    Member

    The CSS Working Group just discussed [css-flexbox] Percentage height resolution of children of flex items with indefinite flex basis, and agreed to the following:

    • RESOLVED: Reject the proposed change but clarify the specification; we invite Sergio to comment further if they have objections to this resolution
    The full IRC log of that discussion <emeyer> Topic: [css-flexbox] Percentage height resolution of children of flex items with indefinite flex basis
    <emeyer> github: https://github.com//issues/6822
    <emeyer> dgrogan: This is also an interop issue. Chrome recently switched from WebKit to Firefox behavior, which we think is what’s specced. Sergei from Igalia has a good argument for why WebKit might be better and should be specced.
    <emeyer> TabAtkins: Elika says she agrees with dgrogan that we should try not to introduce new exceptions unless there’s a strong reason and we can describe it clearly.
    <astearns> aethanyc = Ting-Yu Lin
    <emeyer> dgrogan: When you have a column flexbox and the item’s flex basis is content, and a child of that item has a percent height do we resolve it in the final pass or do we make the flex item’s height indefinite?
    <emeyer> …The spec currently roughly says it’s indefinite. Proposal from Igalia is to make the child of an item with content flex basis resolve its height.
    <emeyer> Rossen: When you say this, are you talking about the measurement of item in the last pass when you would have already computed all of the contributions of flex items?
    <emeyer> fantasai: And the child with the percent is a block level box.
    <emeyer> Rossen: I’m struggling to figure out why we would resolve the percent height, and mroe importantly how, if we don’t do it anywhere else.
    <emeyer> dgrogan: The debate is around when do we resolve that height. Are we on the same page for that?
    <emeyer> dgrogan: The argument is, we already have a bunch of definiteness exceptions, so why not do another?
    <emeyer> Rossen: In the contribution pass, you are not resolving percentages. Which will most likely give the item a height based on its content. When you go to the last pass, if you resolve the percentage of the block child, it will probably overflow or underflow. So do we allow that overflow or underflow to satisfy the percentages?
    <emeyer> dgrogan: That’s a good restatement.
    <emeyer> …I don’t know that I can defend this.
    <emeyer> oriol: I haven’t been following this in detail. I also felt like WebKit is doing something different than the proposal. I don]t have a strong opinion here.
    <emeyer> iank_: I believe WebKit has a bug that it’s resolving during the contribution phase. So looking at its output is a little complicated for teasing out the intended result.
    <emeyer> …I somewhat prefer the Firefox behavior currently. It’s aligned somewhat with other behaviors.
    <emeyer> fremy: If you have the exact same scenario, but the page size is bigger, does that mean the thing won’t get shrunk?
    <emeyer> dholbert: I think it would depend on other things, and I think uinrelated to this issue.
    <emeyer> Rossen: It’s hard to argue about later stages if the earlier stages are inconsistent.
    <emeyer> …I don’t know if we have good progress here. This should maybe go back to the issue. it sounds like the current spec [carrier lost]
    <emeyer> astearns: It sounds like we’re converging against the original proposal?
    <emeyer> iank_: I think Firefox and Blink are correct as per the spec’s definition of definiteness.
    <emeyer> dgrogan: Correct.
    <dholbert> (agreed)
    <emeyer> astearns: Is what’s in the spec at the moment sufficient or does it need to be more clear?
    <emeyer> dgrogan: I think there’s a part of the contribution spec that needs to be tightened up.
    <emeyer> fantasai: I agree, we need to clarification in the spec.
    <emeyer> RESOLVED: Reject the proposed change but clarify the specification; we invite Sergio to comment further if they have objections to this resolution
  14. fantasai commented on Jan 31, 2022

    @fantasai
    Collaborator

    OK, we've made edits in 82b9212; @svillar @Loirooriol Please let us know if those edits seem to adequately address the issue?

  15. Loirooriol commented on Aug 17, 2022

    @Loirooriol
    Contributor

    LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions