[iOS WK2] Interactive elements of developer.apple.com are broken
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Apr 2015 17:25:31 +0000 (17:25 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Apr 2015 17:25:31 +0000 (17:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=143692
Source/WebCore:

rdar://problem/19320087

Reviewed by Sam Weinig.

When a composited RenderLayer had nodes in the scrolling tree by virtue of
both position and overflow:scroll, and one of those reasons disappeared,
we'd fail to remove the corresponding node from the scrolling tree. This
could leave an overflow:scroll element behaving as if it were position:fixed.

Fix by having RenderLayerCompositor::updateScrollCoordinationForThisFrame()
detach the layer on a per-role basis.

Test: platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html

* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::detachFromScrollingCoordinatorForRole):
* rendering/RenderLayerBacking.h:
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::detachScrollCoordinatedLayerForRole):
(WebCore::RenderLayerCompositor::detachScrollCoordinatedLayer): Just moved.
(WebCore::RenderLayerCompositor::updateScrollCoordinatedLayer): Call detachScrollCoordinatedLayerForRole()
if the layer doesn't have the relevant scrolling reasons.
* rendering/RenderLayerCompositor.h:

LayoutTests:

Reviewed by Sam Weinig.

* platform/ios-simulator-wk2/scrolling/remove-scrolling-role-expected.txt: Added.
* platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/ios-simulator-wk2/scrolling/remove-scrolling-role-expected.txt [new file with mode: 0644]
LayoutTests/platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayerBacking.cpp
Source/WebCore/rendering/RenderLayerBacking.h
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebCore/rendering/RenderLayerCompositor.h

index ccea358..51646a4 100644 (file)
@@ -1,3 +1,13 @@
+2015-04-13  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] Interactive elements of developer.apple.com are broken
+        https://bugs.webkit.org/show_bug.cgi?id=143692
+
+        Reviewed by Sam Weinig.
+
+        * platform/ios-simulator-wk2/scrolling/remove-scrolling-role-expected.txt: Added.
+        * platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html: Added.
+
 2015-04-14  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r182794.
diff --git a/LayoutTests/platform/ios-simulator-wk2/scrolling/remove-scrolling-role-expected.txt b/LayoutTests/platform/ios-simulator-wk2/scrolling/remove-scrolling-role-expected.txt
new file mode 100644 (file)
index 0000000..5b777c9
--- /dev/null
@@ -0,0 +1,12 @@
+(Frame scrolling node
+  (scrollable area size 800 600)
+  (contents size 800 2513)
+  (children 1
+    (Overflow scrolling node
+      (scrollable area size 300 400)
+      (contents size 300 2000)
+      (scrolled contents layer 22)
+    )
+  )
+)
+
diff --git a/LayoutTests/platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html b/LayoutTests/platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html
new file mode 100644 (file)
index 0000000..e01a297
--- /dev/null
@@ -0,0 +1,63 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+    <style>
+        body {
+            margin: 0;
+            height: 2500px;
+        }
+
+        #fixed {
+            position: fixed;
+            left: 10px;
+            top: 10px;
+            -webkit-overflow-scrolling: touch;
+            height: 400px;
+            width: 300px;
+            overflow: auto;
+            border: 4px solid black;
+        }
+
+        #fixed.changed {
+            position: absolute;
+            border: 4px solid green;
+        }
+
+        .content {
+            height: 2000px;
+            background-color: silver;
+        }
+
+    </style>
+    <script>
+        if (window.testRunner) {
+            testRunner.waitUntilDone();
+            testRunner.dumpAsText();
+        }
+
+        function doTest()
+        {
+            window.setTimeout(function() {
+                document.getElementById('fixed').classList.add('changed');
+
+                if (window.internals)
+                    document.getElementById('results').innerText = internals.scrollingStateTreeAsText();
+
+                if (window.testRunner)
+                    testRunner.notifyDone();
+
+                }, 0);
+        }
+
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+  <pre id="results"></pre>
+  <div id="fixed">
+    <div class="content">
+    </div>
+  </div>
+</body>
+</html>
index d72f691..ef951b1 100644 (file)
@@ -1,3 +1,31 @@
+2015-04-13  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] Interactive elements of developer.apple.com are broken
+        https://bugs.webkit.org/show_bug.cgi?id=143692
+        rdar://problem/19320087
+
+        Reviewed by Sam Weinig.
+        
+        When a composited RenderLayer had nodes in the scrolling tree by virtue of
+        both position and overflow:scroll, and one of those reasons disappeared,
+        we'd fail to remove the corresponding node from the scrolling tree. This
+        could leave an overflow:scroll element behaving as if it were position:fixed.
+        
+        Fix by having RenderLayerCompositor::updateScrollCoordinationForThisFrame()
+        detach the layer on a per-role basis.
+
+        Test: platform/ios-simulator-wk2/scrolling/remove-scrolling-role.html
+
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::detachFromScrollingCoordinatorForRole):
+        * rendering/RenderLayerBacking.h:
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::detachScrollCoordinatedLayerForRole):
+        (WebCore::RenderLayerCompositor::detachScrollCoordinatedLayer): Just moved.
+        (WebCore::RenderLayerCompositor::updateScrollCoordinatedLayer): Call detachScrollCoordinatedLayerForRole()
+        if the layer doesn't have the relevant scrolling reasons.
+        * rendering/RenderLayerCompositor.h:
+
 2015-04-14  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r182794.
