Skip to content

[UnifiedPDF][iOS] Relative scroll position is not preserved when resizing web view#46211

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
pxlcoder:eng/293906
Jun 2, 2025
Merged

[UnifiedPDF][iOS] Relative scroll position is not preserved when resizing web view#46211
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
pxlcoder:eng/293906

Conversation

@pxlcoder
Copy link
Member

@pxlcoder pxlcoder commented Jun 2, 2025

4312171

[UnifiedPDF][iOS] Relative scroll position is not preserved when resizing web view
https://bugs.webkit.org/show_bug.cgi?id=293906
rdar://152160616

Reviewed by Richard Robinson, Wenson Hsieh, and Tim Horton.

When the web view is resized, the fitting scale of the PDF changes, resulting
in an individual page becoming taller or shorter, and an overall change to the
content size. However, the scroll view's content offset remains unchanged,
resulting in a "jump" to another page when the web view is resized.

Fix by computing a new content offset when the fitting scale changes. Since the
fitting scale is not actually exposed to the UI process, simply take the ratio
of new content height to the old content height.

* Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:
(-[WKWebView _updateScrollViewForTransaction:]):

Repurpose logic that pins the scroll position for traditional web content to
also support pinning the PDF scroll position.

Note that the added logic is only correct for PDFs that display vertically.
This is fine since WebKit does not support horizontal PDF display.

* Tools/TestWebKitAPI/Tests/WebKitCocoa/UnifiedPDFTests.mm:
(TestWebKitAPI::UNIFIED_PDF_TEST(KeepRelativeScrollPositionAfterAnimatedResize)):
(TestWebKitAPI::UNIFIED_PDF_TEST(ScrollOffsetResetWhenChangingPDF)):

Canonical link: https://commits.webkit.org/295715@main

44f36cb

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@pxlcoder pxlcoder requested a review from cdumez as a code owner June 2, 2025 17:06
@pxlcoder pxlcoder self-assigned this Jun 2, 2025
@pxlcoder pxlcoder added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Jun 2, 2025
@hortont424
Copy link
Contributor

This is fine since iOS WebKit does not support horizontal PDF display.

Neither does Mac! I think this is just an invented feature :P

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the oldContentSize.height > 0 just be oldContentSize.height because of the style rule not to compare to 0?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "compare to 0" style rule applies to equality checks

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically it would be fine since content size shouldn't be negative, but I also don't see a strong reason to change it, so keeping as-is.

@pxlcoder pxlcoder added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 2, 2025
…zing web view

https://bugs.webkit.org/show_bug.cgi?id=293906
rdar://152160616

Reviewed by Richard Robinson, Wenson Hsieh, and Tim Horton.

When the web view is resized, the fitting scale of the PDF changes, resulting
in an individual page becoming taller or shorter, and an overall change to the
content size. However, the scroll view's content offset remains unchanged,
resulting in a "jump" to another page when the web view is resized.

Fix by computing a new content offset when the fitting scale changes. Since the
fitting scale is not actually exposed to the UI process, simply take the ratio
of new content height to the old content height.

* Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:
(-[WKWebView _updateScrollViewForTransaction:]):

Repurpose logic that pins the scroll position for traditional web content to
also support pinning the PDF scroll position.

Note that the added logic is only correct for PDFs that display vertically.
This is fine since WebKit does not support horizontal PDF display.

* Tools/TestWebKitAPI/Tests/WebKitCocoa/UnifiedPDFTests.mm:
(TestWebKitAPI::UNIFIED_PDF_TEST(KeepRelativeScrollPositionAfterAnimatedResize)):
(TestWebKitAPI::UNIFIED_PDF_TEST(ScrollOffsetResetWhenChangingPDF)):

Canonical link: https://commits.webkit.org/295715@main
@webkit-commit-queue
Copy link
Collaborator

Committed 295715@main (4312171): https://commits.webkit.org/295715@main

Reviewed commits have been landed. Closing PR #46211 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 4312171 into WebKit:main Jun 2, 2025
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

New Bugs Unclassified bugs are placed in this component until the correct component can be determined.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants