Bug 106589 – [chromium] Use new-style gesture scrolling events for fling.

• Status: RESOLVED
• Resolution: FIXED
• Component: UI Events

[Robert Kroeger]

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.

[Robert Kroeger]

Created attachment 182192 [details]
Patch

[Robert Kroeger]

Created attachment 183012 [details]
Patch

[James Robinson]

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.

[Robert Kroeger]

Created attachment 183233 [details]
Patch

[Robert Kroeger]

Comment on attachment 183233 [details]
Patch

I modified the patch to use scroll to scroll the plugin contents. Please take a look.

[James Robinson]

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

[Robert Kroeger]

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.

[Robert Kroeger]

Created attachment 183275 [details]
Patch

[Robert Kroeger]

Comment on attachment 183275 [details]
Patch

Ptal.

[James Robinson]

Comment on attachment 183275 [details]
Patch

r=me

[WebKit Review Bot]

Comment on attachment 183275 [details]
Patch

Clearing flags on attachment: 183275

Committed r140183: <http://trac.webkit.org/changeset/140183>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.