Fix state restore bug in FindAndReplaceDialogFragment#19981
Fix state restore bug in FindAndReplaceDialogFragment#19981lukstbit wants to merge 1 commit intoankidroid:mainfrom
Conversation
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
bdffe19 to
a7cd900
Compare
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
Show resolved
Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/browser/FindAndReplaceDialogFragment.kt
Show resolved
Hide resolved
| context: Context, | ||
| noteIds: List<NoteId>, | ||
| ): FindAndReplaceDialogFragment { | ||
| val file = IdsFile(context.cacheDir, noteIds) |
There was a problem hiding this comment.
We have a common issue that IdsFile is not deleted after the activity/fragment/dialog is disposed.
Is it feasible to handle this in the PR for this fragment?
| context: Context, | ||
| noteIds: List<NoteId>, | ||
| ): FindAndReplaceDialogFragment { | ||
| val file = IdsFile(context.cacheDir, noteIds) |
There was a problem hiding this comment.
Please provide a name to the IdsFile, this will make it easier to clean up our 'bad' IdsFile implementation at a future date
🤔 which bug ? |
lukstbit
added
the
Needs Author Reply
|
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 |
|
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 |
3 participants
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. See 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