Changeset 171250 in webkit

Timestamp:

Jul 18, 2014 5:00:52 PM (10 years ago)

Author:

Simon Fraser

Message:

[iOS WK2] position:fixed in iframes with programmatic scroll could end up in the wrong place
​https://bugs.webkit.org/show_bug.cgi?id=135078

Source/WebCore:
<rdar://problem/17401823>

Reviewed by Tim Horton.

When the UI-side scrolling tree receives a requested scroll position update, it scrolls
the node (e.g. a frame) then traverses child nodes to update them, e.g. for fixed position.
However, we would always use a viewport rect for the main document (from the scrolling tree),
which is not appropriate for subframes. Subframes should just use their own visible
rect to position fixed children.

Test: platform/mac-wk2/tiled-drawing/scrolling/frames/fixed-inside-frame.html

• page/scrolling/ios/ScrollingTreeFrameScrollingNodeIOS.mm:

(WebCore::ScrollingTreeFrameScrollingNodeIOS::updateChildNodesAfterScroll):

Source/WebKit2:
<rdar://problem/17401823>

Reviewed by Tim Horton.

Fix the logging of requested scroll position and frame scale factor.

• Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp:

(WebKit::RemoteScrollingTreeTextStream::dump):

LayoutTests:

Reviewed by Tim Horton.

Test with position:fixed inside an iframe that scrolls programmatically.

• platform/mac-wk2/tiled-drawing/scrolling/frames/fixed-inside-frame-expected.txt: Added.
• platform/mac-wk2/tiled-drawing/scrolling/frames/fixed-inside-frame.html: Added.
• platform/mac-wk2/tiled-drawing/scrolling/frames/resources/autoscrolling-frame-with-fixed.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/frames/fixed-inside-frame-expected.txt (added)
• LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/frames/fixed-inside-frame.html (added)
• LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/frames/resources/autoscrolling-frame-with-fixed.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/scrolling/ios/ScrollingTreeFrameScrollingNodeIOS.mm (modified) (1 diff)
• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2014-07-18  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] position:fixed in iframes with programmatic scroll could end up in the wrong place
+        https://bugs.webkit.org/show_bug.cgi?id=135078
+
+        Reviewed by Tim Horton.
+        
+        Test with position:fixed inside an iframe that scrolls programmatically.
+
+        * platform/mac-wk2/tiled-drawing/scrolling/frames/fixed-inside-frame-expected.txt: Added.
+        * platform/mac-wk2/tiled-drawing/scrolling/frames/fixed-inside-frame.html: Added.
+        * platform/mac-wk2/tiled-drawing/scrolling/frames/resources/autoscrolling-frame-with-fixed.html: Added.
+
 2014-07-18  Alexey Proskuryakov  <ap@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-07-18  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] position:fixed in iframes with programmatic scroll could end up in the wrong place
+        https://bugs.webkit.org/show_bug.cgi?id=135078
+        <rdar://problem/17401823>
+
+        Reviewed by Tim Horton.
+        
+        When the UI-side scrolling tree receives a requested scroll position update, it scrolls
+        the node (e.g. a frame) then traverses child nodes to update them, e.g. for fixed position.
+        However, we would always use a viewport rect for the main document (from the scrolling tree),
+        which is not appropriate for subframes. Subframes should just use their own visible
+        rect to position fixed children.
+
+        Test: platform/mac-wk2/tiled-drawing/scrolling/frames/fixed-inside-frame.html
+
+        * page/scrolling/ios/ScrollingTreeFrameScrollingNodeIOS.mm:
+        (WebCore::ScrollingTreeFrameScrollingNodeIOS::updateChildNodesAfterScroll):
+
 2014-07-18  Beth Dakin  <bdakin@apple.com>
 

trunk/Source/WebCore/page/scrolling/ios/ScrollingTreeFrameScrollingNodeIOS.mm

@@ -171 +171 @@
         return;
 
-    FloatRect fixedPositionRect = scrollingTree().fixedPositionRect();
-    
+
+    FloatRect fixedPositionRect;
+    if (!parent())
+        fixedPositionRect = scrollingTree().fixedPositionRect();
+    else
+        fixedPositionRect = FloatRect(scrollOffset, scrollableAreaSize());
+
     for (auto& child : *m_children)
         child->updateLayersAfterAncestorChange(*this, fixedPositionRect, FloatSize());

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2014-07-18  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] position:fixed in iframes with programmatic scroll could end up in the wrong place
+        https://bugs.webkit.org/show_bug.cgi?id=135078
+        <rdar://problem/17401823>
+
+        Reviewed by Tim Horton.
+        
+        Fix the logging of requested scroll position and frame scale factor.
+
+        * Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp:
+        (WebKit::RemoteScrollingTreeTextStream::dump):
+
 2014-07-18  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/Source/WebKit2/Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp

@@ -582 +582 @@
         dumpProperty(ts, "scroll-origin", node.scrollOrigin());
 
-    if (!changedPropertiesOnly || node.hasChangedProperty(ScrollingStateFrameScrollingNode::FrameScaleFactor))
     if (!changedPropertiesOnly || node.hasChangedProperty(ScrollingStateScrollingNode::RequestedScrollPosition)) {
         dumpProperty(ts, "requested-scroll-position", node.requestedScrollPosition());
@@ -628 +627 @@
         dumpProperty(ts, "top-content-inset", node.topContentInset());
 
+    if (!changedPropertiesOnly || node.hasChangedProperty(ScrollingStateFrameScrollingNode::FrameScaleFactor))
+        dumpProperty(ts, "frame-scale-factor", node.frameScaleFactor());
+
     if (!changedPropertiesOnly || node.hasChangedProperty(ScrollingStateFrameScrollingNode::ScrolledContentsLayer))
         dumpProperty(ts, "scrolled-contents-layer", static_cast<GraphicsLayer::PlatformLayerID>(node.scrolledContentsLayer()));