REGRESSION (r173484): Reducing content of scrollable region does not reset scroll
authorbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 31 Mar 2015 18:26:16 +0000 (18:26 +0000)
committerbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 31 Mar 2015 18:26:16 +0000 (18:26 +0000)
position
https://bugs.webkit.org/show_bug.cgi?id=138525
-and corresponding-
rdar://problem/18166043

Reviewed by Simon Fraser.

Source/WebCore:

The change that caused this regression was correct. That change does not allow
RenderLayer to update scroll position after a layout if a rubber-band is currently
happening. The change caused this regression because all of the member variables
in ScrollController that attempt to keep track of the current state of the scroll
gesture (m_inScrollGesture, m_momentumScrollInProgress, and
m_snapRubberbandTimerIsActive) all indicated that a momentum scroll gesture was
still in action for this div even though it very much is not when the bug happens.
Those variables were never properly re-set because the
PlatformWheelEventPhaseEnded events never got dispatched to the ScrollController,
which brought the investigation back to Element.

We must still dispatch events that have zero delta so that the default event
handlers can handle them, but we should stopPropagation() so that these events are
not sent to the DOM. Websites will break if they get wheel events with no delta.
* dom/Element.cpp:
(WebCore::Element::dispatchWheelEvent):

LayoutTests:

* platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content-expected.txt: Added.
* platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content.html: Added.
* platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events-expected.txt: Added.
* platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content.html [new file with mode: 0644]
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp

index 2f4be24..f898c8d 100644 (file)
@@ -1,3 +1,18 @@
+2015-03-31  Beth Dakin  <bdakin@apple.com>
+
+        REGRESSION (r173484): Reducing content of scrollable region does not reset scroll 
+        position
+        https://bugs.webkit.org/show_bug.cgi?id=138525
+        -and corresponding-
+        rdar://problem/18166043
+
+        Reviewed by Simon Fraser.
+
+        * platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content-expected.txt: Added.
+        * platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content.html: Added.
+        * platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events-expected.txt: Added.
+        * platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events.html: Added.
+
 2015-03-31  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [ES6] Object type restrictions on a first parameter of several Object.* functions are relaxed
