Bug 147668 – REGRESSION (r173784): [Mac] Correct latching error for non-scrollable iframe nested inside scrollable div.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Brent Fulgham]

The WebKit latching logic improperly causes the main frame to scroll in the following case:

1. The page contains an iframe that is sized so that its contents do not need to scroll.
2. The iframe is marked as "overflow: hidden"
3. The iframe is contained in a div, which is sized so that it needs to scroll.

When this happens, the latching logic improperly skips the containing div, and passes the wheel events to the main frame.

[Brent Fulgham]

<rdar://problem/21870332>

[Brent Fulgham]

This bug was caused by the following:

1. When latching, we evaluate each new wheel event and decide if we should identify the element to latch to, or if we are continuing a set of gestures that are already tied to an existing element.

2. If we are already latched, we pass wheel events to the latched element until we hit the edge of the scrollable region.

3. However, if we have nested scrolling contexts, and we are processing wheel events in the context of a nested frame, we do not want to pass events to the latched element. Consequently, in EventHandler::platformPrepareForWheelEvents, if we find that latchingIsLockedToAncestorOfThisFrame is true we exit early and do not pass events to the latched element.

The bug was that EventHandler::platformCompleteWheelEvent was not handling the nested latch case properly. It was grabbing the current latching state, regardless of whether it applied to the current frame or not. In the case where we were latched to an element in an enclosing frame, we would reach up to the enclosing frame and pass the wheel events to its enclosing view (in this case, the Main Frame).

Instead, we need to perform the same short-circuit we do when beginning processing the wheel event. If we are latched to an enclosing context, we should skip the logic of finding the scrolling container for the latched element. Instead, we should just give the current element the opportunity to handle the wheel event (in case the user provided handlers for it), and then return.

[Brent Fulgham]

Created attachment 258250 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 258250 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Test: platform/mac/fast/scrolling/scroll-div-with-nested-nonscrollable-iframe.html

The new hotness is to put the test in fast/scrolling/scroll-div-with-nested-nonscrollable-iframe.html but I haven't moved platform/mac/fast/scrolling yet so you're OK.

[Brent Fulgham]

Committed r187930: <http://trac.webkit.org/changeset/187930>

[Simon Fraser (smfr)]

The new test is timing out:
https://build.webkit.org/results/Apple%20Yosemite%20Release%20WK1%20(Tests)/r187935%20(7289)/results.html

[Antti Koivisto]

Comment on attachment 258250 [details]
Patch

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

> LayoutTests/platform/mac/fast/scrolling/scroll-div-with-nested-nonscrollable-iframe.html:87
> +    if (window.eventSender) {
> +        testRunner.waitUntilDone();

Not sure if this is why it is timing out but the correct way to make an async test using js-test-post-pre/js-test-post-post is 

jsTestIsAsync = true;

on top of the test and finishJSTest() to end it. Explicit testRunner.waitUntilDone()/testRunner.notifyDone() are not needed.