Latching algorithm in findEnclosingOverflowScroll is broken
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Jun 2015 01:35:40 +0000 (01:35 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Jun 2015 01:35:40 +0000 (01:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=145642
<rdar://problem/21242308>

Reviewed by Simon Fraser.

Source/WebCore:

Test: platform/mac-wk2/tiled-drawing/scrolling/iframe_in_iframe.html

* page/mac/EventHandlerMac.mm:
(WebCore::scrollableAreaForBox): Added helper function.
(WebCore::findEnclosingScrollableContainer): Renamed from findEnclosingOverflowScroll.
Only identify something as our scroll element if it can be scrolled in either
axis of the gesture's motion.
(WebCore::scrollableAreaForContainerNode): Use new helper function.
(WebCore::EventHandler::platformPrepareForWheelEvents): Use new function
name, and pass horizontal and vertical deltas.
(WebCore::findEnclosingOverflowScroll): Deleted.

LayoutTests:

Revise the new latching test to remove the image "max-width: 100%" style, which was
preventing this example from having a horizontal scrollable region at the top of the
file.

It still correctly tests the subpixel bug from Bug 145637.

* platform/mac-wk2/tiled-drawing/scrolling/resources/inner_content.html: Remove width
adjustment on image.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@185234 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/resources/inner_content.html
Source/WebCore/ChangeLog
Source/WebCore/page/mac/EventHandlerMac.mm

index cf3f7cf..7b77aa5 100644 (file)
@@ -1,3 +1,20 @@
+2015-06-04  Brent Fulgham  <bfulgham@apple.com>
+
+        Latching algorithm in findEnclosingOverflowScroll is broken
+        https://bugs.webkit.org/show_bug.cgi?id=145642
+        <rdar://problem/21242308>
+
+        Reviewed by Simon Fraser.
+
+        Revise the new latching test to remove the image "max-width: 100%" style, which was
+        preventing this example from having a horizontal scrollable region at the top of the
+        file.
+        
+        It still correctly tests the subpixel bug from Bug 145637.
+
+        * platform/mac-wk2/tiled-drawing/scrolling/resources/inner_content.html: Remove width
+        adjustment on image.
+
 2015-06-04  Benjamin Poulain  <bpoulain@apple.com>
 
         Combine tiny DFAs into slightly larger ones
index af796cd..540dd01 100644 (file)
@@ -3,12 +3,6 @@
     <head>
         <title>Inner iFrame Example</title>
         <meta charset="utf-8">
-        <style>
-        img {
-            display:block;
-            max-width:100%;
-        }
-        </style>
     </head>
     <body style="position: relative; min-height: 100%; top: 0px;">
         <div style="overflow:auto;">
index 4f014b7..8995fcc 100644 (file)
@@ -1,3 +1,23 @@
+2015-06-04  Brent Fulgham  <bfulgham@apple.com>
+
+        Latching algorithm in findEnclosingOverflowScroll is broken
+        https://bugs.webkit.org/show_bug.cgi?id=145642
+        <rdar://problem/21242308>
+
+        Reviewed by Simon Fraser.
+
+        Test: platform/mac-wk2/tiled-drawing/scrolling/iframe_in_iframe.html
+
+        * page/mac/EventHandlerMac.mm:
+        (WebCore::scrollableAreaForBox): Added helper function.
+        (WebCore::findEnclosingScrollableContainer): Renamed from findEnclosingOverflowScroll.
+        Only identify something as our scroll element if it can be scrolled in either
+        axis of the gesture's motion.
+        (WebCore::scrollableAreaForContainerNode): Use new helper function.
+        (WebCore::EventHandler::platformPrepareForWheelEvents): Use new function
+        name, and pass horizontal and vertical deltas.
+        (WebCore::findEnclosingOverflowScroll): Deleted.
+
 2015-06-04  Gyuyoung Kim  <gyuyoung.kim@webkit.org>
 
         REGRESSION(r185091): Crash happens on indexdb tests
index 35de1c1..5d5c8e2 100644 (file)
@@ -771,7 +771,15 @@ unsigned EventHandler::accessKeyModifiers()
     return PlatformEvent::CtrlKey | PlatformEvent::AltKey;
 }
 
-static ContainerNode* findEnclosingOverflowScroll(ContainerNode* node)
+static ScrollableArea* scrollableAreaForBox(RenderBox& box)
+{
+    if (is<RenderListBox>(box))
+        return downcast<RenderListBox>(&box);
+
+    return box.layer();
+}
+    
+static ContainerNode* findEnclosingScrollableContainer(ContainerNode* node, float deltaX, float deltaY)
 {
     // Find the first node with a valid scrollable area starting with the current
     // node and traversing its parents (or shadow hosts).
@@ -783,8 +791,14 @@ static ContainerNode* findEnclosingOverflowScroll(ContainerNode* node)
             return nullptr;
 
         RenderBox* box = candidate->renderBox();
-        if (box && box->canBeScrolledAndHasScrollableArea())
-            return candidate;
+        if (box && box->canBeScrolledAndHasScrollableArea()) {
+            if (ScrollableArea* scrollableArea = scrollableAreaForBox(*box)) {
+                if (((deltaY > 0) && !scrollableArea->scrolledToTop()) || ((deltaY < 0) && !scrollableArea->scrolledToBottom())
+                    || ((deltaX > 0) && !scrollableArea->scrolledToRight()) || ((deltaX < 0) && !scrollableArea->scrolledToLeft())) {
+                    return candidate;
+                }
+            }
+        }
     }
     
     return nullptr;
@@ -887,12 +901,8 @@ 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();
-    }
+    if (RenderBox* box = container.renderBox())
+        scrollableArea = scrollableAreaForBox(*box);
 
     return scrollableArea;
 }
@@ -916,7 +926,7 @@ void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent& wheel
             scrollableContainer = wheelEventTarget;
             scrollableArea = scrollViewForEventTarget(wheelEventTarget.get());
         } else {
-            scrollableContainer = findEnclosingOverflowScroll(wheelEventTarget.get());
+            scrollableContainer = findEnclosingScrollableContainer(wheelEventTarget.get(), wheelEvent.deltaX(), wheelEvent.deltaY());
             if (scrollableContainer)
                 scrollableArea = scrollableAreaForContainerNode(*scrollableContainer);
             else {