[css-grid][css-flexbox][quirks] Avoid percentage height quirk in new layout models

[Loirooriol]

Quirks mode has this percentage quirk which skips ancestors with height: auto when resolving height: <percentage>. Basically, it's resolved against the nearest ancestor in the containing block chain that doesn't have height: auto.

There are some exceptions: grid and flex containers are not skipped.

But we still have an interoperability problem: Firefox doesn't skip grid and flex items, while Chromium does. Testcase

<div style="display: flex; height: 100px; width: max-content; border: 5px solid blue">
  <div style="width: 100px; align-self: center; border: 3px solid lime">
    <div style="height: 100%; background: red"></div>
  </div>
</div>
<div style="display: grid; grid-template-rows: 100px; width: max-content; border: 5px solid blue">
  <div style="width: 100px; align-self: center; border: 3px solid lime">
    <div style="height: 100%; background: red"></div>
  </div>
</div>

I think Firefox is right and the quirk should be avoided for elements that participate in new layout models (like grid-level and flex-level boxes).

It's not just that quirks shouldn't infect new features. There is also the thing that stretched grid/flex items are considered to have a definite height, even if they have height: auto. So skipping them in that case doesn't even make sense. And skipping them depending on whether the height will end up being definite or not would just make it harder for implementations.

[dholbert]

For reference, I implemented Firefox's behavior on this scenario in https://bugzilla.mozilla.org/show_bug.cgi?id=1578586 .

[chrishtr]

@bfgeek

[Loirooriol]

BTW, I said "would just make it harder for implementations" because in Chromium I tried checking HasPercentHeightDescendants(), but it returned false if the grid item was skipped. Turns out that MaybeHasPercentHeightDescendant() did the trick. So this point may not hold, but I would still prefer to avoid the quirk in new layout models.

[bfgeek]

Due to flexbox and definiteness changing (with flexing/stretching) you need to perform very precise tracking of %-block-size descendants as an arbitrary descendant may trigger a relayout. E.g.
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=8709

[css-meeting-bot]

The CSS Working Group just discussed [css-grid][css-flexbox][quirks] Avoid percentage height quirk in new layout models, and agreed to the following:

• RESOLVED: dholbert to make a change to quirks spec to clarify behavior for flex and grid items and containers

The full IRC log of that discussion

