Bug 106589 - [chromium] Use new-style gesture scrolling events for fling.
Summary: [chromium] Use new-style gesture scrolling events for fling.
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: Robert Kroeger
URL:
Keywords:
Depends on: 103952
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-10 11:47 PST by Robert Kroeger
Modified: 2013-01-18 11:08 PST (History)
4 users (show)

See Also:


Attachments
Patch (9.42 KB, patch)
2013-01-10 12:38 PST, Robert Kroeger
no flags Details | Formatted Diff | Diff
Patch (8.60 KB, patch)
2013-01-16 11:29 PST, Robert Kroeger
no flags Details | Formatted Diff | Diff
Patch (8.95 KB, patch)
2013-01-17 11:48 PST, Robert Kroeger
no flags Details | Formatted Diff | Diff
Patch (8.93 KB, patch)
2013-01-17 14:11 PST, Robert Kroeger
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Kroeger 2013-01-10 11:47:57 PST
https://bugs.webkit.org/show_bug.cgi?id=103952 brings new subtypes of PlatformGestureEvent that provide a superior scroll implementation. Use these from the chromium WebKit layer for flings.
Comment 1 Robert Kroeger 2013-01-10 12:38:51 PST
Created attachment 182192 [details]
Patch
Comment 2 Robert Kroeger 2013-01-16 11:29:59 PST
Created attachment 183012 [details]
Patch
Comment 3 James Robinson 2013-01-16 20:42:02 PST
Comment on attachment 183012 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183012&action=review

> Source/WebKit/chromium/src/WebPluginContainerImpl.cpp:790
> +    WebMouseWheelEventBuilder wheelEvent(this, m_element->renderer(), *event);

copying feedback from bug 103952 so it's here in context:

No - this isn't right.  We never want to synthesize mousewheels for plugins (I checked with cpu and brettw).  There are a few cases for plugins we care about in chromium:

Plugins that use the WebScrollbarGroup system (pdf, for instance).  Detect these by checking if m_scrollGroup is NULL.  If the m_scrollbarGroup is not null, we should handle the scroll directly in this file by adjusting the appropriate scrollbar offset(s).  That's all the pdf plugin actually ends up doing with mousewheel events.

Plugins that don't use this system - for these, we want to pass the touch events to the plugin but not pass any gesture or wheel events for scrolling.  Flash, for instance, never handles mousewheels.
Comment 4 Robert Kroeger 2013-01-17 11:48:14 PST
Created attachment 183233 [details]
Patch
Comment 5 Robert Kroeger 2013-01-17 11:51:06 PST
Comment on attachment 183233 [details]
Patch

I modified the patch to use scroll to scroll the plugin contents. Please take a look.
Comment 6 James Robinson 2013-01-17 12:40:24 PST
Comment on attachment 183233 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183233&action=review

> Source/WebKit/chromium/src/WebPluginContainerImpl.cpp:799
> +        ScrollbarGroup* scroller = scrollbarGroup();

if you read the ::scrollbarGroup() you'll see that it constructs a group if one isn't already there.  You don't want this function to create a new scrollbar group!  Just check m_scrollbarGroup
Comment 7 Robert Kroeger 2013-01-17 14:10:21 PST
Comment on attachment 183233 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183233&action=review

>> Source/WebKit/chromium/src/WebPluginContainerImpl.cpp:799
>> +        ScrollbarGroup* scroller = scrollbarGroup();
> 
> if you read the ::scrollbarGroup() you'll see that it constructs a group if one isn't already there.  You don't want this function to create a new scrollbar group!  Just check m_scrollbarGroup

Thanks. Done in p4.
Comment 8 Robert Kroeger 2013-01-17 14:11:47 PST
Created attachment 183275 [details]
Patch
Comment 9 Robert Kroeger 2013-01-17 14:12:17 PST
Comment on attachment 183275 [details]
Patch

Ptal.
Comment 10 James Robinson 2013-01-17 14:17:11 PST
Comment on attachment 183275 [details]
Patch

r=me
Comment 11 WebKit Review Bot 2013-01-18 11:08:16 PST
Comment on attachment 183275 [details]
Patch

Clearing flags on attachment: 183275

Committed r140183: <http://trac.webkit.org/changeset/140183>
Comment 12 WebKit Review Bot 2013-01-18 11:08:20 PST
All reviewed patches have been landed.  Closing bug.