Bug 106589

Summary: [chromium] Use new-style gesture scrolling events for fling.
Product: WebKit Reporter: Robert Kroeger <rjkroege>
Component: UI EventsAssignee: Robert Kroeger <rjkroege>
Status: RESOLVED FIXED    
Severity: Normal CC: jamesr, rjkroege, tdanderson, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 103952    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

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.