Cannot rely on auto-positioned absolutely positioned descendants being marked for layout

alanbaradlay · alanbaradlay · commit cad87d87fcb0 · 2026-01-07T12:21:20.000-08:00
https://bugs.webkit.org/show_bug.cgi?id=276350 <rdar://problem/131806062> Reviewed by Antti Koivisto. This is a relatively inexpensive way to determine whether a statically positioned (in the inline direction) out-of-flow box needs its position adjusted due to parent border box move (e.g. margin changes). (ideally, the parent would signal to this out-of-flow box that a positional adjustment is required, which would then be picked up here during the out-of-flow box's layout) Test: fast/block/positioning/abspos-auto-left-fixed-top-change-parent-margin-left.html * LayoutTests/fast/block/positioning/abspos-auto-left-fixed-top-change-parent-margin-left-expected.html: Added. * LayoutTests/fast/block/positioning/abspos-auto-left-fixed-top-change-parent-margin-left.html: Added. * Source/WebCore/rendering/PositionedLayoutConstraints.h: * Source/WebCore/rendering/RenderBlock.cpp: (WebCore::RenderBlock::layoutOutOfFlowBox): Canonical link: https://commits.webkit.org/305229@main
diff --git a/LayoutTests/fast/block/positioning/abspos-auto-left-fixed-top-change-parent-margin-left-expected.html b/LayoutTests/fast/block/positioning/abspos-auto-left-fixed-top-change-parent-margin-left-expected.html
@@ -0,0 +1,3 @@
+<!DOCTYPE html>
+<p>There should be no red on this page.</p>
+<div style="margin-left:200px; width:200px; height:200px; background:green;"></div>
diff --git a/LayoutTests/fast/block/positioning/abspos-auto-left-fixed-top-change-parent-margin-left.html b/LayoutTests/fast/block/positioning/abspos-auto-left-fixed-top-change-parent-margin-left.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<p>There should be no red on this page.</p>
+<div style="position:relative;">
+    <div id="elm" style="width:200px; height:200px; background:red;">
+        <div style="position:absolute; top:0; width:200px; height:200px; background:green;"></div>
+    </div>
+</div>
+<script>
+    document.body.offsetTop;
+    document.getElementById("elm").style.marginLeft = "200px";
+</script>
diff --git a/Source/WebCore/rendering/PositionedLayoutConstraints.h b/Source/WebCore/rendering/PositionedLayoutConstraints.h
@@ -91,6 +91,8 @@ class PositionedLayoutConstraints {
     void fixupLogicalLeftPosition(RenderBox::LogicalExtentComputedValues&) const;
     void adjustLogicalTopWithLogicalHeightIfNeeded(RenderBox::LogicalExtentComputedValues&) const;
 
+    LayoutUnit computedInlineStaticDistance() const;
+
 private:
     bool containingCoordsAreFlipped() const;
 
@@ -104,7 +106,6 @@ class PositionedLayoutConstraints {
     bool needsGridAreaAdjustmentBeforeStaticPositioning() const;
     std::optional<LayoutUnit> remainingSpaceForStaticAlignment(LayoutUnit itemSize) const;
     void computeStaticPosition();
-    LayoutUnit computedInlineStaticDistance() const;
     LayoutUnit computedBlockStaticDistance() const;
 
     CheckedRef<const RenderBox> m_renderer;
diff --git a/Source/WebCore/rendering/RenderBlock.cpp b/Source/WebCore/rendering/RenderBlock.cpp
@@ -815,11 +815,25 @@ void RenderBlock::layoutOutOfFlowBox(RenderBox& outOfFlowBox, RelayoutChildren r
         return;
     }
 
+    auto& outOfFlowBoxStyle = outOfFlowBox.style();
     // When a non-positioned block element moves, it may have positioned children that are implicitly positioned relative to the
     // non-positioned block.  Rather than trying to detect all of these movement cases, we just always lay out positioned
     // objects that are positioned implicitly like this.  Such objects are rare, and so in typical DHTML menu usage (where everything is
     // positioned explicitly) this should not incur a performance penalty.
-    if (relayoutChildren == RelayoutChildren::Yes || (outOfFlowBox.style().hasStaticBlockPosition(isHorizontalWritingMode()) && outOfFlowBox.parent() != this))
+    auto needsLayout = [&] {
+        if (relayoutChildren == RelayoutChildren::Yes)
+            return true;
+        if (outOfFlowBox.parent() == this)
+            return false;
+        if (outOfFlowBoxStyle.hasStaticBlockPosition(isHorizontalWritingMode()))
+            return true;
+        if (outOfFlowBoxStyle.hasStaticInlinePosition(isHorizontalWritingMode())) {
+            // FIXME: We could just set the logical left on this out-of-flow box since the box itself does not really need layout (we are just moving an out-of-flow renderer here).
+            return outOfFlowBox.logicalLeft() != PositionedLayoutConstraints { outOfFlowBox, LogicalBoxAxis::Inline }.computedInlineStaticDistance();
+        }
+        return false;
+    };
+    if (needsLayout())
         outOfFlowBox.setChildNeedsLayout(MarkOnlyThis);
 
     // If relayoutChildren is set and the child has percentage padding or an embedded content box, we also need to invalidate the childs pref widths.
@@ -831,7 +845,7 @@ void RenderBlock::layoutOutOfFlowBox(RenderBox& outOfFlowBox, RelayoutChildren r
     // We don't have to do a full layout.  We just have to update our position. Try that first. If we have shrink-to-fit width
     // and we hit the available width constraint, the layoutIfNeeded() will catch it and do a full layout.
     if (outOfFlowBox.needsOutOfFlowMovementLayoutOnly() && outOfFlowBox.tryLayoutDoingOutOfFlowMovementOnly()) {
-        if (Style::AnchorPositionEvaluator::isAnchorPositioned(outOfFlowBox.style()))
+        if (Style::AnchorPositionEvaluator::isAnchorPositioned(outOfFlowBoxStyle))
             Style::AnchorPositionEvaluator::captureScrollSnapshots(outOfFlowBox);
         outOfFlowBox.clearNeedsLayout();
     }

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+<!DOCTYPE html>`
	`2`	`+<p>There should be no red on this page.</p>`
	`3`	`+<div style="margin-left:200px; width:200px; height:200px; background:green;"></div>`