Summary: | REGRESSION (r173784): [Mac] Correct latching error for non-scrollable iframe nested inside scrollable div. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> | ||||
Component: | Layout and Rendering | Assignee: | Brent Fulgham <bfulgham> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bfulgham, simon.fraser | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Mac | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Brent Fulgham
2015-08-04 17:46:41 PDT
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. Created attachment 258250 [details]
Patch
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. Committed r187930: <http://trac.webkit.org/changeset/187930> The new test is timing out: https://build.webkit.org/results/Apple%20Yosemite%20Release%20WK1%20(Tests)/r187935%20(7289)/results.html 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. |