Fixed elements should not rubber-band in WK2, nor remain at negative offsets
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Jan 2017 19:46:40 +0000 (19:46 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Jan 2017 19:46:40 +0000 (19:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=167484
rdar://problem/29453068

Reviewed by Dean Jackson.
Source/WebCore:

There were various problems with the layout rect computation:
1. It ignored the scrollBehaviorForFixedElements() which we use to avoid rubber-banding fixed
   elements in WK2, but allow in WK1, so make use of that.
2. Sometimes layouts/paints of fixed elements would be triggered when coalesced calls to
   AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll() failed to
   copy the layoutViewportOrigin to the scheduled update.
3. The layout viewport could be left with a negative top/left after rubber-banding.

Also add a way to do unconstrained scrollTo(), so that a test can call window.scrollTo(-10, -10) to
simulate rubberbanding.

Tests: fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html
       fast/visual-viewport/rubberbanding-viewport-rects.html

* page/FrameView.cpp:
(WebCore::FrameView::computeLayoutViewportOrigin): Handle ScrollBehaviorForFixedElements, incorporating it
into logic that clamps layoutViewportOrigin between min/max when rubberbanding is not allowed, or not in progress.
(WebCore::FrameView::updateLayoutViewport): Pass in scrollBehaviorForFixedElements().
(WebCore::FrameView::visibleDocumentRect): The clamping here was preventing the visible rect from
escaping the document bounds, which caused fixed elements to bounce with rubber-banding, so remove the clamping,
and fix the logic to allow rubber-banding while taking headers and footers into account.
* page/FrameView.h:
* page/scrolling/AsyncScrollingCoordinator.cpp:
(WebCore::AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll): layoutViewportOrigin has to
be pushed onto the scheduled update, just like scroll position.
* page/scrolling/ScrollingTreeFrameScrollingNode.cpp:
(WebCore::ScrollingTreeFrameScrollingNode::layoutViewportForScrollPosition): Pass in m_behaviorForFixed.
* platform/ScrollView.cpp:
(WebCore::ScrollView::ScrollView):
(WebCore::ScrollView::adjustScrollPositionWithinRange):
(WebCore::ScrollView::setScrollOffset):
* platform/ScrollView.h:
(WebCore::ScrollView::setAllowsUnclampedScrollPositionForTesting):
(WebCore::ScrollView::allowsUnclampedScrollPosition):
* testing/InternalSettings.cpp:
(WebCore::InternalSettings::setAllowUnclampedScrollPosition):
* testing/InternalSettings.h:
* testing/InternalSettings.idl:

Source/WebKit2:

Pass in StickToViewportBounds as we did before visual viewports.

* UIProcess/ios/WebPageProxyIOS.mm:
(WebKit::WebPageProxy::computeCustomFixedPositionRect):

LayoutTests:

Add two tests that use internals.settings.setAllowUnclampedScrollPosition(true) and then
over-scroll to simulator rubber-banding, dumping viewport rects.

setAllowUnclampedScrollPosition() only works in WebKit2, so skip the tests elsewhere.

* TestExpectations:
* fast/visual-viewport/rubberbanding-viewport-rects-expected.txt: Added.
* fast/visual-viewport/rubberbanding-viewport-rects-header-footer-expected.txt: Added.
* fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html: Added.
* fast/visual-viewport/rubberbanding-viewport-rects.html: Added.
* platform/ios-simulator-wk2/TestExpectations:
* platform/ios-simulator-wk2/fast/visual-viewport/rubberbanding-viewport-rects-expected.txt: Added.
* platform/ios-simulator-wk2/fast/visual-viewport/rubberbanding-viewport-rects-header-footer-expected.txt: Added.
* platform/mac-wk2/TestExpectations:

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

22 files changed:
LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/fast/visual-viewport/rubberbanding-viewport-rects-expected.txt [new file with mode: 0644]
LayoutTests/fast/visual-viewport/rubberbanding-viewport-rects-header-footer-expected.txt [new file with mode: 0644]
LayoutTests/fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html [new file with mode: 0644]
LayoutTests/fast/visual-viewport/rubberbanding-viewport-rects.html [new file with mode: 0644]
LayoutTests/platform/ios-simulator-wk2/TestExpectations
LayoutTests/platform/ios-simulator-wk2/fast/visual-viewport/rubberbanding-viewport-rects-expected.txt [new file with mode: 0644]
LayoutTests/platform/ios-simulator-wk2/fast/visual-viewport/rubberbanding-viewport-rects-header-footer-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
Source/WebCore/page/scrolling/ScrollingTreeFrameScrollingNode.cpp
Source/WebCore/platform/ScrollView.cpp
Source/WebCore/platform/ScrollView.h
Source/WebCore/testing/InternalSettings.cpp
Source/WebCore/testing/InternalSettings.h
Source/WebCore/testing/InternalSettings.idl
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm

index 7b74686..72f7fc2 100644 (file)
@@ -1,3 +1,26 @@
+2017-01-30  Simon Fraser  <simon.fraser@apple.com>
+
+        Fixed elements should not rubber-band in WK2, nor remain at negative offsets
+        https://bugs.webkit.org/show_bug.cgi?id=167484
+        rdar://problem/29453068
+
+        Reviewed by Dean Jackson.
+        
+        Add two tests that use internals.settings.setAllowUnclampedScrollPosition(true) and then
+        over-scroll to simulator rubber-banding, dumping viewport rects.
+        
+        setAllowUnclampedScrollPosition() only works in WebKit2, so skip the tests elsewhere.
+
+        * TestExpectations:
+        * fast/visual-viewport/rubberbanding-viewport-rects-expected.txt: Added.
+        * fast/visual-viewport/rubberbanding-viewport-rects-header-footer-expected.txt: Added.
+        * fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html: Added.
+        * fast/visual-viewport/rubberbanding-viewport-rects.html: Added.
+        * platform/ios-simulator-wk2/TestExpectations:
+        * platform/ios-simulator-wk2/fast/visual-viewport/rubberbanding-viewport-rects-expected.txt: Added.
+        * platform/ios-simulator-wk2/fast/visual-viewport/rubberbanding-viewport-rects-header-footer-expected.txt: Added.
+        * platform/mac-wk2/TestExpectations:
+
 2017-01-30  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rollout r211235 Pointer lock events should be delivered directly to the target element.
index bb2b6c1..7bb6513 100644 (file)
@@ -89,6 +89,10 @@ fast/media/mq-prefers-reduced-motion-live-update.html [ Skip ]
 # ApplePay is only available on iOS (greater than iOS 10) and macOS (greater than macOS 10.12) and only for WebKit2.
 http/tests/ssl/applepay/ [ Skip ]
 
+# Only WK2 allows unconstrained scrolling
+fast/visual-viewport/rubberbanding-viewport-rects.html [ Skip ]
+fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html  [ Skip ]
+
 #//////////////////////////////////////////////////////////////////////////////////////////
 # End platform-specific tests.
 #//////////////////////////////////////////////////////////////////////////////////////////
diff --git a/LayoutTests/fast/visual-viewport/rubberbanding-viewport-rects-expected.txt b/LayoutTests/fast/visual-viewport/rubberbanding-viewport-rects-expected.txt
new file mode 100644 (file)
index 0000000..066defe
--- /dev/null
@@ -0,0 +1,20 @@
+This test scrolls the page and checks that the layout and visual viewports respond as expected.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Scrolled to -123, -234
+JSON.stringify(layoutViewport) is {"top":0,"right":785,"bottom":585,"left":0,"width":785,"height":585}
+JSON.stringify(visualViewport) is {"top":-234,"right":662,"bottom":351,"left":-123,"width":785,"height":585}
+
+Scrolled to 475, 525
+JSON.stringify(layoutViewport) is {"top":525,"right":1260,"bottom":1110,"left":475,"width":785,"height":585}
+JSON.stringify(visualViewport) is {"top":525,"right":1260,"bottom":1110,"left":475,"width":785,"height":585}
+
+Scrolled to 1800, 1700
+JSON.stringify(layoutViewport) is {"top":1690,"right":2008,"bottom":2275,"left":1223,"width":785,"height":585}
+JSON.stringify(visualViewport) is {"top":1700,"right":2585,"bottom":2275,"left":1800,"width":785,"height":575}
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/visual-viewport/rubberbanding-viewport-rects-header-footer-expected.txt b/LayoutTests/fast/visual-viewport/rubberbanding-viewport-rects-header-footer-expected.txt
new file mode 100644 (file)
index 0000000..12c84a9
--- /dev/null
@@ -0,0 +1,20 @@
+This test scrolls the page and checks that the layout and visual viewports respond as expected.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Scrolled to -123, -234
+JSON.stringify(layoutViewport) is {"top":0,"right":785,"bottom":585,"left":0,"width":785,"height":585}
+JSON.stringify(visualViewport) is {"top":-234,"right":662,"bottom":299,"left":-123,"width":785,"height":533}
+
+Scrolled to 475, 525
+JSON.stringify(layoutViewport) is {"top":473,"right":1260,"bottom":1058,"left":475,"width":785,"height":585}
+JSON.stringify(visualViewport) is {"top":473,"right":1260,"bottom":1058,"left":475,"width":785,"height":585}
+
+Scrolled to 1800, 1700
+JSON.stringify(layoutViewport) is {"top":1648,"right":2008,"bottom":2233,"left":1223,"width":785,"height":585}
+JSON.stringify(visualViewport) is {"top":1648,"right":2585,"bottom":2233,"left":1800,"width":785,"height":585}
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html b/LayoutTests/fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html
new file mode 100644 (file)
index 0000000..4e7f257
--- /dev/null
@@ -0,0 +1,66 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <script src="../../resources/js-test-pre.js"></script>
+    <style>
+        body {
+            height: 2000px;
+            width: 2000px;
+        }
+    </style>
+    <script>
+        description("This test scrolls the page and checks that the layout and visual viewports respond as expected.");
+
+        if (window.internals) {
+            internals.settings.setVisualViewportEnabled(true);
+            internals.settings.setAllowUnclampedScrollPosition(true);
+            internals.setHeaderHeight(52);
+            internals.setFooterHeight(43);
+        }
+
+        window.jsTestIsAsync = true;
+
+        var visualViewport;
+        function doTest()
+        {
+            // Zooming may scroll the view away from the origin.
+            window.scrollTo(-123, -234);
+            visualViewport = internals.visualViewportRect();
+            layoutViewport = internals.layoutViewportRect();
+            debug('Scrolled to ' + window.scrollX + ', ' + window.scrollY);
+
+            // Don't use shouldBeEqualToString() to avoid showing failures when correct output differs between platforms. 
+            evalAndLogResult("JSON.stringify(layoutViewport)");
+            evalAndLogResult("JSON.stringify(visualViewport)");
+
+            debug('');
+            window.scrollTo(475, 525);
+            visualViewport = internals.visualViewportRect();
+            layoutViewport = internals.layoutViewportRect();
+            debug('Scrolled to ' + window.scrollX + ', ' + window.scrollY);
+
+            evalAndLogResult("JSON.stringify(layoutViewport)");
+            evalAndLogResult("JSON.stringify(visualViewport)");
+
+            debug('');
+            window.scrollTo(1800, 1700);
+            visualViewport = internals.visualViewportRect();
+            layoutViewport = internals.layoutViewportRect();
+            debug('Scrolled to ' + window.scrollX + ', ' + window.scrollY);
+
+            evalAndLogResult("JSON.stringify(layoutViewport)");
+            evalAndLogResult("JSON.stringify(visualViewport)");
+
+            window.scrollTo(0, 0);
+
+            finishJSTest();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/visual-viewport/rubberbanding-viewport-rects.html b/LayoutTests/fast/visual-viewport/rubberbanding-viewport-rects.html
new file mode 100644 (file)
index 0000000..c8bb3ce
--- /dev/null
@@ -0,0 +1,64 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <script src="../../resources/js-test-pre.js"></script>
+    <style>
+        body {
+            height: 2000px;
+            width: 2000px;
+        }
+    </style>
+    <script>
+        description("This test scrolls the page and checks that the layout and visual viewports respond as expected.");
+
+        if (window.internals) {
+            internals.settings.setVisualViewportEnabled(true);
+            internals.settings.setAllowUnclampedScrollPosition(true);
+        }
+
+        window.jsTestIsAsync = true;
+
+        var visualViewport;
+        function doTest()
+        {
+            // Zooming may scroll the view away from the origin.
+            window.scrollTo(-123, -234);
+            visualViewport = internals.visualViewportRect();
+            layoutViewport = internals.layoutViewportRect();
+            debug('Scrolled to ' + window.scrollX + ', ' + window.scrollY);
+
+            // Don't use shouldBeEqualToString() to avoid showing failures when correct output differs between platforms. 
+            evalAndLogResult("JSON.stringify(layoutViewport)");
+            evalAndLogResult("JSON.stringify(visualViewport)");
+
+            debug('');
+            window.scrollTo(475, 525);
+            visualViewport = internals.visualViewportRect();
+            layoutViewport = internals.layoutViewportRect();
+            debug('Scrolled to ' + window.scrollX + ', ' + window.scrollY);
+
+            evalAndLogResult("JSON.stringify(layoutViewport)");
+            evalAndLogResult("JSON.stringify(visualViewport)");
+
+            debug('');
+            window.scrollTo(1800, 1700);
+            visualViewport = internals.visualViewportRect();
+            layoutViewport = internals.layoutViewportRect();
+            debug('Scrolled to ' + window.scrollX + ', ' + window.scrollY);
+
+            evalAndLogResult("JSON.stringify(layoutViewport)");
+            evalAndLogResult("JSON.stringify(visualViewport)");
+
+            window.scrollTo(0, 0);
+
+            finishJSTest();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
index 162856a..3efbedc 100644 (file)
@@ -17,6 +17,9 @@ fast/media/mq-inverted-colors-live-update-in-subframes.html [ Pass ]
 fast/media/mq-monochrome-live-update.html [ Pass ]
 fast/media/mq-prefers-reduced-motion-live-update.html [ Pass ]
 
+fast/visual-viewport/rubberbanding-viewport-rects.html [ Pass ]
+fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html  [ Pass ]
+
 #//////////////////////////////////////////////////////////////////////////////////////////
 # End platform-specific directories.
 #//////////////////////////////////////////////////////////////////////////////////////////
diff --git a/LayoutTests/platform/ios-simulator-wk2/fast/visual-viewport/rubberbanding-viewport-rects-expected.txt b/LayoutTests/platform/ios-simulator-wk2/fast/visual-viewport/rubberbanding-viewport-rects-expected.txt
new file mode 100644 (file)
index 0000000..65589d0
--- /dev/null
@@ -0,0 +1,20 @@
+This test scrolls the page and checks that the layout and visual viewports respond as expected.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Scrolled to -123, -234
+JSON.stringify(layoutViewport) is {"top":0,"right":800,"bottom":600,"left":0,"width":800,"height":600}
+JSON.stringify(visualViewport) is {"top":-234,"right":677,"bottom":366,"left":-123,"width":800,"height":600}
+
+Scrolled to 475, 525
+JSON.stringify(layoutViewport) is {"top":525,"right":1275,"bottom":1125,"left":475,"width":800,"height":600}
+JSON.stringify(visualViewport) is {"top":525,"right":1275,"bottom":1125,"left":475,"width":800,"height":600}
+
+Scrolled to 1800, 1700
+JSON.stringify(layoutViewport) is {"top":1616,"right":2008,"bottom":2216,"left":1208,"width":800,"height":600}
+JSON.stringify(visualViewport) is {"top":1700,"right":2600,"bottom":2216,"left":1800,"width":800,"height":516}
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/platform/ios-simulator-wk2/fast/visual-viewport/rubberbanding-viewport-rects-header-footer-expected.txt b/LayoutTests/platform/ios-simulator-wk2/fast/visual-viewport/rubberbanding-viewport-rects-header-footer-expected.txt
new file mode 100644 (file)
index 0000000..9e4037f
--- /dev/null
@@ -0,0 +1,20 @@
+This test scrolls the page and checks that the layout and visual viewports respond as expected.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Scrolled to -123, -234
+JSON.stringify(layoutViewport) is {"top":0,"right":800,"bottom":600,"left":0,"width":800,"height":600}
+JSON.stringify(visualViewport) is {"top":-234,"right":677,"bottom":314,"left":-123,"width":800,"height":548}
+
+Scrolled to 475, 525
+JSON.stringify(layoutViewport) is {"top":473,"right":1275,"bottom":1073,"left":475,"width":800,"height":600}
+JSON.stringify(visualViewport) is {"top":473,"right":1275,"bottom":1073,"left":475,"width":800,"height":600}
+
+Scrolled to 1800, 1700
+JSON.stringify(layoutViewport) is {"top":1648,"right":2008,"bottom":2248,"left":1208,"width":800,"height":600}
+JSON.stringify(visualViewport) is {"top":1648,"right":2600,"bottom":2248,"left":1800,"width":800,"height":600}
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
index 047822c..8a5513e 100644 (file)
@@ -23,6 +23,9 @@ fast/media/mq-prefers-reduced-motion-live-update.html [ Pass ]
 
 [ Sierra+ ] http/tests/ssl/applepay/ [ Pass ]
 
+fast/visual-viewport/rubberbanding-viewport-rects.html [ Pass ]
+fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html  [ Pass ]
+
 #//////////////////////////////////////////////////////////////////////////////////////////
 # End platform-specific directories.
 #//////////////////////////////////////////////////////////////////////////////////////////
index 262ad35..c7b2333 100644 (file)
@@ -1,3 +1,50 @@
+2017-01-30  Simon Fraser  <simon.fraser@apple.com>
+
+        Fixed elements should not rubber-band in WK2, nor remain at negative offsets
+        https://bugs.webkit.org/show_bug.cgi?id=167484
+        rdar://problem/29453068
+
+        Reviewed by Dean Jackson.
+
+        There were various problems with the layout rect computation:
+        1. It ignored the scrollBehaviorForFixedElements() which we use to avoid rubber-banding fixed
+           elements in WK2, but allow in WK1, so make use of that.
+        2. Sometimes layouts/paints of fixed elements would be triggered when coalesced calls to
+           AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll() failed to
+           copy the layoutViewportOrigin to the scheduled update.
+        3. The layout viewport could be left with a negative top/left after rubber-banding.
+        
+        Also add a way to do unconstrained scrollTo(), so that a test can call window.scrollTo(-10, -10) to
+        simulate rubberbanding.
+
+        Tests: fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html
+               fast/visual-viewport/rubberbanding-viewport-rects.html
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::computeLayoutViewportOrigin): Handle ScrollBehaviorForFixedElements, incorporating it
+        into logic that clamps layoutViewportOrigin between min/max when rubberbanding is not allowed, or not in progress.
+        (WebCore::FrameView::updateLayoutViewport): Pass in scrollBehaviorForFixedElements().
+        (WebCore::FrameView::visibleDocumentRect): The clamping here was preventing the visible rect from
+        escaping the document bounds, which caused fixed elements to bounce with rubber-banding, so remove the clamping,
+        and fix the logic to allow rubber-banding while taking headers and footers into account.
+        * page/FrameView.h:
+        * page/scrolling/AsyncScrollingCoordinator.cpp:
+        (WebCore::AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll): layoutViewportOrigin has to
+        be pushed onto the scheduled update, just like scroll position.
+        * page/scrolling/ScrollingTreeFrameScrollingNode.cpp:
+        (WebCore::ScrollingTreeFrameScrollingNode::layoutViewportForScrollPosition): Pass in m_behaviorForFixed.
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::ScrollView):
+        (WebCore::ScrollView::adjustScrollPositionWithinRange):
+        (WebCore::ScrollView::setScrollOffset):
+        * platform/ScrollView.h:
+        (WebCore::ScrollView::setAllowsUnclampedScrollPositionForTesting):
+        (WebCore::ScrollView::allowsUnclampedScrollPosition):
+        * testing/InternalSettings.cpp:
+        (WebCore::InternalSettings::setAllowUnclampedScrollPosition):
+        * testing/InternalSettings.h:
+        * testing/InternalSettings.idl:
+
 2017-01-30  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Web content process crashes when initiating a drag on a very large image
index 1de401f..cb1ba1d 100644 (file)
@@ -1806,28 +1806,47 @@ void FrameView::removeViewportConstrainedObject(RenderElement* object)
 }
 
 // visualViewport and layoutViewport are both in content coordinates (unzoomed).
-LayoutPoint FrameView::computeLayoutViewportOrigin(const LayoutRect& visualViewport, const LayoutPoint& stableLayoutViewportOriginMin, const LayoutPoint& stableLayoutViewportOriginMax, const LayoutRect& layoutViewport)
+LayoutPoint FrameView::computeLayoutViewportOrigin(const LayoutRect& visualViewport, const LayoutPoint& stableLayoutViewportOriginMin, const LayoutPoint& stableLayoutViewportOriginMax, const LayoutRect& layoutViewport, ScrollBehaviorForFixedElements fixedBehavior)
 {
     LayoutPoint layoutViewportOrigin = layoutViewport.location();
+    bool allowRubberBanding = fixedBehavior == StickToViewportBounds;
 
     if (visualViewport.width() > layoutViewport.width())
         layoutViewportOrigin.setX(visualViewport.x());
     else {
-        if (visualViewport.x() < layoutViewport.x() || visualViewport.x() < stableLayoutViewportOriginMin.x())
+        bool rubberbandingAtLeft = allowRubberBanding && visualViewport.x() < stableLayoutViewportOriginMin.x();
+        bool rubberbandingAtRight = allowRubberBanding && (visualViewport.maxX() - layoutViewport.width()) > stableLayoutViewportOriginMax.x();
+
+        if (visualViewport.x() < layoutViewport.x() || rubberbandingAtLeft)
             layoutViewportOrigin.setX(visualViewport.x());
 
-        if (visualViewport.maxX() > layoutViewport.maxX() || (visualViewport.maxX() - layoutViewport.width()) > stableLayoutViewportOriginMax.x())
+        if (visualViewport.maxX() > layoutViewport.maxX() || rubberbandingAtRight)
             layoutViewportOrigin.setX(visualViewport.maxX() - layoutViewport.width());
+        
+        if (!rubberbandingAtLeft && layoutViewportOrigin.x() < stableLayoutViewportOriginMin.x())
+            layoutViewportOrigin.setX(stableLayoutViewportOriginMin.x());
+        
+        if (!rubberbandingAtRight && layoutViewportOrigin.x() > stableLayoutViewportOriginMax.x())
+            layoutViewportOrigin.setX(stableLayoutViewportOriginMax.x());
     }
 
     if (visualViewport.height() > layoutViewport.height())
         layoutViewportOrigin.setY(visualViewport.y());
     else {
-        if (visualViewport.y() < layoutViewport.y() || visualViewport.y() < stableLayoutViewportOriginMin.y())
+        bool rubberbandingAtTop = allowRubberBanding && visualViewport.y() < stableLayoutViewportOriginMin.y();
+        bool rubberbandingAtBottom = allowRubberBanding && (visualViewport.maxY() - layoutViewport.height()) > stableLayoutViewportOriginMax.y();
+
+        if (visualViewport.y() < layoutViewport.y() || rubberbandingAtTop)
             layoutViewportOrigin.setY(visualViewport.y());
 
-        if (visualViewport.maxY() > layoutViewport.maxY() || (visualViewport.maxY() - layoutViewport.height()) > stableLayoutViewportOriginMax.y())
+        if (visualViewport.maxY() > layoutViewport.maxY() || rubberbandingAtBottom)
             layoutViewportOrigin.setY(visualViewport.maxY() - layoutViewport.height());
+        
+        if (!rubberbandingAtTop && layoutViewportOrigin.y() < stableLayoutViewportOriginMin.y())
+            layoutViewportOrigin.setY(stableLayoutViewportOriginMin.y());
+        
+        if (!rubberbandingAtBottom && layoutViewportOrigin.y() > stableLayoutViewportOriginMax.y())
+            layoutViewportOrigin.setY(stableLayoutViewportOriginMax.y());
     }
 
     return layoutViewportOrigin;
@@ -1892,7 +1911,7 @@ void FrameView::updateLayoutViewport()
     LOG_WITH_STREAM(Scrolling, stream << "visualViewport: " << visualViewportRect());
     LOG_WITH_STREAM(Scrolling, stream << "scroll positions: min: " << unscaledMinimumScrollPosition() << " max: "<< unscaledMaximumScrollPosition());
 
-    LayoutPoint newLayoutViewportOrigin = computeLayoutViewportOrigin(visualViewportRect(), minStableLayoutViewportOrigin(), maxStableLayoutViewportOrigin(), layoutViewport);
+    LayoutPoint newLayoutViewportOrigin = computeLayoutViewportOrigin(visualViewportRect(), minStableLayoutViewportOrigin(), maxStableLayoutViewportOrigin(), layoutViewport, scrollBehaviorForFixedElements());
     if (newLayoutViewportOrigin != m_layoutViewportOrigin) {
         setBaseLayoutViewportOrigin(newLayoutViewportOrigin);
         LOG_WITH_STREAM(Scrolling, stream << "layoutViewport changed to " << layoutViewportRect());
@@ -1934,16 +1953,19 @@ LayoutRect FrameView::layoutViewportRect() const
 // On iOS, pageScaleFactor is always 1 here, and we never have headers and footers.
 LayoutRect FrameView::visibleDocumentRect(const FloatRect& visibleContentRect, float headerHeight, float footerHeight, const FloatSize& totalContentsSize, float pageScaleFactor)
 {
-    FloatRect visibleDocumentRect = visibleContentRect;
-
     float contentsHeight = totalContentsSize.height() - headerHeight - footerHeight;
 
-    float visibleScaledDocumentTop = std::max<float>(visibleContentRect.y() - headerHeight, 0);
-    float visibleScaledDocumentBottom = std::min<float>(visibleContentRect.maxY() - headerHeight, contentsHeight);
+    float rubberBandTop = std::min<float>(visibleContentRect.y(), 0);
+    float visibleScaledDocumentTop = std::max<float>(visibleContentRect.y() - headerHeight, 0) + rubberBandTop;
+    
+    float rubberBandBottom = std::min<float>((totalContentsSize.height() - visibleContentRect.y()) - visibleContentRect.height(), 0);
+    float visibleScaledDocumentBottom = std::min<float>(visibleContentRect.maxY() - headerHeight, contentsHeight) + rubberBandBottom;
 
+    FloatRect visibleDocumentRect = visibleContentRect;
     visibleDocumentRect.setY(visibleScaledDocumentTop);
     visibleDocumentRect.setHeight(std::max<float>(visibleScaledDocumentBottom - visibleScaledDocumentTop, 0));
     visibleDocumentRect.scale(1 / pageScaleFactor);
+    
     return LayoutRect(visibleDocumentRect);
 }
 
index c7d00a7..99d459f 100644 (file)
@@ -304,7 +304,7 @@ public:
     // Static function can be called from another thread.
     static LayoutPoint scrollPositionForFixedPosition(const LayoutRect& visibleContentRect, const LayoutSize& totalContentsSize, const LayoutPoint& scrollPosition, const LayoutPoint& scrollOrigin, float frameScaleFactor, bool fixedElementsLayoutRelativeToFrame, ScrollBehaviorForFixedElements, int headerHeight, int footerHeight);
 
-    WEBCORE_EXPORT static LayoutPoint computeLayoutViewportOrigin(const LayoutRect& visualViewport, const LayoutPoint& stableLayoutViewportOriginMin, const LayoutPoint& stableLayoutViewportOriginMax, const LayoutRect& layoutViewport);
+    WEBCORE_EXPORT static LayoutPoint computeLayoutViewportOrigin(const LayoutRect& visualViewport, const LayoutPoint& stableLayoutViewportOriginMin, const LayoutPoint& stableLayoutViewportOriginMax, const LayoutRect& layoutViewport, ScrollBehaviorForFixedElements fixedBehavior);
 
     // These layers are positioned differently when there is a topContentInset, a header, or a footer. These value need to be computed
     // on both the main thread and the scrolling thread.
index 74cf83d..d08f5a8 100644 (file)
@@ -269,6 +269,7 @@ void AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll(Scr
     if (m_updateNodeScrollPositionTimer.isActive()) {
         if (m_scheduledScrollUpdate.matchesUpdateType(scrollUpdate)) {
             m_scheduledScrollUpdate.scrollPosition = scrollPosition;
+            m_scheduledScrollUpdate.layoutViewportOrigin = layoutViewportOrigin;
             return;
         }
     
index 0f5e691..1f48f15 100644 (file)
@@ -111,7 +111,7 @@ FloatRect ScrollingTreeFrameScrollingNode::layoutViewportForScrollPosition(const
     LOG_WITH_STREAM(Scrolling, stream << "  visualViewport: " << visualViewport);
     LOG_WITH_STREAM(Scrolling, stream << "  scroll positions: min: " << minLayoutViewportOrigin() << " max: "<< maxLayoutViewportOrigin());
 
-    LayoutPoint newLocation = FrameView::computeLayoutViewportOrigin(LayoutRect(visualViewport), LayoutPoint(minLayoutViewportOrigin()), LayoutPoint(maxLayoutViewportOrigin()), layoutViewport);
+    LayoutPoint newLocation = FrameView::computeLayoutViewportOrigin(LayoutRect(visualViewport), LayoutPoint(minLayoutViewportOrigin()), LayoutPoint(maxLayoutViewportOrigin()), layoutViewport, m_behaviorForFixed);
 
     if (layoutViewport.location() != newLocation) {
         layoutViewport.setLocation(newLocation);
index b89d566..a425f6b 100644 (file)
 namespace WebCore {
 
 ScrollView::ScrollView()
-    : m_horizontalScrollbarMode(ScrollbarAuto)
-    , m_verticalScrollbarMode(ScrollbarAuto)
-    , m_horizontalScrollbarLock(false)
-    , m_verticalScrollbarLock(false)
-    , m_prohibitsScrolling(false)
-    , m_canBlitOnScroll(true)
-    , m_scrollbarsSuppressed(false)
-    , m_inUpdateScrollbars(false)
-    , m_updateScrollbarsPass(0)
-    , m_drawPanScrollIcon(false)
-    , m_useFixedLayout(false)
-    , m_paintsEntireContents(false)
-    , m_clipsRepaints(true)
-    , m_delegatesScrolling(false)
 {
 }
 
@@ -381,7 +367,7 @@ ScrollPosition ScrollView::maximumScrollPosition() const
 
 ScrollPosition ScrollView::adjustScrollPositionWithinRange(const ScrollPosition& scrollPoint) const
 {
-    if (!constrainsScrollingToContentEdge())
+    if (!constrainsScrollingToContentEdge() || m_allowsUnclampedScrollPosition)
         return scrollPoint;
 
     return scrollPoint.constrainedBetween(minimumScrollPosition(), maximumScrollPosition());
@@ -418,7 +404,7 @@ void ScrollView::notifyPageThatContentAreaWillPaint() const
 
 void ScrollView::setScrollOffset(const ScrollOffset& offset)
 {
-    LOG_WITH_STREAM(Scrolling, stream << "\nScrollView::setScrollOffset " << offset);
+    LOG_WITH_STREAM(Scrolling, stream << "\nScrollView::setScrollOffset " << offset << " constrains " << constrainsScrollingToContentEdge());
 
     IntPoint constrainedOffset = offset;
     if (constrainsScrollingToContentEdge())
index 6b31f98..4a9055b 100644 (file)
@@ -373,6 +373,9 @@ public:
 
     WEBCORE_EXPORT void scrollOffsetChangedViaPlatformWidget(const ScrollOffset& oldOffset, const ScrollOffset& newOffset);
 
+    void setAllowsUnclampedScrollPositionForTesting(bool allowsUnclampedScrollPosition) { m_allowsUnclampedScrollPosition = allowsUnclampedScrollPosition; }
+    bool allowsUnclampedScrollPosition() const { return m_allowsUnclampedScrollPosition; }
+
 protected:
     ScrollView();
 
@@ -433,21 +436,12 @@ private:
 
     bool isScrollView() const final { return true; }
 
-    RefPtr<Scrollbar> m_horizontalScrollbar;
-    RefPtr<Scrollbar> m_verticalScrollbar;
-    ScrollbarMode m_horizontalScrollbarMode;
-    ScrollbarMode m_verticalScrollbarMode;
-
-    bool m_horizontalScrollbarLock;
-    bool m_verticalScrollbarLock;
-
-    bool m_prohibitsScrolling;
-
     HashSet<Ref<Widget>> m_children;
 
-    // This bool is unused on Mac OS because we directly ask the platform widget
-    // whether it is safe to blit on scroll.
-    bool m_canBlitOnScroll;
+    RefPtr<Scrollbar> m_horizontalScrollbar;
+    RefPtr<Scrollbar> m_verticalScrollbar;
+    ScrollbarMode m_horizontalScrollbarMode { ScrollbarAuto };
+    ScrollbarMode m_verticalScrollbarMode { ScrollbarAuto };
 
 #if PLATFORM(IOS)
     // FIXME: exposedContentRect is a very similar concept to fixedVisibleContentRect except it does not differentiate
@@ -466,18 +460,30 @@ private:
     std::optional<IntSize> m_deferredScrollDelta; // Needed for WebKit scrolling
     std::optional<std::pair<ScrollOffset, ScrollOffset>> m_deferredScrollOffsets; // Needed for platform widget scrolling
 
-    bool m_scrollbarsSuppressed;
+    IntPoint m_panScrollIconPoint;
 
-    bool m_inUpdateScrollbars;
-    unsigned m_updateScrollbarsPass;
+    bool m_horizontalScrollbarLock { false };
+    bool m_verticalScrollbarLock { false };
 
-    IntPoint m_panScrollIconPoint;
-    bool m_drawPanScrollIcon;
-    bool m_useFixedLayout;
+    bool m_prohibitsScrolling { false };
+    bool m_allowsUnclampedScrollPosition { false };
+
+    // This bool is unused on Mac OS because we directly ask the platform widget
+    // whether it is safe to blit on scroll.
+    bool m_canBlitOnScroll { true };
+
+    bool m_scrollbarsSuppressed { false };
+
+    bool m_inUpdateScrollbars { false };
+    unsigned m_updateScrollbarsPass { 0 };
+
+    bool m_drawPanScrollIcon { false };
+    bool m_useFixedLayout { false };
+
+    bool m_paintsEntireContents { false };
+    bool m_clipsRepaints { true };
+    bool m_delegatesScrolling { false };
 
-    bool m_paintsEntireContents;
-    bool m_clipsRepaints;
-    bool m_delegatesScrolling;
 
     void init();
     void destroy();
index 28d0b2b..8f43e07 100644 (file)
@@ -640,6 +640,15 @@ ExceptionOr<void> InternalSettings::setScrollingTreeIncludesFrames(bool enabled)
     return { };
 }
 
+ExceptionOr<void> InternalSettings::setAllowUnclampedScrollPosition(bool allowUnclamped)
+{
+    if (!m_page || !m_page->mainFrame().view())
+        return Exception { INVALID_ACCESS_ERR };
+
+    m_page->mainFrame().view()->setAllowsUnclampedScrollPositionForTesting(allowUnclamped);
+    return { };
+}
+
 ExceptionOr<void> InternalSettings::setAllowsInlineMediaPlayback(bool allows)
 {
     if (!m_page)
index 5c9e936..c654a7e 100644 (file)
@@ -87,6 +87,7 @@ public:
     ExceptionOr<void> setBackgroundShouldExtendBeyondPage(bool);
     ExceptionOr<void> setShouldConvertPositionStyleOnCopy(bool);
     ExceptionOr<void> setScrollingTreeIncludesFrames(bool);
+    ExceptionOr<void> setAllowUnclampedScrollPosition(bool);
     ExceptionOr<void> setAllowsInlineMediaPlayback(bool);
     ExceptionOr<void> setAllowsInlineMediaPlaybackAfterFullscreen(bool);
     ExceptionOr<void> setInlineMediaPlaybackRequiresPlaysInlineAttribute(bool);
index 5f4bdec..75a1030 100644 (file)
@@ -75,6 +75,7 @@ enum ForcedAccessibilityValue { "system", "on", "off" };
     [MayThrowException] void setAutoscrollForDragAndDropEnabled(boolean enabled);
     [MayThrowException] void setBackgroundShouldExtendBeyondPage(boolean hasExtendedBackground);
     [MayThrowException] void setScrollingTreeIncludesFrames(boolean enabled);
+    [MayThrowException] void setAllowUnclampedScrollPosition(boolean allowUnclamped);
 
     [MayThrowException] void setMinimumTimerInterval(unrestricted double intervalInSeconds);
     [MayThrowException] void setAllowsInlineMediaPlayback(boolean allows);
index fb65ff5..85da650 100644 (file)
@@ -1,3 +1,16 @@
+2017-01-30  Simon Fraser  <simon.fraser@apple.com>
+
+        Fixed elements should not rubber-band in WK2, nor remain at negative offsets
+        https://bugs.webkit.org/show_bug.cgi?id=167484
+        rdar://problem/29453068
+
+        Reviewed by Dean Jackson.
+
+        Pass in StickToViewportBounds as we did before visual viewports.
+
+        * UIProcess/ios/WebPageProxyIOS.mm:
+        (WebKit::WebPageProxy::computeCustomFixedPositionRect):
+
 2017-01-30  Chris Dumez  <cdumez@apple.com>
 
         Update DiagnosticLoggingClient::logDiagnosticMessageWithValue() to take in the value as a double
index 30fe595..e97985f 100644 (file)
@@ -268,9 +268,9 @@ WebCore::FloatRect WebPageProxy::computeCustomFixedPositionRect(const FloatRect&
 
     LayoutPoint layoutViewportOrigin;
     if (isBelowMinimumScale)
-        layoutViewportOrigin = FrameView::computeLayoutViewportOrigin(enclosingLayoutRect(constrainedUnobscuredRect), m_minStableLayoutViewportOrigin, m_maxStableLayoutViewportOrigin, enclosingLayoutRect(layoutViewportRect));
+        layoutViewportOrigin = FrameView::computeLayoutViewportOrigin(enclosingLayoutRect(constrainedUnobscuredRect), m_minStableLayoutViewportOrigin, m_maxStableLayoutViewportOrigin, enclosingLayoutRect(layoutViewportRect), StickToViewportBounds);
     else
-        layoutViewportOrigin = FrameView::computeLayoutViewportOrigin(enclosingLayoutRect(unobscuredContentRectRespectingInputViewBounds), m_minStableLayoutViewportOrigin, m_maxStableLayoutViewportOrigin, enclosingLayoutRect(layoutViewportRect));
+        layoutViewportOrigin = FrameView::computeLayoutViewportOrigin(enclosingLayoutRect(unobscuredContentRectRespectingInputViewBounds), m_minStableLayoutViewportOrigin, m_maxStableLayoutViewportOrigin, enclosingLayoutRect(layoutViewportRect), StickToViewportBounds);
 
     if (constraint == UnobscuredRectConstraint::ConstrainedToDocumentRect) {
         // The max stable layout viewport origin really depends on the size of the layout viewport itself, so we need to adjust the location of the layout viewport one final time to make sure it does not end up out of bounds of the document.