-
Notifications
You must be signed in to change notification settings - Fork 167
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.
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
Defensive fixes for a UIViewPropertyAnimator crash #1385
Conversation
"Documentation says this is always true" - good place for a link in the PR notes |
do you mind cherry picking this to main_0.9 branch too please? |
Co-authored-by: Anand Rajeswaran <49537028+anandrajeswaran@users.noreply.github.com>
Only in the code docs unfortunately: |
* Drop support for iOS 14 and remove availability checks (#1357) * Update FluentUI project * Update FluentUI xcconfig * Update FluentUI.Demo project * Update MicrosoftFluentUI.podspec * Clean up Button * Clean up CommandBarButton * Clean up CommandBarButtonGroupView * Clean up LargeContentViewerModifier * Clean up Drawer * Clean up NavigationBar * Clean up BadgeLabelButton * Clean up PillButton * Clean up PillButtonBar * Clean up SegmentedPillButton * Clean up SideTabBar * Clean up TabBarView * Clean up PillButton v2 * Clean up PillButton v3 * Revert "Update FluentUI project" This reverts commit 1c97631. * Update README.md * Update Package.swift * Make ShimmerLinesView inherit ShimmerView init (#1362) * Make ShimmerLinesView inherit ShimmerView init * move shimmer to fluent2 and add new api to demo * Bring fluent2-tokens Segmented Control to main (#1363) * Bring in fluent2-token control * Add token set to project * Update dependent controls * Add colored background for demo * Update demos * Update demo order * Fix whitespace error * Add demo to show notification bar from top (#1364) * Update PR Template to include binary change (#1368) * Fix PersonaListViewDemoController navigation bar transparency bug (#1372) * Change from DemoController to UIViewController * Revert "Change from DemoController to UIViewController" This reverts commit 3b3aeb0. * Fix bug * Remove retain cycles (#1374) * update xcode version to 14.1 for ci (#1371) (#1378) * Implement the Unread Dot for TabBar and SideTabBar (#1349) * Implementing the unread dot on TabBarView * Better naming for variables and events * Updating SideTabBar demo to show the unread dot * Pr feedback - reorganizing, renaming, better documentation * Updating variable names and locations to follow guidance * Added accessibility string for unread dot * Switching to using a badge label for the unread dot * Cleaning up commented out code * Simplifying logic, using a single view for both the badge and the unreadDot * PR cleanup * Add isViewLoaded check (#1379) * Revert "Drop support for iOS 14 and remove availability checks (#1357)" (#1383) * Revert "Drop support for iOS 14 and remove availability checks (#1357)" This reverts commit 4fad398. * Fix whitespace error * Defensive fixes for a UIViewPropertyAnimator crash (#1385) * Latest localized resource files from Touchdown Build (#1388) Co-authored-by: edjamesmsft <edjamesmsft@users.noreply.github.com> Co-authored-by: Harrie Shin <hyshin@microsoft.com> * Fix linting issue (#1393) * Latest localized resource files from Touchdown Build (#1391) Co-authored-by: markavitale <markavitale@users.noreply.github.com> Co-authored-by: Harrie Shin <hyshin@microsoft.com> * Update demo page cell style (#1389) * Fix bottom sheet host height change detection (#1396) * Latest localized resource files from Touchdown Build (#1400) Co-authored-by: markavitale <markavitale@users.noreply.github.com> * Latest localized resource files from Touchdown Build (#1401) Co-authored-by: markavitale <markavitale@users.noreply.github.com> * Fix PopupMenuItemCell's token sink to not overwrite custom colors (#1398) * Change TVC updateAppearance to internal * Fix PopupMenu token sink * Fix Segmented Control unread dot for icons (#1408) * Fix unread dot anchor logic * Update demo to have an unread icon Co-authored-by: huwilkes <67026548+huwilkes@users.noreply.github.com> Co-authored-by: Jeanie Huynh <31874971+jeaniehuynh@users.noreply.github.com> Co-authored-by: Lamine Male <106181067+laminesm@users.noreply.github.com> Co-authored-by: Mike Schreiber <mischreiber@microsoft.com> Co-authored-by: Harrie Shin <hyshin@microsoft.com> Co-authored-by: James Edwards <edjamesmsft@outlook.com> Co-authored-by: Lukas Capkovic <3610850+lcapkovic@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: edjamesmsft <edjamesmsft@users.noreply.github.com> Co-authored-by: markavitale <markavitale@users.noreply.github.com>
Platforms Impacted
Description of changes
Binary change:
Problem
Clients can crash if they make certain bottom sheet modifications during
UIViewPropertyAnimator
completion callbacks.Cause
UIViewPropertyAnimator
has a bug / undocumented behavior - when it's calling out to its completion handlers, the animator reportsisRunning==true
and it's in a privateUIViewAnimatingState
(value 5). When it's in this state, callingfinishAnimation
will cause it to throw an exception.We have a
completeAnimationsIfNeeded
helper function that frequently gets called on bottom sheet environment changes, like the host view size / safe area. This helper checks theisRunning
prop to make sure it's only trying to force complete animators that are running. But if an animator is in the process of completing, and the client reacts to that completion in a way that triggerscompleteAnimationsIfNeeded
synchronously, we can end up triggering the exception.Fix
Two fixes to be safe, but either would work to mitigate the problem:
completeAnimationsIfNeeded
helper as it no longer has an animator to act on.completeAnimationsIfNeeded
- in addition to checkingisRunning
, let's also make surestate == .active
. Documentation says this is always true, but it's clearly not.Verification
Force crash condition by modifying safe area in animator completion. Verify crash doesn't occur anymore with the fixes.
Pull request checklist
This PR has considered:
Microsoft Reviewers: Open in CodeFlow