fix(desktop): drop the Paste accelerator on Windows only - #1335
fix(desktop): drop the Paste accelerator on Windows only#1335rezaffikri wants to merge 2 commits into
Conversation
winc fires a menu accelerator from WM_KEYDOWN without consuming the key, so Paste's explicit callback ran on top of the webview's own native paste and the clipboard landed twice. macOS still needs the accelerator, where it does consume the event and is the only path that reaches Monaco. Fixes skyhook-io#1276
|
Thank you, this is an excellent first contribution. Tracing it to the accelerator being registered alongside an explicit callback, and explaining why This also fixes #1276, which another user reported a week ago and which we had not yet got to the bottom of. Two people helped here without knowing it. Queued with @hisco for review. CI is green. Thanks again for including tests. |
The double insert is specific to winc on Windows, which fires the menu action from WM_KEYDOWN and still forwards the key to the webview. AppKit and GTK consume the accelerator, so there it is the only paste path — dropping it would hand Monaco to native webview behavior that hasn't been verified on either.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a744dd2. Configure here.
| if goos == "windows" { | ||
| return nil | ||
| } | ||
| return keys.CmdOrCtrl("v") |
There was a problem hiding this comment.
Linux paste accelerator reintroduced
High Severity
pasteAccelerator now binds Ctrl+V on Linux again. The PR reports double-paste on Windows and Linux when the accelerator and the Paste callback both run, and the intended fix was macOS-only. Linux was not tested, so this can restore two clipboard inserts per keypress there.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit a744dd2. Configure here.
There was a problem hiding this comment.
This asks for the opposite of the review on #1336. There the same code was flagged as:
pasteAcceleratorclearsCtrl+Vfor both Windows and Linux, but the double-paste rationale only matches Windowswinc, which forwards the key to the webview. GTK consumes accelerators, so on Linux the menu callback was the singleCtrl+Vpath and did not double-insert. Dropping it leaves paste to native webview behavior that was never verified on Linux, especially for Monaco.
I agreed with that and changed the code to match. This comment now asks me to undo it.
On the evidence quoted here: "The PR reports double-paste on Windows and Linux" came from my own description, and it was wrong. I corrected it in the same round of changes. The description now says Windows only. So the claim rests on a mistake in my writing rather than on the code.
The load bearing question is whether GTK consumes the accelerator, and that is not addressed. In GTK 3.24 the closure behind gtk_widget_add_accelerator is:
gboolean can_activate = gtk_widget_can_activate_accel (closure->data, aclosure->signal_id);
if (can_activate)
g_signal_emit (closure->data, aclosure->signal_id, 0);
g_value_set_boolean (return_value, can_activate);The "handled" result comes from widget sensitivity, never from whether a handler is connected. gtkwindow.c also runs gtk_window_activate_key before gtk_window_propagate_key_event. So the accel entry beats the focused webview and the key never reaches it. One paste, not two.
You are right that I have not tested Linux on real hardware, and I say so in the description. But that argues for keeping the accelerator, not dropping it. With this change pasteAccelerator("linux") returns keys.CmdOrCtrl("v"), which is what main does today, so Linux behavior is unchanged and there is no new untested path. Dropping the accelerator is the option that would put untested WebKitGTK behavior on the Monaco paste path.
The only platform this PR changes is Windows, and that is the one I tested.
|
@roylibman Thank you, glad it helped. But just to let you know, I pushed |
hisco
left a comment
There was a problem hiding this comment.
Approve. Correct root cause - Windows accelerator doesn't consume the key, so it pastes twice. Windows-only scope is right. Clean fix.


Description
On Windows, one
Ctrl+Vinserts the clipboard twice. Pasting from Edit → Paste with the mouse inserts it once, correctly.The Edit menu registers
Ctrl+Vas a native accelerator and attaches an explicit callback that performs its own paste:On Windows that means two pastes for one keypress, because the accelerator does not consume the key. Wails handles accelerators itself in
WM_KEYDOWNand falls through toDefWindowProc—winc/form.go:So the clipboard lands once from the webview's own paste and once from the menu callback. Clicking the menu item sends no key event to the webview, which is why the mouse path was always correct.
This is the same trap already documented for Reload in this file: "a native menu accelerator fires regardless of webview focus." Reload was made platform-aware; Paste was not.
Fix: drop the Paste accelerator on Windows only. Windows is the only platform where the accelerator fails to consume the key. macOS (AppKit) and Linux (GTK) both consume it, so there the accelerator is the single paste path and must stay — dropping it would hand Monaco to native webview behavior that isn't verified on either.
On GTK the accelerator is registered by
gtk_widget_add_acceleratorand consumed regardless of whether a handler is connected:closure_accel_activateingtkwidget.creturnscan_activate(widget sensitivity), never handler presence. Andgtkwindow.crunsgtk_window_activate_keybeforegtk_window_propagate_key_event, so the accelerator wins over the focused webview.Cut/Copyare unaffected and deliberately left alone here: they passnilcallbacks, andwindows/menu.goonly binds a handler whenClick != nil, so their accelerator fires into an unbound event and is a harmless no-op. Doubling needs a bound callback and the fall-through; only Paste had both.Trade-off
On Windows the Edit menu no longer displays the
Ctrl+Vhint next to Paste. In winc the hint and the key binding are the same thing (initMenuItemInfoFromActionwrites the label text and registersshortcut2Actiontogether), so keeping the hint means keeping the double paste.Ctrl+Vstill works — the webview handles it.Type of change
How has this been tested?
Environment: Windows 11 (26200), Go 1.26.1, Node 24.13.1, Wails v2.12.0.
Tested locally with minikube/kind
Tested against a remote cluster
Added/updated unit tests
Added
TestPasteAcceleratorDroppedOnlyOnWindows(per-GOOS table) andTestCreateMenuPasteKeepsCallbackWithoutDoubleBinding(wiring). The wiring test fails against the unfixed wiring on Windows —Paste accelerator = &{Key:v Modifiers:[cmdorctrl]}, want <nil>— and passes with the fix.The wiring test also pins that the callback must stay: a
nilcallback binds no handler on Windows, which would leave Edit → Paste inert.go build ./...— clean.go test ./cmd/desktop/— pass.go test ./...—internal/serverandinternal/timelinefail, but they fail identically on unmodifiedmain(same 5 + 1 tests), so they are pre-existing and unrelated to this change.cd web && npm run tsc— pass (no frontend files touched).gofmt— clean.Manual check on a
wails build -platform windows/amd64binary from this branch:Ctrl+Vinserts the clipboard once in the resource search box and once in the Monaco YAML editor, and Edit → Paste from the menu bar still works.Not tested on macOS or Linux — I don't have access to either. The change is scoped to
windows, so macOS and Linux behavior is unchanged by construction.Checklist
Related issues
Fixes #1276