Incorrect offset used for scrollWidth/Height calculation
authoreae@chromium.org <eae@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Jul 2012 18:22:09 +0000 (18:22 +0000)
committereae@chromium.org <eae@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Jul 2012 18:22:09 +0000 (18:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=91461

Reviewed by Eric Seidel.

Source/WebCore:

Due to a different offset being used to calculate the scrollWidth/Height
and pixelSnappedClientWidth/Height the scroll value can be off by one in
same cases. This can causes scrollbars to appear even when there is no
overflow.

Test: fast/sub-pixel/block-with-margin-overflow.html

* rendering/RenderBox.cpp:
(WebCore::RenderBox::scrollWidth):
Change location offset passed to snapSizeToPixel to include x() to match
offset used by pixelSnappedClientWidth.

(WebCore::RenderBox::scrollHeight):
Change location offset passed to snapSizeToPixel to include y() to match
offset used by pixelSnappedClientHeight.

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::clampScrollOffset):
Change calculation to use pixelSnappedClientWidth/Height as it is
subtracted from the pixel snapped scrollWidth/Height values.

(WebCore::RenderLayer::scrollWidth):
(WebCore::RenderLayer::scrollHeight):
Change RenderLayer versions of scrollWidth/Height to include x()/y() as
per the RenderBox versions.

LayoutTests:

Add new test ensuring that a block that shouldn't have overflow doesn't
have scrollbars.

* fast/sub-pixel/block-with-margin-overflow-expected.html: Added.
* fast/sub-pixel/block-with-margin-overflow.html: Added.
* platform/chromium-win/fast/block/float/026-expected.txt:
* platform/chromium-win/fast/block/float/028-expected.txt:
* platform/chromium-win/fast/block/float/overhanging-tall-block-expected.txt:
* platform/mac/fast/multicol/shrink-to-column-height-for-pagination-expected.png:
* platform/mac/fast/multicol/shrink-to-column-height-for-pagination-expected.txt:
Update test expectations that incorrectly had overflow.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@122861 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/sub-pixel/block-with-margin-overflow-expected.html [new file with mode: 0644]
LayoutTests/fast/sub-pixel/block-with-margin-overflow.html [new file with mode: 0644]
LayoutTests/platform/chromium-win/fast/block/float/026-expected.txt
LayoutTests/platform/chromium-win/fast/block/float/028-expected.txt
LayoutTests/platform/chromium-win/fast/block/float/overhanging-tall-block-expected.txt
LayoutTests/platform/mac/fast/multicol/shrink-to-column-height-for-pagination-expected.png
LayoutTests/platform/mac/fast/multicol/shrink-to-column-height-for-pagination-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderLayer.cpp

index f2b6e3e..99ac0ee 100644 (file)
@@ -1,3 +1,22 @@
+2012-07-17  Emil A Eklund  <eae@chromium.org>
+
+        Incorrect offset used for scrollWidth/Height calculation
+        https://bugs.webkit.org/show_bug.cgi?id=91461
+
+        Reviewed by Eric Seidel.
+
+        Add new test ensuring that a block that shouldn't have overflow doesn't
+        have scrollbars.
+
+        * fast/sub-pixel/block-with-margin-overflow-expected.html: Added.
+        * fast/sub-pixel/block-with-margin-overflow.html: Added.
+        * platform/chromium-win/fast/block/float/026-expected.txt:
+        * platform/chromium-win/fast/block/float/028-expected.txt:
+        * platform/chromium-win/fast/block/float/overhanging-tall-block-expected.txt:
+        * platform/mac/fast/multicol/shrink-to-column-height-for-pagination-expected.png:
+        * platform/mac/fast/multicol/shrink-to-column-height-for-pagination-expected.txt:
+        Update test expectations that incorrectly had overflow.
+
 2012-07-17  Tony Chang  <tony@chromium.org>
 
         [chromium] Unreviewed. Remove duplicate lines added in r122857.
