Bug 1739894 - Stop including overflowing descendants in outlines. r=jwatt

Keep old behavior behind a pref.

I've kept the tests that failed enabling the pref because they're
somewhat hard to rewrite and I'd rather avoid that until we know this
will stick, specially since they test the guts of the recursive function
that I'm basically ~disabling.

I've checked that the rendering on those is sensible / matches other
browsers.

Differential Revision: https://phabricator.services.mozilla.com/D130717

layout/generic/nsIFrame.cpp

@@ -9338,13 +9338,15 @@ bool nsIFrame::SetOverflowAreas(const OverflowAreas& aOverflowAreas) {
   }
 }
 
+enum class ApplyTransform : bool { No, Yes };
+
 /**
- * Compute the union of the border boxes of aFrame and its descendants,
- * in aFrame's coordinate space (if aApplyTransform is false) or its
- * post-transform coordinate space (if aApplyTransform is true).
+ * Compute the outline inner rect (so without outline-width and outline-offset)
+ * of aFrame, maybe iterating over its descendants, in aFrame's coordinate space
+ * or its post-transform coordinate space (depending on aApplyTransform).
  */
-static nsRect UnionBorderBoxes(
-    nsIFrame* aFrame, bool aApplyTransform, bool& aOutValid,
+static nsRect ComputeOutlineInnerRect(
+    nsIFrame* aFrame, ApplyTransform aApplyTransform, bool& aOutValid,
     const nsSize* aSizeOverride = nullptr,
     const OverflowAreas* aOverflowOverride = nullptr) {
   const nsRect bounds(nsPoint(0, 0),
@@ -9367,7 +9369,8 @@ static nsRect UnionBorderBoxes(
 
   // Start from our border-box, transformed.  See comment below about
   // transform of children.
-  bool doTransform = aApplyTransform && aFrame->IsTransformed();
+  bool doTransform =
+      aApplyTransform == ApplyTransform::Yes && aFrame->IsTransformed();
   TransformReferenceBox boundsRefBox(nullptr, bounds);
   if (doTransform) {
     u = nsDisplayTransform::TransformRect(bounds, aFrame, boundsRefBox);
@@ -9375,6 +9378,10 @@ static nsRect UnionBorderBoxes(
     u = bounds;
   }
 
+  if (aOutValid && !StaticPrefs::layout_outline_include_overflow()) {
+    return u;
+  }
+
   // Only iterate through the children if the overflow areas suggest
   // that we might need to, and if the frame doesn't clip its overflow
   // anyway.
@@ -9419,18 +9426,18 @@ static nsRect UnionBorderBoxes(
         continue;
       }
 
-      // Note that passing |true| for aApplyTransform when
-      // child->Combines3DTransformWithAncestors() is incorrect if our
-      // aApplyTransform is false... but the opposite would be as
-      // well.  This is because elements within a preserve-3d scene
-      // are always transformed up to the top of the scene.  This
-      // means we don't have a mechanism for getting a transform up to
-      // an intermediate point within the scene.  We choose to
-      // over-transform rather than under-transform because this is
-      // consistent with other overflow areas.
+      // Note that passing ApplyTransform::Yes when
+      // child->Combines3DTransformWithAncestors() returns true is incorrect if
+      // our aApplyTransform is No... but the opposite would be as well.
+      // This is because elements within a preserve-3d scene are always
+      // transformed up to the top of the scene.  This means we don't have a
+      // mechanism for getting a transform up to an intermediate point within
+      // the scene.  We choose to over-transform rather than under-transform
+      // because this is consistent with other overflow areas.
       bool validRect = true;
       nsRect childRect =
-          UnionBorderBoxes(child, true, validRect) + child->GetPosition();
+          ComputeOutlineInnerRect(child, ApplyTransform::Yes, validRect) +
+          child->GetPosition();
 
       if (!validRect) {
         continue;
@@ -9505,13 +9512,14 @@ static void ComputeAndIncludeOutlineArea(nsIFrame* aFrame,
   // calling FinishAndStoreOverflow again, which in turn calls this
   // function again.  We still need to deal with preserve-3d a bit.
   nsRect innerRect;
-  bool validRect;
+  bool validRect = false;
   if (frameForArea == aFrame) {
-    innerRect =
-        UnionBorderBoxes(aFrame, false, validRect, &aNewSize, &aOverflowAreas);
+    innerRect = ComputeOutlineInnerRect(aFrame, ApplyTransform::No, validRect,
+                                        &aNewSize, &aOverflowAreas);
   } else {
     for (; frameForArea; frameForArea = frameForArea->GetNextSibling()) {
-      nsRect r(UnionBorderBoxes(frameForArea, true, validRect));
+      nsRect r =
+          ComputeOutlineInnerRect(frameForArea, ApplyTransform::Yes, validRect);
 
       // Adjust for offsets transforms up to aFrame's pre-transform
       // (i.e., normal) coordinate space; see comments in

layout/reftests/bugs/reftest.list

@@ -837,7 +837,7 @@ fails-if(cocoaWidget||winWidget) random-if(!cocoaWidget&&!winWidget) != 399636-q
 == 400813-1.html 400813-1-ref.html
 == 400826-1.html 400826-1-ref.html
 == chrome://reftest/content/bugs/401946-1.xhtml about:blank
-== 402338-1.html 402338-1-ref.html
+pref(layout.outline.include-overflow,true) == 402338-1.html 402338-1-ref.html
 == 402567-1.html 402567-1-ref.html
 fuzzy-if(winWidget&&!nativeThemePref,0-8,0-1) == 402567-2.html 402567-2-ref.html
 == 402567-3.html 402567-3-ref.html

layout/reftests/columns/reftest.list

@@ -11,7 +11,7 @@
 == column-balancing-overflow-002.html column-balancing-overflow-002.ref.html
 == column-balancing-overflow-003.html column-balancing-overflow-003.ref.html
 == column-balancing-overflow-004.html column-balancing-overflow-004.ref.html
-== column-balancing-overflow-005.html column-balancing-overflow-005.ref.html
+pref(layout.outline.include-overflow,true) == column-balancing-overflow-005.html column-balancing-overflow-005.ref.html
 == column-balancing-000.html column-balancing-000.ref.html
 == column-balancing-001.html column-balancing-000.ref.html
 == column-balancing-002.html column-balancing-002.ref.html

layout/reftests/outline/reftest.list

@@ -1,6 +1,6 @@
 fuzzy(0-52,0-1452) == outline-and-box-shadow.html outline-and-box-shadow-ref.html
-== outline-and-3d-transform-1a.html outline-and-3d-transform-1-ref.html
-== outline-and-3d-transform-1b.html outline-and-3d-transform-1-ref.html
+pref(layout.outline.include-overflow,true) == outline-and-3d-transform-1a.html outline-and-3d-transform-1-ref.html
+pref(layout.outline.include-overflow,true) == outline-and-3d-transform-1b.html outline-and-3d-transform-1-ref.html
 fuzzy-if(gtkWidget,0-136,0-120) fuzzy-if(Android,0-255,0-356) fuzzy-if(d2d,0-16,0-96) fuzzy-if(cocoaWidget,0-255,0-120) fuzzy-if(winWidget,0-255,0-216) == outline-and-3d-transform-2.html outline-and-3d-transform-2-ref.html
 == outline-dynamic-change-1a.html outline-dynamic-change-1-ref.html
 == outline-dynamic-change-1b.html outline-dynamic-change-1-ref.html

modules/libpref/init/StaticPrefList.yaml

@@ -7772,6 +7772,15 @@
   value: 0
   mirror: always
 
+# Whether outlines should include all overflowing descendants, or just the
+# border-box of a given element.
+#
+# Historically we have included descendants but other browsers have not.
+- name: layout.outline.include-overflow
+  type: bool
+  value: false
+  mirror: always
+
 # Controls double click and Alt+Arrow word selection behavior.
 - name: layout.word_select.eat_space_to_next_word
   type: bool

testing/web-platform/meta/css/css-ui/outline-027.html.ini

@@ -1,2 +0,0 @@
-[outline-027.html]
-  expected: FAIL

testing/web-platform/meta/css/css-ui/outline-028.html.ini

@@ -1,2 +0,0 @@
-[outline-028.html]
-  expected: FAIL