Skip to content

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

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

Closed
svillar opened this issue Nov 18, 2021 · 14 comments
Labels
Closed Accepted as Editorial Closed Rejected as Wontfix by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-flexbox-1 Current Work

Comments

@svillar
Copy link

svillar commented Nov 18, 2021

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
@svillar svillar 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 Nov 18, 2021
@Loirooriol
Copy link
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).

@svillar
Copy link
Author

svillar commented Nov 19, 2021

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.

@Loirooriol
Copy link
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?

@svillar
Copy link
Author

svillar commented Nov 19, 2021

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.

@Loirooriol
Copy link
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?

@svillar
Copy link
Author

svillar commented Nov 20, 2021

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.

@davidsgrogan
Copy link
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?

@Loirooriol
Copy link
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.

@bfgeek bfgeek added the css-flexbox-1 Current Work label Dec 21, 2021
@davidsgrogan
Copy link
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.

@aethanyc
Copy link

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>

@fantasai
Copy link
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.

@css-meeting-bot
Copy link
Member

css-meeting-bot commented Jan 26, 2022

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

@fantasai
Copy link
Collaborator

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

@Loirooriol
Copy link
Contributor

LGTM

@fantasai fantasai added Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. and removed Commenter Response Pending labels Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed Accepted as Editorial Closed Rejected as Wontfix by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-flexbox-1 Current Work
Projects
None yet
Development

No branches or pull requests

7 participants