<dael> Topic: [css-grid][css-flexbox][quirks] Avoid percentage height quirk in new layout models
<dael> github: https://github.com//issues/5545
<TabAtkins> The nice part of this is that it inverts the grouping from "all variants of a given property" to "all properties associated with a given variant", which can be much more readable in some circumstances.
<dael> oriol: This is about this % quirk. When in the block axis you have % and your containing block has auto height in quirks mode sizing skips the ancestor until it find another with a definite height and resolves % against this
<dael> oriol: For compat reasons must happen in flow layout. Quirks spec says shouldn't happen for grid and flex
<dael> oriol: I'm prop we should say quirk should not skip over grid and flex items
<dael> oriol: The current behavior chromium has where grid item with auto height and inside an element with % height it's resolved from grid area not grid item. If item is stretched the height is supposed to be definite. But knowing if we will stretch before is tricky and i think it's bad to prop this to new layout modes
<Rossen_> q?
<dael> oriol: FF does not skip grid and flexbox items. WK skips flex not grid. Chromium skips both. I think we should align with FF and never skip
<dael> TabAtkins: I'm very satisfied to limit quirks here if other impl are
<dholbert> I'm happy to align with the Firefox behavior. :)
<dael> iank_: I'm okay with this...it's a pretty late hour to do this change to flexbox in particular but also grid. Chromium can try but might have revert if there's web compat issues.
<dael> iank_: I think WK is closer to chromium then issue says and I think it exposes a WK bug. I'm okay with this but want to reserve the right to go back if there's compat issues
<dael> Rossen_: One thing to double check, you said quirk shouldn't apply to anything in grid and flex items. I didn't hear you say anything about grid and flex items themself
<dael> oriol: Grid and flex items have grid or flex container parent and spec says about that already so already not happening.
<dael> Rossen_: Means later subgrid as well. If we all agree with rule, which makes sense, but we should define anything in a grid boundary should not have to take the quirk
<iank_> q+
<dholbert> q+
<dael> oriol: Not sure I would go that far. A grid item can have varied structure. If you have nested block we could keep the quirk. Maybe breaking that on whole tree of grid item would be more problematic
<astearns> ack iank_
<dael> Rossen_: Having had to impl this quirk twice it's super nasty. Less we have to do it the better. If we can't get away it is what it is
<dael> iank_: I forgot to say a couple things. Would it be possible to instead of relying on quirks spec pull it into flexbox and grid specs? Quirks spec doesn't necessary spec correctly and leaves open to interpretation
<dael> iank_: Also, would have been nice if when FF did the change this was brought up
<dholbert> talking, can't hear me?
<astearns> ack dholbert
<dael> dholbert: Sorry, I don't htink I did enough testing of other browsers when impl. I was aligning to quirks spec, I think.
<dael> dholbert: To forbidding quirk inside grid I think best way to think is it tunnels up through blocks and if you hit an unsupporting container it just stops. If you think of flex and grid as an unsupporting container you have it. It's not that flex items are special but that flex and grid container are special
<dael> Rossen_: I believe this is the exact issue. If flex or grid item fits the bill and has a spec height you can use in quirks algo this item can later stretch with a lot of properties that effect fixed height.
<dael> Rossen_: Fully agree we should have a stronger rule here about flex and grid items to have them not partake in quirks.
<astearns> ack fantasai
<dael> Rossen_: I don't disagree with anything you say, but the less we can have the nasty quirk the better
<dael> fantasai: Note on edits, I don't think makes sense to have in flex and grid. It's really a special behavior for block and anything other then that should not have quick. Quirks should say these boxes do this thing and if you hit any other kind you terminate.
<dael> fantasai: In flexbox and grid we'd have to list a lot of exceptions.
<dael> astearns: I agree it should be defined in one place
<dael> fantasai: We don't have a block layout spec so we don't have a plce to put it
<dael> astearns: I was thinking flex and grid specs could have a note saying we're not doing this thing for the reason it defines
<dael> fantasai: I don't think there's a natural place to put that. Update the quirks mode spec, at some point it goes into a block spec
<dael> oriol: Quirks mode mentions block containers, but grid and flex can be block containers
<dael> fantasai: Whatever correct term is. Seems related to block layout and not about flex or grid. It's not an exception, it's block being special and that shouldn't have anything to do with flex and grid
<dael> Rossen_: sizing?
<astearns> s/grid and flex/grid and flex items/
<dael> fantasai: It's special for blocks.
<dael> Rossen_: We have a lot of sizing things that apply to not everything
<dael> TabAtkins: I don't mind sizing since it will apply to table sizing as well
<dael> astearns: Do we need a resolution to move the definition of the quirk to sizing and be explicit about how it does not apply to grid and flex items
<dael> fantasai: Need to say it's only blocks and tables.
<dael> astearns: Might be good to have as an example b/c has been impl confusion
<dael> plinss: Need to be careful about is this really a quirk and only in a compat mode or is this normal css behavior?
<dael> fantasai: It' snot normal.
<dael> plinss: Then it should stay in quirks and not be in a normal module in my opinion
<dael> astearns: Fair. We do need to firm up the definition
<dael> fantasai: dholbert do you want to propose a change to quirks spec?
<dael> dholbert: I can take a look at it.
<dael> dholbert: FF behavior falls out of how I understodd quirks spec. I'll take a look
<jensimmons> I do think we need something to help future implementors / anyone who doesn't remember this call to know what to do if they are implementing Flex/Grid/Future stuff. Something far off on the side is too off in the side. A note, as Alan suggested, or something would be good.
<dael> astearns: Prop: dholbert to make a change to quirks spec to clarify behavior in this case
<dael> astearns: And iank_ is okay with it being tried in chromium to check for compat
<dael> plinss: Is this only in quirks or is it certain conditions of normal css?
<dael> ??: Only quirks mode
<dael> astearns: Objections?
<dael> iank_: Test would be nice as well
<dael> astearns: Yes. Test would have shown this earlier, but thanks oriol for finding. Can you write tests?
<dael> oriol: Didn't hear
<dael> astearns: Will need tests to show we have interop behavior. Could you commit to adding tests?
<dael> oriol: Yeah
<dael> oriol: I think FF already has tests. I'll look better and write some if needed
<dael> astearns: Would be great to have in WPT
<dael> RESOLVED: dholbert to make a change to quirks spec to clarify behavior for flex and grid items and containers
<dael> astearns: Anything else?

[dholbert]

RESOLVED: dholbert to make a change to quirks spec to clarify behavior for flex and grid items and containers

Looking at the quirks spec-text more clearly, I agree that it does need a change in order to match the agreed-upon behavior.

