From 6251d6e18ee29b1f94bd91a2171fb0cf6fde3f65 Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Tue, 11 Aug 2026 17:38:08 -0400 Subject: [PATCH] fix(copy-text): close the menu when decryption fails, key the chooser on what was rendered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups from testing the Copy Original / Copy Translated chooser on device. The menus stopped dismissing for a note that cannot be decrypted. AccountViewModel.decrypt only invokes its callback on success, and the copy flow now dismisses from inside that callback, so a read-only account (or a foreign/corrupt DM, or a signer that refuses) left the popup/sheet sitting open with nothing copied — previously onDismiss ran unconditionally. Adds decryptOrNull, which always answers (null on failure, and still rethrows so launchSigner keeps toasting/logging the signer error), and dismisses on null. The chooser was also missed whenever the string the viewer translated wasn't the note's raw content. RenderTextEvent prepends a NIP-14 subject the body doesn't already repeat, and renders the newest edit of a versioned post; the copy flow looked the translation up by the raw content of the note it was handed, so a subject-carrying note never hit the cache and silently copied the untranslated text. That derivation is now one function, displayedNoteText, used by both sides, and it is also what gets copied — a subject is part of what the user is reading. The handler takes (note, versionShown) so the body comes from the version on screen and the subject from the note itself, as the viewer composes them; the quick-action menu, which passed the pre-edit original, now resolves the newest modification the way the card does. Verified on a tablet: an undecryptable NIP-04 message under a read-only login now closes the sheet, and a translated note carrying a subject tag offers the chooser and copies the subject line with the original. Co-Authored-By: Claude Opus 5 (1M context) --- .../amethyst/ui/note/CopyNoteText.kt | 38 +++++++++++++------ .../amethyst/ui/note/NoteQuickActionMenu.kt | 8 +++- .../ui/note/elements/NoteActionSections.kt | 2 +- .../amethyst/ui/note/types/Text.kt | 31 ++++++++++----- .../ui/screen/loggedIn/AccountViewModel.kt | 23 +++++++++++ .../display/BookmarkGroupItemOptions.kt | 2 +- 6 files changed, 80 insertions(+), 24 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/CopyNoteText.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/CopyNoteText.kt index 74e23ee3f6..09e1ab078c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/CopyNoteText.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/CopyNoteText.kt @@ -34,6 +34,7 @@ import com.vitorpamplona.amethyst.ui.components.M3ActionRow import com.vitorpamplona.amethyst.ui.components.M3ActionSection import com.vitorpamplona.amethyst.ui.components.cachedTranslation import com.vitorpamplona.amethyst.ui.components.util.setText +import com.vitorpamplona.amethyst.ui.note.types.displayedNoteText import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.stringRes import kotlinx.coroutines.launch @@ -55,18 +56,26 @@ data class CopyTextChoice( * rendered, which is what populated that cache — so a hit means the user is looking at a * translation and gets a chooser (Copy Original / Copy Translated); a miss copies directly. * - * Returns the click handler for the menu entry, taking the note whose text to copy (callers - * pass the latest version of a versioned post). [onCopied] runs after the text lands on the - * clipboard, [onDismiss] when the chooser is cancelled without copying; callers must keep - * their menu in composition until one of the two runs, because the chooser dialog is emitted - * from this composable. + * What gets copied — and what the cache is keyed on — is [displayedNoteText], the same string + * the viewer rendered, not the raw event content: a NIP-14 subject is part of what the user is + * reading and of what was translated. + * + * Returns the click handler for the menu entry, taking the note the menu belongs to and the + * version of it the screen is showing (the same note unless the post was edited — the body + * comes from the version, the subject from the note itself, exactly as the viewer composes + * them). [onCopied] runs after the text lands on the + * clipboard, [onDismiss] when the chooser is cancelled without copying **or** when the note + * can't be decrypted at all (a read-only account, a refused signer) so the menu still closes + * instead of hanging on a copy that will never happen. Callers must keep their menu in + * composition until one of the two runs, because the chooser dialog is emitted from this + * composable. */ @Composable fun copyNoteTextAction( accountViewModel: AccountViewModel, onCopied: () -> Unit, onDismiss: () -> Unit, -): (Note) -> Unit { +): (note: Note, versionShown: Note) -> Unit { val clipboardManager = LocalClipboard.current val scope = rememberCoroutineScope() val choice = remember { mutableStateOf(null) } @@ -95,13 +104,18 @@ fun copyNoteTextAction( ) } - return { note -> - accountViewModel.decrypt(note) { original -> - val translated = cachedTranslation(original, accountViewModel) - if (translated == null) { - copy(original) + return { note, versionShown -> + accountViewModel.decryptOrNull(versionShown) { decrypted -> + if (decrypted == null) { + onDismiss() } else { - choice.value = CopyTextChoice(original, translated) + val original = displayedNoteText(note, decrypted) + val translated = cachedTranslation(original, accountViewModel) + if (translated == null) { + copy(original) + } else { + choice.value = CopyTextChoice(original, translated) + } } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteQuickActionMenu.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteQuickActionMenu.kt index fc98c537af..0363421c51 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteQuickActionMenu.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteQuickActionMenu.kt @@ -70,6 +70,7 @@ import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import com.vitorpamplona.amethyst.model.AddressableNote import com.vitorpamplona.amethyst.model.Note import com.vitorpamplona.amethyst.model.User +import com.vitorpamplona.amethyst.model.textNoteModifications import com.vitorpamplona.amethyst.ui.components.util.setText import com.vitorpamplona.amethyst.ui.navigation.navs.INav import com.vitorpamplona.amethyst.ui.navigation.routes.routeEditDraftTo @@ -299,6 +300,11 @@ fun CardBody( ) } + // "Copy Text" copies the version on screen: an edited post renders its newest modification + // by default (EditState.updateModifications), and the 3-dot menu already copies that one. + // Reading `edits` is a hard-referenced in-memory fold, so no cache scan here. + val noteVersionToCopy = remember(note) { note.textNoteModifications().lastOrNull() ?: note } + // When the rendered note was translated, tapping Copy Text opens a chooser // (Copy Original / Copy Translated) on top of this popup; the popup stays up // until the flow resolves so the chooser survives in composition. @@ -318,7 +324,7 @@ fun CardBody( icon = MaterialSymbols.ContentCopy, label = stringRes(R.string.quick_action_copy_text), ) { - copyNoteText(note) + copyNoteText(note, noteVersionToCopy) } VerticalDivider(color = primaryLight) NoteQuickActionItem( diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/elements/NoteActionSections.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/elements/NoteActionSections.kt index 9cb33f5778..ba13c44b22 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/elements/NoteActionSections.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/elements/NoteActionSections.kt @@ -144,7 +144,7 @@ fun noteActionSections( buildList { add( NoteAction(MaterialSymbols.ContentCopy, stringRes(R.string.copy_text)) { - copyNoteText(noteVersionToCopy) + copyNoteText(note, noteVersionToCopy) }, ) add( diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Text.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Text.kt index b9954841e1..02347f4c91 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Text.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Text.kt @@ -69,6 +69,27 @@ enum class ReplyRenderType { NONE, } +/** + * The text [RenderTextEvent] puts on screen for [note] given its decrypted [body]: a NIP-14 + * subject the body doesn't already repeat is prepended to it. + * + * This is the exact string handed to `TranslatableRichTextViewer`, so it is also the key the + * translation cache stores the result under. The copy-text menus look their translation up by + * the same function — keying on the raw body instead would miss the entry for every + * subject-carrying note and silently copy the untranslated text. + */ +fun displayedNoteText( + note: Note, + body: String, +): String { + val subject = (note.event as? TextNoteEvent)?.subject()?.ifBlank { null } + return if (subject != null && !body.contains(subject, ignoreCase = true)) { + "$subject\n\n$body" + } else { + body + } +} + @Composable fun RenderTextEvent( note: Note, @@ -177,15 +198,7 @@ fun RenderTextEvent( body } - val eventContent = - remember(newBody) { - val subject = (note.event as? TextNoteEvent)?.subject()?.ifBlank { null } - if (!subject.isNullOrBlank() && !newBody.contains(subject, ignoreCase = true)) { - "$subject\n\n$newBody" - } else { - newBody - } - } + val eventContent = remember(newBody) { displayedNoteText(note, newBody) } // A boosted note inside a zap/nutzap/onchain activity card is always shown as a // compact 2-line preview, even when the logged-in user is only a zap-split diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index 7963b56c27..951523e1a2 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -1585,6 +1585,29 @@ class AccountViewModel( account.decryptContent(note)?.let { onReady(it) } } + /** + * [decrypt] that always answers: [onReady] gets null when the content can't be read — a + * read-only account holding no key, a DM this account isn't part of, or a signer that + * refused/timed out. [decrypt] stays silent in those cases, which strands callers that must + * finish either way (a menu that only closes once the copy resolves, say). + */ + fun decryptOrNull( + note: Note, + onReady: (String?) -> Unit, + ) = launchSigner { + val decrypted = + try { + account.decryptContent(note) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + // launchSigner still gets the exception to toast/log the signer failure. + onReady(null) + throw e + } + onReady(decrypted) + } + /** * Runs an action that has both a tracked and a direct broadcast variant, * picking the path the user selected via the "Tracked broadcasts" setting. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/bookmarkgroups/display/BookmarkGroupItemOptions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/bookmarkgroups/display/BookmarkGroupItemOptions.kt index 229f21c690..77b637efdb 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/bookmarkgroups/display/BookmarkGroupItemOptions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/bookmarkgroups/display/BookmarkGroupItemOptions.kt @@ -201,7 +201,7 @@ fun BookmarkGroupItemOptionsMenu( M3ActionSection { M3ActionRow(icon = MaterialSymbols.ContentCopy, text = stringRes(R.string.copy_text)) { val lastNoteVersion = (editState?.value as? GenericLoadable.Loaded)?.loaded?.modificationToShow?.value ?: note - copyNoteText(lastNoteVersion) + copyNoteText(note, lastNoteVersion) } M3ActionRow(icon = MaterialSymbols.ContentCopy, text = stringRes(R.string.copy_user_pubkey)) { note.author?.let {