ScrollingTreeOverflowScrollingNodeIOS uses the wrong fixed position rectangle
authorfred.wang@free.fr <fred.wang@free.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Aug 2017 08:08:17 +0000 (08:08 +0000)
committerfred.wang@free.fr <fred.wang@free.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Aug 2017 08:08:17 +0000 (08:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=175135

Patch by Frederic Wang <fwang@igalia.com> on 2017-08-04
Reviewed by Simon Fraser.

Source/WebCore:

This patch modifies ScrollingTreeOverflowScrollingNodeIOS::updateChildNodesAfterScroll so
that it uses the fixed position rectangle relative of the first frame ancestor instead of
the one of the main frame. This makes it consistent with ScrollingTreeFrameScrollingNodeIOS
and RenderLayerCompositor. This fixes some flickering issues on iOS.

Test: fast/scrolling/ios/fixed-inside-overflow-inside-iframe.html

* page/scrolling/ScrollingTreeFrameScrollingNode.h:
(WebCore::ScrollingTreeFrameScrollingNode::fixedPositionRect): Helper function to get the
fixed position rect to use for that frame.
* page/scrolling/ScrollingTreeNode.cpp:
(WebCore::ScrollingTreeNode::enclosingFrameNode const): Helper function to get the enclosing
frame for this scrolling node or null if there is none.
* page/scrolling/ScrollingTreeNode.h: Declare enclosingFrameNode.

Source/WebKit:

This patch modifies ScrollingTreeOverflowScrollingNodeIOS::updateChildNodesAfterScroll so
that it uses the fixed position rectangle relative of the first frame ancestor instead of
the one of the main frame. This makes it consistent with ScrollingTreeFrameScrollingNodeIOS
and RenderLayerCompositor. This fixes some flickering issues on iOS.

* UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm:
(WebKit::ScrollingTreeOverflowScrollingNodeIOS::updateChildNodesAfterScroll): Use the fixed
position rect of a non-main frame if there is one.

LayoutTests:

This patch adds a new test for a position:fixed element inside an overflow node inside an
iframe. When scrolling the overflow node, the position of such an element should remain fixed
relative to the inner frame. Before that change, ScrollingTreeOverflowScrollingNodeIOS used
to take the main frame as a reference instead, causing the element to flicker and even to
disappear when the user scrolls that overflow node. We add a reftest to verify that the
element is visible and positioned at the correct location when the user scrolls.

* fast/scrolling/ios/fixed-inside-overflow-inside-iframe-expected.html: Added.
* fast/scrolling/ios/fixed-inside-overflow-inside-iframe.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/scrolling/ios/fixed-inside-overflow-inside-iframe-expected.html [new file with mode: 0644]
LayoutTests/fast/scrolling/ios/fixed-inside-overflow-inside-iframe.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/ScrollingTreeFrameScrollingNode.h
Source/WebCore/page/scrolling/ScrollingTreeNode.cpp
Source/WebCore/page/scrolling/ScrollingTreeNode.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm

index d405fb26ea67af27d67e9083a3a884ce58c4f012..e355568765f12baec85acf688543b61566c0ae2b 100644 (file)
@@ -1,3 +1,20 @@
+2017-08-04  Frederic Wang  <fwang@igalia.com>
+
+        ScrollingTreeOverflowScrollingNodeIOS uses the wrong fixed position rectangle
+        https://bugs.webkit.org/show_bug.cgi?id=175135
+
+        Reviewed by Simon Fraser.
+
+        This patch adds a new test for a position:fixed element inside an overflow node inside an
+        iframe. When scrolling the overflow node, the position of such an element should remain fixed
+        relative to the inner frame. Before that change, ScrollingTreeOverflowScrollingNodeIOS used
+        to take the main frame as a reference instead, causing the element to flicker and even to
+        disappear when the user scrolls that overflow node. We add a reftest to verify that the
+        element is visible and positioned at the correct location when the user scrolls.
+
+        * fast/scrolling/ios/fixed-inside-overflow-inside-iframe-expected.html: Added.
+        * fast/scrolling/ios/fixed-inside-overflow-inside-iframe.html: Added.
+
 2017-08-04  Zan Dobersek  <zdobersek@igalia.com>
 
         Unreviewed WPE gardening. Update test expectations and layout test baselines
diff --git a/LayoutTests/fast/scrolling/ios/fixed-inside-overflow-inside-iframe-expected.html b/LayoutTests/fast/scrolling/ios/fixed-inside-overflow-inside-iframe-expected.html
new file mode 100644 (file)
index 0000000..0cee2e0
--- /dev/null
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<html
+  <head>
+    <title>position:fixed element inside overflow node inside iframe</title>
+    <style>
+       iframe {
+           position: absolute;
+           border: none;
+           left: 40px;
+           top: 50px;
+           width: 300px;
+           height: 100px;
+       }
+    </style>
+    <script type="text/javascript">
+      if (window.testRunner)
+          testRunner.waitUntilDone();
+
+      function runTest() {
+         testRunner.notifyDone();
+      }
+    </script>
+  </head>
+  <body>
+    <p>The red position:fixed rectangle should not flicker nor disappear when scrolling that iframe.</p>
+
+<iframe srcdoc="
+<!DOCTYPE html>
+<html>
+  <head>
+    <style>
+      html, body {
+          margin: 0;
+          overflow-y: auto;
+          height: 100px;
+         -webkit-overflow-scrolling: touch;
+      }
+      #fixed {
+          position: fixed;
+          top: 10px;
+          right: 0px;
+          height: 40px;
+          width: 50%;
+          background: red;
+      }
+      #content {
+          width: 100%;
+          height: 1000px;
+          background: blue;
+      }
+    </style>
+  </head>
+  <body>
+    <div id='fixed'></div>
+    <div id='content'></div>
+  </body>
+</html>" onload="runTest()"></iframe>
+</body>
+</html>
diff --git a/LayoutTests/fast/scrolling/ios/fixed-inside-overflow-inside-iframe.html b/LayoutTests/fast/scrolling/ios/fixed-inside-overflow-inside-iframe.html
new file mode 100644 (file)
index 0000000..51f8958
--- /dev/null
@@ -0,0 +1,77 @@
+<!DOCTYPE html>
+<html
+  <head>
+    <title>position:fixed element inside overflow node inside iframe</title>
+    <style>
+       iframe {
+           position: absolute;
+           border: none;
+           left: 40px;
+           top: 50px;
+           width: 300px;
+           height: 100px;
+       }
+    </style>
+    <script type="text/javascript">
+      if (window.testRunner)
+          testRunner.waitUntilDone();
+
+      function getSwipeUIScript(fromX, fromY, toX, toY, duration)
+      {
+          return `(() => {
+              uiController.dragFromPointToPoint(${fromX}, ${fromY}, ${toX}, ${toY}, ${duration}, () => {
+                  uiController.uiScriptComplete("");
+              });
+          })();`;
+      }
+
+      function runTest() {
+         var frameBox = document.querySelector("iframe").getBoundingClientRect();
+         var x = frameBox.left + frameBox.width / 4;
+         var ystart = frameBox.top + frameBox.height - 2;
+         var yend = frameBox.top + 2;
+
+         if (window.testRunner && testRunner.runUIScript) {
+             testRunner.runUIScript(getSwipeUIScript(x, ystart, x, yend, 0.05), () => {
+                 setTimeout(function() { testRunner.notifyDone(); }, 4000);
+             });
+         }
+       }
+    </script>
+  </head>
+  <body>
+    <p>The red position:fixed rectangle should not flicker nor disappear when scrolling that iframe.</p>
+
+<iframe srcdoc="
+<!DOCTYPE html>
+<html>
+  <head>
+    <style>
+      html, body {
+          margin: 0;
+          overflow-y: auto;
+          height: 100px;
+         -webkit-overflow-scrolling: touch;
+      }
+      #fixed {
+          position: fixed;
+          top: 10px;
+          right: 0px;
+          height: 40px;
+          width: 50%;
+          background: red;
+      }
+      #content {
+          width: 100%;
+          height: 1000px;
+          background: blue;
+      }
+    </style>
+  </head>
+  <body>
+    <div id='fixed'></div>
+    <div id='content'></div>
+  </body>
+</html>" onload="runTest()"></iframe>
+</body>
+</html>
index 81d46d078b2496ef880310ce8b95ccd7a057e7fb..3386164b376a9a66afed9002f9b12f6c759ae92b 100644 (file)
@@ -1,3 +1,25 @@
+2017-08-04  Frederic Wang  <fwang@igalia.com>
+
+        ScrollingTreeOverflowScrollingNodeIOS uses the wrong fixed position rectangle
+        https://bugs.webkit.org/show_bug.cgi?id=175135
+
+        Reviewed by Simon Fraser.
+
+        This patch modifies ScrollingTreeOverflowScrollingNodeIOS::updateChildNodesAfterScroll so
+        that it uses the fixed position rectangle relative of the first frame ancestor instead of
+        the one of the main frame. This makes it consistent with ScrollingTreeFrameScrollingNodeIOS
+        and RenderLayerCompositor. This fixes some flickering issues on iOS.
+
+        Test: fast/scrolling/ios/fixed-inside-overflow-inside-iframe.html
+
+        * page/scrolling/ScrollingTreeFrameScrollingNode.h:
+        (WebCore::ScrollingTreeFrameScrollingNode::fixedPositionRect): Helper function to get the
+        fixed position rect to use for that frame.
+        * page/scrolling/ScrollingTreeNode.cpp:
+        (WebCore::ScrollingTreeNode::enclosingFrameNode const): Helper function to get the enclosing
+        frame for this scrolling node or null if there is none.
+        * page/scrolling/ScrollingTreeNode.h: Declare enclosingFrameNode.
+
 2017-08-04  Zan Dobersek  <zdobersek@igalia.com>
 
         Unreviewed. Removing redundant NotImplemented.h header inclusions
