| Summary: | backward mouse wheeling does not work when scroll position is below 0 | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Xiaomei Ji <xji> | ||||||
| Component: | Layout and Rendering | Assignee: | Xiaomei Ji <xji> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | hyatt, pkasting, playmobil, simon.fraser, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
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... We have a minimumScrollPosition Peter. I think the patch should have used that instead of explicitly using the origin. 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.
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).
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.
the style-checking error is bogus. There is no style error when I run style checking locally. Comment on attachment 75744 [details]
patch w/ layout test
Looks great. Thanks for testing vertical too!
r=me
Committed r73529: <http://trac.webkit.org/changeset/73529> I can no longer wheel-scroll upwards in WebKit2 windows on TOT. It may be related to this patch. I filed bug 50729. |
=================================================================== --- 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());