From 5e35c7a69e2a25aa5b793b5135cdf051d1cac101 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Tue, 3 Nov 2026 08:53:05 +0100 Subject: fix(app): finish an editor save after recreation A successful save left saving set while the destination committed in the activity's lifecycleScope. An activity recreated then (theme, locale, split-screen resize) cancelled that scope: onSaved never ran, and the surviving ViewModel kept Save disabled and back swallowed. The ViewModel now keeps the success in a memory-only saved state, set inside the NonCancellable write so a cancelled caller cannot lose it. A LaunchedEffect on it runs the commit and onSaved, so a recreated editor finishes the job; "Save and copy" and the stale-original notice run once only, and left() clears it before onSaved navigates. Editing a symlinked entry derived its folder from the canonical path but its name from the link, saving a new entry next to the target. The folder is now relative to the non-canonical repository directory; the save still checks the target is inside the store. Co-Authored-By: Claude Opus 5.5 --- .../passwordstore/ui/editor/EditorDestination.kt | 59 ++++++++++++---------- .../app/passwordstore/ui/editor/EditorViewModel.kt | 36 ++++++++++--- .../java/app/passwordstore/ui/main/MainActivity.kt | 5 +- 3 files changed, 65 insertions(+), 35 deletions(-) (limited to 'app/src') diff --git a/app/src/main/java/app/passwordstore/ui/editor/EditorDestination.kt b/app/src/main/java/app/passwordstore/ui/editor/EditorDestination.kt index 8b66ba5e..2d1e12f7 100644 --- a/app/src/main/java/app/passwordstore/ui/editor/EditorDestination.kt +++ b/app/src/main/java/app/passwordstore/ui/editor/EditorDestination.kt @@ -122,36 +122,41 @@ fun EditorDestination( // Recipients from the nearest .gpg-id, as pass does; null: an invalid ID, already reported. val ids = activity.getPGPIdentifiers(current.dir) ?: return activity.lifecycleScope.launch { - when (val result = viewModel.save(ids, target, original, activity)) { - null -> Unit - is SaveResult.Failed -> failure = activity.getString(result.message, result.arg) - is SaveResult.Saved -> { - if (copy) activity.copyPasswordToClipboard(result.form.password) - result.staleOriginal?.let { - activity.snackbar( - message = - activity.getString( - R.string.password_creation_file_delete_fail_message, - it.nameWithoutExtension, - ), - length = Snackbar.LENGTH_LONG, - ) - } - // ponytail: commit failures are not shown, as for the list's move and delete; a theme - // or locale change in the middle of a save cancels before this commit, and the next - // commit picks the file up. - activity.commitChange( + val result = viewModel.save(ids, target, original, activity, copy) + if (result is SaveResult.Failed) failure = activity.getString(result.message, result.arg) + } + } + + // Finishes a save: run again by an editor recreated in the middle (a theme or locale change + // cancels this effect), so the commit and onSaved always happen; the copy and notice only once. + val saved = viewModel.saved + LaunchedEffect(saved) { + if (saved == null) return@LaunchedEffect + if (viewModel.announce()) { + if (viewModel.copyOnSave) activity.copyPasswordToClipboard(saved.form.password) + saved.staleOriginal?.let { + activity.snackbar( + message = activity.getString( - if (original != null) R.string.git_commit_edit_text else R.string.git_commit_add_text, - target.relativeTo(root).path.removeSuffix(".gpg"), - ) - ) - keyboard?.hide() - focusManager.clearFocus() - onSaved(result) - } + R.string.password_creation_file_delete_fail_message, + it.nameWithoutExtension, + ), + length = Snackbar.LENGTH_LONG, + ) } } + // ponytail: commit failures are not shown, as for the list's move and delete. A commit cut + // short by recreation runs again; one that already went through finds nothing to commit. + activity.commitChange( + activity.getString( + if (original != null) R.string.git_commit_edit_text else R.string.git_commit_add_text, + saved.file.relativeTo(root).path.removeSuffix(".gpg"), + ) + ) + keyboard?.hide() + focusManager.clearFocus() + viewModel.left() + onSaved(saved) } BackHandler(onBack = ::close) diff --git a/app/src/main/java/app/passwordstore/ui/editor/EditorViewModel.kt b/app/src/main/java/app/passwordstore/ui/editor/EditorViewModel.kt index 0c8b4676..6d2fbaec 100644 --- a/app/src/main/java/app/passwordstore/ui/editor/EditorViewModel.kt +++ b/app/src/main/java/app/passwordstore/ui/editor/EditorViewModel.kt @@ -87,6 +87,19 @@ constructor( var saving by mutableStateOf(false) private set + /** + * The successful save, kept until the editor has committed it and left ([left]), so an editor + * recreated in the middle finishes the job. Memory only: it holds the plaintext form. + */ + var saved by mutableStateOf(null) + private set + + /** Whether [saved] is to be copied to the clipboard ("Save and copy"). */ + var copyOnSave = false + private set + + private var announced = false + /** Whether [start] filled the form. */ var started = false private set @@ -175,28 +188,38 @@ constructor( * renamed or moved. Nothing is written (not even the folder) when [target] would replace another * entry or when encryption fails; a recipient whose key is missing fails it (see * CryptoRepository.encrypt). The caller checked [formError] and that [target] is inside the - * store. Returns null when a save already ran or is running. + * store. A success lands in [saved], with [copy] in [copyOnSave]. Returns null when a save + * already ran or is running. */ suspend fun save( ids: List, target: File, original: File?, context: Context, + copy: Boolean, ): SaveResult? { if (saving) return null saving = true - var result: SaveResult? = null + copyOnSave = copy try { - result = write(ids, target, original, context.applicationContext) + return write(ids, target, original, context.applicationContext) } finally { // Stays set after a success (the editor is leaving); cleared on failure or cancellation. - if (result !is SaveResult.Saved) saving = false + if (saved == null) saving = false } - return result + } + + /** True the first time only: the copy and notices of [saved] run once, across recreation. */ + fun announce(): Boolean = !announced.also { announced = true } + + /** The editor committed [saved] and left: nothing is left to finish. */ + fun left() { + saved = null } override fun onCleared() { form = EditorForm() + saved = null } private suspend fun write( @@ -235,7 +258,8 @@ constructor( stale = original } } - SaveResult.Saved(target, form, stale) + // Set here, inside NonCancellable: a written entry is never forgotten by a cancelled caller. + SaveResult.Saved(target, form, stale).also { saved = it } } } diff --git a/app/src/main/java/app/passwordstore/ui/main/MainActivity.kt b/app/src/main/java/app/passwordstore/ui/main/MainActivity.kt index 1a1ff500..da708148 100644 --- a/app/src/main/java/app/passwordstore/ui/main/MainActivity.kt +++ b/app/src/main/java/app/passwordstore/ui/main/MainActivity.kt @@ -122,8 +122,9 @@ class MainActivity : BaseGitActivity() { }, onEdit = { if (nav.currentBackStackEntry?.id == entry.id) { - val root = PasswordRepository.getRepositoryDirectory().canonicalFile - val dir = file.canonicalFile.parentFile?.relativeTo(root)?.path.orEmpty() + // Not canonical: a symlinked entry keeps its own folder and name. + val root = PasswordRepository.getRepositoryDirectory() + val dir = file.parentFile?.relativeTo(root)?.path.orEmpty() nav.navigate(EditRoute(dir, file.nameWithoutExtension)) } }, -- cgit v1.2.3