Skip to content

[css-flexbox] should a definite flex-basis always make the main size be definite? #4311

Closed

Description

@cbiesinger

https://drafts.csswg.org/css-flexbox/#definite-sizes

Currently, a main size is only made definite if the flex container has a definite height.

However, browsers do not quite implement that. See https://crbug.com/1003506 for some discussion, but basically:

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

An element that would have a definite height outside of flex now suddenly has an indefinite height when put in a flexbox, because it's flexible (at least according to the spec, not in impls).

  • Should that change?
  • Would implementing this spec-compliant be web-compatible?

Also note that Chrome currently resolves percentages only if the main size property is definite; a definite flex-basis is not enough. That should probably be changed.

@dholbert @fantasai @tabatkins

Activity

  1. fantasai commented on Jul 15, 2021

    @fantasai
    Collaborator

    The main reason we restrict percentages from resolving in various indefinite circumstance is that, at least historically, it was hard to do. Afaict, generally authors prefer if percentages resolve. So if implementations are resolving in more cases than specced here... seems like we should just update the spec to match implementations?

    Agenda+

  2. css-meeting-bot commented on Jul 21, 2021

    @css-meeting-bot
    Member

    The CSS Working Group just discussed Does definite flex-basis always cause main size definite?.

    The full IRC log of that discussion <fantasai> Topic: Does definite flex-basis always cause main size definite?
    <fantasai> github: https://github.com//issues/4311
    <fantasai> TabAtkins: There's an example in the issue
    <fantasai> TabAtkins: Column flexbox with a flexible child
    <fantasai> TabAtkins: child has a definite height, but it is flexible so can become larger or smaller
    <fantasai> TabAtkins: Flex item has a child with a percentage height
    <TabAtkins> my phone hung up, one sec
    <TabAtkins> OKAY I CAN'T MAKE A CALL ANY MORE
    <TabAtkins> welp
    <fantasai> fantasai: Spec says this case is indefinite, and percentage won't resolve
    <fantasai> fantasai: But implementations do resolve
    <fantasai> fantasai: so should we change the spec to match implementations?
    <fantasai> iank_: I think all of the implementations are following the spec here
    <fantasai> iank_: in that they are not resolving ...
    <fantasai> cbiesinger: Not true
    <fantasai> cbiesinger: Testcase is red, which means it is resolving
    <fantasai> cbiesinger: Interesting thing here is that the percentage would resolve outside a flexbox
    <fantasai> cbiesinger: it doesn't resolve if you put it inside the flexbox (per spec)
    <fantasai> cbiesinger: I doubt authors expect that
    <fantasai> fantasai: Originaly definiteness was identifying cases where percentages are very simple to calculate
    <fantasai> fantasai: and we restricted percentage resolution to these cases
    <fantasai> fantasai: but authors would be happier to resolve more often
    <fantasai> fantasai: so if implementers are already doing it, might as well match the spec
    <fantasai> iank_: this seems fine to me
    <fantasai> fantasai: does mean that concept of definiteness is more complicated
    <fantasai> cbiesinger: Want to mention, in Chrome we only take into account the width/height property, not the flex-basis. But that's probably a Chrome bug
    <fantasai> Rossen_: Do we know if this is safe to change?
    <jensimmons> q+
    <fantasai> fantasai: Planning to match spec to implementations, so shouldn't have any concerns
    <dholbert> q+
    <Rossen_> ack jensimmons
    <fantasai> jensimmons: Seems folks doing a good job of raising flex bugs and addressing them
    <fantasai> jensimmons: Would love to ask if filing bugs against WebKit as well, seems our implementation is same as Chrome
    <fantasai> iank_: No bug required here, spec is aligning ot implementations
    <fantasai> jensimmons: on this issue, but not last one
    <fantasai> cbiesinger: should be a wpt test also
    <fantasai> Rossen_: WPT tests should raise to webkit
    <jensimmons> no
    <Rossen_> ack jensimmons
    <Rossen_> ack dholbert
    <fantasai> dholbert: Wanted to clarify what the proposed change is here. Are we making the spec match implementations in this case?
    <cbiesinger> I think change is: definite flex-basis makes the size definite, even if the item is flexible
    <fantasai> dholbert: I think that doesn't match what we do
    <fantasai> dholbert: It might be that this case is triggering a more subtle situation
    <fantasai> fantasai: That's interesting, we should investigate
    <fantasai> dholbert: I think intent of our behavior is to match the spec
    <fantasai> dholbert: We do check if flex item is flexible when testing for flexible flex-basis, to see if we want to make item definite
    <fantasai> cbiesinger: I feel like you and I had a discussion about this years ago and you were going to match our behavior
    <fantasai> dholbert: We fixed a number of bugs in last 6 months also
    <fantasai> dholbert: so maybe previously did something more esoteric
    <fantasai> fantasai, rossen: maybe need to go back to GH to figure out
    <Rossen_> q?
  3. dholbert commented on Jul 21, 2021

    @dholbert
    Member

    In the CSSWG discussion, I had misremembered that we implemented the "fully inflexible" requirement here, but it seems that in fact we tried but weren't able to do so because it's not web-compatible, as noted in these code-comments:
    https://searchfox.org/mozilla-central/rev/699174544b058f13f02e7586b3c8fdbf438f084b/layout/generic/nsFlexContainerFrame.cpp#2167-2173
    https://searchfox.org/mozilla-central/rev/699174544b058f13f02e7586b3c8fdbf438f084b/layout/generic/nsFlexContainerFrame.cpp#2130,2133-2135
    and this chromium issue:
    https://bugs.chromium.org/p/chromium/issues/detail?id=1003506#c7

    So I agree that we should just remove the "fully inflexible" restriction here, to match implementations & webcompat requirements (which are what caused this implementation behavior).

  4. dholbert commented on Jul 21, 2021

    @dholbert
    Member

    In the meeting, I think we were pretty much ready to resolve here, except for my pushback; and (now that I've refreshed my memory) I'm now withdrawing my pushback & am happy to resolve on removing the "fully-inflexible" requirement (and just considering the main-size to be definite for any flex item with a definite flex basis).

    One observation, though - when we update the spec text for this, we probably need to write the amended condition as actual authoritative spec-text rather than in a Note. (The spec-text under discussion here is currently a "Note" and seems to be written as if it's just highlighting something that is self-evident; but the new condition here won't really be self-evident anymore.)

  5. modified the milestones: APAC VF2F-2021-07-29, , EUR VF2F-2021-07-27 on Jul 24, 2021
  6. davidsgrogan commented on Jul 24, 2021

    @davidsgrogan
    Member

    This change is good.

    Compat will probably be ok, especially if Blink and Gecko sync up shipping this change.

    To be clear, this will be a change for Blink. Blink's existing behavior is intentional that the red block's percent height resolves in the original test case above, but that giving the item height:auto and flex-basis:100px would make it not resolve. This is intentional because my understanding of https://drafts.csswg.org/css-flexbox/#definite-sizes and the prose in https://drafts.csswg.org/css2/#the-height-property indicates this behavior complies with the current specifications.

    One observation, though - when we update the spec text for this, we probably need to write the amended condition as actual authoritative spec-text rather than in a Note.

    +1.
    I further contend that the Note even as written today is inaccurate. Removing the Note and specifying this new behavior as normative would be wonderful. I think it would also let us close issue 4305.

    Last thing. I'm a little worried about performance. The original testcase has min-height: 0. But most flex items on the web will have to rely on automatic minimum sizing like the case below. So in the below case Blink will lay out the item with an indefinite height to get its content size suggestion, which will be 0. Then Blink will lay out the item post-flexing with a definite height which will cause the subtree to need a second layout, doubling layout time. If this pattern is nested the slowdown gets exponential. Would this cause any potential perf regressions for Gecko?

    <div style="display: flex; flex-direction: column; width: 100px; background: yellow;">
      <div style="flex: 1 1 100px; background: green;">
        <div style="height: 100%; background: red;"></div>
      </div>
    </div>

    Anyway, I haven't dug deeper on this potential performance issue, so if no one else is worried about performance, we shouldn't block this change just because of that.

  7. dholbert commented on Jul 29, 2021

    @dholbert
    Member

    Compat will probably be ok, especially if Blink and Gecko sync up shipping this change.
    [...]
    Would this cause any potential perf regressions for Gecko?

    I believe Gecko is already shipping the proposed behavior -- if there's a definite flex-basis, then we treat that as making the resolved main size definite.

    To be clear, this will be a change for Blink. Blink's existing behavior is intentional that the red block's percent height resolves in the original test case above, but that giving the item height:auto and flex-basis:100px would make it not resolve.

    Interesting! That seems like an odd inconsistency which is nice that we can resolve here. :) (It seems quite weird to condition on the definiteness of the specified height in that example, since the specified height is otherwise ignored for the purpose of computing the actual height [aside from indirectly influencing the resolved automatic minimum height].)

    FWIW, Gecko treats the two scenarios the same (flex-basis:100px;height:auto and flex-basis:auto;height:100px), from the perspective of this issue -- we treat the flex item's final height as definite, since the flex basis is definite in both cases. (Per the previous spec text, we technically should have been also checking if it was fully-inflexible, but that requirement didn't seem to be web-compatible.)

  8. css-meeting-bot commented on Jul 29, 2021

    @css-meeting-bot
    Member

    The CSS Working Group just discussed should definite flex-basis make main size definite?, and agreed to the following:

    • RESOLVED: accept proposal and remove note
    The full IRC log of that discussion <fantasai> Topic: should definite flex-basis make main size definite?
    <fantasai> github: https://github.com//issues/4311
    <fantasai> dgrogan: Note in 9.8 ...
    <fantasai> TabAtkins: That note should not be adding any conditions ever
    <fantasai> dgrogan: Suggestion is to make this note normative and expanding cases where flex item can have a definite size
    <fantasai> dholbert: This is issue that ?? brought up last meeting
    <fantasai> dholbert: I wanted to check what Firefox did
    <fantasai> s/??/biesi/
    <fantasai> dholbert: I did follow up, and we do agree with the behavior that cbiesinger and dgrogan are proposing
    <fantasai> TabAtkins: this has nothing to do with the note
    <fantasai> TabAtkins: This issue is about a flexible flex item with a definite flex basis
    <fantasai> dgrogan: That note is inaccurate
    <fantasai> dgrogan: Definition of definite is you don't need to lay out to get its size
    <fantasai> dgrogan: but many cases here has auto min size
    <fantasai> dgrogan: If we go with this idea in the spec issue, can just get rid of the note
    <fantasai> dgrogan: I think we're all on board with the idea behind this
    <fantasai> dgrogan: do have question about a corner case
    <dgrogan> https://jsfiddle.net/dgrogan/4r9npf3z/3/
    <fantasai> dgrogan: the flex basis is 'content', but the height of ??? is definite
    <fantasai> dholbert: you're talking about 'height: definite'
    <fantasai> dholbert: In that case height property is ignored.
    <fantasai> dholbert: only affects flex item if 'flex-basis: auto'
    <fantasai> dgrogan: Child of the flex item with percentage
    <fantasai> dgrogan: resolves against containing block
    <fantasai> dgrogan: "percentage is calculated with respect to the height of the containing block. If not specified explicitly, computes to auto."
    <fantasai> dgrogan: doesn't trigger behaves as auto clause
    <fantasai> dgrogan: I suspect this is minor editorial oversight
    <fantasai> dholbert: This is just CSS2 believing that only 'height' property can affect height
    <fantasai> dholbert: whereas in flex layout, there's other factors
    <fantasai> astearns: So additional normative change that cbiesinger suggested, sounds like we're in agreement to spec
    <fantasai> TabAtkins: we will add 4th condition to definiteness of flex items, that definite flex basis always make correspondign axis of flex item definite
    <fantasai> TabAtkins: consequently can remove the note
    <fantasai> astearns: further comments?
    <fantasai> RESOLVED: accept proposal and remove note
    <fantasai> <br duration=12m>
  9. tabatkins commented on Aug 10, 2021

    @tabatkins
    Member

    All right, we've made a handful of edits here.

    First, we rephrased the definition of "main size" a little, mostly for clarity, but added a line that makes it clear that the post-flexing main size is used when any layout algo is asking for the "used width" or "used height" (as appropriate). Small detail, but tied into this topic:

    The main size of a flex container or flex item refers to its width or height, whichever is in the main dimension. Its main size property is either its width or height property, whichever is in the main dimension. Likewise, its min and max main size properties are its min-width/max-width or min-height/max-height properties, whichever are in the main dimension, and determine its min/max main size.
    In flex layout, the main size is controlled by the flex property rather than directly by the main size property.

    Note: This means any references to a flex item’s used size in the main dimension (width, height, inline size, block size) refers to its post-flexing main size.

    Then we addressed the main topic, ensuring that a definite flex basis is always treated as definite:

    • If the flex container has a definite main size, then the post-flexing main sizes of its flex items are treated as definite.

    • If a flex-item’s flex basis is definite, then its post-flexing main size is also definite.


    One side-issue that showed up in this thread was some minor confusion about applying CSS2 layout definitions, which explicitly reference the 'width' or 'height' properties' computed values for certain things, when for a flex item it's intended that you pay attention to the flex-basis instead. We're not sure how to actually fix this without going in and editting CSS2 in an invasive way and making it reference flexbox. :/ Hopefully it's okay without that clarification? (We can't just say "whenever another spec referenced the main size property, use the flex-basis instead", because things like inheritance do need to actually get the value of the 'width' or 'height' property...)

  10. tabatkins commented on Sep 10, 2021

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    TestedMemory aid - issue has WPT testscss-flexbox-1Current Work

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions