Title: [185156] trunk
Revision
185156
Author
[email protected]
Date
2015-06-03 11:37:23 -0700 (Wed, 03 Jun 2015)

Log Message

REGRESSION: (r181879): Scrolling in select/option region in iFrame scrolls both select and iframe
https://bugs.webkit.org/show_bug.cgi?id=145574
<rdar://problem/20966828>

Reviewed by Simon Fraser.

Source/WebCore:

Tested by platform/mac-wk2/tiled-drawing/scrolling/fast-scroll-iframe-latched-select.html

When the scroll gesture is started when the latched scrollable container is not at the limit of its
scroll region, we are NOT supposed to propagate the scroll event to the enclosing region. However,
we were doing two things wrong:
(1) When we recognized we were latching, we were using the right wheel event target, but not using
    the latched scrollable container.
(2) Likewise, we were not using latched ScrollableArea when handling wheel events.

Instead, we were using the current scrollable container and ScrollableArea under the mouse pointer,
which could be different from the point we started latching as the content scrolled.
        
The fix was to properly track the scrollable container and scrollable area during latching.

I attempted to store the latched ScrollableArea in the latchingState object, like we already do for the
scrollable container, but found that this did not work properly. I think the life cycle of the
ScrollableArea may not match the scrollable container, and since they are not reference counted I
simply retrieve the ScrollableArea when needed.

* page/mac/EventHandlerMac.mm:
(WebCore::scrollableAreaForContainerNode): Helper function to return the correct ScrollableArea
for the two types of RenderBox elements.
(WebCore::latchedToFrameOrBody): Helper predicate to identify Frame and Body elements.
(WebCore::EventHandler::platformPrepareForWheelEvents): Use the correct ScrollableArea for the given
ContainerNode. When latching, make sure to use the ScrollableArea that is related to the latched scrollable
container, not the area currently underneath the mouse pointer.

LayoutTests:

* platform/mac-wk2/tiled-drawing/scrolling/fast-scroll-iframe-latched-select-expected.txt: Added.
* platform/mac-wk2/tiled-drawing/scrolling/fast-scroll-iframe-latched-select.html: Added.
* platform/mac-wk2/tiled-drawing/scrolling/frames/select_iframe.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (185155 => 185156)


--- trunk/LayoutTests/ChangeLog	2015-06-03 17:33:16 UTC (rev 185155)
+++ trunk/LayoutTests/ChangeLog	2015-06-03 18:37:23 UTC (rev 185156)
@@ -1,3 +1,15 @@
+2015-06-03  Brent Fulgham  <[email protected]>
+
+        REGRESSION: (r181879): Scrolling in select/option region in iFrame scrolls both select and iframe
+        https://bugs.webkit.org/show_bug.cgi?id=145574
+        <rdar://problem/20966828>
+
+        Reviewed by Simon Fraser.
+
+        * platform/mac-wk2/tiled-drawing/scrolling/fast-scroll-iframe-latched-select-expected.txt: Added.
+        * platform/mac-wk2/tiled-drawing/scrolling/fast-scroll-iframe-latched-select.html: Added.
+        * platform/mac-wk2/tiled-drawing/scrolling/frames/select_iframe.html: Added.
+
 2015-06-03  Brady Eidson  <[email protected]>
 
         REGRESSION (r183498): Certain types of frame loads in iframes with <base target="_blank"> can open urls in new window/tabs

Added: trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/fast-scroll-iframe-latched-select-expected.txt (0 => 185156)


--- trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/fast-scroll-iframe-latched-select-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/fast-scroll-iframe-latched-select-expected.txt	2015-06-03 18:37:23 UTC (rev 185156)
@@ -0,0 +1,16 @@
+
+Tests that iframe doesn't consume wheel events when scrolling a select in an iframe.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS Page did not receive wheel events.
+PASS IFrame did not receive wheel events.
+PASS Select consumed wheel events.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/fast-scroll-iframe-latched-select.html (0 => 185156)


--- trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/fast-scroll-iframe-latched-select.html	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/fast-scroll-iframe-latched-select.html	2015-06-03 18:37:23 UTC (rev 185156)
@@ -0,0 +1,99 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="help" href=""
+<script src=""
+</head>
+<body _onload_="setupTopLevel();">
+<script>
+var pageScrollPositionBefore;
+var iframeTarget;
+var iFrameScrollPositionBefore;
+var selectTarget;
+var selectTargetScrollPositionBefore;
+
+function checkForScroll()
+{
+    // The IFrame should not have scrolled at all.
+    var pageScrollPositionAfter = document.body.scrollTop;
+    if (pageScrollPositionAfter != pageScrollPositionAfter)
+        testFailed("Page consumed wheel events.");
+    else
+        testPassed("Page did not receive wheel events.");
+
+    var iFrameScrollPositionAfter = window.frames['target'].document.body.scrollTop;
+    if (iFrameScrollPositionBefore != iFrameScrollPositionAfter)
+        testFailed("IFrame consumed wheel events.");
+    else
+        testPassed("IFrame did not receive wheel events.");
+
+    var selectTargetScrollPositionAfter = selectTarget.scrollTop;
+    if (selectTargetScrollPositionBefore != selectTargetScrollPositionAfter)
+        testPassed("Select consumed wheel events.");
+    else
+        testFailed("Select did not receive wheel events.");
+
+    finishJSTest();
+    testRunner.notifyDone();
+}
+
+function scrollTest()
+{
+    pageScrollPositionBefore = document.body.scrollTop;
+    iFrameScrollPositionBefore = window.frames['target'].document.body.scrollTop;
+
+    iframeTarget = document.getElementById('target');
+
+    selectTarget = window.frames['target'].document.getElementById('selectTarget');
+    selectTargetScrollPositionBefore = selectTarget.scrollTop;
+
+    // Scroll the #source until we reach the #target.
+    var startPosX = Math.round(selectTarget.offsetLeft) + 10;
+    var startPosY = Math.round(selectTarget.offsetTop) + 10; // Slightly more than one wheel scroll away from the IFrame
+    eventSender.mouseMoveTo(startPosX, startPosY);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'began', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'ended', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'begin');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'none', 'end');
+    eventSender.callAfterScrollingCompletes(checkForScroll);
+
+    // We should finish via the scroll event; this will fire in the case of failure when the page doesn't scroll.
+}
+
+function setupTopLevel()
+{
+    if (window.eventSender) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+
+        eventSender.monitorWheelEvents();
+        setTimeout(scrollTest, 0);
+    } else {
+        var messageLocation = document.getElementById('parent');
+        var message = document.createElement('div');
+        message.innerHTML = "<p>This test is better run under DumpRenderTree. To manually test it, place the mouse pointer<br/>"
+            + "at the top of the page, and then use the mouse wheel or a two-finger swipe to scroll the<br/>"
+            + "down past the iframe.<br/><br/>"
+            + "The iframe should not scroll.</p>";
+        messageLocation.appendChild(message);
+    }
+}
+
+</script>
+<div id="parent" style="height: 2000px;">
+    <iframe id="target" name="target" height="500" width="600" src=""
+    </iframe>
+</div>
+<div id="console"></div>
+<script>
+description("Tests that iframe doesn't consume wheel events when scrolling a select in an iframe.");
+</script>
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/resources/select_iframe.html (0 => 185156)


--- trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/resources/select_iframe.html	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/resources/select_iframe.html	2015-06-03 18:37:23 UTC (rev 185156)
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<head> 
+<title>Page Title</title>
+</head>
+<body>
+
+<h1>This is a Heading</h1>
+<p>This is a paragraph.</p>
+ <select id="selectTarget" size="5" multiple="multiple" name="driver[]">
+        <option value="" >(No Driver Filter)</option>
+        <option value="drivername=''"></option>
+        <option value="drivername='alex'">alex</option>
+        <option value="drivername='marc'">marc</option>
+        <option value="drivername='frank'">frank</option>
+        <option value="drivername='james'">james</option>
+    <option value="drivername='michael'">michael</option>
+</select>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+<p>This is a paragraph.</p>
+
+
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (185155 => 185156)


