Skip to content

[UnifiedPDF] [iOS] Page number indicator does not have a fade out transition #47370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 30, 2025
Merged

[UnifiedPDF] [iOS] Page number indicator does not have a fade out transition #47370

merged 1 commit into from
Jun 30, 2025
+15 −2

Conversation

aprotyas
Copy link
Member

@aprotyas aprotyas commented Jun 30, 2025 •
edited by webkit-early-warning-system
Loading

143c9bf

[UnifiedPDF] [iOS] Page number indicator does not have a fade out transition
https://bugs.webkit.org/show_bug.cgi?id=295188
rdar://154641207

Reviewed by Wenson Hsieh.

We notice unexpected behavior from +animateWithDuration:animations: as
we try to fade out the page number indicator. This seems to only
reproduce with a UIGlassEffect visual effect configured, as this does
not happen on visionOS (which uses a UIBlurEffect instead).

While this needs to be investigated further in rdar://154649008, this
patch works around the problem by adopting the animation option
`UIViewAnimationOptionFlushUpdates`, which flushes all pending property
updates when the animation context/scope is modified.

* Source/WTF/wtf/PlatformHave.h:
Introduce a HAVE macro to express platform availability information
about UIViewAnimationOptionFlushUpdates.

* Source/WebKit/UIProcess/PDF/WKPDFPageNumberIndicator.mm:
(-[WKPDFPageNumberIndicator hide:]):

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

a74df58

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

@aprotyas aprotyas self-assigned this Jun 30, 2025
@aprotyas aprotyas requested a review from cdumez as a code owner June 30, 2025 09:06
@aprotyas aprotyas added the PDF For bugs in WebKit's built-in PDF support. label Jun 30, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 30, 2025 •
edited
Loading

EWS run on current version of this PR (hash a74df58)

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

@aprotyas
Copy link
Member Author

Thank you for the review!

@aprotyas aprotyas added the merge-queue Applied to send a pull request to merge-queue label Jun 30, 2025
@aprotyas
[UnifiedPDF] [iOS] Page number indicator does not have a fade out tra…
143c9bf 
…nsition

https://bugs.webkit.org/show_bug.cgi?id=295188
rdar://154641207

Reviewed by Wenson Hsieh.

We notice unexpected behavior from +animateWithDuration:animations: as
we try to fade out the page number indicator. This seems to only
reproduce with a UIGlassEffect visual effect configured, as this does
not happen on visionOS (which uses a UIBlurEffect instead).

While this needs to be investigated further in rdar://154649008, this
patch works around the problem by adopting the animation option
`UIViewAnimationOptionFlushUpdates`, which flushes all pending property
updates when the animation context/scope is modified.

* Source/WTF/wtf/PlatformHave.h:
Introduce a HAVE macro to express platform availability information
about UIViewAnimationOptionFlushUpdates.

* Source/WebKit/UIProcess/PDF/WKPDFPageNumberIndicator.mm:
(-[WKPDFPageNumberIndicator hide:]):

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

Committed 296807@main (143c9bf): https://commits.webkit.org/296807@main

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

@webkit-commit-queue webkit-commit-queue merged commit 143c9bf into WebKit:main Jun 30, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 30, 2025
@aprotyas aprotyas deleted the eng/295188 branch June 30, 2025 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whsieh whsieh whsieh approved these changes

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

Assignees

@aprotyas aprotyas

Labels
PDF For bugs in WebKit's built-in PDF support.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aprotyas @webkit-early-warning-system @webkit-commit-queue @whsieh