commit 41d71b9a98260f9a612704e03ae1a31480fa4486
parent 779e85fffe11ef045e40c0448fe6ab25b71f1739
Author: Daniel Holbert <dholbert@cs.stanford.edu>
Date: Sun, 16 Nov 2025 16:32:17 +0000
Bug 1990034: Properly handle 'stretch'-like block-size values in nsIFrame::ComputeISizeValue and several functions that end up indirectly calling it. r=layout-reviewers,TYLin
We need to do this in order to avoid hitting nsIFrame::ComputeISizeValue's call
to 'AsLengthPercentage()' with 'stretch' (which is not a LengthPercentage);
that triggers an abort right now.
The new code in nsIFrame::ComputeISizeValue is the proximal band-aid here, but
that isn't sufficient for correctness, because it relies on knowing the
computed margin, border, and padding; and we might not yet have the correct
values stashed on the frame, as mentioned in a code-comment in this patch. We
do have the resolved margin/border/padding available up the stack in the
callsite in ReflowInput.cpp, though (the callsite in this bug's backtrace) --
so this patch eagerly resolves 'stretch' at that point, so that
nsIFrame::ComputeISizeValue doesn't need to bother handling it.
This patch also updates nsContainerFrame::ComputeSize to match the
corresponding chunk of nsIFrame::ComputeSize (just adding some code to handle
'stretch' there). This is needed to get the correct answer on a few of the
cases in the included WPT. (We had a code-comment noting that we should do
this, but we didn't previously have any tests where it would make a
difference. Now we do - hooray!)
Differential Revision: https://phabricator.services.mozilla.com/D272730
Diffstat:
4 files changed, 241 insertions(+), 16 deletions(-)
diff --git a/layout/generic/ReflowInput.cpp b/layout/generic/ReflowInput.cpp
@@ -357,21 +357,35 @@ nscoord SizeComputationInput::ComputeISizeValue(
const SizeOrMaxSize& aSize) const {
WritingMode wm = GetWritingMode();
const auto borderPadding = ComputedLogicalBorderPadding(wm);
+ const auto margin = ComputedLogicalMargin(wm);
const LogicalSize contentEdgeToBoxSizing =
aBoxSizing == StyleBoxSizing::Border ? borderPadding.Size(wm)
: LogicalSize(wm);
- const nscoord boxSizingToMarginEdgeISize =
- borderPadding.IStartEnd(wm) + ComputedLogicalMargin(wm).IStartEnd(wm) -
- contentEdgeToBoxSizing.ISize(wm);
+ const nscoord boxSizingToMarginEdgeISize = borderPadding.IStartEnd(wm) +
+ margin.IStartEnd(wm) -
+ contentEdgeToBoxSizing.ISize(wm);
+
+ // Get the bSize with anchor functions resolved, and manually resolve
+ // 'stretch'-like sizes as well (we should do this a bit cleaner,
+ // see bug 2000035):
+ auto bSize = mFrame->StylePosition()->BSize(
+ wm, AnchorPosResolutionParams::From(mFrame, mAnchorPosResolutionCache));
+ if (bSize->BehavesLikeStretchOnBlockAxis()) {
+ if (NS_UNCONSTRAINEDSIZE == aContainingBlockSize.BSize(wm)) {
+ bSize = AnchorResolvedSizeHelper::Auto();
+ } else {
+ nscoord stretchBSize = nsLayoutUtils::ComputeStretchBSize(
+ aContainingBlockSize.BSize(wm), margin.BStartEnd(wm),
+ borderPadding.BStartEnd(wm), aBoxSizing);
+ bSize = AnchorResolvedSizeHelper::LengthPercentage(
+ StyleLengthPercentage::FromAppUnits(stretchBSize));
+ }
+ }
return mFrame
->ComputeISizeValue(mRenderingContext, wm, aContainingBlockSize,
contentEdgeToBoxSizing, boxSizingToMarginEdgeISize,
- aSize,
- *mFrame->StylePosition()->BSize(
- wm, AnchorPosResolutionParams::From(
- mFrame, mAnchorPosResolutionCache)),
- mFrame->GetAspectRatio())
+ aSize, *bSize, mFrame->GetAspectRatio())
.mISize;
}
diff --git a/layout/generic/nsContainerFrame.cpp b/layout/generic/nsContainerFrame.cpp
@@ -1962,12 +1962,24 @@ LogicalSize nsContainerFrame::ComputeSizeWithIntrinsicDimensions(
? AnchorResolvedSizeHelper::Overridden(*aSizeOverrides.mStyleISize)
: stylePos->ISize(aWM, anchorResolutionParams);
- // TODO(dholbert): if styleBSize is 'stretch' here, we should probably
- // resolve it like we do in nsIFrame::ComputeSize. See bug 1937275.
- const auto styleBSize =
- aSizeOverrides.mStyleBSize
- ? AnchorResolvedSizeHelper::Overridden(*aSizeOverrides.mStyleBSize)
- : stylePos->BSize(aWM, anchorResolutionParams);
+ const auto styleBSize = [&] {
+ auto styleBSizeConsideringOverrides =
+ aSizeOverrides.mStyleBSize
+ ? AnchorResolvedSizeHelper::Overridden(*aSizeOverrides.mStyleBSize)
+ : stylePos->BSize(aWM, anchorResolutionParams);
+ if (styleBSizeConsideringOverrides->BehavesLikeStretchOnBlockAxis() &&
+ aCBSize.BSize(aWM) != NS_UNCONSTRAINEDSIZE) {
+ // We've got a 'stretch' BSize; resolve it to a length:
+ nscoord stretchBSize = nsLayoutUtils::ComputeStretchBSize(
+ aCBSize.BSize(aWM), aMargin.BSize(aWM), aBorderPadding.BSize(aWM),
+ stylePos->mBoxSizing);
+ // Note(dshin): This allocates.
+ return AnchorResolvedSizeHelper::LengthPercentage(
+ LengthPercentage::FromAppUnits(stretchBSize));
+ }
+ return styleBSizeConsideringOverrides;
+ }();
+
const auto& aspectRatio =
aSizeOverrides.mAspectRatio ? *aSizeOverrides.mAspectRatio : aAspectRatio;
diff --git a/layout/generic/nsIFrame.cpp b/layout/generic/nsIFrame.cpp
@@ -6604,7 +6604,7 @@ nsIFrame::SizeComputationResult nsIFrame::ComputeSize(
// isAutoBSize is true, or styleBSize is of type LengthPercentage()).
const auto styleBSize = [&] {
auto styleBSizeConsideringOverrides =
- (aSizeOverrides.mStyleBSize)
+ aSizeOverrides.mStyleBSize
? AnchorResolvedSizeHelper::Overridden(*aSizeOverrides.mStyleBSize)
: stylePos->BSize(aWM, anchorResolutionParams);
if (styleBSizeConsideringOverrides->BehavesLikeStretchOnBlockAxis() &&
@@ -7351,8 +7351,41 @@ nsIFrame::ISizeComputationResult nsIFrame::ComputeISizeValue(
if (nsLayoutUtils::IsAutoBSize(aStyleBSize, aCBSize.BSize(aWM))) {
return Nothing();
}
+
+ // Helper used below to resolve aStyleBSize if it's 'stretch' or an alias.
+ // XXXdholbert Really we should be resolving 'stretch' and its aliases
+ // sooner; see bug 2000035.
+ auto ResolveStretchBSize = [&]() {
+ MOZ_ASSERT(aStyleBSize.BehavesLikeStretchOnBlockAxis(),
+ "Only call me for 'stretch'-like BSizes");
+ MOZ_ASSERT(aCBSize.BSize(aWM) != NS_UNCONSTRAINEDSIZE,
+ "If aStyleBSize is stretch-like, then unconstrained "
+ "aCBSize.BSize should make us return via the IsAutoBSize "
+ "check above");
+
+ // NOTE: the borderPadding and margin variables might be zero-filled
+ // instead of having the true values, if those values haven't been
+ // stashed in our property-table yet (e.g. if we're in the midst of
+ // setting up a ReflowInput for our first reflow). So ideally, we should
+ // be resolving 'stretch' **in our callers** rather than here, if those
+ // callers have more up-to-date resolved margin/border/padding values.
+ // We'll still make a best-effort attempt to resolve 'stretch' here,
+ // though, for the benefit of callers that might not have handled it, to
+ // be sure we don't abort in aStyleBSize.AsLengthPercentage(). Ultimately
+ // this all can be removed when we fix bug 2000035.
+ const auto borderPadding = GetLogicalUsedBorderAndPadding(aWM);
+ const auto margin = GetLogicalUsedMargin(aWM);
+ nscoord stretchBSize = nsLayoutUtils::ComputeStretchBSize(
+ aCBSize.BSize(aWM), margin.BStartEnd(aWM),
+ borderPadding.BStartEnd(aWM), StylePosition()->mBoxSizing);
+ return LengthPercentage::FromAppUnits(stretchBSize);
+ };
+
return Some(ComputeISizeValueFromAspectRatio(
- aWM, aCBSize, aContentEdgeToBoxSizing, aStyleBSize.AsLengthPercentage(),
+ aWM, aCBSize, aContentEdgeToBoxSizing,
+ aStyleBSize.BehavesLikeStretchOnBlockAxis()
+ ? ResolveStretchBSize()
+ : aStyleBSize.AsLengthPercentage(),
aAspectRatio));
}();
diff --git a/testing/web-platform/tests/css/css-sizing/stretch/aspect-ratio-2.html b/testing/web-platform/tests/css/css-sizing/stretch/aspect-ratio-2.html
@@ -0,0 +1,166 @@
+<!DOCTYPE html>
+<link rel="help"
+ href="https://drafts.csswg.org/css-sizing-4/#stretch-fit-sizing">
+<script src='/resources/testharness.js'></script>
+<script src='/resources/testharnessreport.js'></script>
+<script src="/resources/check-layout-th.js"></script>
+<meta name="assert"
+ content="stretch keyword works as a block-size for replaced elements with intrinsic-size keyword for inline-size (which in turn depends on the stretched block-size)">
+<style>
+.container {
+ width: 20px;
+ height: 20px;
+ margin: 5px;
+ border: 1px solid black;
+}
+canvas {
+ height: -webkit-fill-available;
+ height: stretch;
+ background: purple;
+}
+
+.withBorder {
+ border: 2px solid cyan;
+ border-width: 2px 3px 4px 5px;
+}
+.withPadding {
+ padding: 2px 3px 4px 5px;
+}
+.withPxMargin {
+ margin: 1px 2px 3px 4px;
+}
+.withPctMargin {
+ /* This works out to 1px 2px 3px 4px */
+ margin: 5% 10% 15% 20%;
+}
+</style>
+<body onload="checkLayout('canvas')">
+ <!-- Simplest cases - replaced element with stretched block-size and
+ intrinsic inline-size: -->
+ <div class="container">
+ <canvas width="30" height="60"
+ data-expected-width="10"
+ data-expected-height="20"></canvas>
+ </div>
+ <div class="container">
+ <canvas width="30" height="60"
+ style="width: 1px; min-width: fit-content"
+ data-expected-width="10"
+ data-expected-height="20"></canvas>
+ </div>
+ <div class="container">
+ <canvas width="30" height="60"
+ style="width: 100px; max-width: fit-content"
+ data-expected-width="10"
+ data-expected-height="20"></canvas>
+ </div>
+
+ <!-- Now without any block-size on the containing block (this makes
+ the canvas's bsize be treated as "auto", so it just takes on its
+ regular intrinsic size in both axes): -->
+ <div class="container" style="height: auto">
+ <canvas width="30" height="60"
+ data-expected-width="30"
+ data-expected-height="60"></canvas>
+ </div>
+ <div class="container" style="height: auto">
+ <canvas width="30" height="60"
+ style="width: 1px; min-width: fit-content"
+ data-expected-width="30"
+ data-expected-height="60"></canvas>
+ </div>
+ <div class="container" style="height: auto">
+ <canvas width="30" height="60"
+ style="width: 100px; max-width: fit-content"
+ data-expected-width="30"
+ data-expected-height="60"></canvas>
+ </div>
+
+ <!-- Now with pixel-valued border: -->
+ <div class="container">
+ <canvas width="30" height="60"
+ class="withBorder"
+ data-expected-width="15"
+ data-expected-height="20"></canvas>
+ </div>
+ <div class="container">
+ <canvas width="30" height="60"
+ class="withBorder"
+ style="width: 1px; min-width: fit-content"
+ data-expected-width="15"
+ data-expected-height="20"></canvas>
+ </div>
+ <div class="container">
+ <canvas width="30" height="60"
+ class="withBorder"
+ style="width: 100px; max-width: fit-content"
+ data-expected-width="15"
+ data-expected-height="20"></canvas>
+ </div>
+
+ <!-- Now with pixel-valued padding: -->
+ <div class="container">
+ <canvas width="30" height="60"
+ class="withPadding"
+ data-expected-width="15"
+ data-expected-height="20"></canvas>
+ </div>
+ <div class="container">
+ <canvas width="30" height="60"
+ class="withPadding"
+ style="width: 1px; min-width: fit-content"
+ data-expected-width="15"
+ data-expected-height="20"></canvas>
+ </div>
+ <div class="container">
+ <canvas width="30" height="60"
+ class="withPadding"
+ style="width: 100px; max-width: fit-content"
+ data-expected-width="15"
+ data-expected-height="20"></canvas>
+ </div>
+
+ <!-- Now with a pixel-valued margin: -->
+ <div class="container">
+ <canvas width="30" height="60"
+ class="withPxMargin"
+ data-expected-width="8"
+ data-expected-height="16"></canvas>
+ </div>
+ <div class="container">
+ <canvas width="30" height="60"
+ class="withPxMargin"
+ style="width: 1px; min-width: fit-content"
+ data-expected-width="8"
+ data-expected-height="16"></canvas>
+ </div>
+ <div class="container">
+ <canvas width="30" height="60"
+ class="withPxMargin"
+ style="width: 100px; max-width: fit-content"
+ data-expected-width="8"
+ data-expected-height="16"></canvas>
+ </div>
+
+ <!-- Now with a percent-valued margin: -->
+ <div class="container">
+ <canvas width="30" height="60"
+ class="withPctMargin"
+ data-expected-width="8"
+ data-expected-height="16"></canvas>
+ </div>
+ <div class="container">
+ <canvas width="30" height="60"
+ class="withPctMargin"
+ style="width: 1px; min-width: fit-content"
+ data-expected-width="8"
+ data-expected-height="16"></canvas>
+ </div>
+ <div class="container">
+ <canvas width="30" height="60"
+ class="withPctMargin"
+ style="width: 100px; max-width: fit-content"
+ data-expected-width="8"
+ data-expected-height="16"></canvas>
+ </div>
+</body>