Title: [277775] trunk
Revision
277775
Author
[email protected]
Date
2021-05-19 22:05:40 -0700 (Wed, 19 May 2021)

Log 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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (277774 => 277775)


--- trunk/LayoutTests/ChangeLog	2021-05-20 05:05:19 UTC (rev 277774)
+++ trunk/LayoutTests/ChangeLog	2021-05-20 05:05:40 UTC (rev 277775)
@@ -1,3 +1,15 @@
+2021-05-19  Tomoki Imai  <[email protected]>
+
+        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  <[email protected]>
 
         Add a way to create `"wheel"` events from gesture/touch events

Added: trunk/LayoutTests/fast/scrolling/page-y-offset-should-not-be-changed-after-zoom-expected.txt (0 => 277775)


--- trunk/LayoutTests/fast/scrolling/page-y-offset-should-not-be-changed-after-zoom-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/scrolling/page-y-offset-should-not-be-changed-after-zoom-expected.txt	2021-05-20 05:05:40 UTC (rev 277775)
@@ -0,0 +1,3 @@
+
+PASS page-y-offset-should-not-be-changed-after-zoom
+

Added: trunk/LayoutTests/fast/scrolling/page-y-offset-should-not-be-changed-after-zoom.html (0 => 277775)


--- trunk/LayoutTests/fast/scrolling/page-y-offset-should-not-be-changed-after-zoom.html	                        (rev 0)
+++ trunk/LayoutTests/fast/scrolling/page-y-offset-should-not-be-changed-after-zoom.html	2021-05-20 05:05:40 UTC (rev 277775)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <script src=""
+    <script src=""
+    <style>
+      body {
+        height: 10000px;
+      }
+    </style>
+  </head>
+  <body>
+    <script>
+      if (window.internals) {
+        test(() => {
+          window.scroll(0, 9000);
+          const originalPageYOffset = pageYOffset;
+          window.internals.setPageZoomFactor(2);
+          const afterZoomPageYOffset = pageYOffset;
+          window.internals.setPageZoomFactor(1);
+          const afterResetPageYOffset = pageYOffset;
+
+          assert_equals(originalPageYOffset, afterZoomPageYOffset);
+          assert_equals(originalPageYOffset, afterResetPageYOffset);
+        });
+      }
+    </script>
+  </body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (277774 => 277775)


--- trunk/Source/WebCore/ChangeLog	2021-05-20 05:05:19 UTC (rev 277774)
+++ trunk/Source/WebCore/ChangeLog	2021-05-20 05:05:40 UTC (rev 277775)
@@ -1,3 +1,21 @@
+2021-05-19  Tomoki Imai  <[email protected]>
+
+        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  <[email protected]>
 
         Add a way to create `"wheel"` events from gesture/touch events

Modified: trunk/Source/WebCore/page/Frame.cpp (277774 => 277775)


--- trunk/Source/WebCore/page/Frame.cpp	2021-05-20 05:05:19 UTC (rev 277774)
+++ trunk/Source/WebCore/page/Frame.cpp	2021-05-20 05:05:40 UTC (rev 277775)
@@ -961,15 +961,14 @@
     if (is<SVGDocument>(*document) && !downcast<SVGDocument>(*document).zoomAndPanEnabled())
         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;
 
@@ -981,6 +980,10 @@
     if (FrameView* view = this->view()) {
         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());
     }
 }
 

Modified: trunk/Source/WebKit/ChangeLog (277774 => 277775)


--- trunk/Source/WebKit/ChangeLog	2021-05-20 05:05:19 UTC (rev 277774)
+++ trunk/Source/WebKit/ChangeLog	2021-05-20 05:05:40 UTC (rev 277775)
@@ -1,3 +1,15 @@
+2021-05-19  Tomoki Imai  <[email protected]>
+
+        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  <[email protected]>
 
         [GPUProcess] It is not safe to call GraphicsContext::paintFrameForMedia() off the main thread

Modified: trunk/Source/WebKit/UIProcess/API/C/WKPage.cpp (277774 => 277775)


--- trunk/Source/WebKit/UIProcess/API/C/WKPage.cpp	2021-05-20 05:05:19 UTC (rev 277774)
+++ trunk/Source/WebKit/UIProcess/API/C/WKPage.cpp	2021-05-20 05:05:40 UTC (rev 277775)
@@ -605,6 +605,8 @@
 void WKPageSetPageZoomFactor(WKPageRef pageRef, double zoomFactor)
 {
     CRASH_IF_SUSPENDED;
+    if (zoomFactor <= 0)
+        return;
     toImpl(pageRef)->setPageZoomFactor(zoomFactor);
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to