- Revision
- 220261
- Author
- [email protected]
- Date
- 2017-08-04 01:08:17 -0700 (Fri, 04 Aug 2017)
Log Message
ScrollingTreeOverflowScrollingNodeIOS uses the wrong fixed position rectangle
https://bugs.webkit.org/show_bug.cgi?id=175135
Patch by Frederic Wang <[email protected]> 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (220260 => 220261)
--- trunk/LayoutTests/ChangeLog 2017-08-04 07:56:17 UTC (rev 220260)
+++ trunk/LayoutTests/ChangeLog 2017-08-04 08:08:17 UTC (rev 220261)
@@ -1,3 +1,20 @@
+2017-08-04 Frederic Wang <[email protected]>
+
+ 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 <[email protected]>
Unreviewed WPE gardening. Update test expectations and layout test baselines
Added: trunk/LayoutTests/fast/scrolling/ios/fixed-inside-overflow-inside-iframe-expected.html (0 => 220261)
--- trunk/LayoutTests/fast/scrolling/ios/fixed-inside-overflow-inside-iframe-expected.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/ios/fixed-inside-overflow-inside-iframe-expected.html 2017-08-04 08:08:17 UTC (rev 220261)
@@ -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>
Property changes on: trunk/LayoutTests/fast/scrolling/ios/fixed-inside-overflow-inside-iframe-expected.html
___________________________________________________________________
Added: svn:eol-style
+LF
\ No newline at end of property
Added: svn:mime-type
+text/html
\ No newline at end of property
Added: trunk/LayoutTests/fast/scrolling/ios/fixed-inside-overflow-inside-iframe.html (0 => 220261)
--- trunk/LayoutTests/fast/scrolling/ios/fixed-inside-overflow-inside-iframe.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/ios/fixed-inside-overflow-inside-iframe.html 2017-08-04 08:08:17 UTC (rev 220261)
@@ -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>
Property changes on: trunk/LayoutTests/fast/scrolling/ios/fixed-inside-overflow-inside-iframe.html
___________________________________________________________________
Added: svn:eol-style
+LF
\ No newline at end of property
Added: svn:mime-type
+text/html
\ No newline at end of property
Modified: trunk/Source/WebCore/ChangeLog (220260 => 220261)
--- trunk/Source/WebCore/ChangeLog 2017-08-04 07:56:17 UTC (rev 220260)
+++ trunk/Source/WebCore/ChangeLog 2017-08-04 08:08:17 UTC (rev 220261)
@@ -1,3 +1,25 @@
+2017-08-04 Frederic Wang <[email protected]>
+
+ 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 <[email protected]>
Unreviewed. Removing redundant NotImplemented.h header inclusions
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeFrameScrollingNode.h (220260 => 220261)
--- trunk/Source/WebCore/page/scrolling/ScrollingTreeFrameScrollingNode.h 2017-08-04 07:56:17 UTC (rev 220260)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeFrameScrollingNode.h 2017-08-04 08:08:17 UTC (rev 220261)
@@ -58,6 +58,8 @@
FloatSize viewToContentsOffset(const FloatPoint& scrollPosition) const;
FloatRect layoutViewportForScrollPosition(const FloatPoint& visibleContentOrigin, float scale) const;
+ FloatRect fixedPositionRect() { return FloatRect(lastCommittedScrollPosition(), scrollableAreaSize()); };
+
protected:
ScrollingTreeFrameScrollingNode(ScrollingTree&, ScrollingNodeID);
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeNode.cpp (220260 => 220261)
--- trunk/Source/WebCore/page/scrolling/ScrollingTreeNode.cpp 2017-08-04 07:56:17 UTC (rev 220260)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeNode.cpp 2017-08-04 08:08:17 UTC (rev 220261)
@@ -29,6 +29,7 @@
#if ENABLE(ASYNC_SCROLLING)
#include "ScrollingStateTree.h"
+#include "ScrollingTreeFrameScrollingNode.h"
#include "TextStream.h"
namespace WebCore {
@@ -78,6 +79,15 @@
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);
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeNode.h (220260 => 220261)
--- trunk/Source/WebCore/page/scrolling/ScrollingTreeNode.h 2017-08-04 07:56:17 UTC (rev 220260)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeNode.h 2017-08-04 08:08:17 UTC (rev 220261)
@@ -38,6 +38,7 @@
class ScrollingStateFixedNode;
class ScrollingStateScrollingNode;
+class ScrollingTreeFrameScrollingNode;
class ScrollingTreeNode : public RefCounted<ScrollingTreeNode> {
public:
@@ -65,6 +66,8 @@
void appendChild(Ref<ScrollingTreeNode>&&);
void removeChild(ScrollingTreeNode&);
+ WEBCORE_EXPORT ScrollingTreeFrameScrollingNode* enclosingFrameNode() const;
+
WEBCORE_EXPORT void dump(TextStream&, ScrollingStateTreeAsTextBehavior) const;
protected:
Modified: trunk/Source/WebKit/ChangeLog (220260 => 220261)
--- trunk/Source/WebKit/ChangeLog 2017-08-04 07:56:17 UTC (rev 220260)
+++ trunk/Source/WebKit/ChangeLog 2017-08-04 08:08:17 UTC (rev 220261)
@@ -1,3 +1,19 @@
+2017-08-04 Frederic Wang <[email protected]>
+
+ 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 <[email protected]>
Remove ENABLE(WEB_SOCKET) guards
Modified: trunk/Source/WebKit/UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm (220260 => 220261)
--- trunk/Source/WebKit/UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm 2017-08-04 07:56:17 UTC (rev 220260)
+++ trunk/Source/WebKit/UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm 2017-08-04 08:08:17 UTC (rev 220261)
@@ -30,10 +30,11 @@
#if ENABLE(ASYNC_SCROLLING)
#import <QuartzCore/QuartzCore.h>
+#import <UIKit/UIPanGestureRecognizer.h>
+#import <UIKit/UIScrollView.h>
#import <WebCore/ScrollingStateOverflowScrollingNode.h>
#import <WebCore/ScrollingTree.h>
-#import <UIKit/UIPanGestureRecognizer.h>
-#import <UIKit/UIScrollView.h>
+#import <WebCore/ScrollingTreeFrameScrollingNode.h>
#import <wtf/BlockObjCExceptions.h>
#import <wtf/SetForScope.h>
@@ -272,7 +273,12 @@
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)
