REGRESSION(r120832): RenderLayer::clampScrollOffset doesn't properly clamp
authorjchaffraix@webkit.org <jchaffraix@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Sep 2012 23:39:00 +0000 (23:39 +0000)
committerjchaffraix@webkit.org <jchaffraix@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Sep 2012 23:39:00 +0000 (23:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=95776

Reviewed by Simon Fraser.

.:

* ManualTests/select-menu-list-wrongly-positioned.html: Added.

Source/WebCore:

r120832 consolidated the clamping logic into RenderLayer::clampScrollOffset. The existing code wouldn't properly ensure that
the offset were positive which got exposed to other code paths, leading to the regression.

Tested by ManualTests/select-menu-list-wrongly-positioned.html as I didn't find a way to create a reliable layout test.

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::clampScrollOffset):
Fixed the clamping logic to ensure that the scroll offset's dimensions are positive.

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

ChangeLog
ManualTests/select-menu-list-wrongly-positioned.html [new file with mode: 0755]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayer.cpp

index 1171c39..5982b82 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2012-09-04  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        REGRESSION(r120832): RenderLayer::clampScrollOffset doesn't properly clamp
+        https://bugs.webkit.org/show_bug.cgi?id=95776
+
+        Reviewed by Simon Fraser.
+
+        * ManualTests/select-menu-list-wrongly-positioned.html: Added.
+
 2012-09-04  Michał Pakuła vel Rutka  <m.pakula@samsung.com>
 
         [EFL] Context menu restore.
diff --git a/ManualTests/select-menu-list-wrongly-positioned.html b/ManualTests/select-menu-list-wrongly-positioned.html
new file mode 100755 (executable)
index 0000000..9f1392a
--- /dev/null
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    table {
+        overflow: hidden;
+    }
+</style>
+<script>
+    function onSelectionChange(select)
+    {
+        document.getElementById('hiddenRow').style.display = '';
+    }
+</script>
+</head>
+<body>
+    <div><a href="https://bugs.webkit.org/show_bug.cgi?id=95776">95776</a>: REGRESSION(r120832): RenderLayer::clampScrollOffset doesn't properly clamp</div>
+    <div>Manual test: click on the menu below and &lt;select&gt; the 'shown' option. Click somewhere on the page so that the &lt;select&gt; loses focus and click again on the &lt;select&gt;.</div>
+    <div>To pass the menu should be properly placed below the &lt;select&gt;.</div>
+    <form name="teste">
+        <table>
+        <tr>
+            <th>Test select:</th>
+            <td>
+            <select onchange=onSelectionChange(this)>
+                <option value="0">hidden</option>
+                <option value="1">shown</option>
+            </select>
+            </td>
+        </tr>
+        <tr id="hiddenRow" style="display: none;">
+            <th>Lorem ipsum</th>
+            <td>dolor sic amet.</td>
+        </tr>
+        </table>
+    </form>
+</body>
+</html>
index d3de75d..614af0f 100644 (file)
@@ -1,3 +1,19 @@
+2012-09-04  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        REGRESSION(r120832): RenderLayer::clampScrollOffset doesn't properly clamp
+        https://bugs.webkit.org/show_bug.cgi?id=95776
+
+        Reviewed by Simon Fraser.
+
+        r120832 consolidated the clamping logic into RenderLayer::clampScrollOffset. The existing code wouldn't properly ensure that
+        the offset were positive which got exposed to other code paths, leading to the regression.
+
+        Tested by ManualTests/select-menu-list-wrongly-positioned.html as I didn't find a way to create a reliable layout test.
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::clampScrollOffset):
+        Fixed the clamping logic to ensure that the scroll offset's dimensions are positive.
+
 2012-09-04  Joshua Bell  <jsbell@chromium.org>
 
         IndexedDB: IDBRequest leaks if IDBCursor closes and no further events fired
index c14790a..da70d75 100644 (file)
@@ -1665,8 +1665,8 @@ IntSize RenderLayer::clampScrollOffset(const IntSize& scrollOffset) const
     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);
+    int x = max(min(scrollOffset.width(), maxX), 0);
+    int y = max(min(scrollOffset.height(), maxY), 0);
     return IntSize(x, y);
 }