- Revision
- 182191
- Author
- [email protected]
- Date
- 2015-03-31 11:26:16 -0700 (Tue, 31 Mar 2015)
Log Message
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.
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (182190 => 182191)
--- trunk/LayoutTests/ChangeLog 2015-03-31 18:22:32 UTC (rev 182190)
+++ trunk/LayoutTests/ChangeLog 2015-03-31 18:26:16 UTC (rev 182191)
@@ -1,3 +1,18 @@
+2015-03-31 Beth Dakin <[email protected]>
+
+ 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 <[email protected]>
[ES6] Object type restrictions on a first parameter of several Object.* functions are relaxed
Added: trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content-expected.txt (0 => 182191)
--- trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content-expected.txt 2015-03-31 18:26:16 UTC (rev 182191)
@@ -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.
Added: trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content.html (0 => 182191)
--- trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content.html (rev 0)
+++ trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-reduced-content.html 2015-03-31 18:26:16 UTC (rev 182191)
@@ -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=""
+<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>
+
Added: trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events-expected.txt (0 => 182191)
--- trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events-expected.txt 2015-03-31 18:26:16 UTC (rev 182191)
@@ -0,0 +1,2 @@
+PASS Wheel events with delta of zero were not sent to the DOM.
+
Added: trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events.html (0 => 182191)
--- trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events.html (rev 0)
+++ trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/overflow-scroll-zero-delta-wheel-events.html 2015-03-31 18:26:16 UTC (rev 182191)
@@ -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=""
+<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>
+
Modified: trunk/Source/WebCore/ChangeLog (182190 => 182191)
--- trunk/Source/WebCore/ChangeLog 2015-03-31 18:22:32 UTC (rev 182190)
+++ trunk/Source/WebCore/ChangeLog 2015-03-31 18:26:16 UTC (rev 182191)
@@ -1,3 +1,30 @@
+2015-03-31 Beth Dakin <[email protected]>
+
+ 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 <[email protected]>
[Win] Unreviewed debug build fix after r182186.
Modified: trunk/Source/WebCore/dom/Element.cpp (182190 => 182191)
--- trunk/Source/WebCore/dom/Element.cpp 2015-03-31 18:22:32 UTC (rev 182190)
+++ trunk/Source/WebCore/dom/Element.cpp 2015-03-31 18:26:16 UTC (rev 182191)
@@ -281,10 +281,16 @@
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();
}