I suspect we need to insert a step between steps 1 and 2 that says (hand-waving a bit): "if element has a computed value of the display property that is 'flex' or 'grid', then return the nearest ancestor containing block of the original element that you started with."

The algorithm may need some minor wordsmithing to make that fit in cleanly. I don't have cycles to focus on this edit right away, but I can do that at some point in the next week or so.

[sorry for briefly prematurely closing the issue; I accidentally had the "close with comment button" focused, when I happened to hit Enter while composing this comment]

[Loirooriol]

Isn't it simpler to replace "if element is a not a block container" with "if element is a not a block box nor an inline-block"?
This excludes flex-level and grid-level block containers.

BTW, not sure if a table caption is considered to be a block box or a table-level block container:

<div style="display: table; height:100px">
  <div style="display: table-caption">
    <div style="height: 100%; background: cyan">foobar

the inner <div> is 100px tall in Chromium and WebKit, but as tall as the text in Firefox.

[dholbert]

Isn't it simpler to replace "if element is a not a block container" with "if element is a not a block box nor an inline-block"?
This excludes flex-level and grid-level block containers.

I was originally thinking of something like that (amending step 4 of this quirk's algorithm), but that won't help, because the algorithm can terminate before that and "choose" the flex container as the percent basis in step 3, if the flex container has a definite height.

I think:
(a) we need the algorithm to bail out before step 3, if it has landed on a grid/flex container (otherwise, step 3 could end up choosing that grid/flex container as the quirky percent-basis, to use for the descendant in question).
(b) we need to specify what element gets used as the containing block in that case. (Right now the text says that you use the element that the algorithm returns, i.e. whatever ancestor you'd tunneled up to; but really, I think we want to use the nearest ancestor containing block for the percent-height element, or whatever we would've chosen in standards-mode. And that nearest ancestor presumably will be indefinite height, which means the percent is unresolvable and treated as indefinite, which is OK.)

[Loirooriol]

otherwise, step 3 could end up choosing that grid/flex container as the quirky percent-basis

But it would be chosen anyway at step 4 since it's not a block container? Isn't it fine? Why would we want to skip it if it has a definite height? Not sure if I get it, can you include some markup and which is the original element for which we are finding the containing block?

[dholbert]

Sorry, I think I misunderstood what you were suggesting -- you'd said "a block box", which I interpreted as another way of saying "a block container" / "a display:block box" (which I'd thought you were using as a way to exclude the flex container, when we get to the point in the algorithm where element is pointing at the flex container with e.g. your original testcase here).

If you meant "a block-level box", then I think your suggested change would potentially work, because the flexbox and grid specs do indeed say that their flex/grid items are not block-level boxes (they're flex/grid-level). So your suggested surgical change to the quirks-mode spec would effectively terminate the "digging up" algorithm as soon as element is a flex item (at step 4 of the loop-body), and it would then choose the flex item as the percent basis (even though it doesn't have a definite size).

