Repository navigation
Fix paste into browser routing to terminal text box (text box beta) - #6872
Closed
austinywang wants to merge 10 commits into
Closed
austinywang wants to merge 10 commits into
austinywang wants to merge 10 commits into
Conversation
With a terminal + browser side by side and text box beta enabled, Cmd+V into the focused browser pastes into the terminal's text box instead. Root cause: paste is missing from the browser document-editing commands (only copy/cut/select-all are routed to focused web content first), so Cmd+V falls through to the NSWindow.performKeyEquivalent broadcast where the terminal text box's performKeyEquivalent claims it even though it is not the first responder. These two tests fail on the current tree and pass once the fix lands: - shouldRouteBrowserDocumentEditingCommandEquivalentThroughWebContentFirst must return true for Cmd+V. - TextBoxInputTextView.performKeyEquivalent must decline Cmd+V while another view owns first responder. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…6380) With a terminal + browser side by side and text box beta enabled, pasting into the focused browser pasted into the terminal's text box instead. Two complementary causes, fixed here: 1. Paste was missing from the browser document-editing commands (BrowserDocumentEditingCommandEquivalent had copy/cut/select-all but not paste). Copy/cut/select-all preflight into the focused web view before cmux's menu fallback, but Cmd+V fell through to the NSWindow.performKeyEquivalent broadcast. Add paste (Cmd+V) so the focused web view claims it first, exactly like the other editing commands. Cmd+V only — Cmd+Shift+V keeps its dedicated paste-as-plain-text path. 2. performKeyEquivalent is broadcast to every view in the window, not just the first responder. TextBoxInputTextView's override claimed Cmd+C/X/V and undo/redo unconditionally, so an unfocused text box stole those shortcuts from whatever actually owned focus (browser web content, the browser omnibar, etc.). Gate the standard edit/undo shortcuts on the text view being the first responder; the focus shortcut path is intentionally left ungated so it can still focus the text box. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a ChangesBrowser Paste Shortcut Routing Fix
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant NSWindow
participant BrowserWebView as Focused Web View
participant TextBoxInputTextView
NSWindow->>BrowserWebView: performKeyEquivalent(Cmd+V)
BrowserWebView-->>NSWindow: shouldRouteBrowserDocumentEditingCommandEquivalentThroughWebContentFirst returns true
NSWindow->>TextBoxInputTextView: performKeyEquivalent(Cmd+V) if web view declines
TextBoxInputTextView->>TextBoxInputTextView: isFirstResponderForEditingShortcuts?
TextBoxInputTextView-->>NSWindow: handle if first responder, else fallback to super
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (24 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryTwo-part fix for Cmd+V being intercepted by an unfocused
Confidence Score: 5/5Safe to merge — the change is a targeted, well-documented two-part fix with regression tests that cover the exact failure scenario and several related edge cases. Both production changes are small, follow the established pattern in the file (paste mirrors copy/cut/selectAll/italic in the enum, and the first-responder guard follows the same gating logic as other AppKit performKeyEquivalent overrides), and the fix provides defense-in-depth: the browser preflight ensures the web view claims Cmd+V first, and the first-responder guard ensures an unfocused text box never steals it even if the broadcast reaches it. No new ambient globals, no blocking primitives, no i18n surface, and no test seams introduced. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User as User (Cmd+V)
participant CmuxWebView as CmuxWebView<br/>(focused browser)
participant ShortcutRouting as ShortcutRoutingSupport
participant TextBox as TextBoxInputTextView<br/>(unfocused)
Note over User,TextBox: BEFORE fix
User->>ShortcutRouting: performKeyEquivalent(Cmd+V)
ShortcutRouting-->>User: paste not in BrowserDocumentEditingCommandEquivalent → skip browser preflight
User->>TextBox: NSWindow broadcast performKeyEquivalent
TextBox-->>User: claimed unconditionally → pasted into text box
Note over User,TextBox: AFTER fix
User->>ShortcutRouting: performKeyEquivalent(Cmd+V)
ShortcutRouting->>CmuxWebView: paste now in BrowserDocumentEditingCommandEquivalent → preflight to web content
CmuxWebView-->>User: handled → paste into browser
Note over TextBox: broadcast never reaches text box (browser already consumed event)
Note over TextBox: defense-in-depth: even if broadcast reaches text box, isFirstResponderForEditingShortcuts guard returns false → super()
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User as User (Cmd+V)
participant CmuxWebView as CmuxWebView<br/>(focused browser)
participant ShortcutRouting as ShortcutRoutingSupport
participant TextBox as TextBoxInputTextView<br/>(unfocused)
Note over User,TextBox: BEFORE fix
User->>ShortcutRouting: performKeyEquivalent(Cmd+V)
ShortcutRouting-->>User: paste not in BrowserDocumentEditingCommandEquivalent → skip browser preflight
User->>TextBox: NSWindow broadcast performKeyEquivalent
TextBox-->>User: claimed unconditionally → pasted into text box
Note over User,TextBox: AFTER fix
User->>ShortcutRouting: performKeyEquivalent(Cmd+V)
ShortcutRouting->>CmuxWebView: paste now in BrowserDocumentEditingCommandEquivalent → preflight to web content
CmuxWebView-->>User: handled → paste into browser
Note over TextBox: broadcast never reaches text box (browser already consumed event)
Note over TextBox: defense-in-depth: even if broadcast reaches text box, isFirstResponderForEditingShortcuts guard returns false → super()
Reviews (7): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
…paste-into-browser-always
Contributor
Author
CI note:
|
…paste-into-browser-always # Conflicts: # .github/swift-file-length-budget.tsv
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This gitignored scheduler lock (a stale local artifact) was accidentally tracked on this branch and is unrelated to the paste-routing fix. Remove it from the PR diff so only the intended files remain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
…paste-into-browser-always # Conflicts: # .github/swift-file-length-budget.tsv
…paste-into-browser-always # Conflicts: # .github/swift-file-length-budget.tsv
…paste-into-browser-always
…paste-into-browser-always
Contributor
Author
|
Reviewed the CodeRabbit "Cmux Full Internationalization" summary finding. This PR does not add user-facing app, CLI, schema, docs, or localization strings: the production changes are shortcut routing data and first-responder gating, and the added English strings are source comments/test assertion messages only. No localization catalog update is needed for this PR. CI and autoreview are clean on the current head. |
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
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
3 participants
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.
Fixes #6380
Problem
With a terminal and a browser pane side by side and text box beta enabled, pasting (Cmd+V) into the focused browser pasted into the terminal's text box instead of the browser.
Only paste was affected — not copy/cut/select-all, and not typing — and only with the text box beta enabled. That asymmetry is the tell.
Root cause
Two complementary issues:
Paste was missing from the browser document-editing commands.
BrowserDocumentEditingCommandEquivalentlistedcopy,cut, andselectAllbut notpaste. Those three preflight into the focused web view (shouldRouteBrowserDocumentEditingCommandEquivalentThroughWebContentFirst) before cmux's menu fallback, so the focused browser claims them first. Cmd+V was never routed there, so it fell through to the originalNSWindow.performKeyEquivalent.performKeyEquivalentis broadcast to every view in the window, not just the first responder.TextBoxInputTextViewoverrides it and claimed Cmd+C/X/V (and undo/redo) unconditionally. So in the fall-through broadcast, the unfocused terminal text box grabbed Cmd+V before the menu could route it to the browser — pasting into the text box. Copy/cut weren't affected because they were already routed to web content first (issue 1); typing wasn't affected becausekeyDownonly reaches the first responder.Fix
ShortcutRoutingSupport.swift— addpaste(Cmd+V) toBrowserDocumentEditingCommandEquivalentso the focused web view claims it first, exactly like copy/cut/select-all. Cmd+V only; Cmd+Shift+V (paste-and-match-style) keeps its dedicated path becausematchesrequires an exact modifier match.TextBoxInput.swift— gate the text box's standard edit/undo shortcuts inperformKeyEquivalenton the text view actually owning first responder, so an unfocused text box no longer steals copy/cut/paste/undo from the focused browser (web content or omnibar) or terminal. The focus shortcut path is intentionally left ungated so it can still focus the text box.Tests
Two-commit red/green:
testBrowserPasteCommandRoutesThroughWebContentFirstandtestTextBoxDeclinesPasteShortcutWhenNotFirstResponder.testTextBoxHandlesPasteShortcutWhenFirstResponder(normal paste still works when focused) andtestBrowserPlainTextPasteCommandIsNotADocumentEditingPaste(Cmd+Shift+V keeps its own path).Localization
No user-facing strings, menus, settings rows, shortcut metadata, alerts, tooltips, or docs changed — Cmd+V is a standard system command (not a cmux-owned
KeyboardShortcutSettingsshortcut), matching the existing copy/cut/select-all pattern. No localization updates required.🤖 Generated with Claude Code
Summary by cubic
Fixes Cmd+V being intercepted by the terminal text box when Text Box Beta is enabled. Paste now goes to the focused browser; the text box only handles edit shortcuts when it’s first responder (fixes #6380).
BrowserDocumentEditingCommandEquivalentso the focused web view claims it first; Cmd+Shift+V keeps its dedicated path.TextBoxInputTextView.performKeyEquivalentso copy/cut/paste/undo/redo run only when the text box owns first responder; the focus shortcut stays ungated.Written for commit c388e03. Summary will update on new commits.
Summary by CodeRabbit