Bug 1436881: Remove redundant special-case code for treating flex-basis enum values as 'auto' in vertical axis. r?mats
This patch should not affect behavior.
Logic-wise: the idea behind this patch is to behave as if the
'usingFlexBasisForHeight' variable were always false, which in turn lets us
remove an "if (!usingFlexBasisForHeight || ...)" check, because it trivially
passes when that bool is false.
Background on this special case & why we can remove it:
=======================================================
In the original flexbox implementation, we had some special-case code to be
sure we didn't end up swapping in e.g. "flex-basis:-moz-min-content" for
"height" in these ComputeSize functions, because that was a scenario that
previously would've been prevented at the parser level (height:-moz-min-content
is rejected for now), and hence may not have ended up being handled robustly.
However, nowadays it'll be handled just fine in these functions, and will
produce the same result as our special-case exception tries to achieve.
In particular, for a nsFrame that is a flex item in a flex container with a
vertical main axis (the scenario that these special cases are catching):
- If the (vertical) main axis is this nsFrame's inline axis (i.e. if this
nsFrame has a vertical writing-mode), then Stylo actually converts
enumerated flex-basis values like "-moz-min-content" to "auto" at the parser
level. So it's not actually possible to get a computed "flex-basis" of
-moz-min-content, in that scenario.
- Otherwise, i.e. if the (vertical) main axis is this nsFrame's block axis,
then these ComputeSize functions will now end up getting an enumerated
"blockStyleCoord" (really pointing to flexBasis), but that'll still end up
being treated like 'auto'. This happens by virtue of ComputeSize's calls to
ComputeAutoSize (which initializes the tentative bsize value to
NS_UNCONSTRAINEDSIZE) and to nsLayoutUtils::IsAutoBSize (which returns
"true" for eStyleUnit_Enumerated values and then makes us leave the
ComputeAutoSize result unperturbed).
MozReview-Commit-ID: CU1pFwSoBQ4
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -5639,31 +5639,19 @@ nsFrame::ComputeSize(gfxContext*
usingFlexBasisForISize =
(flexContainerIsRowOriented == inlineAxisSameAsParent);
// NOTE: The logic here should match the similar chunk for determining
// inlineStyleCoord and blockStyleCoord in
// nsFrame::ComputeSizeWithIntrinsicDimensions().
const nsStyleCoord* flexBasis = &(stylePos->mFlexBasis);
if (flexBasis->GetUnit() != eStyleUnit_Auto) {
- // One caveat for when flex-basis is stomping on 'height': We don't
- // support enumerated values (e.g. "max-content") for height yet (that's
- // bug 567039). So, if our computed flex-basis is an enumerated value,
- // we'll just behave as if it were "auto", which means "use the main-size
- // property after all" (which is "height", in this case). NOTE: Once we
- // support intrinsic sizing keywords for "height", we should remove this
- // check.
- bool usingFlexBasisForHeight =
- (usingFlexBasisForISize != aWM.IsVertical());
- if (!usingFlexBasisForHeight ||
- flexBasis->GetUnit() != eStyleUnit_Enumerated) {
- // Override whichever coord we're overriding:
- (usingFlexBasisForISize ? inlineStyleCoord : blockStyleCoord) =
- flexBasis;
- }
+ // Override whichever styleCoord is in flex container's main axis:
+ (usingFlexBasisForISize ? inlineStyleCoord : blockStyleCoord) =
+ flexBasis;
}
}
// Compute inline-axis size
if (inlineStyleCoord->GetUnit() != eStyleUnit_Auto) {
result.ISize(aWM) =
ComputeISizeValue(aRenderingContext, aCBSize.ISize(aWM),
boxSizingAdjust.ISize(aWM), boxSizingToMarginEdgeISize,
@@ -5902,31 +5890,19 @@ nsFrame::ComputeSizeWithIntrinsicDimensi
// Flex items use their "flex-basis" property in place of their main-size
// property (e.g. "width") for sizing purposes, *unless* they have
// "flex-basis:auto", in which case they use their main-size property
// after all.
// NOTE: The logic here should match the similar chunk for determining
// inlineStyleCoord and blockStyleCoord in nsFrame::ComputeSize().
const nsStyleCoord* flexBasis = &(stylePos->mFlexBasis);
if (flexBasis->GetUnit() != eStyleUnit_Auto) {
- // One caveat for when flex-basis is stomping on 'height': We don't
- // support enumerated values (e.g. "max-content") for height yet
- // (that's bug 567039). So, if our computed flex-basis is an enumerated
- // value, we'll just behave as if it were "auto", which means "use the
- // main-size property after all" (which is "height", in this case).
- // NOTE: Once we support intrinsic sizing keywords for "height", we
- // should remove this check.
- bool usingFlexBasisForHeight =
- (usingFlexBasisForISize != aWM.IsVertical());
- if (!usingFlexBasisForHeight ||
- flexBasis->GetUnit() != eStyleUnit_Enumerated) {
- // Override whichever coord we're overriding:
- (usingFlexBasisForISize ? inlineStyleCoord : blockStyleCoord) =
- flexBasis;
- }
+ // Override whichever styleCoord is in flex container's main axis:
+ (usingFlexBasisForISize ? inlineStyleCoord : blockStyleCoord) =
+ flexBasis;
}
}
}
// Handle intrinsic sizes and their interaction with
// {min-,max-,}{width,height} according to the rules in
// http://www.w3.org/TR/CSS21/visudet.html#min-max-widths