github-starred/android
2
0
Fork 0
mirror of https://github.com/bitwarden/android.git synced 2026-03-16 08:33:59 -05:00
Code Issues 144 Packages Projects Releases 116 Wiki Activity

[PR #1842] [MERGED] Fix Crash on Scroll on iOS 15.4 beta #3172

New Issue
Closed
opened 2025-11-26 23:29:14 -06:00 by GiteaMirror · 0 comments
GiteaMirror commented 2025-11-26 23:29:14 -06:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/bitwarden/android/pull/1842
Author: @fedemkr
Created: 3/14/2022
Status: ✅ Merged
Merged: 3/14/2022
Merged by: @fedemkr

Base: master ← Head: bug/ios-15-4-beta-scroll-crash

📝 Commits (1)

  • 433ff3f Fix #1745 crash on scroll of grouped collection view on iOS 15.4 beta

📊 Changes

4 files changed (+9 additions, -9 deletions)

View changed files

📝 src/App/Pages/Send/SendGroupingsPage/SendGroupingsPage.xaml (+1 -2)
📝 src/App/Pages/Settings/SettingsPage/SettingsPage.xaml (+1 -2)
📝 src/App/Pages/Settings/SettingsPage/SettingsPageViewModel.cs (+6 -3)
📝 src/App/Pages/Vault/GroupingsPage/GroupingsPage.xaml (+1 -2)

📄 Description

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

Fixes crash produced on iOS 15.4 beta when:

  • Scrolling on vault
  • Accessing Send list
  • Accessing Settings

There seems to be a different calculation algorithm on iOS 15.4 beta for auto cell sizes. And if the header has something that changes visibility therefore the height of the cell, it's invalidating the context of layout measurement and it seems to be triggering the recursion calculation process again indefinitely. I'm not sure why the recursion process doesn't stop when it should after some recursion cycles but that visibility seems to be the root of the problem.
On a side note, also I've found out that if the header HeightRequest is given a value that is not an int (i.e. has decimals) the recursion is infinite as well so the app crashes. So IMHO, there seems to be something on Xamarin.Forms and/or Apple internal calculations that is comparing with some tolerance that is not expected when using non int fixed values.

Code changes

  • ....xaml: Removed the visibility binding on the top BoxView of the GroupHeaderTemplate so that the auto cell size calculations don't get messed up. Btw I don't think hiding this top line on the first element makes a huge UI/UX difference so I just removed the binding instead of having some logic to just apply the binding on iOS 15.4 downwards.
  • SettingsPageViewModel.cs: Changed the ExtendedObservableCollection to use ObservableRangeCollection from the Community toolkit so that the collection property change notifications are done propertly

Testing requirements

Update to iOS 15.4 RC and check that the app doesn't crash on vault, settings, send lists and when using autofill.
Make some general testing as well just in case I've missed some other form of crash.
Make some tests on the same views on Android just in case something got broken which I don't think.

Before you submit

  • I have added unit tests where it makes sense to do so (encouraged but not required)
  • This change requires a documentation update (notify the documentation team)
  • This change has particular deployment requirements (notify the DevOps team)

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/bitwarden/android/pull/1842 **Author:** [@fedemkr](https://github.com/fedemkr) **Created:** 3/14/2022 **Status:** ✅ Merged **Merged:** 3/14/2022 **Merged by:** [@fedemkr](https://github.com/fedemkr) **Base:** `master` ← **Head:** `bug/ios-15-4-beta-scroll-crash` --- ### 📝 Commits (1) - [`433ff3f`](https://github.com/bitwarden/android/commit/433ff3fe3723725c9405b538c5bc14e330227f7a) Fix #1745 crash on scroll of grouped collection view on iOS 15.4 beta ### 📊 Changes **4 files changed** (+9 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `src/App/Pages/Send/SendGroupingsPage/SendGroupingsPage.xaml` (+1 -2) 📝 `src/App/Pages/Settings/SettingsPage/SettingsPage.xaml` (+1 -2) 📝 `src/App/Pages/Settings/SettingsPage/SettingsPageViewModel.cs` (+6 -3) 📝 `src/App/Pages/Vault/GroupingsPage/GroupingsPage.xaml` (+1 -2) </details> ### 📄 Description ## Type of change - [X] Bug fix - [ ] New feature development - [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc) - [ ] Build/deploy pipeline (DevOps) - [ ] Other ## Objective <!--Describe what the purpose of this PR is. For example: what bug you're fixing or what new feature you're adding--> Fixes crash produced on iOS 15.4 beta when: - Scrolling on vault - Accessing Send list - Accessing Settings There seems to be a different calculation algorithm on iOS 15.4 beta for auto cell sizes. And if the header has something that changes visibility therefore the height of the cell, it's invalidating the context of layout measurement and it seems to be triggering the recursion calculation process again indefinitely. I'm not sure why the recursion process doesn't stop when it should after some recursion cycles but that visibility seems to be the root of the problem. On a side note, also I've found out that if the header `HeightRequest` is given a value that is not an `int` (i.e. has decimals) the recursion is infinite as well so the app crashes. So IMHO, there seems to be something on Xamarin.Forms and/or Apple internal calculations that is comparing with some tolerance that is not expected when using non `int` fixed values. ## Code changes <!--Explain the changes you've made to each file or major component. This should help the reviewer understand your changes--> <!--Also refer to any related changes or PRs in other repositories--> * **....xaml:** Removed the visibility binding on the top `BoxView` of the `GroupHeaderTemplate` so that the auto cell size calculations don't get messed up. Btw I don't think hiding this top line on the first element makes a huge UI/UX difference so I just removed the binding instead of having some logic to just apply the binding on iOS 15.4 downwards. * **SettingsPageViewModel.cs:** Changed the `ExtendedObservableCollection` to use `ObservableRangeCollection` from the Community toolkit so that the collection property change notifications are done propertly ## Testing requirements <!--What functionality requires testing by QA? This includes testing new behavior and regression testing--> Update to iOS 15.4 RC and check that the app doesn't crash on vault, settings, send lists and when using autofill. Make some general testing as well just in case I've missed some other form of crash. Make some tests on the same views on Android just in case something got broken which I don't think. ## Before you submit - [ ] I have added **unit tests** where it makes sense to do so (encouraged but not required) - [ ] This change requires a **documentation update** (notify the documentation team) - [ ] This change has particular **deployment requirements** (notify the DevOps team) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2025-11-26 23:29:14 -06:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
app:authenticator
app:password-manager
bug
bug-passkey
customer-support
duplicate
enhancement
good first issue
help wanted
needs-reply
needs-response
pull-request
Mirrored from GitHub Pull Request
 question
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/android#3172
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?