[Mac] Unable to scroll to bottom of nested scrollable areas
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Aug 2014 18:25:08 +0000 (18:25 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Aug 2014 18:25:08 +0000 (18:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=135637
<rdar://problem/17910241>

Reviewed by Zalan Bujtas.

Source/WebCore:

Test: platform/mac/fast/scrolling/scroll-latched-nested-div.html

Avoid truncating the fractional portion of scroll ranges.

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::updateScrollbarsAfterLayout): Round
the LayoutUnit values for scroll width and height rather than
truncating.

LayoutTests:

* platform/mac/fast/scrolling/scroll-latched-nested-div-expected.txt: Added.
* platform/mac/fast/scrolling/scroll-latched-nested-div.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/mac/fast/scrolling/scroll-latched-nested-div-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/fast/scrolling/scroll-latched-nested-div.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayer.cpp

index bd5a0a7..2559ab2 100644 (file)
@@ -1,3 +1,14 @@
+2014-08-06  Brent Fulgham  <bfulgham@apple.com>
+
+        [Mac] Unable to scroll to bottom of nested scrollable areas
+        https://bugs.webkit.org/show_bug.cgi?id=135637
+        <rdar://problem/17910241>
+
+        Reviewed by Zalan Bujtas.
+
+        * platform/mac/fast/scrolling/scroll-latched-nested-div-expected.txt: Added.
+        * platform/mac/fast/scrolling/scroll-latched-nested-div.html: Added.
+
 2014-08-06  Brian J. Burg  <burg@cs.washington.edu>
 
         Web Inspector: protocol command invocations should return a promise if no callback is supplied
diff --git a/LayoutTests/platform/mac/fast/scrolling/scroll-latched-nested-div-expected.txt b/LayoutTests/platform/mac/fast/scrolling/scroll-latched-nested-div-expected.txt
new file mode 100644 (file)
index 0000000..c5cd2fd
--- /dev/null
@@ -0,0 +1,66 @@
+Put mouse next to the table labeled 'Scrollable Region' and flick downward. The scroll movement should come to a halt. Then flick again. The overall page should scroll.
+Scrollable Region
+
+Count  DATA    Rev Count
+TOP TOP TOP TOP TOP    TOP TOP TOP TOP TOP     TOP TOP TOP TOP TOP
+1      0.1100  40
+2      0.1155  39
+3      0.2200  38
+4      0.2255  37
+5      0.3300  36
+6      0.3355  35
+7      0.4400  34
+8      0.4455  33
+9      0.5500  32
+10     0.5555  31
+11     0.6600  30
+12     0.6655  29
+13     0.7700  28
+14     0.7755  27
+15     0.8800  26
+16     0.8855  25
+17     0.9900  24
+18     0.9955  23
+19     0.9999  22
+20     1.0000  21
+21     1.0000  20
+22     0.9999  19
+23     0.9955  18
+24     0.9900  17
+25     0.8855  16
+26     0.8800  15
+27     0.7755  14
+28     0.7700  13
+29     0.6655  12
+30     0.6600  11
+31     0.5555  10
+32     0.5500  9
+33     0.4455  8
+34     0.4400  7
+35     0.3355  6
+36     0.3300  5
+37     0.2255  4
+38     0.2200  3
+39     0.1155  2
+40     0.1100  1
+END END END END END    END END END END END     END END END END END
+A set of information at the bottom of the table.
+
+Tests that a scrollable div nested inside another scrollable div properly handles wheel events under sub-pixel conditions.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+wrapperTarget = [object HTMLDivElement]
+First scroll event:
+PASS Page did not receive wheel events during the first gesture.
+PASS Wrapper received wheel events during the first gesture.
+PASS div did not receive wheel events during the first gesture.
+Second scroll event:
+PASS Page received wheel events during the second gesture.
+PASS Wrapper did not receive wheel events during the second gesture.
+PASS div did not receive wheel events during the second gesture.
+
diff --git a/LayoutTests/platform/mac/fast/scrolling/scroll-latched-nested-div.html b/LayoutTests/platform/mac/fast/scrolling/scroll-latched-nested-div.html
new file mode 100644 (file)
index 0000000..49a74d0
--- /dev/null
@@ -0,0 +1,224 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.scrollable_region {
+    width: 680px;
+}
+
+.table td, .table th {
+    padding: 2px;
+}
+
+.table th {
+    height: 20px;
+    text-align: left;
+    font-weight: strong;
+}
+
+.table tr:nth-child(odd) {
+    background: #f3f3f3;
+}
+
+.scrollable_select option:nth-child(odd) {
+    background: #f3f3f3;
+}
+
+</style>
+<script src="../../../../resources/js-test-pre.js"></script>
+<script>
+function onLoad() {
+    setupTopLevel();
+}
+</script>
+</head>
+<body onload="onLoad();">
+<script>
+
+var wrapperTarget;
+var divTarget;
+var pageScrollPositionBefore;
+var wrapperScrollPositionBefore;
+var divScrollPositionBefore;
+var continueCount = 5;
+
+function checkForSecondScroll() {
+    // 'parent' should have scrolled, the content of 'target' should
+    // not have scrolled, and 'wrapper' should not have scrolled.
+    var pageScrollPositionAfter = document.body.scrollTop;
+    var wrapperScrollPositionAfter = wrapperTarget.scrollTop;
+    var divScrollPositionAfter = divTarget.scrollTop;
+
+    debug("Second scroll event:");
+    if (pageScrollPositionBefore != pageScrollPositionAfter)
+        testPassed("Page received wheel events during the second gesture.");
+    else
+        testFailed("Page did not receive wheel events during the second gesture.");
+
+    if (wrapperScrollPositionBefore != wrapperScrollPositionAfter)
+        testFailed("Wrapper received wheel events during the second gesture.");
+    else
+        testPassed("Wrapper did not receive wheel events during the second gesture.");
+
+    if (divScrollPositionBefore != divScrollPositionAfter)
+        testFailed("div received wheel events during the second gesture.");
+    else
+        testPassed("div did not receive wheel events during the second gesture.");
+
+    testRunner.notifyDone();
+}
+
+function checkForFirstScroll() {
+    // 'parent' should not have scrolled, and the content of 'target' should
+    // not have moved. However, 'wrapper' should have moved.
+    var pageScrollPositionAfter = document.body.scrollTop;
+    var wrapperScrollPositionAfter = wrapperTarget.scrollTop;
+    var divScrollPositionAfter = divTarget.scrollTop;
+
+    debug("First scroll event:");
+    if (pageScrollPositionBefore != pageScrollPositionAfter)
+        testFailed("Page received wheel events during the first gesture.");
+    else
+        testPassed("Page did not receive wheel events during the first gesture.");
+
+    if (wrapperScrollPositionBefore != wrapperScrollPositionAfter)
+        testPassed("Wrapper received wheel events during the first gesture.");
+    else
+        testFailed("Wrapper did not receive wheel events during the first gesture.");
+
+    if (divScrollPositionBefore != divScrollPositionAfter)
+        testFailed("div received wheel events during the first gesture.");
+    else
+        testPassed("div did not receive wheel events during the first gesture.");
+
+    wrapperScrollPositionBefore = wrapperTarget.scrollTop;
+
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'began', 'none', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'ended', 'none', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'begin', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'none', 'end', true);
+    setTimeout(checkForSecondScroll, 100);
+}
+
+function scrollTest() {
+    // See where our IFrame lives:
+    pageScrollPositionBefore = document.body.scrollTop;
+
+    divTarget = document.getElementById('target');
+    divTarget.scrollTop = divTarget.scrollHeight - divTarget.clientHeight - 100;
+
+    divScrollPositionBefore = divTarget.scrollTop;
+
+    wrapperTarget = document.getElementById('wrapper');
+    wrapperScrollPositionBefore = wrapperTarget.scrollTop;
+    debug("wrapperTarget = " + wrapperTarget);
+
+    // Move mouse to a position to the right side of the table, and near the bottom.
+    var startPosX = divTarget.offsetLeft + divTarget.clientWidth + 50;
+    var startPosY = Math.round(divTarget.offsetTop) - 42; // One wheel turn before end.
+    eventSender.mouseMoveTo(startPosX, startPosY); // Make sure we are just outside the iFrame
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'began', 'none', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'ended', 'none', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -5, 'none', 'begin', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -5, 'none', 'continue', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -5, 'none', 'continue', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -5, 'none', 'continue', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -5, 'none', 'continue', true);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'none', 'end', true);
+    setTimeout(checkForFirstScroll, 100);
+}
+
+function setupTopLevel() {
+
+    if (window.eventSender) {
+        testRunner.waitUntilDone();
+
+        setTimeout(scrollTest, 1000);
+    } else {
+        var messageLocation = document.getElementById('parent');
+        var message = document.createElement('div');
+        message.innerHTML = "<p>This test is better run under DumpRenderTree. To manually test it, place the mouse pointer<br/>"
+            + "at the top of the page, and then use the mouse wheel or a two-finger swipe to scroll the<br/>"
+            + "down past the div.<br/><br/>"
+            + "You should not see the row of END labels if this test is successful.</p>";
+        messageLocation.appendChild(message);
+    }
+}
+
+</script>
+<div id="parent" style="height: 2000px; width: 800px;">
+    <div id="wrapper" style='overflow-y: auto; overflow-x: hidden; max-height: 600px;'>
+        <div id="source" style="height: 100px; width: 600px;">
+            Put mouse next to the table labeled 'Scrollable Region' and flick downward. The scroll movement
+            should come to a halt. Then flick again. The overall page should scroll.
+        </div>
+        <div class="scrollable_region">
+            <h3>Scrollable Region</h3>
+            <div id="target" style='overflow-y: auto; overflow-x: hidden; max-height: 485.55px;'>
+                <table class="table" style='width: 99%'>
+                    <tr><th>Count</th><th>DATA</th><th>Rev Count</th></tr>
+                    <tr><td>TOP TOP TOP TOP TOP</td><td>TOP TOP TOP TOP TOP</td><td>TOP TOP TOP TOP TOP</td></tr>
+                    <tr><td>1</td><td>0.1100</td><td>40</td></tr>
+                    <tr><td>2</td><td>0.1155</td><td>39</td></tr>
+                    <tr><td>3</td><td>0.2200</td><td>38</td></tr>
+                    <tr><td>4</td><td>0.2255</td><td>37</td></tr>
+                    <tr><td>5</td><td>0.3300</td><td>36</td></tr>
+                    <tr><td>6</td><td>0.3355</td><td>35</td></tr>
+                    <tr><td>7</td><td>0.4400</td><td>34</td></tr>
+                    <tr><td>8</td><td>0.4455</td><td>33</td></tr>
+                    <tr><td>9</td><td>0.5500</td><td>32</td></tr>
+                    <tr><td>10</td><td>0.5555</td><td>31</td></tr>
+                    <tr><td>11</td><td>0.6600</td><td>30</td></tr>
+                    <tr><td>12</td><td>0.6655</td><td>29</td></tr>
+                    <tr><td>13</td><td>0.7700</td><td>28</td></tr>
+                    <tr><td>14</td><td>0.7755</td><td>27</td></tr>
+                    <tr><td>15</td><td>0.8800</td><td>26</td></tr>
+                    <tr><td>16</td><td>0.8855</td><td>25</td></tr>
+                    <tr><td>17</td><td>0.9900</td><td>24</td></tr>
+                    <tr><td>18</td><td>0.9955</td><td>23</td></tr>
+                    <tr><td>19</td><td>0.9999</td><td>22</td></tr>
+                    <tr><td>20</td><td>1.0000</td><td>21</td></tr>
+                    <tr><td>21</td><td>1.0000</td><td>20</td></tr>
+                    <tr><td>22</td><td>0.9999</td><td>19</td></tr>
+                    <tr><td>23</td><td>0.9955</td><td>18</td></tr>
+                    <tr><td>24</td><td>0.9900</td><td>17</td></tr>
+                    <tr><td>25</td><td>0.8855</td><td>16</td></tr>
+                    <tr><td>26</td><td>0.8800</td><td>15</td></tr>
+                    <tr><td>27</td><td>0.7755</td><td>14</td></tr>
+                    <tr><td>28</td><td>0.7700</td><td>13</td></tr>
+                    <tr><td>29</td><td>0.6655</td><td>12</td></tr>
+                    <tr><td>30</td><td>0.6600</td><td>11</td></tr>
+                    <tr><td>31</td><td>0.5555</td><td>10</td></tr>
+                    <tr><td>32</td><td>0.5500</td><td>9</td></tr>
+                    <tr><td>33</td><td>0.4455</td><td>8</td></tr>
+                    <tr><td>34</td><td>0.4400</td><td>7</td></tr>
+                    <tr><td>35</td><td>0.3355</td><td>6</td></tr>
+                    <tr><td>36</td><td>0.3300</td><td>5</td></tr>
+                    <tr><td>37</td><td>0.2255</td><td>4</td></tr>
+                    <tr><td>38</td><td>0.2200</td><td>3</td></tr>
+                    <tr><td>39</td><td>0.1155</td><td>2</td></tr>
+                    <tr><td>40</td><td>0.1100</td><td>1</td></tr>
+                    <tr><td>END END END END END</td><td>END END END END END</td><td>END END END END END</td></tr>
+                </table>
+            </div>
+            <div id="bottom" style="height: 100px; width: 600px;">
+                <p>A set of information at the bottom of the table.</p>
+            </div>
+        </div>
+    </div>
+</div>
+<div id="console"></div>
+<script>
+description("Tests that a scrollable div nested inside another scrollable div properly handles wheel events under sub-pixel conditions.");
+</script>
+<script src="../../../../resources/js-test-post.js"></script>
+</body>
+</html>
index bb2a9c3..b9411d5 100644 (file)
@@ -1,3 +1,20 @@
+2014-08-05  Brent Fulgham  <bfulgham@apple.com>
+
+        [Mac] Unable to scroll to bottom of nested scrollable areas
+        https://bugs.webkit.org/show_bug.cgi?id=135637
+        <rdar://problem/17910241>
+
+        Reviewed by Zalan Bujtas.
+
+        Test: platform/mac/fast/scrolling/scroll-latched-nested-div.html
+
+        Avoid truncating the fractional portion of scroll ranges.
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::updateScrollbarsAfterLayout): Round
+        the LayoutUnit values for scroll width and height rather than
+        truncating.
+
 2014-08-06  Andy Estes  <aestes@apple.com>
 
         [iOS] QuickLook returns an invalid MIME type for some documents
