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