Testing with various WebKit1 clients after my change for Iframe scroll gestures has revealed an edge case: Under the following conditions: 1. A Cocoa application embeds a WebView using WK1. 2. The webview consists of an Iframe. 3. The Iframe size is the same as the WebView, so that there is no scrollable region. If the user performs a scroll wheel gesture on the web view, other scrollable portions of the user interface become "locked up".
<rdar://problem/17767169>
Analysis indicates that WebKit is issuing two scrollWheel events during the handling of this gesture. The two events are intercepted at the OS level and result in some undesirable side effects that end up locking the view. It looks like the nuance I missed in my earlier work was that although we bail out early when interacting with IFrames in the general case (under WK1), when we pass through the EventHandler::passWheelEventToWidget for an IFrame we actually DO dispatch the 'scrollWheel' event to the NSScrollView backing the IFrame. This seems to set up the multiple-scrollWheel event condition found in Dictionary.app. I think the correct fix is to recognize a WK1 case where the widget to which we just passed the wheel event is an frame view (IFrame), to not dispatch a second scrollWheel event by returning 'true' in this case.
Created attachment 236736 [details] Patch
Since this depends on host application logic, it might be a good candidate for a TestWebKitAPI test.
Comment on attachment 236736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236736&action=review > Source/WebCore/page/mac/EventHandlerMac.mm:923 > + // If the platform widget is a frame view (IFrame), and we are in WebKit1, 'passWheelEventToWidget' has already > + // called 'scrollWheel' on the underlying platform widget. We need to return true in this case to avoid getting > + // multiple copies of the scrollWheel event sent to the platform widget. IS such a long comment really necessary? I'd suggest no comment, or a one-liner stating the reason.
Committed r172703: <http://trac.webkit.org/changeset/172703>
(In reply to comment #5) > (From update of attachment 236736 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=236736&action=review > > > Source/WebCore/page/mac/EventHandlerMac.mm:923 > > + // If the platform widget is a frame view (IFrame), and we are in WebKit1, 'passWheelEventToWidget' has already > > + // called 'scrollWheel' on the underlying platform widget. We need to return true in this case to avoid getting > > + // multiple copies of the scrollWheel event sent to the platform widget. > > IS such a long comment really necessary? I'd suggest no comment, or a one-liner stating the reason. OK! Fixed while landing.
See Bug 136040 for work on creating a TestWebKitAPI test for this condition.
Comment on attachment 236736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236736&action=review > Source/WebCore/page/EventHandler.cpp:2658 > +bool EventHandler::platformCompletePlatformWidgetWheelEvent(const PlatformWheelEvent&, const Widget*, ContainerNode*) Since this can never be null, it should be a Widget& rather than a Widget*.
Comment on attachment 236736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236736&action=review >> Source/WebCore/page/EventHandler.cpp:2658 >> +bool EventHandler::platformCompletePlatformWidgetWheelEvent(const PlatformWheelEvent&, const Widget*, ContainerNode*) > > Since this can never be null, it should be a Widget& rather than a Widget*. Good point! I'll land this as a follow-up patch.
Added Darin's suggested clean-up: Committed r172705: <http://trac.webkit.org/changeset/172705>
Correct a bad merge from my last change: Committed in r172720. <http://trac.webkit.org/changeset/172720>