Bug 50370 – backward mouse wheeling does not work when scroll position is below 0

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Xiaomei Ji]

===================================================================
--- ScrollView.cpp	(revision 73075)
+++ ScrollView.cpp	(working copy)
@@ -731,9 +731,9 @@
     float deltaY = m_verticalScrollbar ? e.deltaY() : 0;
     IntSize maxScrollDelta = maximumScrollPosition() - scrollPosition();
     if ((deltaX < 0 && maxScrollDelta.width() > 0)
-        || (deltaX > 0 && scrollOffset().width() > 0)
+        || (deltaX > 0 && scrollPosition().x() > -m_scrollOrigin.x())
         || (deltaY < 0 && maxScrollDelta.height() > 0)
-        || (deltaY > 0 && scrollOffset().height() > 0)) {
+        || (deltaY > 0 && scrollPosition().y() > -m_scrollOrigin.y())) {
         e.accept();
         if (e.granularity() == ScrollByPageWheelEvent) {
             ASSERT(!e.deltaX());

[Peter Kasting]

This fix works, but conceptually it's a little weird.

It might be nice to make maximumScrollPosition() return negative values for RTL (or vertical writing mode, for y-coords), so that it works in the same coordinate space that scrollPosition() is in.  At that point, the bounds are always (0,0) and maximumScrollPosition(), the only difference is which one is "left" and which is "right".  That would result in code that, IMO, would be significantly clearer to read.  It might make other parts of ScrollView that rely on maximumScrollPosition() saner, too...

[Dave Hyatt]

We have a minimumScrollPosition Peter.  I think the patch should have used that instead of explicitly using the origin.

[Xiaomei Ji]

Created attachment 75596 [details]
patch w/ layout test

Something is wrong with Mac port (maybe mouseScrollByX in EventSendingController.mm under DRT): backward scrolling below 0 works fine manually, but the tests fails. Need further investigation.

[Xiaomei Ji]

Created attachment 75744 [details]
patch w/ layout test

Looks like you have to do "eventSender.mouseMoveTo()" before "eventSender.continuousMouseScrollBy()" to make the mouseScrollBy() work in Mac port. (mouseScrollByX in EventSendingController.mm needs lastMousePosition).

[WebKit Review Bot]

Attachment 75744 [details] did not pass style-queue:

Failed to run "[u'git', u'reset', u'--hard', u'HEAD']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'HEAD'.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Xiaomei Ji]

the style-checking error is bogus. There is no style error when I run style checking locally.

[Dave Hyatt]

Comment on attachment 75744 [details]
patch w/ layout test

Looks great.  Thanks for testing vertical too!

r=me

[Xiaomei Ji]

Committed r73529: <http://trac.webkit.org/changeset/73529>

[Simon Fraser (smfr)]

I can no longer wheel-scroll upwards in WebKit2 windows on TOT. It may be related to this patch.

[Simon Fraser (smfr)]

I filed bug 50729.