Repository navigation
UI: Paste button and menus paste the system clipboard; regression test for the Paste freeze (#273) - #461
Merged
Conversation
…ion test (storytold#273) In 0.5.0 a click anywhere on the Home ribbon's Paste button (label, icon or dropdown arrow) froze the app for good. home() called paste.secondary_clicked() inside ctx.data_mut(); after any click egui answers that with ctx.input(), which takes the context lock the closure already holds, so the UI thread waited on itself. Sampling the hung 0.5.0 process on macOS shows the main thread parked in RawRwLock::lock_exclusive_slow under Context::write <- data_mut <- ribbon::home. storytold#272 moved the query out of the closure; it landed after the 0.5.0 release, which is the build both reporters used. No test ever clicked the Paste button itself, which is how this shipped. The new test clicks it, then opens its menu with a secondary click and picks Paste Values, on a worker thread with a timeout so a regression fails instead of hanging the run. With storytold#272 reverted it fails (egui's debug lock check panics after 10 s); with it, it passes. The Paste button gets an accessible "Paste" name so the test (and screen readers) can find it, the same way Pictures and Border presets are named. With the freeze gone, the button still did not do what it says: it pasted GridCraft's own copy only (GridState::system_clipboard was never assigned), so text copied in another app gave "Nothing to paste." The ribbon button, its options menu's Paste, the cell and header context menus and the macOS Edit menu now read the system clipboard through a new paste_text host service and paste it like Cmd/Ctrl+V does, falling back to GridCraft's own copy when there is no text (or no service, as on the web). Asking egui for the clipboard instead (RequestPaste) was tried and stalled the macOS event loop until the next input event. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ps a newer programmatic copy
…p (H,V,P) pastes the system clipboard
neilcawse
force-pushed
the
fix/ribbon-paste
branch
from
October 13, 2026 01:19
fa99cb5 to
95012e0
Compare
Member
|
Merged — thank you @neilcawse for the contribution! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The freeze in #273 is already fixed on main by #272. This PR adds the regression test that was missing. It also fixes the next thing a user hits once the freeze is gone: the Paste button ignored text copied in other apps. I didn't put
Fixes #273on it because the freeze fix is #272's. #273 can be closed once a release includes #272.The freeze (#273)
In 0.5.0, clicking anywhere on the Home ribbon's Paste button (the label, the icon or the dropdown arrow) froze the app for good. Both reports use 0.5.0 or older: the macOS comment, and the Fedora RPM
gridcraft 0.5.0in the Linux comment. #272 merged on 2026-10-11, after the v0.5.0 release on 2026-10-10.git tag --contains e1939aalists no tag.Cause:
ribbon::homecalledpaste.secondary_clicked()insideui.ctx().data_mut(|data| …). In egui 0.36,Response::clicked_byisself.flags.contains(CLICKED) && self.ctx.input(…), so it only touches the context after a click. When it does, it tries to take the context lock thatdata_mutalready holds, and the UI thread waits on itself. Which button was clicked and what is on the clipboard make no difference.Reproduced on macOS (arm64) by building
v0.5.0withcargo build --release -p gridcraftand running--sample sales --control 7981:ui.inspectanswered. Thenui.click {x:44,y:100}(the Paste label) went through, and everyui.inspectafter it timed out. I relaunched and repeated withui.click {x:61,y:125}(the dropdown chevron): same freeze.sample <pid> 3, main thread (abridged):RawRwLock::lock_shared_slowbehind it.The Linux stack in the issue fits this: the main thread sits in a bare
syscall(parking_lot's futex wait) and the other threads are idle. In release builds egui's lock just waits forever, so the SIGABRT a few seconds later probably came from outside the process (the session or launcher killing a hung app). That part is an inference; the coredump has no symbols.On 95c8776 (main + #449) I ran 21 combinations: a left click on the label, a left click on the chevron and a right click, each with 7 clipboard contents (empty; plain text; ~10 MB of text; a PNG image; an RTF/HTML table; a file copied in Finder; a GridCraft copy).
ui.inspectanswered within 0.4 s every time.The Paste button ignored the system clipboard
On main, with text copied in another app, clicking Paste showed "Nothing to paste." (control channel:
ui.clickon the button, thensheet.readreturned an empty cell andui.inspectreturnedmessage: "Nothing to paste."). The button pastedGridState::system_clipboard, which nothing ever assigns, so it always ranedit.pastewith no text and pasted only GridCraft's own copy. The options menu's Paste, the cell and header context menus and the macOS Edit menu behaved the same way. Cmd/Ctrl+V did work, because it gets the text from egui'sEvent::Paste.First I tried asking egui for the clipboard (
ViewportCommand::RequestPaste) and waiting for theEvent::Pasteit produces. On macOS that stalled the event loop. After the request, no frame ran until the next real input event. The control channel'ssheet.readtimed out for 4 s at a time, and the log showed the text arriving about 26 s later, when some unrelated event finally woke the loop. Without the request, frames kept running. So I dropped that approach.What this PR changes
Services::paste_text(new,crates/ui-egui/src/lib.rs): the host reads the clipboard's plain text.SheetApp::paste_from_clipboard()pastes that text through the existingpaste_external, the same route Cmd/Ctrl+V uses, so the HTML lookup from Clipboard: pasted currency cells keep the number from the clipboard HTML (#257) #285 still applies. When there is no text (an empty clipboard, an image) or no service (the web build), it pastes GridCraft's own copy, as before. The ribbon button, the options menu's Paste, the cell and header context menus and the macOS Edit menu all call it. The unusedGridState::system_clipboardis removed.SheetApp::copy_to_clipboardinribbon_keys, as the ribbon's Copy and Cut buttons do, so they publish to the system clipboard. Before, they ranedit.copy/edit.cutwithout it, and the Paste button then pasted older text from another app instead of the copy. Excel publishes every Copy to the clipboard.edit.copy/edit.cut, which go throughSheetApp::run) still don't write to it.SheetApp::runnow remembers the system clipboard's text at that moment (clipboard_text_at_copy). While the clipboard still holds that text,paste_from_clipboardpastes GridCraft's copy, because it is newer. Once other text is copied, that text is pasted.copy_to_clipboardclears the remembered text, because it publishes the copy itself; it calls the new privaterun_command(the old body ofrun) so that it doesn't read the clipboard first.SheetAppnow remembers that text (cut_text, cleared by any copy, and by any paste of other text, since the clipboard has then moved on). When GridCraft's clipboard is empty and the pasted text is still that text (trailing line breaks ignored, asedit.pastedoes, and CRLF matching LF),paste_external(the one route for Cmd/Ctrl+V and for the ribbon, menu and keytip Paste) runs the internaledit.paste, which reports "Nothing to paste.". Text copied elsewhere afterwards still pastes, and so does the cut's text once it has been copied afresh after other text.edit.pasteSpecialand pasted only GridCraft's copy.keytips.jsonnow targetsedit.paste(table and generated tests regenerated withcargo xtask keytips), andribbon_keyssendsedit.pastetopaste_from_clipboard, like the button. Its badge now sits on the menu's Paste item instead of theedit.pasteSpecialitems. H,V,S and the other Paste Special keytips are unchanged.apps/gridcraft/src/main.rs):paste_textreads plain text through the same shared arboard handle thatpaste_htmlandcopy_htmlalready use (read_clipboard, factored out of the oldpaste_htmlclosure).crates/ui-egui/tests/clipboard.rs):clicking_the_ribbon_paste_button_or_its_menu_does_not_freezeclicks Paste with real pointer events and checks the pasted cells. Then it secondary-clicks Paste, picks Paste Values and checks those cells. It runs on a worker thread with a 60 srecv_timeout, so a deadlock fails the test instead of hangingcargo test.ribbon_paste_pastes_the_system_clipboard_like_cmd_v: tab-separated CRLF text from the host lands in D4:E5. A GridCraft copy coming back through the clipboard is pasted with its formulas. An empty clipboard pastes GridCraft's own copy. One clipboard read per click.paste_menu_items_paste_the_system_clipboardcovers the options menu's Paste and the cell context menu's Paste.keytip_copy_and_cut_reach_the_system_clipboard_and_ribbon_paste_pastes_them: F10, H, C (and F10, H, X) with "stale text from earlier" on the host clipboard. The host clipboard then holds the copied text, and the ribbon Paste button pastes A1:B2 at D4.a_pasted_ribbon_cut_is_not_pasted_again_from_the_system_clipboard: A1:B3 with B3=B2*2, F10,H,X, Paste at D4, Paste again: E6 keeps its formula and "Nothing to paste." shows; new host text then pastes.cmd_v_after_a_pasted_cut_does_not_paste_the_cut_text_again: A1:B3 with B3=B2*2,Event::Cut,Event::Pasteof the published text at D4, then again with CRLF line endings: E6 keeps its formula.a_pasted_programmatic_cut_does_not_paste_the_older_system_clipboard_text:run("edit.cut")with "stale" on the host, Paste, Paste: D4 stays "Name"; new host text then pastes.the_cut_text_pastes_again_once_the_clipboard_has_held_other_text: cut A1 ("Name"), Cmd/Ctrl+V at D4, paste "other" at G1, then paste "Name" (CRLF) at H1: H1 is "Name", not "Nothing to paste.".the_paste_keytip_pastes_the_system_clipboard_like_the_paste_button: host textexternal\t7, no GridCraft copy, F10,H,V,P pastes it at D4:E4.ribbon_paste_prefers_a_programmatic_copy_until_other_text_is_copied:run("edit.copy")leaves the host's stale text in place, and the Paste button pastes the copy. After the host text changes, the Paste button pastes the new text.crates/ui-egui/src/ribbon.rs:paste.widget_info(…"Paste").big_buttonregisters no accessible name, so neither kittest nor a screen reader could find the button. This uses the same one-line pattern as Pictures and Border presets.docs/ui-parity.md: header line and a revision row. The row no longer says the desktop reads the clipboard off the UI thread with a 5 s timeout;read_clipboardreads it on the UI thread.Verification
secondary_clicked()back insidedata_mut),cargo test -p gridcraft-ui-egui --test clipboard clicking_the_ribbonFAILED in 10.2 s withDEBUG PANIC: Failed to acquire RwLock write after 10s. Deadlock?. That is egui's debug-build lock check; the test's own timeout covers release builds. The two system-clipboard tests useServices::paste_text, which doesn't exist before this PR.keytip_copy_and_cut_reach_the_system_clipboard_and_ribbon_paste_pastes_themfails (the host clipboard still holds the stale text). With theclipboard_text_at_copycheck reverted,ribbon_paste_prefers_a_programmatic_copy_until_other_text_is_copiedfails (D4 gets "stale text from earlier").lib.rsand keytip changes reverted (tests kept), all three new tests fail on e16373d.lib.rschange reverted,cmd_v_after_a_pasted_cut_does_not_paste_the_cut_text_againfails on ea690ce (E6 becomes the constant 84). With thecut_text = Nonereset reverted,the_cut_text_pastes_again_once_the_clipboard_has_held_other_textfails (H1 stays empty).cargo test -p gridcraft-ui-egui --test clipboard: 15 passed.cargo xtask keytips --checkpasses.cargo test -p gridcraft-ui-egui,cargo clippy -p gridcraft-ui-egui --all-targets -- -D warningsandcargo check -p gridcraft-web --target wasm32-unknown-unknownpass.--sample sales --control 7987):ext\t7\nrow2\t8frompbcopy, then a click on the Paste label, and separately on the chevron:sheet.readshowsext/7/row2/8at the selection.menu\t9on the clipboard:W30:X30=menu,9.edit.copyof A1:B2: the button and the chevron paste A1:B2.ui.inspectanswered 1.4–2.1 s after the click.ui.inspectstayed responsive throughout. I saved the clipboard text before the runs and restored it afterwards.cargo xtask ci:CI summary: all 8 steps passed (fmt, clippy, test, assets, layers, wasm, parity, keytips).Not done
big_split_buttonwidget; Paste should move to it once UI: theme tokens with a contrast test, ribbon overflow arrows and split button, multi-line formula bar, code editor widget #456 merges, as a follow-up.paste_textread (on Paste, and inSheetApp::runafter a control-channel or MCP copy) has the same exposure as the existingpaste_htmlread on Cmd/Ctrl+V; I haven't seen it hang, so this PR doesn't change it.Event::Paste, and that path still pastes it, as on main. Only the ribbon and menu Paste useclipboard_text_at_copy.