index aeafcc9..055442a 100644 (file)
@@ -1570,6 +1570,20 @@ void RenderLayerBacking::detachFromScrollingCoordinator()
     m_viewportConstrainedNodeID = 0;
 }
 
+void RenderLayerBacking::detachFromScrollingCoordinatorForRole(ScrollingNodeType role)
+{
+    ScrollingNodeID nodeID = scrollingNodeIDForRole(role);
+    if (!nodeID)
+        return;
+
+    ScrollingCoordinator* scrollingCoordinator = scrollingCoordinatorFromLayer(m_owningLayer);
+    if (!scrollingCoordinator)
+        return;
+
+    scrollingCoordinator->detachFromStateTree(nodeID);
+    setScrollingNodeIDForRole(0, role);
+}
+
 GraphicsLayerPaintingPhase RenderLayerBacking::paintingPhaseForPrimaryLayer() const
 {
     unsigned phase = 0;
index 4a4a467..e74fddd 100644 (file)
@@ -108,6 +108,7 @@ public:
     GraphicsLayer* scrollingContentsLayer() const { return m_scrollingContentsLayer.get(); }
 
     void detachFromScrollingCoordinator();
+    void detachFromScrollingCoordinatorForRole(ScrollingNodeType);
     
     ScrollingNodeID scrollingNodeIDForRole(ScrollingNodeType nodeType) const
     {
index c72287e..7642243 100644 (file)
@@ -3813,6 +3813,33 @@ ScrollingNodeID RenderLayerCompositor::attachScrollingNode(RenderLayer& layer, S
     return nodeID;
 }
 
+void RenderLayerCompositor::detachScrollCoordinatedLayerForRole(RenderLayer& layer, ScrollingNodeType role)
+{
+    RenderLayerBacking* backing = layer.backing();
+    if (!backing)
+        return;
+
+    if (ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(role))
+        m_scrollingNodeToLayerMap.remove(nodeID);
+
+    backing->detachFromScrollingCoordinatorForRole(role);
+}
+
+void RenderLayerCompositor::detachScrollCoordinatedLayer(RenderLayer& layer)
+{
+    RenderLayerBacking* backing = layer.backing();
+    if (!backing)
+        return;
+
+    if (ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(FrameScrollingNode))
+        m_scrollingNodeToLayerMap.remove(nodeID);
+
+    if (ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(FixedNode))
+        m_scrollingNodeToLayerMap.remove(nodeID);
+
+    backing->detachFromScrollingCoordinator();
+}
+
 void RenderLayerCompositor::updateScrollCoordinationForThisFrame(ScrollingNodeID parentNodeID)
 {
     ScrollingCoordinator* scrollingCoordinator = this->scrollingCoordinator();
@@ -3883,7 +3910,8 @@ void RenderLayerCompositor::updateScrollCoordinatedLayer(RenderLayer& layer, Scr
         }
         
         parentNodeID = nodeID;
-    }
+    } else
+        detachScrollCoordinatedLayerForRole(layer, FixedNode);
 
     if (reasons & Scrolling) {
         if (isRootLayer)
@@ -3908,22 +3936,8 @@ void RenderLayerCompositor::updateScrollCoordinatedLayer(RenderLayer& layer, Scr
 #endif
             scrollingCoordinator->updateOverflowScrollingNode(nodeID, backing->scrollingLayer(), backing->scrollingContentsLayer(), &scrollingGeometry);
         }
-    }
-}
-
-void RenderLayerCompositor::detachScrollCoordinatedLayer(RenderLayer& layer)
-{
-    RenderLayerBacking* backing = layer.backing();
-    if (!backing)
-        return;
-
-    if (ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(FrameScrollingNode))
-        m_scrollingNodeToLayerMap.remove(nodeID);
-
-    if (ScrollingNodeID nodeID = backing->scrollingNodeIDForRole(FixedNode))
-        m_scrollingNodeToLayerMap.remove(nodeID);
-
-    backing->detachFromScrollingCoordinator();
+    } else
+        detachScrollCoordinatedLayerForRole(layer, OverflowScrollingNode);
 }
 
 ScrollableArea* RenderLayerCompositor::scrollableAreaForScrollLayerID(ScrollingNodeID nodeID) const
index 0cf0256..baa665f 100644 (file)
@@ -434,6 +434,7 @@ private:
     ScrollingNodeID attachScrollingNode(RenderLayer&, ScrollingNodeType, ScrollingNodeID parentNodeID);
     void updateScrollCoordinatedLayer(RenderLayer&, ScrollCoordinationReasons);
     void detachScrollCoordinatedLayer(RenderLayer&);
+    void detachScrollCoordinatedLayerForRole(RenderLayer&, ScrollingNodeType);
     void reattachSubframeScrollLayers();
     
     FixedPositionViewportConstraints computeFixedViewportConstraints(RenderLayer&) const;