In a column flexbox, the height of a fully inflexible item with fixed flex-basis is definite, and percent children resolve against the flex basis, not the specified height. See: https://drafts.csswg.org/css-flexbox-1/#definite-sizes https://drafts.csswg.org/css-flexbox/#change-2016-inflexible-definite This is tested by WPT test http://wpt.live/css/css-flexbox/percentage-heights-006.html which fails on WebKit
Created attachment 426701 [details] Patch
Comment on attachment 426701 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426701&action=review > Source/WebCore/rendering/RenderFlexibleBox.cpp:1244 > +// This method is only called whenever a descendant of a flex item wants to resolve a percentage in its I’m not a huge fan of the word "method" to mean "member function". Can we just say "function" instead? > Source/WebCore/rendering/RenderFlexibleBox.cpp:1255 > + if (child.style().flexGrow() == 0.0 && child.style().flexShrink() == 0.0 && childMainSizeIsDefinite(child, flexBasisForChild(child))) Is this really needed only for the values of "exactly zero"? Are other very small values covered sufficiently by test cases? > Source/WebCore/rendering/RenderFlexibleBox.cpp:1261 > - auto updateDescendants = m_inLayout ? UpdatePercentageHeightDescendants::Yes : UpdatePercentageHeightDescendants::No; > - if (!canComputePercentageFlexBasis(child, Length(0, LengthType::Percent), updateDescendants)) > + if (!canComputePercentageFlexBasis(child, Length(0, LengthType::Percent), UpdatePercentageHeightDescendants::Yes)) What makes the m_inLayout check no longer necessary?
Comment on attachment 426701 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426701&action=review >> Source/WebCore/rendering/RenderFlexibleBox.cpp:1244 >> +// This method is only called whenever a descendant of a flex item wants to resolve a percentage in its > > I’m not a huge fan of the word "method" to mean "member function". Can we just say "function" instead? Sure >> Source/WebCore/rendering/RenderFlexibleBox.cpp:1255 >> + if (child.style().flexGrow() == 0.0 && child.style().flexShrink() == 0.0 && childMainSizeIsDefinite(child, flexBasisForChild(child))) > > Is this really needed only for the values of "exactly zero"? Are other very small values covered sufficiently by test cases? Yes the specs define "inflexible item" as an item whose flex-growth and flex-shrink are zero https://drafts.csswg.org/css-flexbox/#fully-inflexible The main doubt I had was whether to express this as !child.style().flexGrow() or as in the patch. Being a float I decided to use the other syntax but not strong opinion here. Regarding testing, using arbitrary low values are in practice the same as using arbitrary big ones in the sense that we'd end up distributing big chunks of space. There is one test checking that a huge value for shrink/grow does not crash, but apart from that I haven't seen others. >> Source/WebCore/rendering/RenderFlexibleBox.cpp:1261 >> + if (!canComputePercentageFlexBasis(child, Length(0, LengthType::Percent), UpdatePercentageHeightDescendants::Yes)) > > What makes the m_inLayout check no longer necessary? Because it was actually wrong. We do always want to update the hasPercentageHeightDescendants flag whenever this function is called. It used to work because in most of the cases this is called as part of layout.
Committed r276634 (237062@main): <https://commits.webkit.org/237062@main>
<rdar://problem/77201507>