Attachment 479608 Details for Bug 314554 – Patch

[patch] Patch

bug-314554-20260511080743.patch (text/plain), 12.22 KB, created by alan on 2026-05-11 08:07:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: alan

Created: 2026-05-11 08:07:44 PDT

Size: 12.22 KB

patch

obsolete

>From 18e11285d414e65cf60b5db80d39ab7fb62dadae Mon Sep 17 00:00:00 2001
>From: Alan Baradlay <zalan@apple.com>
>Date: Mon, 11 May 2026 07:57:26 -0700
>Subject: [PATCH] Percentage max-width on aspect-ratio box resolves against
> wrong axis in perpendicular writing mode
> https://bugs.webkit.org/show_bug.cgi?id=314554
>
>Reviewed by NOBODY (OOPS!).
>
>We had two functions:
>- computeLogicalWidthFromAspectRatioInternal: height x aspect-ratio, no clamping
>- computeLogicalWidthFromAspectRatio: calls Internal, then clamps with min/max
>
>The "Internal" naming was confusing - callers had to know which one to pick
>based on context. But the split is simple: preferred-width computation never
>wants clamping (min/max is applied later by a shared helper), layout always does.
>
>Remove the split. Keep one function (the unconstrained one) named
>computeLogicalWidthFromAspectRatio(). The two layout callsites now call
>constrainLogicalWidthByMinMax() inline, just like the normal width path already
>does. This also fixes an inconsistency (see test case): the old wrapper used
>containingBlockLogicalWidthForContent() to resolve percentage min/max, but the
>normal path in computeLogicalWidth uses a containerWidthInInlineDirection
>variable that accounts for perpendicular writing modes. The aspect-ratio path
>now uses the same variable.
>
>* Source/WebCore/rendering/RenderBlock.cpp:
>(WebCore::RenderBlock::computePreferredLogicalWidths):
>* Source/WebCore/rendering/RenderBox.cpp:
>(WebCore::RenderBox::computeLogicalWidth):
>(WebCore::RenderBox::computeIntrinsicKeywordLogicalWidths):
>(WebCore::RenderBox::computeOutOfFlowPositionedLogicalWidth):
>(WebCore::RenderBox::computeLogicalWidthFromAspectRatio):
>* Source/WebCore/rendering/RenderBox.h:
>* Source/WebCore/rendering/RenderReplaced.cpp:
>(WebCore::RenderReplaced::computePreferredLogicalWidths):
>* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008-expected.html: Added.
>* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008-ref.html: Added.
>* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008.html: Added.
>---
> Source/WebCore/rendering/RenderBlock.cpp      |  2 +-
> Source/WebCore/rendering/RenderBox.cpp        | 18 +++++--------
> Source/WebCore/rendering/RenderBox.h          |  1 -
> Source/WebCore/rendering/RenderReplaced.cpp   |  2 +-
> ...sic-percent-non-replaced-008-expected.html | 20 +++++++++++++++
> ...ntrinsic-percent-non-replaced-008-ref.html | 20 +++++++++++++++
> .../intrinsic-percent-non-replaced-008.html   | 25 +++++++++++++++++++
> 7 files changed, 73 insertions(+), 15 deletions(-)
> create mode 100644 LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008-expected.html
> create mode 100644 LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008-ref.html
> create mode 100644 LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008.html
>
>diff --git a/Source/WebCore/rendering/RenderBlock.cpp b/Source/WebCore/rendering/RenderBlock.cpp
>index 458988851a57..64eacd3c43bd 100644
>--- a/Source/WebCore/rendering/RenderBlock.cpp
>+++ b/Source/WebCore/rendering/RenderBlock.cpp
>@@ -2243,7 +2243,7 @@ void RenderBlock::computePreferredLogicalWidths()
>         computeIntrinsicLogicalWidths(m_minPreferredLogicalWidth, m_maxPreferredLogicalWidth);
>         m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth;
>     } else if (shouldComputeLogicalWidthFromAspectRatio()) {
>-        m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth = (computeLogicalWidthFromAspectRatioInternal() - borderAndPaddingLogicalWidth());
>+        m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth = (computeLogicalWidthFromAspectRatio() - borderAndPaddingLogicalWidth());
>         m_minPreferredLogicalWidth = std::max(0_lu, m_minPreferredLogicalWidth);
>         m_maxPreferredLogicalWidth = std::max(0_lu, m_maxPreferredLogicalWidth);
>     } else 
>diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp
>index cdab24f161d7..5ef9e6f72192 100644
>--- a/Source/WebCore/rendering/RenderBox.cpp
>+++ b/Source/WebCore/rendering/RenderBox.cpp
>@@ -2842,10 +2842,11 @@ void RenderBox::computeLogicalWidth(LogicalExtentComputedValues& computedValues)
>             return *overridingLogicalWidth;
>         if (treatAsReplaced)
>             return downcast<RenderReplaced>(*this).computeReplacedLogicalWidth() + borderAndPaddingLogicalWidth();
>-        if (shouldComputeLogicalWidthFromAspectRatio() && style().logicalWidth().isAuto())
>-            return computeLogicalWidthFromAspectRatio();
> 
>         auto containerWidthInInlineDirection = !hasPerpendicularContainingBlock ? containerLogicalWidth : perpendicularContainingBlockLogicalHeight();
>+        if (shouldComputeLogicalWidthFromAspectRatio() && style().logicalWidth().isAuto())
>+            return constrainLogicalWidthByMinMax(computeLogicalWidthFromAspectRatio(), containerWidthInInlineDirection, containingBlock, AllowIntrinsic::No);
>+
>         auto preferredWidth = computeLogicalWidthUsing(usedLogicalWidthLength, containerWidthInInlineDirection, containingBlock);
>         return constrainLogicalWidthByMinMax(preferredWidth, containerWidthInInlineDirection, containingBlock);
>     };
>@@ -2927,7 +2928,7 @@ template<typename Keyword> void RenderBox::computeIntrinsicKeywordLogicalWidths(
>         computeIntrinsicKeywordLogicalWidths(minLogicalWidth, maxLogicalWidth);
>     else {
>         if (shouldComputeLogicalWidthFromAspectRatio()) {
>-            minLogicalWidth = maxLogicalWidth = computeLogicalWidthFromAspectRatioInternal() - borderAndPadding;
>+            minLogicalWidth = maxLogicalWidth = computeLogicalWidthFromAspectRatio() - borderAndPadding;
>             if (firstChild() && style().logicalMinWidth().isAuto()) {
>                 LayoutUnit minChildrenLogicalWidth;
>                 LayoutUnit maxChildrenLogicalWidth;
>@@ -4375,7 +4376,7 @@ template<typename SizeType> LayoutUnit RenderBox::computeOutOfFlowPositionedLogi
>                 return 0_lu;
>             } else {
>                 if (shouldComputeLogicalWidthFromAspectRatio()) {
>-                    auto logicalWidth = computeLogicalWidthFromAspectRatio();
>+                    auto logicalWidth = constrainLogicalWidthByMinMax(computeLogicalWidthFromAspectRatio(), inlineConstraints.containingSize(), *containingBlock(), AllowIntrinsic::No);
>                     return style().boxSizingForAspectRatio() == BoxSizing::BorderBox ? logicalWidth : logicalWidth - inlineConstraints.bordersPlusPadding();
>                 }
>                 return fallback();
>@@ -5237,7 +5238,7 @@ bool RenderBox::shouldComputeLogicalWidthFromAspectRatio() const
>     return overridingBorderBoxLogicalHeight() || shouldComputeLogicalWidthFromAspectRatioAndInsets(*this) || style().logicalHeight().isFixed() || isResolvablePercentageHeight() || isResolveableStretchSize(style().logicalHeight());
> }
> 
>-LayoutUnit RenderBox::computeLogicalWidthFromAspectRatioInternal() const
>+LayoutUnit RenderBox::computeLogicalWidthFromAspectRatio() const
> {
>     ASSERT(preferredAspectRatio());
> 
>@@ -5246,13 +5247,6 @@ LayoutUnit RenderBox::computeLogicalWidthFromAspectRatioInternal() const
>     return inlineSizeFromAspectRatio(horizontalBorderAndPaddingExtent(), verticalBorderAndPaddingExtent(), preferredAspectRatio().value_or(0.f), style().boxSizingForAspectRatio(), logicalHeightforAspectRatio, style().aspectRatio(), isRenderReplaced());
> }
> 
>-LayoutUnit RenderBox::computeLogicalWidthFromAspectRatio() const
>-{
>-    auto logicalWidth = computeLogicalWidthFromAspectRatioInternal();
>-    LayoutUnit containerWidthInInlineDirection = std::max<LayoutUnit>(0, containingBlockLogicalWidthForContent());
>-    return constrainLogicalWidthByMinMax(logicalWidth, containerWidthInInlineDirection, *containingBlock(), AllowIntrinsic::No);
>-}
>-
> std::optional<double> RenderBox::preferredAspectRatio() const
> {
>     if (style().aspectRatio().hasRatio())
>diff --git a/Source/WebCore/rendering/RenderBox.h b/Source/WebCore/rendering/RenderBox.h
>index d1120256491a..b34b56ed9fa2 100644
>--- a/Source/WebCore/rendering/RenderBox.h
>+++ b/Source/WebCore/rendering/RenderBox.h
>@@ -690,7 +690,6 @@ protected:
>     bool NODELETE shouldIgnoreAspectRatio() const;
>     bool isResolveableStretchSize(const auto& size) const { return size.isStretch() && containingBlockHasDefiniteBlockSize(); }
>     bool isUnresolveableStretchSize(const auto& size) const { return size.isStretch() && !containingBlockHasDefiniteBlockSize(); }
>-    LayoutUnit computeLogicalWidthFromAspectRatioInternal() const;
>     LayoutUnit computeLogicalWidthFromAspectRatio() const;
>     void applyTransferredMinMaxSizesFromAspectRatio();
>     std::pair<LayoutUnit, LayoutUnit> computeMinMaxLogicalWidthFromAspectRatio() const;
>diff --git a/Source/WebCore/rendering/RenderReplaced.cpp b/Source/WebCore/rendering/RenderReplaced.cpp
>index e35b211be596..924616dc3059 100644
>--- a/Source/WebCore/rendering/RenderReplaced.cpp
>+++ b/Source/WebCore/rendering/RenderReplaced.cpp
>@@ -870,7 +870,7 @@ void RenderReplaced::computePreferredLogicalWidths()
>     // width may not be set on our containing block.
>     if (style().logicalWidth().isPercentOrCalculated()) {
>         if (canDerivePreferredWidthFromAspectRatio(*this)) {
>-            m_maxPreferredLogicalWidth = computeLogicalWidthFromAspectRatioInternal() - borderAndPaddingLogicalWidth();
>+            m_maxPreferredLogicalWidth = computeLogicalWidthFromAspectRatio() - borderAndPaddingLogicalWidth();
>             m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth;
>         } else {
>             computeIntrinsicLogicalWidths(m_minPreferredLogicalWidth, m_maxPreferredLogicalWidth);
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008-expected.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008-expected.html
>new file mode 100644
>index 000000000000..c735a6032c9c
>--- /dev/null
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008-expected.html
>@@ -0,0 +1,20 @@
>+<!DOCTYPE html>
>+<title>Reference</title>
>+<style>
>+.container {
>+  writing-mode: vertical-rl;
>+  width: 300px;
>+  height: 200px;
>+}
>+
>+.ref {
>+  writing-mode: horizontal-tb;
>+  width: 150px;
>+  height: 50px;
>+  background: green;
>+}
>+</style>
>+<p>Test passes if there is a 150x50 green rectangle.</p>
>+<div class="container">
>+  <div class="ref"></div>
>+</div>
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008-ref.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008-ref.html
>new file mode 100644
>index 000000000000..c735a6032c9c
>--- /dev/null
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008-ref.html
>@@ -0,0 +1,20 @@
>+<!DOCTYPE html>
>+<title>Reference</title>
>+<style>
>+.container {
>+  writing-mode: vertical-rl;
>+  width: 300px;
>+  height: 200px;
>+}
>+
>+.ref {
>+  writing-mode: horizontal-tb;
>+  width: 150px;
>+  height: 50px;
>+  background: green;
>+}
>+</style>
>+<p>Test passes if there is a 150x50 green rectangle.</p>
>+<div class="container">
>+  <div class="ref"></div>
>+</div>
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008.html
>new file mode 100644
>index 000000000000..47d7a3055dae
>--- /dev/null
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/intrinsic-percent-non-replaced-008.html
>@@ -0,0 +1,25 @@
>+<!DOCTYPE html>
>+<title>Percentage max-width with aspect-ratio resolves against correct axis in perpendicular writing mode</title>
>+<link rel="help" href="https://www.w3.org/TR/css-sizing-3/#cyclic-percentage-contribution">
>+<link rel="stylesheet" type="text/css" href="/fonts/ahem.css">
>+<link rel="match" href="intrinsic-percent-non-replaced-008-ref.html">
>+<style>
>+.container {
>+  writing-mode: vertical-rl;
>+  width: 300px;
>+  height: 200px;
>+}
>+
>+.child {
>+  writing-mode: horizontal-tb;
>+  aspect-ratio: 4 / 1;
>+  height: 50px;
>+  max-width: 50%;
>+  background: green;
>+  font: 20px/1 Ahem;
>+}
>+</style>
>+<p>Test passes if there is a 150x50 green rectangle.</p>
>+<div class="container">
>+  <div class="child"></div>
>+</div>

Actions: View | Formatted Diff | Diff

Attachments on bug 314554: 479608 | 479611 | 479630