Attachment 479638 Details for Bug 265243 – Patch

[patch] Patch

bug-265243-20260512103501.patch (text/plain), 8.77 KB, created by alan on 2026-05-12 10:35:01 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: alan

Created: 2026-05-12 10:35:01 PDT

Size: 8.77 KB

patch

obsolete

>From e088747a2195b136c98cf4cabcc0adcfe11a84ae Mon Sep 17 00:00:00 2001
>From: Alan Baradlay <zalan@apple.com>
>Date: Tue, 12 May 2026 10:18:37 -0700
>Subject: [PATCH] Flex item with aspect-ratio keeps stale height after
> container resize https://bugs.webkit.org/show_bug.cgi?id=265243
>
>Reviewed by NOBODY (OOPS!).
>
>  <div style="display: flex; width: 100%">
>    <div style="width: 100%; aspect-ratio: 1/1">
>      <img style="width: 100%; height: 100%">
>    </div>
>  </div>
>
>After resizing the container (e.g. viewport resize or script changing width),
>the wrapper div keeps its old height instead of updating to match the new width
>via aspect-ratio. A reload fixes it.
>
>The flex stretch algorithm caches each item's "intrinsic content height" so it
>can restore it after stretch relayout (since stretch changes the height and
>would contaminate the intrinsic value). The problem: for items with aspect-ratio,
>this cache was never invalidated - the guard in cacheIntrinsicContentLogicalHeightForFlexItem
>correctly prevented the normal layout path from caching (because the height
>depends on the width and changes on resize), but applyStretchAlignmentToFlexItem
>bypassed that guard and wrote to the cache directly. On subsequent layouts, the
>stale cached value was used to determine the flex line cross size, which then
>stretched the item back to the old height.
>
>The fix: extract shouldCacheIntrinsicContentLogicalHeightForFlexItem() so both
>code paths share the same caching policy. Items with aspect-ratio are excluded
>from caching because their content height is always derivable from their current
>width - no preservation needed.
>
>* Source/WebCore/rendering/RenderBox.cpp:
>(WebCore::RenderBox::shouldCacheIntrinsicContentLogicalHeightForFlexItem):
>(WebCore::RenderBox::cacheIntrinsicContentLogicalHeightForFlexItem):
>* Source/WebCore/rendering/RenderBox.h:
>* Source/WebCore/rendering/RenderFlexibleBox.cpp:
>(WebCore::RenderFlexibleBox::applyStretchAlignmentToFlexItem):
>* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001-expected.html: Added.
>* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001-ref.html: Added.
>* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001.html: Added.
>---
> Source/WebCore/rendering/RenderBox.cpp        | 20 +++++++----
> Source/WebCore/rendering/RenderBox.h          |  1 +
> .../WebCore/rendering/RenderFlexibleBox.cpp   |  3 +-
> ...flex-aspect-ratio-resize-001-expected.html | 11 +++++++
> .../flex-aspect-ratio-resize-001-ref.html     | 11 +++++++
> .../flex-aspect-ratio-resize-001.html         | 33 +++++++++++++++++++
> 6 files changed, 72 insertions(+), 7 deletions(-)
> create mode 100644 LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001-expected.html
> create mode 100644 LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001-ref.html
> create mode 100644 LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001.html
>
>diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp
>index 7b2309542556..cde1a98ed155 100644
>--- a/Source/WebCore/rendering/RenderBox.cpp
>+++ b/Source/WebCore/rendering/RenderBox.cpp
>@@ -3345,19 +3345,27 @@ static bool NODELETE shouldFlipBeforeAfterMargins(WritingMode containingBlockWri
>     return shouldFlip;
> }
> 
>+bool RenderBox::shouldCacheIntrinsicContentLogicalHeightForFlexItem() const
>+{
>+    // The flex stretch algorithm needs to know the item's intrinsic content height
>+    // before stretch was applied. We cache it so the stretch phase can read back
>+    // the pre-stretch value after relayout. Items with aspect-ratio don't need
>+    // caching because their content height is always derivable from the current width.
>+    return isFlexItem() && !isFloatingOrOutOfFlowPositioned() && !shouldComputeLogicalHeightFromAspectRatio();
>+}
>+
> void RenderBox::cacheIntrinsicContentLogicalHeightForFlexItem(LayoutUnit height) const
> {
>     // FIXME: it should be enough with checking hasOverridingLogicalHeight() as this logic could be shared
>     // by any layout system using overrides like grid or flex. However this causes a never ending sequence of calls
>     // between layoutBlock() <-> relayoutToAvoidWidows().
>-    if (isFloatingOrOutOfFlowPositioned())
>-        return;
>-    CheckedPtr flexibleBox = dynamicDowncast<RenderFlexibleBox>(parent());
>-    if (!flexibleBox)
>+    if (!shouldCacheIntrinsicContentLogicalHeightForFlexItem())
>         return;
>-    if (overridingBorderBoxLogicalHeight() || shouldComputeLogicalHeightFromAspectRatio())
>+    ASSERT(is<RenderFlexibleBox>(parent());
>+    if (overridingBorderBoxLogicalHeight())
>         return;
>-    flexibleBox->setCachedFlexItemIntrinsicContentLogicalHeight(*this, height);
>+    if (CheckedPtr flexibleBox = dynamicDowncast<RenderFlexibleBox>(parent());
>+        flexibleBox->setCachedFlexItemIntrinsicContentLogicalHeight(*this, height);
> }
> 
> void RenderBox::overrideLogicalHeightForSizeContainment()
>diff --git a/Source/WebCore/rendering/RenderBox.h b/Source/WebCore/rendering/RenderBox.h
>index ef9a4ece9163..a59be426b61f 100644
>--- a/Source/WebCore/rendering/RenderBox.h
>+++ b/Source/WebCore/rendering/RenderBox.h
>@@ -375,6 +375,7 @@ public:
> 
>     void overrideLogicalHeightForSizeContainment();
> 
>+    bool shouldCacheIntrinsicContentLogicalHeightForFlexItem() const;
>     void cacheIntrinsicContentLogicalHeightForFlexItem(LayoutUnit) const;
>     
>     // This function will compute the logical border-box height, without laying
>diff --git a/Source/WebCore/rendering/RenderFlexibleBox.cpp b/Source/WebCore/rendering/RenderFlexibleBox.cpp
>index fbcc6e27e7b9..50b3732565c8 100644
>--- a/Source/WebCore/rendering/RenderFlexibleBox.cpp
>+++ b/Source/WebCore/rendering/RenderFlexibleBox.cpp
>@@ -3020,7 +3020,8 @@ void RenderFlexibleBox::applyStretchAlignmentToFlexItem(RenderBox& flexItem, Lay
>             // Don't use layoutChildIfNeeded to avoid setting cross axis cached size twice.
>             flexItem.layoutIfNeeded();
> 
>-            setCachedFlexItemIntrinsicContentLogicalHeight(flexItem, flexItemIntrinsicContentLogicalHeight);
>+            if (flexItem.shouldCacheIntrinsicContentLogicalHeightForFlexItem())
>+                setCachedFlexItemIntrinsicContentLogicalHeight(flexItem, flexItemIntrinsicContentLogicalHeight);
>         }
>         return;
>     }
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001-expected.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001-expected.html
>new file mode 100644
>index 000000000000..6432a01427c5
>--- /dev/null
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001-expected.html
>@@ -0,0 +1,11 @@
>+<!DOCTYPE html>
>+<title>Reference</title>
>+<style>
>+.ref {
>+  width: 200px;
>+  height: 200px;
>+  background: green;
>+}
>+</style>
>+<p>Test passes if there is a 200x200 green square with no red.</p>
>+<div class="ref"></div>
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001-ref.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001-ref.html
>new file mode 100644
>index 000000000000..6432a01427c5
>--- /dev/null
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001-ref.html
>@@ -0,0 +1,11 @@
>+<!DOCTYPE html>
>+<title>Reference</title>
>+<style>
>+.ref {
>+  width: 200px;
>+  height: 200px;
>+  background: green;
>+}
>+</style>
>+<p>Test passes if there is a 200x200 green square with no red.</p>
>+<div class="ref"></div>
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001.html
>new file mode 100644
>index 000000000000..6d2e0155950f
>--- /dev/null
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-resize-001.html
>@@ -0,0 +1,33 @@
>+<!DOCTYPE html>
>+<title>Flex item with aspect-ratio updates height correctly on container resize</title>
>+<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#algo-stretch">
>+<link rel="stylesheet" type="text/css" href="/fonts/ahem.css">
>+<link rel="match" href="flex-aspect-ratio-resize-001-ref.html">
>+<style>
>+.container {
>+  display: flex;
>+  width: 500px;
>+}
>+
>+.wrapper {
>+  width: 100%;
>+  aspect-ratio: 1 / 1;
>+  background: green;
>+}
>+
>+.image {
>+  display: block;
>+  width: 100%;
>+  height: 100%;
>+}
>+</style>
>+<p>Test passes if there is a 200x200 green square with no red.</p>
>+<div id="container" class="container" style="background: red;">
>+  <div class="wrapper">
>+    <img src="../support/1x1-green.png" class="image">
>+  </div>
>+</div>
>+<script>
>+document.body.offsetHeight;
>+document.getElementById("container").style.width = "200px";
>+</script>

Actions: View | Formatted Diff | Diff

Attachments on bug 265243: 468724 | 479638 | 479639 | 479640