Changeset 277775 in webkit

Timestamp:

May 19, 2021 10:05:40 PM (14 months ago)

Author:

Tomoki Imai

Message:

Scrolling must be done after the layout when doing full page zoom
​https://bugs.webkit.org/show_bug.cgi?id=225730

Reviewed by Simon Fraser.

Source/WebCore:

Previously, the actual scroll was executed before the layout with the zoomed position.
It sometimes makes the scroll position exceed the page height, and cannot retain pageYOffset.
In the user experience perspective, the user may miss what they are looking at after zoom.

This patch makes the scroll happen after the layout, with the zoomed position.

Test: LayoutTests\fast\scrolling\page-y-offset-should-not-be-changed-after-zoom.html

• page/Frame.cpp:

(WebCore::Frame::setPageAndTextZoomFactors): Makes the scroll happen after the layout.

Source/WebKit:

Add check to disallow negative or zero zoom value.

• UIProcess/API/C/WKPage.cpp:

(WKPageSetPageZoomFactor):

LayoutTests:

Added a testcase to ensure that scroll position is not changed after zoom.

• fast/scrolling/page-y-offset-should-not-be-changed-after-zoom-expected.txt: Added.
• fast/scrolling/page-y-offset-should-not-be-changed-after-zoom.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/scrolling/page-y-offset-should-not-be-changed-after-zoom-expected.txt (added)
• LayoutTests/fast/scrolling/page-y-offset-should-not-be-changed-after-zoom.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/Frame.cpp (modified) (2 diffs)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/API/C/WKPage.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-05-19  Tomoki Imai  <Tomoki.Imai@sony.com>
+
+        Scrolling must be done after the layout when doing full page zoom
+        https://bugs.webkit.org/show_bug.cgi?id=225730
+
+        Reviewed by Simon Fraser.
+
+        Added a testcase to ensure that scroll position is not changed after zoom.
+
+        * fast/scrolling/page-y-offset-should-not-be-changed-after-zoom-expected.txt: Added.
+        * fast/scrolling/page-y-offset-should-not-be-changed-after-zoom.html: Added.
+
 2021-05-19  Devin Rousso  <drousso@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-05-19  Tomoki Imai  <Tomoki.Imai@sony.com>
+
+        Scrolling must be done after the layout when doing full page zoom
+        https://bugs.webkit.org/show_bug.cgi?id=225730
+
+        Reviewed by Simon Fraser.
+
+        Previously, the actual scroll was executed before the layout with the zoomed position.
+        It sometimes makes the scroll position exceed the page height, and cannot retain pageYOffset.
+        In the user experience perspective, the user may miss what they are looking at after zoom.
+
+        This patch makes the scroll happen after the layout, with the zoomed position.
+
+        Test: LayoutTests\fast\scrolling\page-y-offset-should-not-be-changed-after-zoom.html
+
+        * page/Frame.cpp:
+        (WebCore::Frame::setPageAndTextZoomFactors): Makes the scroll happen after the layout.
+
 2021-05-19  Devin Rousso  <drousso@apple.com>
 

trunk/Source/WebCore/page/Frame.cpp

@@ -962 +962 @@
         return;
 
+    Optional<ScrollPosition> scrollPositionAfterZoomed;
     if (m_pageZoomFactor != pageZoomFactor) {
+        // Compute the scroll position with scale after zooming to stay the same position in the content.
         if (FrameView* view = this->view()) {
-            // Update the scroll position when doing a full page zoom, so the content stays in relatively the same position.
-            LayoutPoint scrollPosition = view->scrollPosition();
-            float percentDifference = (pageZoomFactor / m_pageZoomFactor);
-            view->setScrollPosition(IntPoint(scrollPosition.x() * percentDifference, scrollPosition.y() * percentDifference));
+            scrollPositionAfterZoomed = view->scrollPosition();
+            scrollPositionAfterZoomed->scale(pageZoomFactor / m_pageZoomFactor);
         }
     }
-
     m_pageZoomFactor = pageZoomFactor;
     m_textZoomFactor = textZoomFactor;
@@ -982 +981 @@
         if (document->renderView() && document->renderView()->needsLayout() && view->didFirstLayout())
             view->layoutContext().layout();
+
+        // Scrolling to the calculated position must be done after the layout.
+        if (scrollPositionAfterZoomed)
+            view->setScrollPosition(scrollPositionAfterZoomed.value());
     }
 }

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-05-19  Tomoki Imai  <Tomoki.Imai@sony.com>
+
+        Scrolling must be done after the layout when doing full page zoom
+        https://bugs.webkit.org/show_bug.cgi?id=225730
+
+        Reviewed by Simon Fraser.
+
+        Add check to disallow negative or zero zoom value.
+
+        * UIProcess/API/C/WKPage.cpp:
+        (WKPageSetPageZoomFactor):
+
 2021-05-19  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebKit/UIProcess/API/C/WKPage.cpp

@@ -606 +606 @@
 {
     CRASH_IF_SUSPENDED;
+    if (zoomFactor <= 0)
+        return;
     toImpl(pageRef)->setPageZoomFactor(zoomFactor);
 }