From 9f019edfebfe53881e3a968fe975c8ba9466c612 Mon Sep 17 00:00:00 2001 From: 2ro <17595647+2ro@users.noreply.github.com> Date: Sun, 5 Jul 2026 00:41:33 -0400 Subject: [PATCH] Goblin: fix desktop clipboard (copy/paste lost the selection) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On desktop the Paste button in the wallet-restore seed entry did nothing, and the mirror copy on the new-wallet creation screen was equally broken: a copied recovery phrase never made it to the clipboard. Root cause (inherited from GRIM, not a Goblin regression — the two clipboard impls are byte-identical): copy_string_to_buffer / get_string_from_buffer each created a fresh `arboard::Clipboard` and dropped it the instant the function returned. On Linux (X11 AND Wayland) the clipboard selection is owned by the live Clipboard instance, so dropping it immediately released ownership and the copied text vanished before it could be pasted. Reproduced with arboard 3.6.1 on a Wayland session: fresh-instance-per-call read back empty; one persistent instance across set+get read back correctly. Fix: keep ONE long-lived `arboard::Clipboard` on the Desktop platform (behind a Mutex, since the trait methods take &self and arboard's take &mut self), so our process stays the durable selection owner. Also stop unwrap()-panicking on a clipboard backend error — a failed copy/paste now logs instead of crashing. Every clipboard button in the app routes through this one platform pair (onboarding paste/copy, GRIM creation paste/copy, proof, slatepack message entry, tx/receipt copy, receive copy, npub/nsec/backup copy, the shared TextEdit paste), so this single change repairs the whole class at once. Verified end-to-end: an #[ignore]d clipboard_roundtrip test drives the real Desktop impl on the live session clipboard and asserts a 24-word phrase survives copy -> paste (run with --ignored). It passes on Wayland now and would fail under the old pattern. --- src/gui/platform/desktop/mod.rs | 74 +++++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 4 deletions(-) diff --git a/src/gui/platform/desktop/mod.rs b/src/gui/platform/desktop/mod.rs index 74380eaf..d4302bba 100644 --- a/src/gui/platform/desktop/mod.rs +++ b/src/gui/platform/desktop/mod.rs @@ -39,6 +39,17 @@ pub struct Desktop { /// Flag to check if attention required after window focusing. attention_required: Arc, + + /// Long-lived clipboard owner. On Linux (X11 AND Wayland) the clipboard + /// selection is owned by the live `arboard::Clipboard` instance: the prior + /// code created a fresh instance per call and dropped it the moment the + /// function returned, so the selection ownership was released immediately and + /// copied text (e.g. a recovery phrase) vanished before it could be pasted — + /// the "Paste does nothing on desktop" bug. Keeping ONE instance alive for + /// the app lifetime makes our process the durable selection owner, so a copy + /// survives long enough to paste (in-app or into another window). Held behind + /// a Mutex because the trait methods take `&self` and arboard's take `&mut`. + clipboard: Arc>>, } impl Desktop { @@ -49,9 +60,28 @@ impl Desktop { camera_index: Arc::new(AtomicUsize::new(0)), stop_camera: Arc::new(AtomicBool::new(false)), attention_required: Arc::new(AtomicBool::new(false)), + clipboard: Arc::new(parking_lot::Mutex::new(None)), } } + /// Run `f` against the process-wide, lazily-created clipboard instance, + /// returning `default` if the clipboard backend can't be opened. Reusing one + /// instance (rather than `Clipboard::new()` per call) is what keeps a copied + /// selection alive on Linux — see the `clipboard` field. + fn with_clipboard(&self, f: impl FnOnce(&mut arboard::Clipboard) -> R, default: R) -> R { + let mut guard = self.clipboard.lock(); + if guard.is_none() { + match arboard::Clipboard::new() { + Ok(c) => *guard = Some(c), + Err(e) => { + log::error!("clipboard: failed to open: {e}"); + return default; + } + } + } + f(guard.as_mut().unwrap()) + } + // #[allow(dead_code)] #[cfg(not(target_os = "macos"))] fn start_camera_capture( @@ -211,13 +241,24 @@ impl PlatformCallbacks for Desktop { } fn copy_string_to_buffer(&self, data: String) { - let mut clipboard = arboard::Clipboard::new().unwrap(); - clipboard.set_text(data).unwrap(); + // Reuse the long-lived instance so the selection survives the call (see + // the `clipboard` field). A backend error is logged, never a panic — a + // failed copy must not crash the wallet. + self.with_clipboard( + |clipboard| { + if let Err(e) = clipboard.set_text(data) { + log::error!("clipboard: set_text failed: {e}"); + } + }, + (), + ); } fn get_string_from_buffer(&self) -> String { - let mut clipboard = arboard::Clipboard::new().unwrap(); - clipboard.get_text().unwrap_or("".to_string()) + self.with_clipboard( + |clipboard| clipboard.get_text().unwrap_or_default(), + String::new(), + ) } fn start_camera(&self) { @@ -359,3 +400,28 @@ lazy_static! { /// Last captured image from started camera. static ref LAST_CAMERA_IMAGE: Arc, u32)>>> = Arc::new(RwLock::new(None)); } + +#[cfg(test)] +mod clipboard_tests { + use super::*; + use crate::gui::platform::PlatformCallbacks; + + /// Round-trips a copy then paste through the REAL `Desktop` platform impl on + /// the live session clipboard. Ignored by default (needs a display/clipboard + /// backend); run manually with a Wayland/X11 session: + /// cargo test --lib clipboard_roundtrip -- --ignored --nocapture + /// Proves the persistent-instance fix: with the old fresh-instance-per-call + /// pattern this read back empty on Wayland/X11. + #[test] + #[ignore] + fn clipboard_roundtrip() { + let d = Desktop::new(); + let phrase = "abandon ability able about above absent absorb abstract \ + absurd abuse access accident account accuse achieve acid acoustic \ + acquire across act action actor actress"; + d.copy_string_to_buffer(phrase.to_string()); + std::thread::sleep(std::time::Duration::from_millis(200)); + let got = d.get_string_from_buffer(); + assert_eq!(got, phrase, "clipboard round-trip lost the copied text"); + } +}