Bug 833542. Make scrollWidth/scrollHeight for overflow:visible match what they would be for overflow:hidden on the same element. r=mats

content/base/public/Element.h

@@ -1036,13 +1036,6 @@ protected:
    */
   virtual Element* GetOffsetRect(nsRect& aRect);
 
-  /**
-   * Retrieve the size of the padding rect of this element.
-   *
-   * @param aSize the size of the padding rect
-   */
-  nsIntSize GetPaddingRectSize();
-
   nsIFrame* GetStyledFrame();
 
   virtual Element* GetNameSpaceElement()

content/base/src/Element.cpp

@@ -514,20 +514,6 @@ Element::GetOffsetRect(nsRect& aRect)
   return nullptr;
 }
 
-nsIntSize
-Element::GetPaddingRectSize()
-{
-  nsIFrame* frame = GetStyledFrame();
-  if (!frame) {
-    return nsIntSize(0, 0);
-  }
-
-  NS_ASSERTION(frame->GetParent(), "Styled frame has no parent");
-  nsRect rcFrame = nsLayoutUtils::GetAllInFlowPaddingRectsUnion(frame, frame->GetParent());
-  return nsIntSize(nsPresContext::AppUnitsToIntCSSPixels(rcFrame.width),
-                   nsPresContext::AppUnitsToIntCSSPixels(rcFrame.height));
-}
-
 nsIScrollableFrame*
 Element::GetScrollFrame(nsIFrame **aStyledFrame)
 {
@@ -597,6 +583,20 @@ Element::ScrollIntoView(bool aTop)
                                    nsIPresShell::SCROLL_OVERFLOW_HIDDEN);
 }
 
+static nsSize GetScrollRectSizeForOverflowVisibleFrame(nsIFrame* aFrame)
+{
+  if (!aFrame) {
+    return nsSize(0,0);
+  }
+
+  nsRect paddingRect = aFrame->GetPaddingRectRelativeToSelf();
+  nsOverflowAreas overflowAreas(paddingRect, paddingRect);
+  nsLayoutUtils::UnionChildOverflow(aFrame, overflowAreas);
+  return nsLayoutUtils::GetScrolledRect(aFrame,
+      overflowAreas.ScrollableOverflow(), paddingRect.Size(),
+      aFrame->GetStyleVisibility()->mDirection).Size();
+}
+
 int32_t
 Element::ScrollHeight()
 {
@@ -604,11 +604,13 @@ Element::ScrollHeight()
     return 0;
 
   nsIScrollableFrame* sf = GetScrollFrame();
-  if (!sf) {
+  nscoord height;
-    return GetPaddingRectSize().height;
+  if (sf) {
+    height = sf->GetScrollRange().height + sf->GetScrollPortRect().height;
+  } else {
+    height = GetScrollRectSizeForOverflowVisibleFrame(GetStyledFrame()).height;
   }
 
-  nscoord height = sf->GetScrollRange().height + sf->GetScrollPortRect().height;
   return nsPresContext::AppUnitsToIntCSSPixels(height);
 }
 
@@ -619,11 +621,13 @@ Element::ScrollWidth()
     return 0;
 
   nsIScrollableFrame* sf = GetScrollFrame();
-  if (!sf) {
+  nscoord width;
-    return GetPaddingRectSize().width;
+  if (sf) {
+    width = sf->GetScrollRange().width + sf->GetScrollPortRect().width;
+  } else {
+    width = GetScrollRectSizeForOverflowVisibleFrame(GetStyledFrame()).width;
   }
 
-  nscoord width = sf->GetScrollRange().width + sf->GetScrollPortRect().width;
   return nsPresContext::AppUnitsToIntCSSPixels(width);
 }
 

dom/tests/mochitest/general/test_offsets.html

@@ -60,6 +60,11 @@
                                value="This button is much longer than the others">
   </p></div>
 
+  <div id="overflow-visible" style="width:100px; height:100px;">
+    <div id="overflow-visible-1" style="width:200px; height:1px; background:yellow;"></div>
+    <div id="overflow-visible-2" style="height:200px; background:lime;"></div>
+  </div>
+
   <input id="input-displaynone" style="display: none; border: 0; padding: 0;"
          _offsetParent="null">
   <p id="p3" style="margin: 2px; border: 0; padding: 1px;"

dom/tests/mochitest/general/test_offsets.js

@@ -60,6 +60,7 @@ function testElement(element)
                               offsetParent, element.id);
 
   var scrollWidth, scrollHeight, clientWidth, clientHeight;
