overflow-clip-margin: Ignore margin if overflow:scroll/hidden.

This patch ensures that we overflow:scroll and overflow:hidden do not expand the bounds of the clip, even if we have contain: paint (which clips to overflow clip edge). R=ikilpatrick@chromium.org, chrishtr@chromium.org Fixed: 1252885 Change-Id: Ie52be3b9201a70e3de014f190cbd3ce00b170d9b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3330347 Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Commit-Queue: Vladimir Levin <vmpstr@chromium.org> Cr-Commit-Position: refs/heads/main@{#952433}
FairyWorld · Dec 16, 2021 · f0753f9 · f0753f9
1 parent 9df15b0
commit f0753f9
Show file tree

Hide file tree

Showing 9 changed files with 97 additions and 36 deletions.
diff --git a/third_party/blink/renderer/core/intersection_observer/intersection_geometry.cc b/third_party/blink/renderer/core/intersection_observer/intersection_geometry.cc
@@ -101,9 +101,10 @@ PhysicalRect InitializeRootRect(const LayoutObject* root,
 
 PhysicalRect GetBoxBounds(const LayoutBox* box, bool use_overflow_clip_edge) {
   PhysicalRect bounds(box->PhysicalBorderBoxRect());
-  // OverflowClipMargin() should only apply if clipping occurs on both axis.
-  if (use_overflow_clip_edge && box->ShouldClipOverflowAlongBothAxis() &&
-      box->StyleRef().OverflowClipMargin() != LayoutUnit()) {
+  // Only use overflow clip rect if we need to use overflow clip edge and
+  // overflow clip margin may have an effect, meaning we clip to the overflow
+  // clip edge and not something else.
+  if (use_overflow_clip_edge && box->ShouldApplyOverflowClipMargin()) {
     // OverflowClipRect() may be smaller than PhysicalBorderBoxRect().
     bounds.Unite(box->OverflowClipRect(PhysicalOffset()));
   }
@@ -124,8 +125,7 @@ std::pair<PhysicalRect, bool> InitializeTargetRect(const LayoutObject* target,
   } else if (target->IsBox()) {
     result.first =
         GetBoxBounds(To<LayoutBox>(target),
-                     (flags & IntersectionGeometry::kUseOverflowClipEdge) ==
-                         IntersectionGeometry::kUseOverflowClipEdge);
+                     flags & IntersectionGeometry::kUseOverflowClipEdge);
   } else if (target->IsLayoutInline()) {
     result.first = PhysicalRect::EnclosingRect(
         To<LayoutBoxModelObject>(target)->LocalBoundingBoxRectF());

diff --git a/third_party/blink/renderer/core/layout/layout_block.cc b/third_party/blink/renderer/core/layout/layout_block.cc
@@ -289,12 +289,9 @@ void LayoutBlock::UpdateFromStyle() {
   NOT_DESTROYED();
   LayoutBox::UpdateFromStyle();
 
-  // OverflowClipMargin() is only set if overflow is 'clip' along both axis, or
-  // 'contain: paint'. The later implies clipping along both axis.
-  bool should_clip_overflow =
-      (!StyleRef().IsOverflowVisibleAlongBothAxes() ||
-       StyleRef().OverflowClipMargin() != LayoutUnit()) &&
-      AllowsNonVisibleOverflow();
+  bool should_clip_overflow = (!StyleRef().IsOverflowVisibleAlongBothAxes() ||
+                               ShouldApplyPaintContainment()) &&
+                              AllowsNonVisibleOverflow();
   if (should_clip_overflow != HasNonVisibleOverflow()) {
     if (GetScrollableArea())
       GetScrollableArea()->InvalidateAllStickyConstraints();

diff --git a/third_party/blink/renderer/core/layout/layout_box.cc b/third_party/blink/renderer/core/layout/layout_box.cc
@@ -1966,9 +1966,7 @@ PhysicalRect LayoutBox::ClippingRect(const PhysicalOffset& location) const {
 
 void LayoutBox::ApplyVisibleOverflowToClipRect(PhysicalRect& clip_rect) const {
   const OverflowClipAxes overflow_clip = GetOverflowClipAxes();
-  if (overflow_clip == kOverflowClipBothAxis) {
-    clip_rect.Inflate(StyleRef().OverflowClipMargin());
-  } else {
+  if (overflow_clip != kOverflowClipBothAxis) {
     const LayoutRect infinite_rect(LayoutRect::InfiniteIntRect());
     if ((overflow_clip & kOverflowClipX) == kNoOverflowClip) {
       clip_rect.offset.left = infinite_rect.X();
@@ -1978,6 +1976,8 @@ void LayoutBox::ApplyVisibleOverflowToClipRect(PhysicalRect& clip_rect) const {
       clip_rect.offset.top = infinite_rect.Y();
       clip_rect.size.height = infinite_rect.Height();
     }
+  } else if (ShouldApplyOverflowClipMargin()) {
+    clip_rect.Inflate(StyleRef().OverflowClipMargin());
   }
 }
 
@@ -7716,19 +7716,17 @@ LayoutRect LayoutBox::LayoutOverflowRectForPropagation(
   LayoutRect rect = BorderBoxRect();
 
   if (!ShouldApplyLayoutContainment() &&
-      (!ShouldClipOverflowAlongBothAxis() ||
-       StyleRef().OverflowClipMargin() != LayoutUnit())) {
+      (!ShouldClipOverflowAlongBothAxis() || ShouldApplyOverflowClipMargin())) {
     LayoutRect overflow = LayoutOverflowRect();
     if (HasNonVisibleOverflow()) {
       const OverflowClipAxes overflow_clip_axes = GetOverflowClipAxes();
-      const LayoutUnit overflow_clip_margin = StyleRef().OverflowClipMargin();
       LayoutRect clip_rect = rect;
-      if (overflow_clip_margin != LayoutUnit()) {
-        // overflow_clip_margin should only be set if 'overflow' is 'clip' along
-        // both axis.
+      if (ShouldApplyOverflowClipMargin()) {
+        // We should apply overflow clip margin only if we clip overflow on both
+        // axes.
         DCHECK_EQ(overflow_clip_axes, kOverflowClipBothAxis);
         clip_rect.Contract(BorderBoxOutsets());
-        clip_rect.Inflate(overflow_clip_margin);
+        clip_rect.Inflate(StyleRef().OverflowClipMargin());
         overflow.Intersect(clip_rect);
       } else {
         ApplyOverflowClip(overflow_clip_axes, clip_rect, overflow);
@@ -7782,16 +7780,15 @@ LayoutRect LayoutBox::VisualOverflowRect() const {
     return self_visual_overflow_rect;
 
   const OverflowClipAxes overflow_clip_axes = GetOverflowClipAxes();
-  const LayoutUnit overflow_clip_margin = StyleRef().OverflowClipMargin();
-  if (overflow_clip_margin != LayoutUnit()) {
-    // overflow_clip_margin should only be set if 'overflow' is 'clip' along
-    // both axis.
+  if (ShouldApplyOverflowClipMargin()) {
+    // We should apply overflow clip margin only if we clip overflow on both
+    // axis.
     DCHECK_EQ(overflow_clip_axes, kOverflowClipBothAxis);
     const LayoutRect& contents_visual_overflow_rect =
         overflow_->visual_overflow->ContentsVisualOverflowRect();
     if (!contents_visual_overflow_rect.IsEmpty()) {
       LayoutRect result = BorderBoxRect();
-      result.Inflate(overflow_clip_margin);
+      result.Inflate(StyleRef().OverflowClipMargin());
       result.Intersect(contents_visual_overflow_rect);
       result.Unite(self_visual_overflow_rect);
       return result;

diff --git a/third_party/blink/renderer/core/layout/layout_object.h b/third_party/blink/renderer/core/layout/layout_object.h
@@ -600,6 +600,25 @@ class CORE_EXPORT LayoutObject : public GarbageCollected<LayoutObject>,
     return fragment_->UniqueId();
   }
 
+  inline bool ShouldApplyOverflowClipMargin() const {
+    NOT_DESTROYED();
+    // If the object is clipped by something other than overflow:clip (i.e. it's
+    // a scroll container), then we should not apply overflow-clip-margin.
+    if (IsScrollContainer())
+      return false;
+
+    const auto& style = StyleRef();
+    // Nothing to apply if there is no margin.
+    if (!style.OverflowClipMargin())
+      return false;
+
+    // In all other cases, we apply overflow-clip-margin when we clip to
+    // overflow clip edge, meaning we have overflow: clip or paint containment.
+    return (style.OverflowX() == EOverflow::kClip &&
+            style.OverflowY() == EOverflow::kClip) ||
+           ShouldApplyPaintContainment();
+  }
+
   inline bool IsEligibleForPaintOrLayoutContainment() const {
     NOT_DESTROYED();
     return (!IsInline() || IsAtomicInlineLevel()) && !IsRubyText() &&

diff --git a/third_party/blink/renderer/core/layout/ng/ng_layout_overflow_calculator.cc b/third_party/blink/renderer/core/layout/ng/ng_layout_overflow_calculator.cc
@@ -291,22 +291,21 @@ PhysicalRect NGLayoutOverflowCalculator::LayoutOverflowForPropagation(
       child_fragment.ShouldApplyLayoutContainment() ||
       child_fragment.IsInlineBox() ||
       (child_fragment.ShouldClipOverflowAlongBothAxis() &&
-       child_style.OverflowClipMargin() == LayoutUnit()) ||
+       !child_fragment.ShouldApplyOverflowClipMargin()) ||
       child_fragment.IsHiddenForPaint();
 
   if (!ignore_layout_overflow) {
     PhysicalRect child_overflow = child_fragment.LayoutOverflow();
     if (child_fragment.HasNonVisibleOverflow()) {
       const OverflowClipAxes overflow_clip_axes =
           child_fragment.GetOverflowClipAxes();
-      const LayoutUnit overflow_clip_margin = child_style.OverflowClipMargin();
-      if (overflow_clip_margin != LayoutUnit()) {
-        // overflow_clip_margin should only be set if 'overflow' is 'clip' along
-        // both axis.
+      if (child_fragment.ShouldApplyOverflowClipMargin()) {
+        // ShouldApplyOverflowClipMargin should only be true if we're clipping
+        // overflow in both axes.
         DCHECK_EQ(overflow_clip_axes, kOverflowClipBothAxis);
         PhysicalRect child_padding_rect({}, child_fragment.Size());
         child_padding_rect.Contract(child_fragment.Borders());
-        child_padding_rect.Inflate(overflow_clip_margin);
+        child_padding_rect.Inflate(child_style.OverflowClipMargin());
         child_overflow.Intersect(child_padding_rect);
       } else {
         if (overflow_clip_axes & kOverflowClipX) {

diff --git a/third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.cc b/third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.cc
@@ -713,8 +713,7 @@ PhysicalRect NGPhysicalBoxFragment::InkOverflow() const {
     return self_rect;
 
   const OverflowClipAxes overflow_clip_axes = GetOverflowClipAxes();
-  // overflow_clip_margin should only be set if 'overflow' is 'clip' along
-  // both axis.
+  // overflow_clip_margin should only be set if we clip both axes.
   DCHECK(overflow_clip_axes == kOverflowClipBothAxis ||
          !style.OverflowClipMargin());
   if (overflow_clip_axes == kNoOverflowClip) {
@@ -723,12 +722,12 @@ PhysicalRect NGPhysicalBoxFragment::InkOverflow() const {
   }
 
   if (overflow_clip_axes == kOverflowClipBothAxis) {
-    if (const LayoutUnit overflow_clip_margin = style.OverflowClipMargin()) {
+    if (ShouldApplyOverflowClipMargin()) {
       const PhysicalRect& contents_rect =
           ink_overflow_.Contents(InkOverflowType(), Size());
       if (!contents_rect.IsEmpty()) {
         PhysicalRect result = LocalRect();
-        result.Inflate(overflow_clip_margin);
+        result.Inflate(style.OverflowClipMargin());
         result.Intersect(contents_rect);
         result.Unite(self_rect);
         return result;

diff --git a/third_party/blink/renderer/core/layout/ng/ng_physical_fragment.h b/third_party/blink/renderer/core/layout/ng/ng_physical_fragment.h
@@ -354,6 +354,10 @@ class CORE_EXPORT NGPhysicalFragment
     return IsCSSBox() && layout_object_->ShouldClipOverflowAlongBothAxis();
   }
 
+  bool ShouldApplyOverflowClipMargin() const {
+    return IsCSSBox() && layout_object_->ShouldApplyOverflowClipMargin();
+  }
+
   // Return whether we can traverse this fragment and its children directly, for
   // painting, hit-testing and other layout read operations. If false is
   // returned, we need to traverse the layout object tree instead.

diff --git a/third_party/blink/web_tests/external/wpt/css/css-overflow/overflow-clip-margin-008-ref.html b/third_party/blink/web_tests/external/wpt/css/css-overflow/overflow-clip-margin-008-ref.html
@@ -0,0 +1,22 @@
+<!doctype html>
+<html class="reftest">
+<meta charset="utf-8">
+<title>Overflow-clip-margin has no effect with overflow:scroll and paint containment (ref)</title>
+<link rel="help" href="https://www.w3.org/TR/css-overflow-3/#propdef-overflow-clip-margin">
+<link rel="author" title="Vladimir Levin" href="mailto:vmpstr@chromium.org">
+<style>
+.container {
+  width: 100px;
+  height: 100px;
+  contain: paint;
+  overflow: scroll;
+}
+.child {
+  width: 200px;
+  height: 200px;
+  background: lightblue;
+}
+</style>
+<div class=container>
+  <div class=child></div>
+</div>
diff --git a/third_party/blink/web_tests/external/wpt/css/css-overflow/overflow-clip-margin-008.html b/third_party/blink/web_tests/external/wpt/css/css-overflow/overflow-clip-margin-008.html
@@ -0,0 +1,24 @@
+<!doctype html>
+<html class="reftest">
+<meta charset="utf-8">
+<title>Overflow-clip-margin has no effect with overflow:scroll and paint containment </title>
+<link rel="help" href="https://www.w3.org/TR/css-overflow-3/#propdef-overflow-clip-margin">
+<link rel="author" title="Vladimir Levin" href="mailto:vmpstr@chromium.org">
+<link rel="match" href="overflow-clip-margin-008-ref.html">
+<style>
+.container {
+  width: 100px;
+  height: 100px;
+  overflow-clip-margin: 20px;
+  contain: paint;
+  overflow: scroll;
+}
+.child {
+  width: 200px;
+  height: 200px;
+  background: lightblue;
+}
+</style>
+<div class=container>
+  <div class=child></div>
+</div>