I discovered this problem while debugging Bug 145637. The logic in findEnclosingOverflowScroll uses the RenderBox "canBeScrolledAndHasScrollableArea" to identify the nearest enclosing element that can be scrolled. However, this method considers horizontal and vertical scrolling as equally relevant. This is a problem when we want to perform a vertical scroll, but the element underneath the mouse pointer can only be scrolled in a horizontal direction. When we have this scenario, the latching logic chooses the horizontally scrollable region and attempts to latch to it. When it discovers that it cannot be scrolled in the user's gesture direction, it gives up and passes control to the next enclosing latching context. When we are working on a single page, this means the document body, and things generally work as the user expects. But when we have nested iframes, this causes us to pop up to the enclosing iframe, which looks wrong to the user. The test case I added in Bug 145637 highlights this problem if you modify the <img> style to remove the "max-width:100%" in 'inner_content.html'. I think the right fix here is to modify 'findEnclosingOverflowScroll' so that it takes the gesture's dominant direction into account.
<rdar://problem/21242308>
Created attachment 254281 [details] Patch
Comment on attachment 254281 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=254281&action=review > Source/WebCore/page/mac/EventHandlerMac.mm:795 > + if ((verticalScroll && box->hasScrollableOverflowY()) || (!verticalScroll && box->hasScrollableOverflowX())) > + return candidate; I'm not sure I like the idea of adding "predominantly vertical" logic here. The event could have non-zero X delta, and box->hasScrollableOverflowX() could be true, but we would not return that box as a candidate.
(In reply to comment #3) > Comment on attachment 254281 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=254281&action=review > > > Source/WebCore/page/mac/EventHandlerMac.mm:795 > > + if ((verticalScroll && box->hasScrollableOverflowY()) || (!verticalScroll && box->hasScrollableOverflowX())) > > + return candidate; > > I'm not sure I like the idea of adding "predominantly vertical" logic here. > The event could have non-zero X delta, and box->hasScrollableOverflowX() > could be true, but we would not return that box as a candidate. Ah! Good point. Maybe something like "deltaX && box->hasScrollableOverflowX() || deltaY && box->hasScrollableOverflowY()"
Created attachment 254282 [details] Patch
Committed r185208: <http://trac.webkit.org/changeset/185208>
This broke platform/mac-wk2/tiled-drawing/scrolling/fast-scroll-iframe-latched-select.html
@@ -8,7 +8,7 @@ TEST COMPLETE PASS Page did not receive wheel events. -PASS IFrame did not receive wheel events. +FAIL IFrame consumed wheel events. PASS Select consumed wheel events. PASS successfullyParsed is true Rolling out.
Re-opened since this is blocked by bug 145655
Rolling out r185211 too (which just added tests that now fail).
(In reply to comment #7) > This broke > platform/mac-wk2/tiled-drawing/scrolling/fast-scroll-iframe-latched-select. > html Ah! Indeed it does. The <select> case doesn't work the same way because it doesn't treat its scrollable area as overflow. So the "scrollsOverflowY()" returns false for <select>, even though it does satisfy "canBeScrolledAndHasScrollableArea".
Created attachment 254323 [details] Patch
Created attachment 254325 [details] Patch
Comment on attachment 254325 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=254325&action=review > Source/WebCore/page/mac/EventHandlerMac.mm:782 > +static ContainerNode* findEnclosingOverflowScrollForDominantDirection(ContainerNode* node, float deltaX, float deltaY) This isn't really "dominant direction" any more.
Committed r185234: <http://trac.webkit.org/changeset/185234>