Bug 833542. Make scrollWidth/scrollHeight for overflow:visible match what they would be for overflow:hidden on the same element. r=mats
authorRobert O'Callahan <robert@ocallahan.org>
Tue, 29 Jan 2013 14:38:22 +1300
changeset 120178 8e1f8a3c55acf29f3cfba1719b8b3ae601cd503c
parent 120177 954fc89373b8e19044d9da472b2d2b11af8cef9d
child 120179 0d4873554deefef2140c7c979c30ec8f8f00cc15
push id22081
push userrocallahan@mozilla.com
push dateTue, 29 Jan 2013 08:58:48 +0000
treeherdermozilla-inbound@7e9f3cc6cc0a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs833542
milestone21.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
content/base/src/Element.cpp
dom/tests/mochitest/general/test_offsets.html
dom/tests/mochitest/general/test_offsets.js
layout/base/nsLayoutUtils.cpp
layout/base/nsLayoutUtils.h
layout/generic/nsGfxScrollFrame.cpp
--- a/content/base/public/Element.h
+++ b/content/base/public/Element.h
@@ -1031,23 +1031,16 @@ protected:
   /**
    * Retrieve the rectangle for the offsetX properties, which
    * are coordinates relative to the returned element.
    *
    * @param aRect offset rectangle
    */
   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()
   {
     return this;
   }
 
   nsIDOMAttr* GetAttributeNodeNSInternal(const nsAString& aNamespaceURI,
--- a/content/base/src/Element.cpp
+++ b/content/base/src/Element.cpp
@@ -509,30 +509,16 @@ Element::GetOffsetRect(nsRect& aRect)
   nsIFrame* parent = frame->GetParent() ? frame->GetParent() : frame;
   nsRect rcFrame = nsLayoutUtils::GetAllInFlowRectsUnion(frame, parent);
   aRect.width = nsPresContext::AppUnitsToIntCSSPixels(rcFrame.width);
   aRect.height = nsPresContext::AppUnitsToIntCSSPixels(rcFrame.height);
 
   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)
 {
   // it isn't clear what to return for SVG nodes, so just return nothing
   if (IsSVG()) {
     if (aStyledFrame) {
       *aStyledFrame = nullptr;
     }
@@ -592,43 +578,61 @@ Element::ScrollIntoView(bool aTop)
   presShell->ScrollContentIntoView(this,
                                    nsIPresShell::ScrollAxis(
                                      vpercent,
                                      nsIPresShell::SCROLL_ALWAYS),
                                    nsIPresShell::ScrollAxis(),
                                    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()
 {
   if (IsSVG())
     return 0;
 
   nsIScrollableFrame* sf = GetScrollFrame();
-  if (!sf) {
-    return GetPaddingRectSize().height;
+  nscoord 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);
 }
 
 int32_t
 Element::ScrollWidth()
 {
   if (IsSVG())
     return 0;
 
   nsIScrollableFrame* sf = GetScrollFrame();
-  if (!sf) {
-    return GetPaddingRectSize().width;
+  nscoord 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);
 }
 
 nsRect
 Element::GetClientAreaRect()
 {
   nsIFrame* styledFrame;
   nsIScrollableFrame* sf = GetScrollFrame(&styledFrame);
--- a/dom/tests/mochitest/general/test_offsets.html
+++ b/dom/tests/mochitest/general/test_offsets.html
@@ -55,16 +55,21 @@
        _clientLeft="1" _clientTop="1" _clientWidth="62" _clientHeight="32"><p id="p1" style="margin: 0; padding: 0;">One</p>
     <p id="p2">Two</p>
     <p id="scrollchild">Three</p>
     <p id="lastlinebox" style="margin: 0; padding: 0;"><input id="lastline" type="button"
                                style="margin: 0px; border: 2px solid red;"
                                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;"
          _offsetLeft="9" _offsetTop="9" _offsetWidth="64" _offsetHeight="34"
          _scrollWidth="62" _scrollHeight="32"
          _clientLeft="1" _clientTop="1" _clientWidth="62" _clientHeight="32">
     <input id="input-nosize" style="width: 0; height: 0; margin: 0; border: 0; padding: 0;">
   </p>
--- a/dom/tests/mochitest/general/test_offsets.js
+++ b/dom/tests/mochitest/general/test_offsets.js
@@ -55,37 +55,49 @@ function testElement(element)
 
   if (element instanceof HTMLElement)
     checkOffsetState(element, -10000, -10000,
                               borderLeft + paddingLeft + width + paddingRight + borderRight,
                               borderTop + paddingTop + height + paddingBottom + borderBottom,
                               offsetParent, element.id);
 
   var scrollWidth, scrollHeight, clientWidth, clientHeight;
+  var doScrollCheck = true;
   if (element.id == "scrollbox") {
     var lastchild = $("lastline");
     scrollWidth = lastchild.getBoundingClientRect().width + paddingLeft + paddingRight;
     var top = element.firstChild.getBoundingClientRect().top;
     var bottom = element.lastChild.getBoundingClientRect().bottom;
     var contentsHeight = bottom - top;
     scrollHeight = contentsHeight + paddingTop + paddingBottom;
     clientWidth = paddingLeft + width + paddingRight - scrollbarWidth;
     clientHeight = paddingTop + height + paddingBottom - scrollbarHeight;
-  }
-  else {
-    scrollWidth = paddingLeft + width + paddingRight;
-    scrollHeight = paddingTop + height + paddingBottom;
+  } else {
     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 (element instanceof SVGElement)
-    checkScrollState(element, 0, 0, 0, 0, element.id);
-  else
-    checkScrollState(element, 0, 0, scrollWidth, scrollHeight, element.id);
+  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);
   else
     checkClientState(element, borderLeft, borderTop, clientWidth, clientHeight, element.id);
 
   var boundingrect = element.getBoundingClientRect();
   isEqualAppunits(boundingrect.width, borderLeft + paddingLeft + width + paddingRight + borderRight,
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -1148,16 +1148,50 @@ nsLayoutUtils::GetNearestScrollableFrame
       if (ss.mVertical != NS_STYLE_OVERFLOW_HIDDEN ||
           ss.mHorizontal != NS_STYLE_OVERFLOW_HIDDEN)
         return scrollableFrame;
     }
   }
   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,
                               nsStyleContext* aStyleContext,
                               nsCSSPseudoElements::Type aPseudoElement,
                               nsPresContext* aPresContext)
 {
   NS_PRECONDITION(aPresContext, "Must have a prescontext");
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@@ -370,16 +370,27 @@ public:
    * The search extends across document boundaries.
    *
    * @param  aFrame the frame to start with
    * @return the nearest scrollable frame or nullptr if not found
    */
   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
    * attached to it; returns false otherwise.
    *
    * @param aContent the content node we're looking at
    * @param aStyleContext aContent's style context
    * @param aPseudoElement the id of the pseudo style we care about
    * @param aPresContext the presentation context
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -3723,38 +3723,19 @@ nsGfxScrollFrameInner::GetScrolledRect()
                "Scrolled rect smaller than scrollport?");
   return result;
 }
 
 nsRect
 nsGfxScrollFrameInner::GetScrolledRectInternal(const nsRect& aScrolledFrameOverflowArea,
                                                const nsSize& aScrollPortSize) const
 {
-  nscoord x1 = aScrolledFrameOverflowArea.x,
-          x2 = aScrolledFrameOverflowArea.XMost(),
-          y1 = aScrolledFrameOverflowArea.y,
-          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);
+  return nsLayoutUtils::GetScrolledRect(mScrolledFrame,
+      aScrolledFrameOverflowArea, aScrollPortSize,
+      IsLTR() ? NS_STYLE_DIRECTION_LTR : NS_STYLE_DIRECTION_RTL);
 }
 
 nsMargin
 nsGfxScrollFrameInner::GetActualScrollbarSizes() const
 {
   nsRect r = mOuter->GetPaddingRect() - mOuter->GetPosition();
 
   return nsMargin(mScrollPort.x - r.x, mScrollPort.y - r.y,