github-starred/android
2
0
Fork 0
mirror of https://github.com/bitwarden/android.git synced 2026-03-22 12:32:53 -05:00
Code Issues 144 Packages Projects Releases 93 Wiki Activity

[PR #1879] [MERGED] EC-149 Fix crash when trying to Focus on Lock page #3195

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

📋 Pull Request Information

Original PR: https://github.com/bitwarden/android/pull/1879
Author: @fedemkr
Created: 4/13/2022
Status: ✅ Merged
Merged: 7/5/2022
Merged by: @fedemkr

Base: master ← Head: bug/fix-crash-becomefirstresponder

📝 Commits (1)

  • 9b201c1 Fix crash when trying to Focus an Entry from a background thread and improved the code so there are fewer direct access from the VM to the View

📊 Changes

2 files changed (+39 additions, -30 deletions)

View changed files

📝 src/App/Pages/Accounts/LockPage.xaml.cs (+27 -12)
📝 src/App/Pages/Accounts/LockPageViewModel.cs (+12 -18)

📄 Description

Type of change

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

Objective

Fix crash produced when focussing the pin/master password entry on the lock page after choosing the fallback method on a failed biometric scan.
I couldn't reproduced this but there are a lot of crashes reported on AppCenter so just fixed it where it's appearing to crash.
Also improved the code so that the ViewModel have fewer direct accesses to the View/Page.

Code changes

  • LockPage.xaml.cs: Improved the code making the SecretEntry centralizing the logic with the PinLock to get the correct entry. Also perform the logic of Focusing on the Page invoking it on the main thread to avoid the crash.
  • LockPageViewModel.cs: Moved the focus action to the page, given that the VM should not do this directly on MVVM. Also added WeakEventManager so we don't have to worry about memory leaks.

Testing requirements

I couldn't reproduce the issue but given the crash report it would be like:

  1. Enter you account
  2. Configure biometrics
  3. Configure pin
  4. Lock the account
  5. Try to unlock with fail biometric (i.e. don't use the correct one)
  6. Try again until the fallback appears (Master Password / Pin)
  7. Choose the fallback
  8. Then it should crash

Also it could be like that but with the automatic prompting of biometrics when minimizing the app and starting it again.

Btw, if no biometrics is setup the pin/master password entry should be automatically focused (i.e. should work as before) and if biometrics are configured and the fallback is selected the same should happen, i.e. it should automatically focus the pin/master password.

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/1879 **Author:** [@fedemkr](https://github.com/fedemkr) **Created:** 4/13/2022 **Status:** ✅ Merged **Merged:** 7/5/2022 **Merged by:** [@fedemkr](https://github.com/fedemkr) **Base:** `master` ← **Head:** `bug/fix-crash-becomefirstresponder` --- ### 📝 Commits (1) - [`9b201c1`](https://github.com/bitwarden/android/commit/9b201c131cfecd28ddec5b008aa9c6afa8912060) Fix crash when trying to Focus an Entry from a background thread and improved the code so there are fewer direct access from the VM to the View ### 📊 Changes **2 files changed** (+39 additions, -30 deletions) <details> <summary>View changed files</summary> 📝 `src/App/Pages/Accounts/LockPage.xaml.cs` (+27 -12) 📝 `src/App/Pages/Accounts/LockPageViewModel.cs` (+12 -18) </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--> Fix crash produced when focussing the pin/master password entry on the lock page after choosing the fallback method on a failed biometric scan. I couldn't reproduced this but there are a lot of crashes reported on AppCenter so just fixed it where it's appearing to crash. Also improved the code so that the ViewModel have fewer direct accesses to the View/Page. ## 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--> * **LockPage.xaml.cs:** Improved the code making the `SecretEntry` centralizing the logic with the `PinLock` to get the correct entry. Also perform the logic of Focusing on the `Page` invoking it on the main thread to avoid the crash. * **LockPageViewModel.cs:** Moved the focus action to the page, given that the VM should not do this directly on MVVM. Also added `WeakEventManager` so we don't have to worry about memory leaks. ## Testing requirements <!--What functionality requires testing by QA? This includes testing new behavior and regression testing--> I couldn't reproduce the issue but given the crash report it would be like: 1. Enter you account 2. Configure biometrics 3. Configure pin 4. Lock the account 5. Try to unlock with fail biometric (i.e. don't use the correct one) 6. Try again until the fallback appears (Master Password / Pin) 7. Choose the fallback 8. Then it should crash Also it could be like that but with the automatic prompting of biometrics when minimizing the app and starting it again. Btw, if no biometrics is setup the pin/master password entry should be automatically focused (i.e. should work as before) and if biometrics are configured and the fallback is selected the same should happen, i.e. it should automatically focus the pin/master password. ## 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:32 -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#3195
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?