fix: handle import dialog events via ViewModel lifecycle#20992
Conversation
|
@mixelas Are you able to reproduce this issue ? If yes, May I know the exact reproduction steps, coz I can't reproduce |
|
@sanjaysargam I wasn’t able to reproduce it reliably either. The issue appears to be timing/lifecycle dependent, so there isn’t a deterministic set of steps that reproduces it every time. The closest scenario I could infer from the crash reports and code path is: |
|
I'd propose closing this until reproduction steps and a regression test exists, |
|
@lukstbit assigned this to me and explicitly approved this ViewModel approach as the right solution. I followed his guidance and implemented it. This issue has real ACRA crash reports in production and the fix uses a standard pattern already in AnkiDroid. I can add unit/integration tests to strengthen confidence. The timing-dependent nature makes 100% repro hard, but the fix is architecturally sound. |
|
I'll wait for a reply, but it's unexpected for someone to produce a fix before reproducing an issue. Our contributor guide rules aren't "hard" rules, but I feel skipping reproduction steps often requires a strong justification. I've had cases where I was able to remove hundreds of lines of code once a problem was properly understood |
There was a problem hiding this comment.
I think this is a step in the right direction but:
- I expect the previous related message code to be removed
- I expect that the import dialog isn't shown for activities that can't handle it(only DeckPicker should handle it)
- I expect that if we end up in another activity by the time the dialog should be shown to have DeckPicker show it when coming back
Note: I've made a PR for one of the other async dialogs having this issue in #21018
Kind of reproduce this by adding the patch below in DeckPicker:
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
index 823542920d..d0906c4185 100644
--- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
@@ -197,6 +197,7 @@ import com.ichi2.utils.show
import com.ichi2.utils.title
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
+import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.launch
@@ -357,6 +358,7 @@ open class DeckPicker :
lifecycleScope.launch {
withProgress(message = getString(R.string.import_preparing_file)) {
withContext(Dispatchers.IO) {
+ delay(4000)
onSelectedPackageToImport(it.data!!)
}
}
Start the import process of an apkg -> select a file -> come back and seeing the progress -> while the progress is running go to home -> wait a bit more until the delay passes -> start CardBrowser through a shortcut -> see the ImportAdd dialog -> confirm and crash
While we can't reproduce this exactly it's clear IMO what's happening. Plus this back and forth with destinations through a ViewModel is not something new and we use it. |
1a7c390 to
29ad77b
Compare
beab2fe to
7dc302e
Compare
|
Given Let's go back to the ViewModel approach. Sorry for the mixed messages |
9e668d7 to
8ce046f
Compare
8ce046f to
d24f09a
Compare
|
I switched it to the ViewModel-based approach from #21018, so the import dialog is now handled the same way as that async dialog fix. |
3ef66f1 to
cb9a264
Compare
|
Thanks a lot for the detailed review and patience here. I really appreciate it. I’m very open to further changes. I’m trying to learn the preferred project patterns as I go. |
|
Snapshot diff report vs
All 1 changed screenshotsPreferencesScreenshotTest
|
| /* | ||
| * Copyright (c) 2026 Georgios Michelakis <michelakisgio@gmail.com> | ||
| * | ||
| * This program is free software; you can redistribute it and/or modify it under | ||
| * the terms of the GNU General Public License as published by the Free Software | ||
| * Foundation; either version 3 of the License, or (at your option) any later | ||
| * version. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, but WITHOUT ANY | ||
| * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A | ||
| * PARTICULAR PURPOSE. See the GNU General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU General Public License along with | ||
| * this program. If not, see <http://www.gnu.org/licenses/>. | ||
| */ |
There was a problem hiding this comment.
I improved our process for licenses this week. The linux kernel improved this in 2017: https://github.com/ankidroid/Anki-Android/blob/main/CONTRIBUTING.md#licensecopyright-headers
Please note that the SPDX-FileCopyrightText line is optional, and you may remove it at your discretion.
| /* | |
| * Copyright (c) 2026 Georgios Michelakis <michelakisgio@gmail.com> | |
| * | |
| * This program is free software; you can redistribute it and/or modify it under | |
| * the terms of the GNU General Public License as published by the Free Software | |
| * Foundation; either version 3 of the License, or (at your option) any later | |
| * version. | |
| * | |
| * This program is distributed in the hope that it will be useful, but WITHOUT ANY | |
| * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A | |
| * PARTICULAR PURPOSE. See the GNU General Public License for more details. | |
| * | |
| * You should have received a copy of the GNU General Public License along with | |
| * this program. If not, see <http://www.gnu.org/licenses/>. | |
| */ | |
| // SPDX-License-Identifier: GPL-3.0-or-later | |
| // SPDX-FileCopyrightText: 2026 Georgios Michelakis <michelakisgio@gmail.com> |
There was a problem hiding this comment.
If you're going to resolve comments without acting on them, aim to explain why
| showImportDialog(request.dialogType, request.importPath) | ||
| }.onFailure { exception -> | ||
| if (exception is IllegalStateException) { | ||
| showSimpleNotification( |
There was a problem hiding this comment.
See lukstbit's PR for how to handle this (as far as I recall, with a TODO)
487bda8 to
2559802
Compare
67754ac to
a8889c8
Compare
david-allison
left a comment
There was a problem hiding this comment.
Feels like this needs more testing - I can't see how the SharedPrefs would have executed
|
please rebase onto main rather than including merge commits |
1a510ab to
148ff21
Compare
| private lateinit var importViewModel: ImportViewModel | ||
|
|
||
| override fun onCreate(savedInstanceState: Bundle?) { | ||
| super.onCreate(savedInstanceState) | ||
| importViewModel = ViewModelProvider(requireActivity()).get(ImportViewModel::class.java) | ||
| } |
There was a problem hiding this comment.
why not by activityViewModels()
david-allison
left a comment
There was a problem hiding this comment.
Needs a rebase - the flow also appears to be unused
General mentoring: Check the "Files changed" tab, and make sure it matches the code which you wrote
| class ImportViewModel : ViewModel() { | ||
| private val pendingImportRequestState = MutableStateFlow<ImportRequest?>(null) | ||
|
|
||
| val pendingImportRequest: StateFlow<ImportRequest?> = pendingImportRequestState |
| import timber.log.Timber | ||
|
|
||
| class ImportViewModel : ViewModel() { | ||
| private val pendingImportRequestState = MutableStateFlow<ImportRequest?>(null) |
There was a problem hiding this comment.
nit: kotlin 2.3 introduced explicit backing fields
Index: AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ImportViewModel.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ImportViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ImportViewModel.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ImportViewModel.kt (revision e795549502be2f470389931aa561671adcb837f8)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ImportViewModel.kt (date 1780051930315)
@@ -22,18 +22,17 @@
import timber.log.Timber
class ImportViewModel : ViewModel() {
- private val pendingImportRequestState = MutableStateFlow<ImportRequest?>(null)
-
- val pendingImportRequest: StateFlow<ImportRequest?> = pendingImportRequestState
+ val pendingImportRequest: StateFlow<ImportRequest?>
+ field = MutableStateFlow<ImportRequest?>(null)
fun registerImportRequest(request: ImportRequest) {
Timber.d("Import dialog requested: %s", request.dialogType)
- pendingImportRequestState.value = request
+ pendingImportRequest.value = request
}
fun clearImportRequest() {
Timber.d("Clearing pending import dialog request")
- pendingImportRequestState.value = null
+ pendingImportRequest.value = null
}
data class ImportRequest(
51a2898 to
a8889c8
Compare
|
|
||
| fun setPendingImportRequest(request: ImportRequest) { | ||
| Timber.d("Setting pending import request: %s", request.dialogType) | ||
| savedStateHandle[PENDING_IMPORT_REQUEST_KEY] = request |
There was a problem hiding this comment.
This seems to have been reverted, why?

Purpose / Description
Fix a crash where import dialog handling could run after activity changes and assume a DeckPicker-like context.
The issue is an async UI event path that may outlive the originating screen, causing invalid activity assumptions and ClassCastException in rare lifecycle transitions.
Fixes
Approach
Introduced an import event pipeline through a dedicated ImportViewModel.
ImportDialog now emits import actions as ViewModel events instead of relying only on direct activity casting.
AnkiActivity observes those events with lifecycle awareness, so dialog-triggered import actions are consumed only when the UI is in a safe state.
Kept backward-compatible behavior: if the current activity already implements ImportDialogListener, handling still works directly.
Net effect: removes brittle activity assumptions and aligns async dialog consumption with lifecycle-safe delivery.
How Has This Been Tested?
Build and run AnkiDroid in debug mode.
Start app flow that can trigger import confirmation dialogs.
Trigger lifecycle transitions around dialog timing
Test configuration used:
Android debug build
Emulator/device setup used for local development
Pre-commit hooks passed during commit, including ktlint
Checklist
Please, go through these checks before submitting the PR.