index 43488c4faf117881c9b9837ef58571146c9108a7..760bb06e4b17cce0c1daedb7d952f6c64f72552a 100644 (file)
@@ -58,6 +58,8 @@ public:
     FloatSize viewToContentsOffset(const FloatPoint& scrollPosition) const;
     FloatRect layoutViewportForScrollPosition(const FloatPoint& visibleContentOrigin, float scale) const;
 
+    FloatRect fixedPositionRect() { return FloatRect(lastCommittedScrollPosition(), scrollableAreaSize()); };
+
 protected:
     ScrollingTreeFrameScrollingNode(ScrollingTree&, ScrollingNodeID);
 
index 0f6616d0f3dc35786306fd466f7a696415b2eee6..ba5ba3e44d93bca16d49e6c238fb14017d924b51 100644 (file)
@@ -29,6 +29,7 @@
 #if ENABLE(ASYNC_SCROLLING)
 
 #include "ScrollingStateTree.h"
+#include "ScrollingTreeFrameScrollingNode.h"
 #include "TextStream.h"
 
 namespace WebCore {
@@ -78,6 +79,15 @@ void ScrollingTreeNode::dumpProperties(TextStream& ts, ScrollingStateTreeAsTextB
         ts.dumpProperty("nodeID", scrollingNodeID());
 }
 
+ScrollingTreeFrameScrollingNode* ScrollingTreeNode::enclosingFrameNode() const
+{
+    ScrollingTreeNode* node = parent();
+    while (node && node->nodeType() != FrameScrollingNode)
+        node = node->parent();
+
+    return downcast<ScrollingTreeFrameScrollingNode>(node);
+}
+
 void ScrollingTreeNode::dump(TextStream& ts, ScrollingStateTreeAsTextBehavior behavior) const
 {
     dumpProperties(ts, behavior);
index 2e163246bb96e33fef62ada71b41a21392127932..93fded19b02a2d64bcea51a74cf368288ea86866 100644 (file)
@@ -38,6 +38,7 @@ namespace WebCore {
 
 class ScrollingStateFixedNode;
 class ScrollingStateScrollingNode;
+class ScrollingTreeFrameScrollingNode;
 
 class ScrollingTreeNode : public RefCounted<ScrollingTreeNode> {
 public:
@@ -65,6 +66,8 @@ public:
     void appendChild(Ref<ScrollingTreeNode>&&);
     void removeChild(ScrollingTreeNode&);
 
+    WEBCORE_EXPORT ScrollingTreeFrameScrollingNode* enclosingFrameNode() const;
+
     WEBCORE_EXPORT void dump(TextStream&, ScrollingStateTreeAsTextBehavior) const;
 
 protected:
index 534e23e8d708449ac1fde52ba62992430a8912ef..b51fc715cb241cdfb3fb9c2ca2373cbf73708fab 100644 (file)
@@ -1,3 +1,19 @@
+2017-08-04  Frederic Wang  <fwang@igalia.com>
+
+        ScrollingTreeOverflowScrollingNodeIOS uses the wrong fixed position rectangle
+        https://bugs.webkit.org/show_bug.cgi?id=175135
+
+        Reviewed by Simon Fraser.
+
+        This patch modifies ScrollingTreeOverflowScrollingNodeIOS::updateChildNodesAfterScroll so
+        that it uses the fixed position rectangle relative of the first frame ancestor instead of
+        the one of the main frame. This makes it consistent with ScrollingTreeFrameScrollingNodeIOS
+        and RenderLayerCompositor. This fixes some flickering issues on iOS.
+
+        * UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm:
+        (WebKit::ScrollingTreeOverflowScrollingNodeIOS::updateChildNodesAfterScroll): Use the fixed
+        position rect of a non-main frame if there is one.
+
 2017-08-03  Brian Burg  <bburg@apple.com>
 
         Remove ENABLE(WEB_SOCKET) guards
index c9f0fa7a240822d5c8e4abf2e5ca435db830e10d..ac1cce4f21ba88fda0c06265de3a550f0f9a53a5 100644 (file)
 #if ENABLE(ASYNC_SCROLLING)
 
 #import <QuartzCore/QuartzCore.h>
-#import <WebCore/ScrollingStateOverflowScrollingNode.h>
-#import <WebCore/ScrollingTree.h>
 #import <UIKit/UIPanGestureRecognizer.h>
 #import <UIKit/UIScrollView.h>
+#import <WebCore/ScrollingStateOverflowScrollingNode.h>
+#import <WebCore/ScrollingTree.h>
+#import <WebCore/ScrollingTreeFrameScrollingNode.h>
 #import <wtf/BlockObjCExceptions.h>
 #import <wtf/SetForScope.h>
 
@@ -272,7 +273,12 @@ void ScrollingTreeOverflowScrollingNodeIOS::updateChildNodesAfterScroll(const Fl
     if (!m_children)
         return;
 
-    FloatRect fixedPositionRect = scrollingTree().fixedPositionRect();
+    FloatRect fixedPositionRect;
+    ScrollingTreeFrameScrollingNode* frameNode = enclosingFrameNode();
+    if (frameNode && frameNode->parent())
+        fixedPositionRect = frameNode->fixedPositionRect();
+    else
+        fixedPositionRect = scrollingTree().fixedPositionRect();
     FloatSize scrollDelta = lastCommittedScrollPosition() - scrollPosition;
 
     for (auto& child : *m_children)