Bug 1465250 - Make contain:paint trigger clipping independent of the overflow property. r=mattwoodrow draft
authorYusuf Sermet <ysermet@mozilla.com>
Wed, 30 May 2018 14:28:53 -0700
changeset 805456 3c73973bd6373889ce1d147e348e584036782d1d
parent 800800 ec6bd5d3de4635acd2e2d0dcd2222232524d9459
child 805467 c164fffa15d20eb9765e59129aee02f550f8f557
child 805483 42dcbf6f8ccbf6f4b76ebbfb0a408952be89345a
child 805559 6772555b53426dbba675b27589067992d7f75643
push id112664
push userbmo:ysermet@mozilla.com
push dateThu, 07 Jun 2018 20:37:41 +0000
reviewersmattwoodrow
bugs1465250
milestone62.0a1
Bug 1465250 - Make contain:paint trigger clipping independent of the overflow property. r=mattwoodrow MozReview-Commit-ID: 2QbfZD1jnWX
layout/generic/nsFrame.cpp
layout/generic/nsFrame.h
layout/generic/nsIFrame.h
layout/reftests/w3c-css/submitted/contain/contain-paint-clip-001.html
layout/reftests/w3c-css/submitted/contain/contain-paint-clip-002-ref.html
layout/reftests/w3c-css/submitted/contain/contain-paint-clip-002.html
layout/reftests/w3c-css/submitted/contain/contain-paint-clip-006-ref.html
layout/reftests/w3c-css/submitted/contain/contain-paint-clip-006.html
layout/reftests/w3c-css/submitted/contain/reftest.list
layout/style/test/mochitest.ini
layout/style/test/test_contain_formatting_context.html
servo/components/style/style_adjuster.rs
testing/web-platform/meta/css/css-contain/contain-paint-002.html.ini
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -1813,47 +1813,41 @@ nsIFrame::GetBorderRadii(nscoord aRadii[
 {
   nsSize sz = GetSize();
   return GetBorderRadii(sz, sz, GetSkipSides(), aRadii);
 }
 
 bool
 nsIFrame::GetMarginBoxBorderRadii(nscoord aRadii[8]) const
 {
-  if (!GetBorderRadii(aRadii)) {
-    return false;
-  }
-  OutsetBorderRadii(aRadii, GetUsedMargin());
-  NS_FOR_CSS_HALF_CORNERS(corner) {
-    if (aRadii[corner]) {
-      return true;
-    }
-  }
-  return false;
+  return GetBoxBorderRadii(aRadii, GetUsedMargin(), true);
 }
 
 bool
 nsIFrame::GetPaddingBoxBorderRadii(nscoord aRadii[8]) const
 {
-  if (!GetBorderRadii(aRadii))
-    return false;
-  InsetBorderRadii(aRadii, GetUsedBorder());
-  NS_FOR_CSS_HALF_CORNERS(corner) {
-    if (aRadii[corner])
-      return true;
-  }
-  return false;
+  return GetBoxBorderRadii(aRadii, GetUsedBorder(), false);
 }
 
 bool
 nsIFrame::GetContentBoxBorderRadii(nscoord aRadii[8]) const
 {
+  return GetBoxBorderRadii(aRadii, GetUsedBorderAndPadding(), false);
+}
+
+bool
+nsIFrame::GetBoxBorderRadii(nscoord aRadii[8], nsMargin aOffset, bool aIsOutset) const
+{
   if (!GetBorderRadii(aRadii))
     return false;
-  InsetBorderRadii(aRadii, GetUsedBorderAndPadding());
+  if (aIsOutset) {
+    OutsetBorderRadii(aRadii, aOffset);
+  } else {
+    InsetBorderRadii(aRadii, aOffset);
+  }
   NS_FOR_CSS_HALF_CORNERS(corner) {
     if (aRadii[corner])
       return true;
   }
   return false;
 }
 
 bool
@@ -2497,45 +2491,38 @@ ApplyOverflowClipping(nsDisplayListBuild
   // have clipping.
   if (!nsFrame::ShouldApplyOverflowClipping(aFrame, aDisp)) {
     return false;
   }
   nsRect clipRect;
   bool haveRadii = false;
   nscoord radii[8];
   auto* disp = aFrame->StyleDisplay();
-  if (disp->mOverflowClipBoxBlock == NS_STYLE_OVERFLOW_CLIP_BOX_PADDING_BOX &&
-      disp->mOverflowClipBoxInline == NS_STYLE_OVERFLOW_CLIP_BOX_PADDING_BOX) {
-    clipRect = aFrame->GetPaddingRectRelativeToSelf() +
-      aBuilder->ToReferenceFrame(aFrame);
-    haveRadii = aFrame->GetPaddingBoxBorderRadii(radii);
-  } else {
-    // Only deflate the padding if we clip to the content-box in that axis.
-    auto wm = aFrame->GetWritingMode();
-    bool cbH = (wm.IsVertical() ? disp->mOverflowClipBoxBlock
-                                : disp->mOverflowClipBoxInline) ==
-               NS_STYLE_OVERFLOW_CLIP_BOX_CONTENT_BOX;
-    bool cbV = (wm.IsVertical() ? disp->mOverflowClipBoxInline
-                                : disp->mOverflowClipBoxBlock) ==
-               NS_STYLE_OVERFLOW_CLIP_BOX_CONTENT_BOX;
-    nsMargin bp = aFrame->GetUsedPadding();
-    if (!cbH) {
-      bp.left = bp.right = nscoord(0);
-    }
-    if (!cbV) {
-      bp.top = bp.bottom = nscoord(0);
-    }
-
-    bp += aFrame->GetUsedBorder();
-    bp.ApplySkipSides(aFrame->GetSkipSides());
-    nsRect rect(nsPoint(0, 0), aFrame->GetSize());
-    rect.Deflate(bp);
-    clipRect = rect + aBuilder->ToReferenceFrame(aFrame);
-    // XXX border-radius
-  }
+  // Only deflate the padding if we clip to the content-box in that axis.
+  auto wm = aFrame->GetWritingMode();
+  bool cbH = (wm.IsVertical() ? disp->mOverflowClipBoxBlock
+                              : disp->mOverflowClipBoxInline) ==
+             NS_STYLE_OVERFLOW_CLIP_BOX_CONTENT_BOX;
+  bool cbV = (wm.IsVertical() ? disp->mOverflowClipBoxInline
+                              : disp->mOverflowClipBoxBlock) ==
+             NS_STYLE_OVERFLOW_CLIP_BOX_CONTENT_BOX;
+  nsMargin bp = aFrame->GetUsedPadding();
+  if (!cbH) {
+    bp.left = bp.right = nscoord(0);
+  }
+  if (!cbV) {
+    bp.top = bp.bottom = nscoord(0);
+  }
+
+  bp += aFrame->GetUsedBorder();
+  bp.ApplySkipSides(aFrame->GetSkipSides());
+  nsRect rect(nsPoint(0, 0), aFrame->GetSize());
+  rect.Deflate(bp);
+  clipRect = rect + aBuilder->ToReferenceFrame(aFrame);
+  haveRadii = aFrame->GetBoxBorderRadii(radii, bp, false);
   aClipState.ClipContainingBlockDescendantsExtra(clipRect, haveRadii ? radii : nullptr);
   return true;
 }
 
 #ifdef DEBUG
 static void PaintDebugBorder(nsIFrame* aFrame, DrawTarget* aDrawTarget,
      const nsRect& aDirtyRect, nsPoint aPt)
 {
--- a/layout/generic/nsFrame.h
+++ b/layout/generic/nsFrame.h
@@ -621,16 +621,22 @@ public:
   {
     // clip overflow:-moz-hidden-unscrollable, except for nsListControlFrame,
     // which is an nsHTMLScrollFrame.
     if (MOZ_UNLIKELY(aDisp->mOverflowX == NS_STYLE_OVERFLOW_CLIP &&
                      !aFrame->IsListControlFrame())) {
       return true;
     }
 
+    // contain: paint, which we should interpret as -moz-hidden-unscrollable
+    // by default.
+    if (aDisp->IsContainPaint()) {
+      return true;
+    }
+
     // and overflow:hidden that we should interpret as -moz-hidden-unscrollable
     if (aDisp->mOverflowX == NS_STYLE_OVERFLOW_HIDDEN &&
         aDisp->mOverflowY == NS_STYLE_OVERFLOW_HIDDEN) {
       // REVIEW: these are the frame types that set up clipping.
       mozilla::LayoutFrameType type = aFrame->Type();
       if (type == mozilla::LayoutFrameType::Table ||
           type == mozilla::LayoutFrameType::TableCell ||
           type == mozilla::LayoutFrameType::BCTableCell ||
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -1406,16 +1406,17 @@ public:
   virtual bool GetBorderRadii(const nsSize& aFrameSize,
                               const nsSize& aBorderArea,
                               Sides aSkipSides,
                               nscoord aRadii[8]) const;
   bool GetBorderRadii(nscoord aRadii[8]) const;
   bool GetMarginBoxBorderRadii(nscoord aRadii[8]) const;
   bool GetPaddingBoxBorderRadii(nscoord aRadii[8]) const;
   bool GetContentBoxBorderRadii(nscoord aRadii[8]) const;
+  bool GetBoxBorderRadii(nscoord aRadii[8], nsMargin aOffset, bool aIsOutset) const;
   bool GetShapeBoxBorderRadii(nscoord aRadii[8]) const;
 
   /**
    * XXX: this method will likely be replaced by GetVerticalAlignBaseline
    * Get the position of the frame's baseline, relative to the top of
    * the frame (its top border edge).  Only valid when Reflow is not
    * needed.
    * @note You should only call this on frames with a WM that's parallel to aWM.
--- a/layout/reftests/w3c-css/submitted/contain/contain-paint-clip-001.html
+++ b/layout/reftests/w3c-css/submitted/contain/contain-paint-clip-001.html
@@ -9,17 +9,17 @@
   <style>
   body {
     margin: 0;
   }
   .root {
     contain: paint;
     width: 100px;
     height: 100px;
-    background: green;
+    background: blue;
     margin: 25px;
     padding: 25px;
   }
   .a {
     width: 100px;
     height: 200px;
     background: red;
   }
--- a/layout/reftests/w3c-css/submitted/contain/contain-paint-clip-002-ref.html
+++ b/layout/reftests/w3c-css/submitted/contain/contain-paint-clip-002-ref.html
@@ -1,25 +1,27 @@
 <!DOCTYPE HTML>
 <html>
 <head>
   <meta charset="utf-8">
   <title>CSS Reftest Reference</title>
+  <link rel="author" title="Yusuf Sermet" href="mailto:ysermet@mozilla.com">
   <link rel="author" title="Kyle Zentner" href="mailto:zentner.kyle@gmail.com">
   <style>
   body {
     margin: 0;
   }
   .root {
     overflow: hidden;
     width: 100px;
     height: 100px;
     background: green;
     margin: 25px;
-    padding: 25px;
+    padding: 10px;
+    border-radius: 4em;
   }
   </style>
 </head>
 <body>
   <div class="root">
     AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA This text should
     be clipped to the box. Lorem ipsum dolor sit amet, consectetur adipiscing
     elit. Integer nec odio. Praesent libero. Sed cursus ante dapibus diam. Sed
--- a/layout/reftests/w3c-css/submitted/contain/contain-paint-clip-002.html
+++ b/layout/reftests/w3c-css/submitted/contain/contain-paint-clip-002.html
@@ -1,27 +1,29 @@
 <!DOCTYPE HTML>
 <html>
 <head>
   <meta charset="utf-8">
-  <title>CSS Test: 'contain: paint' with overflowing text contents.</title>
+  <title>CSS Test: 'contain: paint' with overflowing text contents inside a rounded rectangle box.</title>
+  <link rel="author" title="Yusuf Sermet" href="mailto:ysermet@mozilla.com">
   <link rel="author" title="Kyle Zentner" href="mailto:zentner.kyle@gmail.com">
   <link rel="help" href="http://www.w3.org/TR/css-containment-1/#containment-paint">
   <link rel="match" href="contain-paint-clip-002-ref.html">
   <style>
   body {
     margin: 0;
   }
   .root {
     contain: paint;
     width: 100px;
     height: 100px;
     background: green;
     margin: 25px;
-    padding: 25px;
+    padding: 10px;
+    border-radius: 4em;
   }
   </style>
 </head>
 <body>
   <div class="root">
     AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA This text should
     be clipped to the box. Lorem ipsum dolor sit amet, consectetur adipiscing
     elit. Integer nec odio. Praesent libero. Sed cursus ante dapibus diam. Sed
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/contain/contain-paint-clip-006-ref.html
@@ -0,0 +1,35 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>CSS Reftest Reference</title>
+  <link rel="author" title="Yusuf Sermet" href="mailto:ysermet@mozilla.com">
+  <link rel="author" title="Kyle Zentner" href="mailto:zentner.kyle@gmail.com">
+  <style>
+  body {
+    margin: 0;
+  }
+  .root {
+    width: 100px;
+    height: 100px;
+    background: green;
+    margin: 25px;
+    padding: 25px;
+    overflow: hidden;
+    overflow-clip-box: content-box;
+  }
+  </style>
+</head>
+<body>
+  <div class="root">
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA This text should
+    be clipped to the content box. Lorem ipsum dolor sit amet, consectetur adipiscing
+    elit. Integer nec odio. Praesent libero. Sed cursus ante dapibus diam. Sed
+    nisi. Nulla quis sem at nibh elementum imperdiet. Duis sagittis ipsum.
+    Praesent mauris. Fusce nec tellus sed augue semper porta. Mauris massa.
+    Vestibulum lacinia arcu eget nulla. Class aptent taciti sociosqu ad litora
+    torquent per conubia nostra, per inceptos himenaeos. Curabitur sodales
+    ligula in libero.
+  </div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/contain/contain-paint-clip-006.html
@@ -0,0 +1,37 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>CSS Test: 'contain: paint' with overflowing text contents while "overflow-clip-box: content-box" enabled.</title>
+  <link rel="author" title="Yusuf Sermet" href="mailto:ysermet@mozilla.com">
+  <link rel="author" title="Kyle Zentner" href="mailto:zentner.kyle@gmail.com">
+  <link rel="help" href="http://www.w3.org/TR/css-containment-1/#containment-paint">
+  <link rel="match" href="contain-paint-clip-006-ref.html">
+  <style>
+  body {
+    margin: 0;
+  }
+  .root {
+    contain: paint;
+    width: 100px;
+    height: 100px;
+    background: green;
+    margin: 25px;
+    padding: 25px;
+    overflow-clip-box: content-box;
+  }
+  </style>
+</head>
+<body>
+  <div class="root">
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA This text should
+    be clipped to the content box. Lorem ipsum dolor sit amet, consectetur adipiscing
+    elit. Integer nec odio. Praesent libero. Sed cursus ante dapibus diam. Sed
+    nisi. Nulla quis sem at nibh elementum imperdiet. Duis sagittis ipsum.
+    Praesent mauris. Fusce nec tellus sed augue semper porta. Mauris massa.
+    Vestibulum lacinia arcu eget nulla. Class aptent taciti sociosqu ad litora
+    torquent per conubia nostra, per inceptos himenaeos. Curabitur sodales
+    ligula in libero.
+  </div>
+</body>
+</html>
--- a/layout/reftests/w3c-css/submitted/contain/reftest.list
+++ b/layout/reftests/w3c-css/submitted/contain/reftest.list
@@ -1,13 +1,14 @@
 default-preferences pref(layout.css.contain.enabled,true)
 
 == contain-paint-clip-001.html contain-paint-clip-001-ref.html
 == contain-paint-clip-002.html contain-paint-clip-002-ref.html
 == contain-paint-clip-003.html contain-paint-clip-003-ref.html
 == contain-paint-clip-004.html contain-paint-clip-004-ref.html
 == contain-paint-clip-005.html contain-paint-clip-003-ref.html
+pref(layout.css.overflow-clip-box.enabled,true) == contain-paint-clip-006.html contain-paint-clip-006-ref.html
 == contain-paint-containing-block-absolute-001.html contain-paint-containing-block-absolute-001-ref.html
 == contain-paint-containing-block-fixed-001.html contain-paint-containing-block-fixed-001-ref.html
 == contain-paint-formatting-context-float-001.html contain-paint-formatting-context-float-001-ref.html
 == contain-paint-formatting-context-margin-001.html contain-paint-formatting-context-margin-001-ref.html
 == contain-paint-stacking-context-001a.html contain-paint-stacking-context-001-ref.html
 == contain-paint-stacking-context-001b.html contain-paint-stacking-context-001-ref.html
--- a/layout/style/test/mochitest.ini
+++ b/layout/style/test/mochitest.ini
@@ -175,17 +175,16 @@ skip-if = toolkit == 'android'
 [test_computed_style_grid_with_pseudo.html]
 [test_computed_style_in_created_document.html]
 [test_computed_style_min_size_auto.html]
 [test_computed_style_no_flush.html]
 [test_computed_style_no_pseudo.html]
 [test_computed_style_prefs.html]
 [test_condition_text.html]
 [test_condition_text_assignment.html]
-[test_contain_formatting_context.html]
 [test_counter_descriptor_storage.html]
 [test_counter_style.html]
 [test_crash_with_content_policy.html]
 support-files = file_bug1381233.html
 [test_css_cross_domain.html]
 skip-if = toolkit == 'android' #bug 536603
 [test_css_eof_handling.html]
 [test_css_escape_api.html]
deleted file mode 100644
--- a/layout/style/test/test_contain_formatting_context.html
+++ /dev/null
@@ -1,40 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<!--
-https://bugzilla.mozilla.org/show_bug.cgi?id=1170781
--->
-<head>
-  <meta charset="utf-8">
-  <title>Test that 'contain: paint' updates 'display' correctly</title>
-  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
-  <script type="text/javascript" src="property_database.js"></script>
-  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
-</head>
-<body>
-  <div id="test" style="contain: paint"></div>
-  <script>
-  // This mapping is currently mostly based off of a bugzilla comment by Tab
-  // Atkins Jr. in bug 1179349. Ultimately, it should be based on a
-  // specification.
-
-  // XXX 'ruby[-*]' and 'run-in' might need to be added here.
-  var expectedChanges = {
-    'inline'              : 'inline-block',
-  };
-  var displayInfo = gCSSProperties["display"];
-  var test = document.getElementById('test');
-  var displayVals = displayInfo.initial_values.concat(displayInfo.other_values);
-  for (dispVal of displayVals) {
-    test.style.display = dispVal;
-    if (expectedChanges.hasOwnProperty(dispVal)) {
-      is(getComputedStyle(test).display, expectedChanges[dispVal],
-          `'contain: paint' should change 'display: ${dispVal}' to ` +
-          `'display: ${expectedChanges[dispVal]}'`);
-    } else {
-      is(getComputedStyle(test).display, dispVal,
-          `'contain: paint' should not change 'display: ${dispVal}'`);
-    }
-  }
-  </script>
-</body>
-</html>
--- a/servo/components/style/style_adjuster.rs
+++ b/servo/components/style/style_adjuster.rs
@@ -310,49 +310,16 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
                     .mutate_box()
                     .set_adjusted_display(Display::InlineBlock, false);
             } else {
                 self.style.mutate_box().set_display(Display::InlineBlock);
             }
         }
     }
 
-    #[cfg(feature = "gecko")]
-    fn adjust_for_contain(&mut self) {
-        use properties::longhands::contain::SpecifiedValue;
-
-        // An element with contain: paint needs to be a formatting context, and
-        // also implies overflow: clip.
-        //
-        // TODO(emilio): This mimics Gecko, but spec links are missing!
-        let contain = self.style.get_box().clone_contain();
-        if !contain.contains(SpecifiedValue::PAINT) {
-            return;
-        }
-
-        if self.style.get_box().clone_display() == Display::Inline {
-            self.style
-                .mutate_box()
-                .set_adjusted_display(Display::InlineBlock, false);
-        }
-
-        // When 'contain: paint', update overflow from 'visible' to 'clip'.
-        if self.style
-            .get_box()
-            .clone_contain()
-            .contains(SpecifiedValue::PAINT)
-        {
-            if self.style.get_box().clone_overflow_x() == Overflow::Visible {
-                let box_style = self.style.mutate_box();
-                box_style.set_overflow_x(Overflow::MozHiddenUnscrollable);
-                box_style.set_overflow_y(Overflow::MozHiddenUnscrollable);
-            }
-        }
-    }
-
     /// When mathvariant is not "none", font-weight and font-style are
     /// both forced to "normal".
     #[cfg(feature = "gecko")]
     fn adjust_for_mathvariant(&mut self) {
         use properties::longhands::_moz_math_variant::computed_value::T as MozMathVariant;
         use properties::longhands::font_weight::computed_value::T as FontWeight;
         use values::generics::font::FontStyle;
         if self.style.get_font().clone__moz_math_variant() != MozMathVariant::None {
@@ -747,17 +714,16 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
         }
         self.adjust_for_top_layer();
         self.blockify_if_necessary(layout_parent_style, element);
         self.adjust_for_position();
         self.adjust_for_overflow();
         #[cfg(feature = "gecko")]
         {
             self.adjust_for_table_text_align();
-            self.adjust_for_contain();
             self.adjust_for_mathvariant();
             self.adjust_for_justify_items();
         }
         #[cfg(feature = "servo")]
         {
             self.adjust_for_alignment(layout_parent_style);
         }
         self.adjust_for_border_width();
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-contain/contain-paint-002.html.ini
+++ /dev/null
@@ -1,2 +0,0 @@
-[contain-paint-002.html]
-  expected: FAIL