Summary: | REGRESSION(r137726): Spring Loaded Pan Scrolling doesn't stop | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | yosin | ||||||
Component: | UI Events | Assignee: | yosin | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | dglazkov, morrita, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
yosin
2013-01-17 17:32:31 PST
Created attachment 183364 [details]
Patch 1
Comment on attachment 183364 [details]
Patch 1
Could you review this patch?
Thanks in advance.
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 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.
Created attachment 183385 [details]
Patch 2
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 on attachment 183385 [details] Patch 2 Clearing flags on attachment: 183385 Committed r140104: <http://trac.webkit.org/changeset/140104> All reviewed patches have been landed. Closing bug. |