Bug 1379332: When computing abspos CB content-box size, don't bother subtracting borderpadding if CB is a 0-sized child of a XUL-collapsed frame. r?mats draft
authorDaniel Holbert <dholbert@cs.stanford.edu>
Fri, 20 Oct 2017 16:40:43 -0700
changeset 684288 a7e5502cc9c960253e2db33ec2d38955d4b7b3e7
parent 683863 d1e995c8640a191cd127e87273ec96cb2fabffa9
child 684292 431fbafd2beab7a9466bc5e6d4e9ab99a7f4c036
child 684293 5115185f68b98d51e41f30aff4863515bef93bc0
push id85580
push userdholbert@mozilla.com
push dateFri, 20 Oct 2017 23:40:57 +0000
reviewersmats
bugs1379332
milestone58.0a1
Bug 1379332: When computing abspos CB content-box size, don't bother subtracting borderpadding if CB is a 0-sized child of a XUL-collapsed frame. r?mats If border & padding were ignored for sizing the containing block (which can happen, if the containing block is a chlid of a XUL-collapsed frame), then we don't need to subtract border & padding when computing the frame's content-box size for its abspos descendants. MozReview-Commit-ID: JGnzShl8m67
layout/generic/ReflowInput.cpp
layout/xul/crashtests/1379332-1.xul
layout/xul/crashtests/1379332-2.xul
layout/xul/crashtests/crashtests.list
--- a/layout/generic/ReflowInput.cpp
+++ b/layout/generic/ReflowInput.cpp
@@ -1088,16 +1088,24 @@ ReflowInput::ApplyRelativePositioning(ns
     StickyScrollContainer* ssc =
       StickyScrollContainer::GetStickyScrollContainerForFrame(aFrame);
     if (ssc) {
       *aPosition = ssc->ComputePosition(aFrame);
     }
   }
 }
 
+// Returns true if aFrame is non-null, a XUL frame, and "XUL-collapsed" (which
+// only becomes a valid question to ask if we know it's a XUL frame).
+static bool
+IsXULCollapsedXULFrame(nsIFrame* aFrame)
+{
+  return aFrame && aFrame->IsXULBoxFrame() && aFrame->IsXULCollapsed();
+}
+
 nsIFrame*
 ReflowInput::GetHypotheticalBoxContainer(nsIFrame*    aFrame,
                                                nscoord&     aCBIStartEdge,
                                                LogicalSize& aCBSize) const
 {
   aFrame = aFrame->GetContainingBlock();
   NS_ASSERTION(aFrame != mFrame, "How did that happen?");
 
@@ -1123,19 +1131,33 @@ ReflowInput::GetHypotheticalBoxContainer
     aCBSize = reflowInput->ComputedSize(wm);
   } else {
     /* Didn't find a reflow reflowInput for aFrame.  Just compute the information we
        want, on the assumption that aFrame already knows its size.  This really
        ought to be true by now. */
     NS_ASSERTION(!(aFrame->GetStateBits() & NS_FRAME_IN_REFLOW),
                  "aFrame shouldn't be in reflow; we'll lie if it is");
     WritingMode wm = aFrame->GetWritingMode();
-    LogicalMargin borderPadding = aFrame->GetLogicalUsedBorderAndPadding(wm);
-    aCBIStartEdge = borderPadding.IStart(wm);
-    aCBSize = aFrame->GetLogicalSize(wm) - borderPadding.Size(wm);
+    // Compute CB's offset & content-box size by subtracting borderpadding from
+    // frame size.  Exception: if the CB is 0-sized, it *might* be a child of a
+    // XUL-collapsed frame and might have nonzero borderpadding that was simply
+    // discarded during its layout. (See the child-zero-sizing in
+    // nsSprocketLayout::XULLayout()).  In that case, we ignore the
+    // borderpadding here (just like we did when laying it out), or else we'd
+    // produce a bogus negative content-box size.
+    aCBIStartEdge = 0;
+    aCBSize = aFrame->GetLogicalSize(wm);
+    if (!aCBSize.IsAllZero() ||
+        (!IsXULCollapsedXULFrame(aFrame->GetParent()))) {
+      // aFrame is not XUL-collapsed (nor is it a child of a XUL-collapsed
+      // frame), so we can go ahead and subtract out border padding.
+      LogicalMargin borderPadding = aFrame->GetLogicalUsedBorderAndPadding(wm);
+      aCBIStartEdge += borderPadding.IStart(wm);
+      aCBSize -= borderPadding.Size(wm);
+    }
   }
 
   return aFrame;
 }
 
 struct nsHypotheticalPosition {
   // offset from inline-start edge of containing block (which is a padding edge)
   nscoord       mIStart;
new file mode 100644
--- /dev/null
+++ b/layout/xul/crashtests/1379332-1.xul
@@ -0,0 +1,9 @@
+<?xml version="1.0"?>
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <hbox style="visibility: collapse; position: relative">
+    <textbox>
+      <hbox style="position: absolute; width: 10px; height: 10px">
+      </hbox>
+    </textbox>
+  </hbox>
+</window>
new file mode 100644
--- /dev/null
+++ b/layout/xul/crashtests/1379332-2.xul
@@ -0,0 +1,9 @@
+<?xml version="1.0"?>
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <hbox style="position: relative;visibility: collapse;">
+    <hbox style="padding:5px; border: 5px solid black">
+      <hbox style="position: absolute; width: 10px; height: 10px">
+      </hbox>
+    </hbox>
+  </hbox>
+</window>
--- a/layout/xul/crashtests/crashtests.list
+++ b/layout/xul/crashtests/crashtests.list
@@ -92,8 +92,14 @@ load 514300-1.xul
 load 536931-1.xhtml
 asserts-if(stylo,4) load 538308-1.xul # bug 1397644
 load 557174-1.xml
 load 564705-1.xul
 load 583957-1.html
 load 617089.html
 load menulist-focused.xhtml
 load 716503.html
+
+# Bug 1379332's tests need stylo to be disabled in order for the
+# 'visiblity:collapse' to take effect (which is needed to trigger the assertion
+# that these tests are screening for, in unpatched builds):
+pref(layout.css.servo.enabled,false) load 1379332-1.xul
+pref(layout.css.servo.enabled,false) load 1379332-2.xul