index 49f2abc..1bec8f2 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2014 Apple Inc. All rights reserved.
  *
  * Portions are Copyright (C) 1998 Netscape Communications Corporation.
  *
@@ -3245,13 +3245,13 @@ void RenderLayer::updateScrollbarsAfterLayout()
         int clientWidth = box->pixelSnappedClientWidth();
         int pageStep = std::max(std::max<int>(clientWidth * Scrollbar::minFractionToStepWhenPaging(), clientWidth - Scrollbar::maxOverlapBetweenPages()), 1);
         m_hBar->setSteps(Scrollbar::pixelsPerLineStep(), pageStep);
-        m_hBar->setProportion(clientWidth, m_scrollSize.width());
+        m_hBar->setProportion(clientWidth, m_scrollSize.width().round());
     }
     if (m_vBar) {
         int clientHeight = box->pixelSnappedClientHeight();
         int pageStep = std::max(std::max<int>(clientHeight * Scrollbar::minFractionToStepWhenPaging(), clientHeight - Scrollbar::maxOverlapBetweenPages()), 1);
         m_vBar->setSteps(Scrollbar::pixelsPerLineStep(), pageStep);
-        m_vBar->setProportion(clientHeight, m_scrollSize.height());
+        m_vBar->setProportion(clientHeight, m_scrollSize.height().round());
     }
 
     updateScrollableAreaSet(hasScrollableHorizontalOverflow() || hasScrollableVerticalOverflow());