+  var doScrollCheck = true;
   if (element.id == "scrollbox") {
     var lastchild = $("lastline");
     scrollWidth = lastchild.getBoundingClientRect().width + paddingLeft + paddingRight;
@@ -69,18 +70,29 @@ function testElement(element)
     scrollHeight = contentsHeight + paddingTop + paddingBottom;
     clientWidth = paddingLeft + width + paddingRight - scrollbarWidth;
     clientHeight = paddingTop + height + paddingBottom - scrollbarHeight;
-  }
+  } else {
-  else {
-    scrollWidth = paddingLeft + width + paddingRight;
-    scrollHeight = paddingTop + height + paddingBottom;
     clientWidth = paddingLeft + width + paddingRight;
     clientHeight = paddingTop + height + paddingBottom;
+    if (element.id == "overflow-visible") {
+      scrollWidth = 200;
+      scrollHeight = 201;
+    } else if (element.scrollWidth > clientWidth ||
+               element.scrollHeight > clientHeight) {
+      // The element overflows. Don't check scrollWidth/scrollHeight since the
+      // above calculation is not correct.
+      doScrollCheck = false;
+    } else {
+      scrollWidth = clientWidth;
+      scrollHeight = clientHeight;
+    }
   }
 
+  if (doScrollCheck) {
     if (element instanceof SVGElement)
       checkScrollState(element, 0, 0, 0, 0, element.id);
      else
       checkScrollState(element, 0, 0, scrollWidth, scrollHeight, element.id);
+  }
 
   if (element instanceof SVGElement)
     checkClientState(element, 0, 0, 0, 0, element.id);

layout/base/nsLayoutUtils.cpp

@@ -1153,6 +1153,40 @@ nsLayoutUtils::GetNearestScrollableFrame(nsIFrame* aFrame)
   return nullptr;
 }
 
+// static
+nsRect
+nsLayoutUtils::GetScrolledRect(nsIFrame* aScrolledFrame,
+                               const nsRect& aScrolledFrameOverflowArea,
+                               const nsSize& aScrollPortSize,
+                               uint8_t aDirection)
+{
+  nscoord x1 = aScrolledFrameOverflowArea.x,
+          x2 = aScrolledFrameOverflowArea.XMost(),
+          y1 = aScrolledFrameOverflowArea.y,
+          y2 = aScrolledFrameOverflowArea.YMost();
+  if (y1 < 0) {
+    y1 = 0;
+  }
+  if (aDirection != NS_STYLE_DIRECTION_RTL) {
+    if (x1 < 0) {
+      x1 = 0;
+    }
+  } else {
+    if (x2 > aScrollPortSize.width) {
+      x2 = aScrollPortSize.width;
+    }
+    // When the scrolled frame chooses a size larger than its available width (because
+    // its padding alone is larger than the available width), we need to keep the
+    // start-edge of the scroll frame anchored to the start-edge of the scrollport.
+    // When the scrolled frame is RTL, this means moving it in our left-based
+    // coordinate system, so we need to compensate for its extra width here by
+    // effectively repositioning the frame.
+    nscoord extraWidth = std::max(0, aScrolledFrame->GetSize().width - aScrollPortSize.width);
+    x2 += extraWidth;
+  }
+  return nsRect(x1, y1, x2 - x1, y2 - y1);
+}
+
 //static
 bool
 nsLayoutUtils::HasPseudoStyle(nsIContent* aContent,

layout/base/nsLayoutUtils.h

@@ -374,6 +374,17 @@ public:
    */
   static nsIScrollableFrame* GetNearestScrollableFrame(nsIFrame* aFrame);
 
+  /**
+   * GetScrolledRect returns the range of allowable scroll offsets
+   * for aScrolledFrame, assuming the scrollable overflow area is
+   * aScrolledFrameOverflowArea and the scrollport size is aScrollPortSize.
+   * aDirection is either NS_STYLE_DIRECTION_LTR or NS_STYLE_DIRECTION_RTL.
+   */
+  static nsRect GetScrolledRect(nsIFrame* aScrolledFrame,
+                                const nsRect& aScrolledFrameOverflowArea,
+                                const nsSize& aScrollPortSize,
+                                uint8_t aDirection);
+
   /**
    * HasPseudoStyle returns true if aContent (whose primary style
    * context is aStyleContext) has the aPseudoElement pseudo-style

layout/generic/nsGfxScrollFrame.cpp

@@ -3728,28 +3728,9 @@ nsRect
 nsGfxScrollFrameInner::GetScrolledRectInternal(const nsRect& aScrolledFrameOverflowArea,
                                                const nsSize& aScrollPortSize) const
 {
-  nscoord x1 = aScrolledFrameOverflowArea.x,
+  return nsLayoutUtils::GetScrolledRect(mScrolledFrame,
-          x2 = aScrolledFrameOverflowArea.XMost(),
+      aScrolledFrameOverflowArea, aScrollPortSize,
-          y1 = aScrolledFrameOverflowArea.y,
+      IsLTR() ? NS_STYLE_DIRECTION_LTR : NS_STYLE_DIRECTION_RTL);
-          y2 = aScrolledFrameOverflowArea.YMost();
-  if (y1 < 0)
-    y1 = 0;
-  if (IsLTR()) {
-    if (x1 < 0)
-      x1 = 0;
-  } else {
-    if (x2 > aScrollPortSize.width)
-      x2 = aScrollPortSize.width;
-    // When the scrolled frame chooses a size larger than its available width (because
-    // its padding alone is larger than the available width), we need to keep the
-    // start-edge of the scroll frame anchored to the start-edge of the scrollport. 
-    // When the scrolled frame is RTL, this means moving it in our left-based
-    // coordinate system, so we need to compensate for its extra width here by
-    // effectively repositioning the frame.
-    nscoord extraWidth = std::max(0, mScrolledFrame->GetSize().width - aScrollPortSize.width);
-    x2 += extraWidth;
-  }
-  return nsRect(x1, y1, x2 - x1, y2 - y1);
 }
 
 nsMargin