diff --git a/LayoutTests/fast/sub-pixel/block-with-margin-overflow-expected.html b/LayoutTests/fast/sub-pixel/block-with-margin-overflow-expected.html
new file mode 100644 (file)
index 0000000..bc173d1
--- /dev/null
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <style>
+            .block {
+                overflow: auto;
+                margin: 0 1px;
+            }
+        </style>
+    </head>
+    <body>
+        <div class="block">
+            Should neither overflow nor display a scrollbar.
+        </div>
+    </body>
+</html>
+
diff --git a/LayoutTests/fast/sub-pixel/block-with-margin-overflow.html b/LayoutTests/fast/sub-pixel/block-with-margin-overflow.html
new file mode 100644 (file)
index 0000000..69b2a03
--- /dev/null
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <style>
+            .block {
+                overflow: auto;
+                margin: 0 0.6px;
+            }
+        </style>
+    </head>
+    <body>
+        <div class="block">
+            Should neither overflow nor display a scrollbar.
+        </div>
+    </body>
+</html>
+
index 1a6fd8a..728fbff 100644 (file)
@@ -42,7 +42,7 @@ layer at (8,208) size 569x59 clip at (25,210) size 550x40
       text run at (39,2) width 513: "This is an overflow:scroll region. It should sit on the same line as the float and wrap as"
       text run at (290,22) width 4 RTL: "."
       text run at (294,22) width 258: "needed to fit within the remaining line width"