--- trunk/Source/WebCore/ChangeLog	2015-06-03 17:33:16 UTC (rev 185155)
+++ trunk/Source/WebCore/ChangeLog	2015-06-03 18:37:23 UTC (rev 185156)
@@ -1,3 +1,38 @@
+2015-06-03  Brent Fulgham  <[email protected]>
+
+        REGRESSION: (r181879): Scrolling in select/option region in iFrame scrolls both select and iframe
+        https://bugs.webkit.org/show_bug.cgi?id=145574
+        <rdar://problem/20966828>
+
+        Reviewed by Simon Fraser.
+
+        Tested by platform/mac-wk2/tiled-drawing/scrolling/fast-scroll-iframe-latched-select.html
+
+        When the scroll gesture is started when the latched scrollable container is not at the limit of its
+        scroll region, we are NOT supposed to propagate the scroll event to the enclosing region. However,
+        we were doing two things wrong:
+        (1) When we recognized we were latching, we were using the right wheel event target, but not using
+            the latched scrollable container.
+        (2) Likewise, we were not using latched ScrollableArea when handling wheel events.
+
+        Instead, we were using the current scrollable container and ScrollableArea under the mouse pointer,
+        which could be different from the point we started latching as the content scrolled.
+        
+        The fix was to properly track the scrollable container and scrollable area during latching.
+
+        I attempted to store the latched ScrollableArea in the latchingState object, like we already do for the
+        scrollable container, but found that this did not work properly. I think the life cycle of the
+        ScrollableArea may not match the scrollable container, and since they are not reference counted I
+        simply retrieve the ScrollableArea when needed.
+
+        * page/mac/EventHandlerMac.mm:
+        (WebCore::scrollableAreaForContainerNode): Helper function to return the correct ScrollableArea
+        for the two types of RenderBox elements.
+        (WebCore::latchedToFrameOrBody): Helper predicate to identify Frame and Body elements.
+        (WebCore::EventHandler::platformPrepareForWheelEvents): Use the correct ScrollableArea for the given
+        ContainerNode. When latching, make sure to use the ScrollableArea that is related to the latched scrollable
+        container, not the area currently underneath the mouse pointer.
+
 2015-06-03  Brady Eidson  <[email protected]>
 
         REGRESSION (r183498): Certain types of frame loads in iframes with <base target="_blank"> can open urls in new window/tabs

Modified: trunk/Source/WebCore/page/mac/EventHandlerMac.mm (185155 => 185156)


--- trunk/Source/WebCore/page/mac/EventHandlerMac.mm	2015-06-03 17:33:16 UTC (rev 185155)
+++ trunk/Source/WebCore/page/mac/EventHandlerMac.mm	2015-06-03 18:37:23 UTC (rev 185156)
@@ -883,6 +883,26 @@
     return false;
 }
 
+static ScrollableArea* scrollableAreaForContainerNode(ContainerNode& container)
+{
+    ScrollableArea* scrollableArea = nullptr;
+
+    if (RenderBox* box = container.renderBox()) {
+        if (is<RenderListBox>(*box))
+            scrollableArea = downcast<RenderListBox>(box);
+        else
+            scrollableArea = box->layer();
+    }
+
+    return scrollableArea;
+}
+
+static bool latchedToFrameOrBody(ContainerNode& container)
+{
+    // FIXME(106133): We might need to add or switch to is<HTMLDocumentElement> when this bug is fixed.
+    return is<HTMLFrameSetElement>(container) || is<HTMLBodyElement>(container);
+}
+
 void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent& wheelEvent, const HitTestResult& result, RefPtr<Element>& wheelEventTarget, RefPtr<ContainerNode>& scrollableContainer, ScrollableArea*& scrollableArea, bool& isOverWidget)
 {
     FrameView* view = m_frame.view();
@@ -897,14 +917,9 @@
             scrollableArea = scrollViewForEventTarget(wheelEventTarget.get());
         } else {
             scrollableContainer = findEnclosingOverflowScroll(wheelEventTarget.get());
-            if (scrollableContainer) {
-                if (RenderBox* box = scrollableContainer->renderBox()) {
-                    if (is<RenderListBox>(*box))
-                        scrollableArea = downcast<RenderListBox>(box);
-                    else
-                        scrollableArea = box->layer();
-                }
-            } else {
+            if (scrollableContainer)
+                scrollableArea = scrollableAreaForContainerNode(*scrollableContainer);
+            else {
                 scrollableContainer = view->frame().document()->bodyOrFrameset();
                 scrollableArea = view;
             }
@@ -944,6 +959,12 @@
 
         wheelEventTarget = latchingState->wheelEventElement();
         isOverWidget = latchingState->widgetIsLatched();
+        scrollableContainer = latchingState->scrollableContainer();
+
+        if (scrollableContainer) {
+            if (!latchedToFrameOrBody(*scrollableContainer) && !latchingState->widgetIsLatched())
+                scrollableArea = scrollableAreaForContainerNode(*scrollableContainer);
+        }
     }
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to