Fix state restore bug in FindAndReplaceDialogFragment#19981
Open
lukstbit wants to merge 1 commit intoankidroid:mainfrom
Open
Fix state restore bug in FindAndReplaceDialogFragment#19981lukstbit wants to merge 1 commit intoankidroid:mainfrom
lukstbit wants to merge 1 commit intoankidroid:mainfrom
Conversation
4 tasks
lukstbit
force-pushed
the
fix_findReplaceBackgroundRestore
branch
from
bdffe19 to
a7cd900
Compare
david-allison
requested changes
Jan 5, 2026
Member
david-allison
left a comment
david-allison
left a comment
There was a problem hiding this comment.
Needs to handle IdsFile being missing
This is a common issue
AnkiDroid/src/main/java/com/ichi2/anki/browser/FindAndReplaceDialogFragment.kt
Outdated
Show resolved
Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/browser/FindAndReplaceDialogFragment.kt
Outdated
Show resolved
Hide resolved
david-allison
previously requested changes
Jan 5, 2026
AnkiDroid/src/main/java/com/ichi2/anki/browser/FindAndReplaceDialogFragment.kt
Outdated
Show resolved
Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/browser/FindAndReplaceDialogFragment.kt
Outdated
Show resolved
Hide resolved
Member
🤔 which bug ? |
lukstbit
added
the
Needs Author Reply
Contributor
|
Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically |
Contributor
|
Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically |
To get the current selected notes the fragment is querying the browser's ViewModel for the selected notes. This works most of the times but fails(we lose the selection and will target everything instead of only the selected notes) when going to the background and coming back(and fragment gets recreated). At this point the fragment tries to access again the selected notes but these are not available as they are being manually restored from a file. This bug can be seen by using the "Don't keep activities" dev option. The fix consists in using an IdsFile so the fragment always has a reference to the selected notes from the moment of its creation.
lukstbit
force-pushed
the
fix_findReplaceBackgroundRestore
branch
from
a7cd900 to
13e7a13
Compare
Member
Author
|
I fixed all raised issues and added an entry about how to reproduce the bug. |
lukstbit
removed
Needs Author Reply
david-allison
approved these changes
Mar 4, 2026
| private fun removeIdsFile() { | ||
| runCatching { idsFile.delete() } | ||
| .onFailure { throwable -> | ||
| Timber.e( |
Member
There was a problem hiding this comment.
Suggested change
| Timber.e( | |
| Timber.w( |
david-allison
added
Needs Second Approval
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose / Description
Fixes a state restore bug for find and replace fragment where previously selected notes would get ignored when coming back from the background. This is not great because if the user doesn't notice he's going to run the find and replace operation over his entire collection.
Bug reproduction:
Select a row in CardBrowser -> start FindReplace dialog -> notice that the "Selected notes only" is checked -> send dialog to background(with "Don't keep activities" enabled) -> bring the app back to foreground -> at this moment the "Selected notes only" checkbox is unchecked and disabled -> Clicking OK at this point will run the operation against all notes(the previous selection can still be seen in the browser)
Video with the bug:
Screen_recording_20251230_192801.webm
I used our IdsFile implementation to not query the browser's ViewModel. I'm not enthusiastic about this but it works, the browser could probably be changed to fix this but I didn't look into it.
With the code in this PR:
Screen_recording_20251230_191350.webm
Fixes
How Has This Been Tested?
Checked the bug scenario, ran tests.
Checklist