Bug 107205 - REGRESSION(r137726): Spring Loaded Pan Scrolling doesn't stop
Summary: REGRESSION(r137726): Spring Loaded Pan Scrolling doesn't stop
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: yosin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-17 17:32 PST by yosin
Modified: 2013-01-18 00:24 PST (History)
3 users (show)

See Also:

Attachments
Patch 1 (15.76 KB, patch)
2013-01-17 21:07 PST, yosin 		no flags Details | Formatted Diff | Diff
Patch 2 (16.09 KB, patch)
2013-01-17 23:17 PST, yosin 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description yosin 2013-01-17 17:32:31 PST 
Import from http://crbug.com/167238

= Re-produce steps =
1. Open scrollable page.
2. Hold middle mouse button and move pointer around to start scrolling
3. Release middle mouse button

= Result =
Continue scrolling

= Expectation =
Stop scrolling when releaseing mouse.
Comment 1 yosin 2013-01-17 21:07:26 PST 
Created attachment 183364 [details]
Patch 1
Comment 2 yosin 2013-01-17 21:08:23 PST 
Comment on attachment 183364 [details]
Patch 1

Could you review this patch?
Thanks in advance.
Comment 3 WebKit Review Bot 2013-01-17 22:53:42 PST 
Comment on attachment 183364 [details]
Patch 1

Attachment 183364 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15937553

New failing tests:
platform/chromium-win/fast/events/panScroll-drag.html
platform/chromium-win/fast/events/panScroll-click.html
Comment 4 Hajime Morrita 2013-01-17 22:56:59 PST 
Comment on attachment 183364 [details]
Patch 1

An approach looks good. It's great to get rid of extra flags.

About state name.
MouseDown/MouseUp is unclear here AutoScrollController doesn't handle mouse event directly for pan scroll.
It looks we have existing AutoscrollForPan and an extra state which indicates AutoscrollForPan is finishing. 
Can we just add an enum for that instead of replace AutoScroll into two state?

Also, we need TestExpectations for non-windows platforms.
Comment 5 yosin 2013-01-17 23:17:55 PST 
Created attachment 183385 [details]
Patch 2
Comment 6 yosin 2013-01-17 23:19:50 PST 
Comment on attachment 183385 [details]
Patch 2

Could you review this patch?
Thanks in advance.

= Changes since the last review =
* Update platform/chromium/TestExpecation for skipping newly added pan scroll tests.
* Use AutoscrollForPanCanStop instead of AutoscrollForPan{Before,After}MouseUp.
Comment 7 WebKit Review Bot 2013-01-18 00:24:02 PST 
Comment on attachment 183385 [details]
Patch 2

Clearing flags on attachment: 183385

Committed r140104: <http://trac.webkit.org/changeset/140104>
Comment 8 WebKit Review Bot 2013-01-18 00:24:06 PST 
All reviewed patches have been landed.  Closing bug.