[iOS] Programmaic scroll of "scrolling=no" iframe fails
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 May 2020 18:23:57 +0000 (18:23 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 May 2020 18:23:57 +0000 (18:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=212063
<rdar://problem/57049514>

Reviewed by Antti Koivisto.
Source/WebCore:

ScrollView::setScrollPosition() calls requestScrollPositionUpdate(), and if this returns
false it relies on the confusingly-named updateScrollbars() to actually do the scroll.
This code path is hit for "scrolling=no" frames, which are not scroll-coordinated.

ScrollView::updateScrollbars() fails to set the scroll position on iOS, where managesScrollbars()
returns false, so fix that.

Test: fast/scrolling/progammatic-scroll-scrolling-no-frame.html

* platform/ScrollView.cpp:
(WebCore::ScrollView::updateScrollbars):

LayoutTests:

New passing results for some WPT tests (iOS-specific results are removed).

fast/frames/flattening/scrolling-in-object.html relies on mouse events, so skip on iOS. It happened
to pass because both test and reference showed the red square.

fast/loader/scroll-position-restored-on-back.html needs to wait for the UI process to restore the scroll position.

* fast/loader/scroll-position-restored-on-back.html:
* fast/scrolling/progammatic-scroll-scrolling-no-frame-expected.txt: Added.
* fast/scrolling/progammatic-scroll-scrolling-no-frame.html: Added.
* platform/ios-wk2/fast/overflow/scrollRevealButton-expected.txt:
* platform/ios-wk2/imported/w3c/web-platform-tests/html/interaction/focus/processing-model/preventScroll-expected.txt:
* platform/ios/TestExpectations:
* platform/ios/fast/visual-viewport/viewport-dimensions-iframe-expected.txt: Removed.
* platform/ios/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/history-traversal/persisted-user-state-restoration/scroll-restoration-navigation-cross-origin-expected.txt:
* platform/ios/imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location_hash-expected.txt:
* platform/ios/imported/w3c/web-platform-tests/intersection-observer/cross-origin-iframe.sub-expected.txt: Removed.
* platform/ios/imported/w3c/web-platform-tests/intersection-observer/iframe-no-root-expected.txt: Removed.
* platform/ios/imported/w3c/web-platform-tests/intersection-observer/iframe-no-root-with-wrapping-scroller-expected.txt: Removed.
* platform/ios/imported/w3c/web-platform-tests/intersection-observer/nested-cross-origin-iframe.sub-expected.txt: Removed.
* platform/ios/imported/w3c/web-platform-tests/visual-viewport/viewport-unscaled-scroll-iframe-expected.txt: Removed.

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

17 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/loader/scroll-position-restored-on-back.html
LayoutTests/fast/scrolling/progammatic-scroll-scrolling-no-frame-expected.txt [new file with mode: 0644]
LayoutTests/fast/scrolling/progammatic-scroll-scrolling-no-frame.html [new file with mode: 0644]
LayoutTests/platform/ios-wk2/fast/overflow/scrollRevealButton-expected.txt
LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/html/interaction/focus/processing-model/preventScroll-expected.txt
LayoutTests/platform/ios/TestExpectations
LayoutTests/platform/ios/fast/visual-viewport/viewport-dimensions-iframe-expected.txt [deleted file]
LayoutTests/platform/ios/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/history-traversal/persisted-user-state-restoration/scroll-restoration-navigation-cross-origin-expected.txt
LayoutTests/platform/ios/imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location_hash-expected.txt
LayoutTests/platform/ios/imported/w3c/web-platform-tests/intersection-observer/cross-origin-iframe.sub-expected.txt [deleted file]
LayoutTests/platform/ios/imported/w3c/web-platform-tests/intersection-observer/iframe-no-root-expected.txt [deleted file]
LayoutTests/platform/ios/imported/w3c/web-platform-tests/intersection-observer/iframe-no-root-with-wrapping-scroller-expected.txt [deleted file]
LayoutTests/platform/ios/imported/w3c/web-platform-tests/intersection-observer/nested-cross-origin-iframe.sub-expected.txt [deleted file]
LayoutTests/platform/ios/imported/w3c/web-platform-tests/visual-viewport/viewport-unscaled-scroll-iframe-expected.txt [deleted file]
Source/WebCore/ChangeLog
Source/WebCore/platform/ScrollView.cpp

index eddd6d9..5bd1091 100644 (file)
@@ -1,3 +1,33 @@
+2020-05-19  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS] Programmaic scroll of "scrolling=no" iframe fails
+        https://bugs.webkit.org/show_bug.cgi?id=212063
+        <rdar://problem/57049514>
+
+        Reviewed by Antti Koivisto.
+        
+        New passing results for some WPT tests (iOS-specific results are removed).
+
+        fast/frames/flattening/scrolling-in-object.html relies on mouse events, so skip on iOS. It happened
+        to pass because both test and reference showed the red square.
+
+        fast/loader/scroll-position-restored-on-back.html needs to wait for the UI process to restore the scroll position.
+
+        * fast/loader/scroll-position-restored-on-back.html:
+        * fast/scrolling/progammatic-scroll-scrolling-no-frame-expected.txt: Added.
+        * fast/scrolling/progammatic-scroll-scrolling-no-frame.html: Added.
+        * platform/ios-wk2/fast/overflow/scrollRevealButton-expected.txt:
+        * platform/ios-wk2/imported/w3c/web-platform-tests/html/interaction/focus/processing-model/preventScroll-expected.txt:
+        * platform/ios/TestExpectations:
+        * platform/ios/fast/visual-viewport/viewport-dimensions-iframe-expected.txt: Removed.
+        * platform/ios/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/history-traversal/persisted-user-state-restoration/scroll-restoration-navigation-cross-origin-expected.txt:
+        * platform/ios/imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location_hash-expected.txt:
+        * platform/ios/imported/w3c/web-platform-tests/intersection-observer/cross-origin-iframe.sub-expected.txt: Removed.
+        * platform/ios/imported/w3c/web-platform-tests/intersection-observer/iframe-no-root-expected.txt: Removed.
+        * platform/ios/imported/w3c/web-platform-tests/intersection-observer/iframe-no-root-with-wrapping-scroller-expected.txt: Removed.
+        * platform/ios/imported/w3c/web-platform-tests/intersection-observer/nested-cross-origin-iframe.sub-expected.txt: Removed.
+        * platform/ios/imported/w3c/web-platform-tests/visual-viewport/viewport-unscaled-scroll-iframe-expected.txt: Removed.
+
 2020-05-19  Andy Estes  <aestes@apple.com>
 
         [Apple Pay] Add testing and logging for ApplePaySetup
index 11314a5..b3ce608 100644 (file)
@@ -1,8 +1,10 @@
 <!-- webkit-test-runner [ enableBackForwardCache=true ] -->
 <html>
 <head>
+    <script src="../../resources/ui-helper.js"></script>
+    
 <script>
-function navigate()
+async function navigate()
 {
     if (location.hash == "") {
         if (window.testRunner) {
@@ -16,14 +18,14 @@ function navigate()
         return;
     }
 
-    setTimeout(function () { 
-        var scrollPosition = document.scrollingElement.scrollTop;
-        var result = document.getElementById("result");
-        if (scrollPosition == 100)
-            result.innerHTML = "Success! The scroll position was restored after navigation."
-        if (window.testRunner)
-            testRunner.notifyDone();
-    }, 0);
+    await UIHelper.ensureStablePresentationUpdate();
+
+    var scrollPosition = document.scrollingElement.scrollTop;
+    var result = document.getElementById("result");
+    if (scrollPosition == 100)
+        result.innerHTML = "Success! The scroll position was restored after navigation."
+    if (window.testRunner)
+        testRunner.notifyDone();
 }
 
 </script>
diff --git a/LayoutTests/fast/scrolling/progammatic-scroll-scrolling-no-frame-expected.txt b/LayoutTests/fast/scrolling/progammatic-scroll-scrolling-no-frame-expected.txt
new file mode 100644 (file)
index 0000000..7672f0f
--- /dev/null
@@ -0,0 +1,7 @@
+
+PASS iframe.contentWindow.scrollY is 0
+PASS iframe.contentWindow.scrollY is 400
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/scrolling/progammatic-scroll-scrolling-no-frame.html b/LayoutTests/fast/scrolling/progammatic-scroll-scrolling-no-frame.html
new file mode 100644 (file)
index 0000000..7964c0a
--- /dev/null
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<head>
+       <script src="../../resources/js-test-pre.js"></script>
+    <script>
+        jsTestIsAsync = true;
+        var iframe;
+        window.addEventListener('load', () => {
+            iframe = document.getElementsByTagName('iframe')[0];
+            shouldBe('iframe.contentWindow.scrollY', '0');
+            iframe.contentWindow.scrollTo(0, 400);
+            shouldBe('iframe.contentWindow.scrollY', '400');
+            finishJSTest();
+        }, false);
+    </script>
+</head>
+<body>
+    <iframe scrolling="no" srcdoc="
+    <style>
+        body {
+            height: 2000px;
+        }
+    </style>
+    text here
+    "></iframe>
+<div id="console"></div>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
index 97760eb..cb3185e 100644 (file)
@@ -38,3 +38,4 @@ layer at (0,0) size 800x1190
               RenderBlock {DIV} at (0,564) size 135x300
         RenderText {#text} at (0,0) size 0x0
       RenderBlock {DIV} at (0,674) size 784x500
+frame 'fr' scrolled to 0,18
index 162685c..3d1623d 100644 (file)
@@ -1,6 +1,6 @@
 
 
-FAIL Sanity test assert_true: after elm.scrollIntoView() expected true got false
+PASS Sanity test 
 FAIL elm.focus() without arguments assert_true: expected true got false
 FAIL elm.focus(undefined) assert_true: expected true got false
 FAIL elm.focus(null) assert_true: expected true got false
index 5d7f683..aef95e9 100644 (file)
@@ -1053,6 +1053,7 @@ fast/css/resize-single-axis.html [ Skip ]
 fast/css/user-drag-none.html [ Skip ]
 fast/forms/range/disabled-while-dragging.html [ Skip ]
 fast/forms/range/range-drag-when-toggled-disabled.html [ Skip ]
+fast/frames/flattening/scrolling-in-object.html [ Skip ]
 fast/media/video-element-in-details-collapse.html [ Skip ]
 fast/frames/user-gesture-timestamp-propagation.html [ Failure ]
 fast/events/mouse-click-different-mouseDown-mouseUp-nodes.html [ Skip ]
diff --git a/LayoutTests/platform/ios/fast/visual-viewport/viewport-dimensions-iframe-expected.txt b/LayoutTests/platform/ios/fast/visual-viewport/viewport-dimensions-iframe-expected.txt
deleted file mode 100644 (file)
index fa8f9d2..0000000
+++ /dev/null
@@ -1,4 +0,0 @@
-
-
-FAIL Verify viewport dimensions for iframe. assert_equals: pageLeft expected 10 but got 0
-
index b63bc57..fb86570 100644 (file)
@@ -1,6 +1,6 @@
 
 PASS location hash 
-FAIL Setting location.hash on srcdoc iframe assert_true: Should have scrolled by more than one viewport height expected true got false
+PASS Setting location.hash on srcdoc iframe 
 PASS Setting hash should automatically include hash character 
 FAIL Setting hash should encode incompatible characters assert_equals: expected "#not%20encoded" but got "#not encoded"
 PASS Setting hash to an already encoded value should not double encode it 
diff --git a/LayoutTests/platform/ios/imported/w3c/web-platform-tests/intersection-observer/cross-origin-iframe.sub-expected.txt b/LayoutTests/platform/ios/imported/w3c/web-platform-tests/intersection-observer/cross-origin-iframe.sub-expected.txt
deleted file mode 100644 (file)
index 556bf27..0000000
+++ /dev/null
@@ -1,8 +0,0 @@
-
-
-PASS Intersection observer test with no explicit root and target in a cross-origin iframe. 
-PASS First rAF 
-PASS topDocument.scrollingElement.scrollTop = 200 
-FAIL iframeDocument.scrollingElement.scrollTop = 250 assert_equals: expected 2 but got 0
-FAIL topDocument.scrollingElement.scrollTop = 100 assert_equals: expected 2 but got 0
-
diff --git a/LayoutTests/platform/ios/imported/w3c/web-platform-tests/intersection-observer/iframe-no-root-expected.txt b/LayoutTests/platform/ios/imported/w3c/web-platform-tests/intersection-observer/iframe-no-root-expected.txt
deleted file mode 100644 (file)
index fd52d5b..0000000
+++ /dev/null
@@ -1,8 +0,0 @@
-
-
-PASS Observer with the implicit root; target in a same-origin iframe. 
-PASS First rAF. 
-PASS document.scrollingElement.scrollTop = 200 
-FAIL iframe.contentDocument.scrollingElement.scrollTop = 250 assert_equals: entries.length expected 2 but got 1
-FAIL document.scrollingElement.scrollTop = 100 assert_equals: entries.length expected 3 but got 1
-
diff --git a/LayoutTests/platform/ios/imported/w3c/web-platform-tests/intersection-observer/iframe-no-root-with-wrapping-scroller-expected.txt b/LayoutTests/platform/ios/imported/w3c/web-platform-tests/intersection-observer/iframe-no-root-with-wrapping-scroller-expected.txt
deleted file mode 100644 (file)
index fd52d5b..0000000
+++ /dev/null
@@ -1,8 +0,0 @@
-
-
-PASS Observer with the implicit root; target in a same-origin iframe. 
-PASS First rAF. 
-PASS document.scrollingElement.scrollTop = 200 
-FAIL iframe.contentDocument.scrollingElement.scrollTop = 250 assert_equals: entries.length expected 2 but got 1
-FAIL document.scrollingElement.scrollTop = 100 assert_equals: entries.length expected 3 but got 1
-
diff --git a/LayoutTests/platform/ios/imported/w3c/web-platform-tests/intersection-observer/nested-cross-origin-iframe.sub-expected.txt b/LayoutTests/platform/ios/imported/w3c/web-platform-tests/intersection-observer/nested-cross-origin-iframe.sub-expected.txt
deleted file mode 100644 (file)
index b695f5f..0000000
+++ /dev/null
@@ -1,4 +0,0 @@
-
-
-FAIL IntersectionObserver with `implicit root` in a nested cross-origin iframe works assert_true: The target element is now intersecting in all ancestor viewports expected true got false
-
diff --git a/LayoutTests/platform/ios/imported/w3c/web-platform-tests/visual-viewport/viewport-unscaled-scroll-iframe-expected.txt b/LayoutTests/platform/ios/imported/w3c/web-platform-tests/visual-viewport/viewport-unscaled-scroll-iframe-expected.txt
deleted file mode 100644 (file)
index 5e8c657..0000000
+++ /dev/null
@@ -1,15 +0,0 @@
-Viewport: Scroll in iframe - no page scale
-
-Test Description: This test checks that window.visualViewport returns correct offset and scroll values without any pinch-zoom page scale applied.
-
-
-frames[0].window.visualViewport's offsetLeft and offsetTop is (0, 0).
-
-frames[0].window.visualViewport's pageLeft and pageTop is (0, 0).
-
-
-PASS offsetLeft must be 0. 
-PASS offsetTop must be 0. 
-FAIL pageLeft must reflect location of viewport in document. assert_equals: expected 1000 but got 0
-FAIL pageTop must reflect location of viewport in document. assert_equals: expected 1200 but got 0
-
index 3352e38..8341c87 100644 (file)
@@ -1,3 +1,23 @@
+2020-05-19  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS] Programmaic scroll of "scrolling=no" iframe fails
+        https://bugs.webkit.org/show_bug.cgi?id=212063
+        <rdar://problem/57049514>
+
+        Reviewed by Antti Koivisto.
+
+        ScrollView::setScrollPosition() calls requestScrollPositionUpdate(), and if this returns
+        false it relies on the confusingly-named updateScrollbars() to actually do the scroll.
+        This code path is hit for "scrolling=no" frames, which are not scroll-coordinated.
+
+        ScrollView::updateScrollbars() fails to set the scroll position on iOS, where managesScrollbars()
+        returns false, so fix that.
+
+        Test: fast/scrolling/progammatic-scroll-scrolling-no-frame.html
+
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::updateScrollbars):
+
 2020-05-19  Andy Estes  <aestes@apple.com>
 
         [Apple Pay] Add testing and logging for ApplePaySetup
index 3aee3f9..e822d0f 100644 (file)
@@ -591,11 +591,19 @@ void ScrollView::updateScrollbars(const ScrollPosition& desiredPosition)
     if (m_inUpdateScrollbars || prohibitsScrolling() || platformWidget())
         return;
     
-    if (!managesScrollbars()) {
-        if (scrollOriginChanged()) {
-            ScrollableArea::scrollToOffsetWithoutAnimation(scrollOffsetFromPosition(desiredPosition));
+    auto scrollToPosition = [&](ScrollPosition desiredPosition) {
+        auto adjustedScrollPosition = desiredPosition;
+        if (!isRubberBandInProgress())
+            adjustedScrollPosition = adjustScrollPositionWithinRange(adjustedScrollPosition);
+
+        if (adjustedScrollPosition != scrollPosition() || scrollOriginChanged()) {
+            ScrollableArea::scrollToOffsetWithoutAnimation(scrollOffsetFromPosition(adjustedScrollPosition));
             resetScrollOriginChanged();
         }
+    };
+
+    if (!managesScrollbars()) {
+        scrollToPosition(desiredPosition);
         return;
     }
 
@@ -772,14 +780,7 @@ void ScrollView::updateScrollbars(const ScrollPosition& desiredPosition)
             invalidateScrollCornerRect(oldScrollCornerRect);
     }
 
-    IntPoint adjustedScrollPosition = desiredPosition;
-    if (!isRubberBandInProgress())
-        adjustedScrollPosition = adjustScrollPositionWithinRange(adjustedScrollPosition);
-
-    if (adjustedScrollPosition != scrollPosition() || scrollOriginChanged()) {
-        ScrollableArea::scrollToOffsetWithoutAnimation(scrollOffsetFromPosition(adjustedScrollPosition));
-        resetScrollOriginChanged();
-    }
+    scrollToPosition(desiredPosition);
 
     // Make sure the scrollbar offsets are up to date.
     if (m_horizontalScrollbar)