I'm still not sure it's useful to choose the flex item (rather than the percent-height-element's nearest containing block) as the percent basis in this scenario; but maybe it doesn't matter which one we choose, since I think both are indefinite-height, as they must be if we end up terminating the algorithm in this way.

[Loirooriol]

With block box I meant "block-level block container", i.e. excluding flex/grid items (which are not block-level) and excluding flex/grid containers (which are not block containers). Both need to be excluded because by using position, it can happen that the flex/grid item isn't in the containing block chain, but the flex/grid container is.

[dholbert]

Thanks - that makes sense. I had forgotten that "block box" was a specially defined term.

[Loirooriol]

Ping @dholbert for the spec update.

[dholbert]

Yup, sorry for going quiet here. There's a closely-related bug report that I want to better-understand & close out before fixing this - https://bugzilla.mozilla.org/show_bug.cgi?id=1687557 . Hoping to take care of both soon.

[Loirooriol]

@dholbert Any updates?

[dholbert]

Sorry, only a little bit of an update; I have a patch posted on the bug that I mentioned (it was just an issue with one particular edge case), but the reviewer wants me to do a more thorough analysis of how this quirk applies with respect to orthogonal writing-modes (and how that matches/doesn't-match other browsers) before landing my patch, and I haven't had time for that yet. And that analysis probably needs to be done before I write any spec text here, too.

(See https://bugzilla.mozilla.org/show_bug.cgi?id=1687557#c9 and subsequent testcases/comments from Mats in particular. the tl;dr, as I recall, is: Firefox applies this "percentage height quirk" strictly for heights (as you might expect), and Firefox bails out if the quirk's "walk up the tree" traverses any element that has a vertical writing mode. Whereas, Chromium seems to apply the quirk in the block axis, regardless of writing mode; i.e. it's more of a "percentage block-size quirk", in Chromium.)

So: I need to do some more thorough investigation/test-crafting, and possibly raise some CSSWG discussion or file some Chrome bugs on the block-axis vertical-writing-mode stuff before I can usefully spec this (since that part was out of scope of the earlier csswg discussion/resolution). Unfortunately I've had other tasks occupying my attention and haven't been able to get to that yet. (Also, on a personal note: I'll be out for the next ~3 weeks to move house, so that will be occupying most of my attention for the extremely-immediate future.)

[dholbert]

Firefox applies this "percentage height quirk" strictly for heights (as you might expect), and Firefox bails out if the quirk's "walk up the tree" traverses any element that has a vertical writing mode. Whereas, Chromium seems to apply the quirk in the block axis, regardless of writing mode; i.e. it's more of a "percentage block-size quirk", in Chromium.)

For reference, I just filed https://bugzilla.mozilla.org/show_bug.cgi?id=1879957 on this behavior difference.

[css-meeting-bot]

The CSS Working Group just discussed [css-grid][css-flexbox][quirks] Avoid percentage height quirk in new layout models, and agreed to the following:

• RESOLVED: Add "block box" (block level block containers), "flex item" and "grid item" to the quirks spec

The full IRC log of that discussion

<emilio> oriol: we also discussed this in the past
<emilio> ... when in quirks mode percentage in block axis skip over ancestors with auto-heights
<emilio> ... this was required for all kind of boxes
<emilio> ... spec says not to do this for grid/flex containers but grid/flex items seem to be included
<emilio> ... impls do different things
<emilio> ... ff has no quirk for flex/grid items
<emilio> ... chromium has the quirk for both flex and grid containers
<emilio> ... webkit only has it for flex but not grid
<emilio> ... we resolved that dholbert would change the compat spec to change the container and item cases
<emilio> ... he investigated some cases he wasn't sure about
<emilio> dholbert: the most recent bit that caused it to be more complicated
<emilio> ... was that webkit / blink use the block axis rather than the vertical axis
<emilio> ... probably makes sense for ff to align with that
<emilio> ... but need some test-cases specially with mixed writing-modes
<emilio> ... the spirit of the FF behavior was that you stop at a flex/grid container
<emilio> ... if you hit an item then it's a block
<emilio> ... I don't remember to what extent we have more variability there
<emilio> oriol: reading in a comment that there's a condition that checks for "if an element is not a block container", we should substitute it by "block or inline-block container", to exclude grid/flex block-level containers
<oriol> https://quirks.spec.whatwg.org/#the-percentage-height-calculation-quirk
<emilio> oriol: so idea was to align with ff in not having the quirk for flex / grid items
<emilio> iank_: I think you need to mention grid / flex item in the compat spec for this to be correct
<emilio> dholbert: I think ff behavior is that we do allow flex / grid items
<emilio> ... if they happen to be blocks
<astearns> ack dbaron
<Zakim> dbaron, you wanted to support notion about what you traverse across
<emilio> oriol: per spec those are block containers but they are flex level, not block level
<emilio> iank_: let's be super-explicit
<emilio> dbaron: I support the idea of limiting the quirk in terms of what you're traversing through
<emilio> dholbert: one thing I'm not quite clear on whether we include flex items as termination points
<emilio> iank_: yes
<emilio> dholbert: then I don't think you need to mention them in the compat spec
<emilio> iank_: I think you do
<dbaron> https://quirks.spec.whatwg.org/#the-percentage-height-calculation-quirk
<emilio> dbaron: > If element has a computed value of the position property that is absolute, or if element is a not a block container or a table wrapper box, then return element.
<dbaron> 4. If element has a computed value of the position property that is absolute, or if element is a not a block container or a table wrapper box, then return element.
<emilio> dbaron: that needs to also say grid / flex item
<emilio> iank_: then you don't ever hit a flex / grid container
<emilio> oriol: we could just say block level box
<emilio> emilio: let's be extra-explicit
<emilio> RESOLVE: Add "block box" (block level block containers), "flex item" and "grid item" to the quirks spec
<astearns> RESOLVED: Add "block box" (block level block containers), "flex item" and "grid item" to the quirks spec
<emilio> dbaron: to be clear the new algo is applicable to new layout models that are also block containers
<emilio> ... since the existing spec already restricted others

[zcorpan]

PR: whatwg/quirks#76