diff --git a/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content-expected.txt b/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content-expected.txt
new file mode 100644 (file)
index 0000000..1b34305
--- /dev/null
@@ -0,0 +1,2 @@
+PASS Re-sizing the content of the scrolled div correctly set a new scroll position.
+This test should be run in the test harness.
diff --git a/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content.html b/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content.html
new file mode 100644 (file)
index 0000000..66d773f
--- /dev/null
@@ -0,0 +1,86 @@
+<html>
+<head>
+<style>
+.outer {
+    position: relative;
+    margin: 100px;
+    height: 400px;
+    width: 200px;
+    border: 1px solid blue;
+}
+
+#inner {
+    position: absolute;
+    top: 0;
+    left: 0;
+    right: 0;
+    bottom: 0;
+
+    overflow-x: hidden;
+    overflow-y: auto;    
+}
+
+.big {
+    height: 2000px;
+}
+</style>
+
+<script src="../../../../resources/js-test-pre.js"></script>
+<script>
+
+function decreaseContentSize()
+{
+    var content = document.getElementById('content');
+    content.classList.remove("big");
+    internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
+
+    var divTarget = document.getElementById('inner');
+    var divScrollPos = divTarget.scrollTop;
+    if (divScrollPos == 0)
+        testPassed("Re-sizing the content of the scrolled div correctly set a new scroll position.");
+    else
+        testFailed("Re-sizing the content of the scrolled div failed to correctly set a new scroll position. ");
+
+    testRunner.notifyDone();
+}
+
+function scrollTest()
+{
+    var startPosX = 150;
+    var startPosY = 150;
+    eventSender.mouseMoveTo(startPosX, startPosY);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'began', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'ended', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'begin');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'none', 'end');
+
+    setTimeout(decreaseContentSize, 100);
+}
+
+function setUp() {
+    if (window.eventSender) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+
+        setTimeout(scrollTest, 0);
+    }
+}
+</script>
+</head>
+
+<body onload="setUp();">
+
+<div class="outer">
+    <div id="inner">
+        <div id="content" class="big">This test should be run in the test harness.</div>
+    </div>
+</div>
+</body>
+</html>
+
diff --git a/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events-expected.txt b/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events-expected.txt
new file mode 100644 (file)
index 0000000..f7edf71
--- /dev/null
@@ -0,0 +1,2 @@
+PASS Wheel events with delta of zero were not sent to the DOM.
+
diff --git a/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events.html b/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events.html
new file mode 100644 (file)
index 0000000..7f41e22
--- /dev/null
@@ -0,0 +1,78 @@
+<html>
+<head>
+<style>
+#scrolly {
+    height: 400px;
+    width: 200px;
+    border: 1px solid blue;
+
+    overflow-x: hidden;
+    overflow-y: auto;
+}
+
+#content {
+    height: 2000px;
+}
+</style>
+
+<script src="../../../../resources/js-test-pre.js"></script>
+<script>
+
+var deltaOfZero = false;
+
+function checkForZero()
+{
+    if (deltaOfZero)
+        testFailed("Wheel events with zero delta were sent to the DOM. ");
+    else
+        testPassed("Wheel events with delta of zero were not sent to the DOM.");
+
+    testRunner.notifyDone();
+}
+
+function didScroll(event) {
+    if (event.wheelDeltaX == 0 && event.wheelDeltaY == 0)
+        deltaOfZero = true;
+}
+
+function scrollTest()
+{
+    var startPosX = 100;
+    var startPosY = 100;
+    eventSender.mouseMoveTo(startPosX, startPosY);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'began', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'ended', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'begin');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'none', 'end');
+
+    setTimeout(checkForZero, 0);
+}
+
+function setUp() {
+    var scrolly = document.getElementById("scrolly");
+    scrolly.addEventListener("mousewheel", didScroll);
+
+    if (window.eventSender) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+
+        setTimeout(scrollTest, 0);
+    }
+}
+</script>
+</head>
+
+<body onload="setUp();">
+
+<div id="scrolly">
+    <div id="content"></div>
+</div>
+</body>
+</html>
+
index 55d32ff..d571404 100644 (file)
@@ -1,3 +1,30 @@
+2015-03-31  Beth Dakin  <bdakin@apple.com>
+
+        REGRESSION (r173484): Reducing content of scrollable region does not reset scroll 
+        position
+        https://bugs.webkit.org/show_bug.cgi?id=138525
+        -and corresponding-
+        rdar://problem/18166043
+
+        Reviewed by Simon Fraser.
+
+        The change that caused this regression was correct. That change does not allow 
+        RenderLayer to update scroll position after a layout if a rubber-band is currently 
+        happening. The change caused this regression because all of the member variables 
+        in ScrollController that attempt to keep track of the current state of the scroll 
+        gesture (m_inScrollGesture, m_momentumScrollInProgress, and 
+        m_snapRubberbandTimerIsActive) all indicated that a momentum scroll gesture was 
+        still in action for this div even though it very much is not when the bug happens. 
+        Those variables were never properly re-set because the 
+        PlatformWheelEventPhaseEnded events never got dispatched to the ScrollController, 
+        which brought the investigation back to Element.
+
+        We must still dispatch events that have zero delta so that the default event 
+        handlers can handle them, but we should stopPropagation() so that these events are 
+        not sent to the DOM. Websites will break if they get wheel events with no delta.
+        * dom/Element.cpp:
+        (WebCore::Element::dispatchWheelEvent):
+
 2015-03-31  Alex Christensen  <achristensen@webkit.org>
 
         [Win] Unreviewed debug build fix after r182186.
index 8c33e8f..1ed7360 100644 (file)
@@ -281,10 +281,16 @@ bool Element::dispatchMouseEvent(const PlatformMouseEvent& platformEvent, const
 
 bool Element::dispatchWheelEvent(const PlatformWheelEvent& event)
 {
+    RefPtr<WheelEvent> wheelEvent = WheelEvent::create(event, document().defaultView());
+
+    // Events with no deltas are important because they convey platform information about scroll gestures
+    // and momentum beginning or ending. However, those events should not be sent to the DOM since some
+    // websites will break. They need to be dispatched because dispatching them will call into the default
+    // event handler, and our platform code will correctly handle the phase changes. Calling stopPropogation()
+    // will prevent the event from being sent to the DOM, but will still call the default event handler.
     if (!event.deltaX() && !event.deltaY())
-        return true;
+        wheelEvent->stopPropagation();
 
-    RefPtr<WheelEvent> wheelEvent = WheelEvent::create(event, document().defaultView());
     return EventDispatcher::dispatchEvent(this, wheelEvent) && !wheelEvent->defaultHandled();
 }