Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-19 16:25:38 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1092007 part 2: Treat a flex item's main-size as indefinite if the item and the container both have an indefinite size in that axis. r=mats

Browse Source 
Flex containers resolve a main-axis size for each of their flex items, and they
impose this size on the flex items by stomping on the items'
ReflowInput::ComputedISize() or ComputedBSize() data.  For cases when we're
stomping on the flex item's block-axis, then this can make us improperly treat
this size as definite (i.e. use it for percent resolution).

The flexbox spec does call out some cases where the item's size *is* supposed
to be considered definite[1], but if we're not in one of those cases, we need
to be careful to treat the size as indefinite even though it's been resolved.

This patch achieves this by:
 - adding a flag to FlexItem, which gets set in cases where we know we need to
   treat the size as indefinite.
 - adding a flag to ReflowInput, which gets set whenever a ComputedBSize is
   imposed (if the FlexItem had its flag set).
 - adding some code to the ReflowInput percent-resolution codepath to test for
   this flag and skip past percent-resolution if it's set.

This patch makes us pass all of the existing testcases in the web-platform-test
"percentage-heights-003.html". This patch also adds a few more subtests
there to exercise cases where the flex container and item have orthogonal
writing-modes.

(Note the XXXdholbert comments in the patch about items being "fully
inflexible" in some cases in order to be treated as having a definite size.
If I were to address that comment here, then we would start failing the
web-platform-test "percentage-heights-005.html", which all browsers currently
pass[2]. Therefore, I'm not adding that restriction at this point.)

[1] https://drafts.csswg.org/css-flexbox/#definite-sizes
[2] https://wpt.fyi/results/css/css-flexbox/percentage-heights-005.html

Differential Revision: https://phabricator.services.mozilla.com/D43705

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Daniel Holbert 2019-09-04 22:45:08 +00:00
parent a347fd9495
commit 2933f10325
7 changed files with 147 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
layout/generic/ReflowInput.cpp
View File

@ -208,6 +208,7 @@ ReflowInput::ReflowInput(nsPresContext* aPresContext,
mFlags.mIsColumnBalancing = false;
mFlags.mColumnSetWrapperHasNoBSizeLeft = false;
mFlags.mIsFlexContainerMeasuringBSize = false;
mFlags.mTreatBSizeAsIndefinite = false;
mFlags.mDummyParentReflowInput = false;
mFlags.mShrinkWrap = !!(aFlags & COMPUTE_SIZE_SHRINK_WRAP);
mFlags.mUseAutoBSize = !!(aFlags & COMPUTE_SIZE_USE_AUTO_BSIZE);
@ -2091,6 +2092,10 @@ LogicalSize ReflowInput::ComputeContainingBlockRectangle(
WritingMode wm = aContainingBlockRI->GetWritingMode();
if (aContainingBlockRI->mFlags.mTreatBSizeAsIndefinite) {
cbSize.BSize(wm) = NS_UNCONSTRAINEDSIZE;
}
// mFrameType for abs-pos tables is NS_CSS_FRAME_TYPE_BLOCK, so we need to
// special case them here.
if (NS_FRAME_GET_TYPE(mFrameType) == NS_CSS_FRAME_TYPE_ABSOLUTE ||

9
layout/generic/ReflowInput.h
View File

@ -248,6 +248,15 @@ struct SizeComputationInput {
// BSize.
bool mIsFlexContainerMeasuringBSize : 1;
// If this flag is set, the BSize of this frame should be considered
// indefinite for the purposes of percent resolution on child frames (we
// should behave as if ComputedBSize() were NS_INTRINSIC_ISIZE when doing
// percent resolution against this.ComputedBSize()). For example: flex
// items may have their ComputedBSize() resolved ahead-of-time by their
// flex container, and yet their BSize might have to be considered
// indefinite per https://drafts.csswg.org/css-flexbox/#definite-sizes
bool mTreatBSizeAsIndefinite : 1;
// a "fake" reflow input made in order to be the parent of a real one
bool mDummyParentReflowInput : 1;

67
layout/generic/nsFlexContainerFrame.cpp
View File

@ -623,6 +623,8 @@ class nsFlexContainerFrame::FlexItem : public LinkedListElement<FlexItem> {
return mFlexShrink * mFlexBaseSize;
}
bool TreatBSizeAsIndefinite() const { return mTreatBSizeAsIndefinite; }
const AspectRatio& IntrinsicRatio() const { return mIntrinsicRatio; }
bool HasIntrinsicRatio() const { return !!mIntrinsicRatio; }
@ -891,6 +893,9 @@ class nsFlexContainerFrame::FlexItem : public LinkedListElement<FlexItem> {
// Does this item need to resolve a min-[width|height]:auto (in main-axis).
bool mNeedsMinSizeAutoResolution;
// Should we take care to treat this item's resolved BSize as indefinite?
bool mTreatBSizeAsIndefinite;
// Does this item have an auto margin in either main or cross axis?
bool mHasAnyAutoMargin;
@ -1942,6 +1947,57 @@ FlexItem::FlexItem(ReflowInput& aFlexItemReflowInput, float aFlexGrow,
mAlignSelf &= ~NS_STYLE_ALIGN_FLAG_BITS;
}
// Our main-size is considered definite if any of these are true:
// (a) main axis is the item's inline axis.
// (b) flex container has definite main size.
// (c) flex item has a definite flex basis and is fully inflexible
// (NOTE: We don't actually check "fully inflexible" because webcompat
// may not agree with that restriction...)
//
// Hence, we need to take care to treat the final main-size as *indefinite*
// if none of these conditions are satisfied.
if (mIsInlineAxisMainAxis) {
// The item's block-axis is the flex container's cross axis. We don't need
// any special handling to treat cross sizes as indefinite, because the
// cases where we stomp on the cross size with a definite value are all...
// - situations where the spec requires us to treat the cross size as
// definite; specifically, `align-self:stretch` whose cross size is
// definite.
// - situations where definiteness doesn't matter (e.g. for an element with
// an intrinsic aspect ratio, which for now are all leaf nodes and hence
// can't have any percent-height descendants that would care about the
// definiteness of its size. (Once bug 1528375 is fixed, we might need to
// be more careful about definite vs. indefinite sizing on flex items with
// aspect ratios.)
mTreatBSizeAsIndefinite = false;
} else {
// The item's block-axis is the flex container's main axis. So, the flex
// item's main size is its BSize, and is considered definite under certain
// conditions laid out for definite flex-item main-sizes in the spec.
if (aAxisTracker.IsRowOriented() ||
(containerRS->ComputedBSize() != NS_UNCONSTRAINEDSIZE &&
!containerRS->mFlags.mTreatBSizeAsIndefinite)) {
// The flex *container* has a definite main-size (either by being
// row-oriented [and using its own inline size which is by definition
// definite, or by being column-oriented and having a definite
// block-size). The spec says this means all of the flex items'
// post-flexing main sizes should *also* be treated as definite.
mTreatBSizeAsIndefinite = false;
} else if (aFlexBaseSize != NS_UNCONSTRAINEDSIZE) {
// The flex item has a definite flex basis, which we'll treat as making
// its main-size definite.
// XXXdholbert Technically the spec requires the flex item to *also* be
// fully inflexible in order to have its size treated as definite in this
// scenario, but no browser implements that additional restriction, so
// it's not clear that this additional requirement would be
// web-compatible...
mTreatBSizeAsIndefinite = false;
} else {
// Otherwise, we have to treat the item's BSize as indefinite.
mTreatBSizeAsIndefinite = true;
}
}
SetFlexBaseSizeAndMainSize(aFlexBaseSize);
CheckForMinSizeAuto(aFlexItemReflowInput, aAxisTracker);
@ -2010,6 +2066,7 @@ FlexItem::FlexItem(nsIFrame* aChildFrame, nscoord aCrossSize,
mIsStrut(true), // (this is the constructor for making struts, after all)
mIsInlineAxisMainAxis(true), // (doesn't matter, we're not doing layout)
mNeedsMinSizeAutoResolution(false),
mTreatBSizeAsIndefinite(false),
mHasAnyAutoMargin(false),
mAlignSelf(NS_STYLE_ALIGN_FLEX_START),
mAlignSelfFlags(0) {
@ -4738,6 +4795,9 @@ void nsFlexContainerFrame::DoFlexLayout(
childReflowInput.SetComputedISize(item->GetMainSize());
} else {
childReflowInput.SetComputedBSize(item->GetMainSize());
if (item->TreatBSizeAsIndefinite()) {
childReflowInput.mFlags.mTreatBSizeAsIndefinite = true;
}
}
}
@ -5145,6 +5205,9 @@ void nsFlexContainerFrame::ReflowFlexItem(
} else {
childReflowInput.SetComputedBSize(aItem.GetMainSize());
didOverrideComputedBSize = true;
if (aItem.TreatBSizeAsIndefinite()) {
childReflowInput.mFlags.mTreatBSizeAsIndefinite = true;
}
}
// Override reflow input's computed cross-size if either:
@ -5161,6 +5224,10 @@ void nsFlexContainerFrame::ReflowFlexItem(
childReflowInput.SetComputedISize(aItem.GetCrossSize());
didOverrideComputedISize = true;
} else {
// Note that in the above cases we don't need to worry about the BSize
// needing to be treated as indefinite, because this is for cases where
// the block size would always be considered definite (or where its
// definiteness would be irrelevant).
childReflowInput.SetComputedBSize(aItem.GetCrossSize());
didOverrideComputedBSize = true;
}

17
layout/reftests/bugs/1128354-1-ref.html
View File

@ -14,9 +14,22 @@
.flexInnerHoriz {
display: flex;
/* In this reference case, we make the inner elements be auto-height
* and aligned to the main-axis start side ("align-self:flex-start").
* That's how the corresponding piece of the testcase should render,
* given its "height: [unresolvable percent]" and implicit
* "align-self: stretch". */
align-self: flex-start;
background: pink;
}
.height50pct {
align-self: flex-start;
background: brown;
}
.height0pct {
align-self: flex-start;
background: yellow;
}
.spacer {
background: lightblue;
height: 200px;
@ -28,6 +41,8 @@
<div class="flexVert">
<div class="flexIntermediateHoriz">
<div class="flexInnerHoriz">text</div>
<div class="height50pct">fifty</div>
<div class="height0pct">zero</div>
<div class="spacer"></div>
</div>
</div>

23
layout/reftests/bugs/1128354-1.html
View File

@ -14,11 +14,26 @@
.flexInnerHoriz {
display: flex;
height: 100%; /* If you delete this line, then pink area is stretched
to have its height match blue area. */
/* This percent should not be resolvable, because our parent's
* height is indefinite (because our parent is a flex item with an
* indefinite flex basis, in an indefinite-main-sized flex container).
* So we just end up with our content height.
*/
height: 100%;
background: pink;
}
.height50pct {
/* This percent should not be resolvable, for the same reason as above.
*/
height: 50%;
background: brown;
}
.height0pct {
/* This percent should not be resolvable, for the same reason as above.
*/
height: 0%;
background: yellow;
}
.spacer {
background: lightblue;
height: 200px;
@ -30,6 +45,8 @@
<div class="flexVert">
<div class="flexIntermediateHoriz">
<div class="flexInnerHoriz">text</div>
<div class="height50pct">fifty</div>
<div class="height0pct">zero</div>
<div class="spacer"></div>
</div>
</div>

7
testing/web-platform/meta/css/css-flexbox/percentage-heights-003.html.ini
View File

@ -1,7 +0,0 @@
[percentage-heights-003.html]
[.flexbox 4]
expected: FAIL
[.flexbox 3]
expected: FAIL

30
testing/web-platform/tests/css/css-flexbox/percentage-heights-003.html
View File

@ -31,6 +31,12 @@
background: orange;
display: block;
}
.vert-wm {
writing-mode: vertical-rl;
}
.horiz-wm {
writing-mode: horizontal-tb;
}
</style>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
@ -82,5 +88,29 @@
</div>
</div>
<!-- indefinite unwrapped column flexbox, with orthogonal-flow flex item. The
flex item's main size (height) is definite, since it's the item's inline
size, and inline sizes always end up definite. -->
<div style="height: 100px;">
<div class="flexbox column">
<div class="vert-wm">
<span data-expected-height="100"></span>
</div>
</div>
</div>
<!-- indefinite unwrapped row-oriented vertical-writing-mode flexbox, with
orthogonal-flow (horizontal-writing-mode) flex item. The flex item's
height (main size) is definite, since its parent flex container has a
definite main size, because the flex container's main axis is its
inline-axis and inline sizes are definite. -->
<div style="height: 100px;">
<div class="flexbox vert-wm">
<div class="horiz-wm">
<span data-expected-height="100"></span>
</div>
</div>
</div>
</body>
</html>