-layer at (189,816) size 388x79 clip at (206,818) size 369x60 scrollWidth 370
+layer at (189,816) size 388x79 clip at (206,818) size 369x60
   RenderBlock {DIV} at (180,0) size 389x79 [border: (2px solid #0000FF)]
     RenderText {#text} at (5,2) size 368x59
       text run at (5,2) width 366: "This is an overflow:scroll region. It should sit on the same line"
index 1a6fd8a..728fbff 100644 (file)
@@ -42,7 +42,7 @@ layer at (8,208) size 569x59 clip at (25,210) size 550x40
       text run at (39,2) width 513: "This is an overflow:scroll region. It should sit on the same line as the float and wrap as"
       text run at (290,22) width 4 RTL: "."
       text run at (294,22) width 258: "needed to fit within the remaining line width"
-layer at (189,816) size 388x79 clip at (206,818) size 369x60 scrollWidth 370
+layer at (189,816) size 388x79 clip at (206,818) size 369x60
   RenderBlock {DIV} at (180,0) size 389x79 [border: (2px solid #0000FF)]
     RenderText {#text} at (5,2) size 368x59
       text run at (5,2) width 366: "This is an overflow:scroll region. It should sit on the same line"
index cda4179..6c79241 100644 (file)
@@ -9,6 +9,6 @@ layer at (0,0) size 800x25178684 backgroundClip at (0,0) size 800x17895697 clip
 layer at (10,10) size 179x25178662 backgroundClip at (10,10) size 179x17895687 clip at (11,11) size 177x17895686 outlineClip at (0,0) size 800x17895697
   RenderTextControl {TEXTAREA} at (2,1) size 179x25178664 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
     RenderBlock {DIV} at (3,3) size 175x16
-layer at (10,-21225448) size 179x25178662 backgroundClip at (10,0) size 179x3953214 clip at (11,0) size 162x3953213 outlineClip at (0,0) size 800x17895697 scrollHeight 25178660
+layer at (10,-21225448) size 179x25178662 backgroundClip at (10,0) size 179x3953214 clip at (11,0) size 177x3953213 outlineClip at (0,0) size 800x17895697
   RenderTextControl {TEXTAREA} at (2,25178664) size 179x-46404128 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
-    RenderBlock {DIV} at (3,3) size 160x16
+    RenderBlock {DIV} at (3,3) size 175x16
index 4c38cf8..5f4bc10 100644 (file)
Binary files a/LayoutTests/platform/mac/fast/multicol/shrink-to-column-height-for-pagination-expected.png and b/LayoutTests/platform/mac/fast/multicol/shrink-to-column-height-for-pagination-expected.png differ
index 7b7f89d..c93a3f3 100644 (file)
@@ -6,4 +6,3 @@ layer at (-400,0) size 800x600 backgroundClip at (0,0) size 800x600 clip at (0,0
       RenderBlock {DIV} at (0,0) size 400x600
         RenderImage {IMG} at (392,0) size 400x600
         RenderText {#text} at (0,0) size 0x0
-scrolled to -385,0
index 0f20ac7..d8715dc 100644 (file)
@@ -1,3 +1,36 @@
+2012-07-17  Emil A Eklund  <eae@chromium.org>
+
+        Incorrect offset used for scrollWidth/Height calculation
+        https://bugs.webkit.org/show_bug.cgi?id=91461
+
+        Reviewed by Eric Seidel.
+
+        Due to a different offset being used to calculate the scrollWidth/Height
+        and pixelSnappedClientWidth/Height the scroll value can be off by one in
+        same cases. This can causes scrollbars to appear even when there is no
+        overflow.
+
+        Test: fast/sub-pixel/block-with-margin-overflow.html
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::scrollWidth):
+        Change location offset passed to snapSizeToPixel to include x() to match
+        offset used by pixelSnappedClientWidth.
+        
+        (WebCore::RenderBox::scrollHeight):
+        Change location offset passed to snapSizeToPixel to include y() to match
+        offset used by pixelSnappedClientHeight.
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::clampScrollOffset):
+        Change calculation to use pixelSnappedClientWidth/Height as it is
+        subtracted from the pixel snapped scrollWidth/Height values.
+        
+        (WebCore::RenderLayer::scrollWidth):
+        (WebCore::RenderLayer::scrollHeight):
+        Change RenderLayer versions of scrollWidth/Height to include x()/y() as
+        per the RenderBox versions.
+
 2012-07-17  Hans Muller  <hmuller@adobe.com>
 
         Rename CSS Exclusions CSSWrapShape class properties to match Exclusion shape function parameters
index 2f2ab24..131ebca 100644 (file)
@@ -385,7 +385,7 @@ int RenderBox::scrollWidth() const
     // For objects with visible overflow, this matches IE.
     // FIXME: Need to work right with writing modes.
     if (style()->isLeftToRightDirection())
-        return snapSizeToPixel(max(clientWidth(), layoutOverflowRect().maxX() - borderLeft()), clientLeft());
+        return snapSizeToPixel(max(clientWidth(), layoutOverflowRect().maxX() - borderLeft()), x() + clientLeft());
     return clientWidth() - min(ZERO_LAYOUT_UNIT, layoutOverflowRect().x() - borderLeft());
 }
 
@@ -395,7 +395,7 @@ int RenderBox::scrollHeight() const
         return layer()->scrollHeight();
     // For objects with visible overflow, this matches IE.
     // FIXME: Need to work right with writing modes.
-    return snapSizeToPixel(max(clientHeight(), layoutOverflowRect().maxY() - borderTop()), clientTop());
+    return snapSizeToPixel(max(clientHeight(), layoutOverflowRect().maxY() - borderTop()), y() + clientTop());
 }
 
 int RenderBox::scrollLeft() const
index a59a1b3..441c0be 100644 (file)
@@ -1619,8 +1619,8 @@ IntSize RenderLayer::clampScrollOffset(const IntSize& scrollOffset) const
     RenderBox* box = renderBox();
     ASSERT(box);
 
-    int maxX = scrollWidth() - box->clientWidth();
-    int maxY = scrollHeight() - box->clientHeight();
+    int maxX = scrollWidth() - box->pixelSnappedClientWidth();
+    int maxY = scrollHeight() - box->pixelSnappedClientHeight();
 
     int x = min(max(scrollOffset.width(), 0), maxX);
     int y = min(max(scrollOffset.height(), 0), maxY);
@@ -2442,7 +2442,7 @@ int RenderLayer::scrollWidth() const
     ASSERT(renderBox());
     if (m_scrollDimensionsDirty)
         const_cast<RenderLayer*>(this)->computeScrollDimensions();
-    return snapSizeToPixel(m_scrollSize.width(), renderBox()->clientLeft());
+    return snapSizeToPixel(m_scrollSize.width(), renderBox()->clientLeft() + renderBox()->x());
 }
 
 int RenderLayer::scrollHeight() const
@@ -2450,7 +2450,7 @@ int RenderLayer::scrollHeight() const
     ASSERT(renderBox());
     if (m_scrollDimensionsDirty)
         const_cast<RenderLayer*>(this)->computeScrollDimensions();
-    return snapSizeToPixel(m_scrollSize.height(), renderBox()->clientTop());
+    return snapSizeToPixel(m_scrollSize.height(), renderBox()->clientTop() + renderBox()->y());
 }
 
 LayoutUnit RenderLayer::overflowTop() const