From fa306937f7ef0ad7afd11d9374fb68bba1827d9d Mon Sep 17 00:00:00 2001 From: bench Date: Sun, 20 Sep 2026 14:13:06 -0700 Subject: [PATCH] fix(terminal): address review nits on the #21762 middle-click fix - Fix a mis-attributing comment: the suppression window (not preventDefault, which only helps on mousedown while Chromium's native paste fires on mouseup) is what swallows the duplicate native paste. - Drop the now-unused getPrimarySelectionMiddleClickPane. - Assert stopPropagation is/isn't called per tracking mode in the repro test. --- ...de-middle-click-double-paste.repro.test.ts | 26 +++++++++++++------ .../use-terminal-pane-mobile-actions.ts | 20 +++++--------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/issue-21762-tracking-mode-middle-click-double-paste.repro.test.ts b/src/renderer/src/components/terminal-pane/issue-21762-tracking-mode-middle-click-double-paste.repro.test.ts index 473912bbdac..19dab3e0eda 100644 --- a/src/renderer/src/components/terminal-pane/issue-21762-tracking-mode-middle-click-double-paste.repro.test.ts +++ b/src/renderer/src/components/terminal-pane/issue-21762-tracking-mode-middle-click-double-paste.repro.test.ts @@ -89,8 +89,9 @@ function buildController(pane: ManagedPane): TerminalPaneContextController { function fireMiddleMouseDown( handler: (event: React.MouseEvent) => void, target: EventTarget -): { defaultPrevented: boolean } { +): { defaultPrevented: boolean; propagationStopped: boolean } { let defaultPrevented = false + let propagationStopped = false // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test-only stub; the handler only calls button/target/preventDefault/stopPropagation off the event. handler({ button: 1, @@ -98,9 +99,11 @@ function fireMiddleMouseDown( preventDefault: () => { defaultPrevented = true }, - stopPropagation: () => {} + stopPropagation: () => { + propagationStopped = true + } } as unknown as React.MouseEvent) - return { defaultPrevented } + return { defaultPrevented, propagationStopped } } describe('issue 21762: middle-click native-paste suppression in mouse-tracking TUIs', () => { @@ -113,7 +116,7 @@ describe('issue 21762: middle-click native-paste suppression in mouse-tracking T document.body.replaceChildren() }) - it('arms native-paste suppression even when the pane is in mouse-tracking mode', () => { + it('arms native-paste suppression without stopping propagation when the pane is in mouse-tracking mode', () => { const pane = buildTrackedPane('sgr') const { result } = renderHook(() => useTerminalPaneMobileActions(buildController(pane))) @@ -122,20 +125,27 @@ describe('issue 21762: middle-click native-paste suppression in mouse-tracking T pane.container ) - // Chromium's native middle-click paste must be blocked regardless of tracking mode. - expect(outcome.defaultPrevented).toBe(true) // The #8993 suppression window must be armed so the native follow-up paste // doesn't reach xterm's helper textarea and duplicate the TUI's own paste. expect(armPrimarySelectionNativePasteSuppressionMock).toHaveBeenCalled() + // Propagation must NOT be stopped here — xterm's own mousedown listener + // (a descendant of this capture handler) still needs to see the event so + // the tracking TUI receives the click as a mouse report. + expect(outcome.propagationStopped).toBe(false) + expect(pane.terminal.focus).not.toHaveBeenCalled() }) - it('still pastes directly to the PTY when the pane is not in mouse-tracking mode', () => { + it('stops propagation and pastes directly to the PTY when the pane is not in mouse-tracking mode', () => { const pane = buildTrackedPane('none') const { result } = renderHook(() => useTerminalPaneMobileActions(buildController(pane))) - fireMiddleMouseDown(result.current.handlePrimarySelectionMiddleMouseDown, pane.container) + const outcome = fireMiddleMouseDown( + result.current.handlePrimarySelectionMiddleMouseDown, + pane.container + ) expect(armPrimarySelectionNativePasteSuppressionMock).toHaveBeenCalled() + expect(outcome.propagationStopped).toBe(true) expect(pane.terminal.focus).toHaveBeenCalled() }) }) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-mobile-actions.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-mobile-actions.ts index 6a94386625f..ba47eaa7d5c 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-mobile-actions.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-mobile-actions.ts @@ -113,15 +113,6 @@ export function useTerminalPaneMobileActions(controller: TerminalPaneContextCont // oxlint-disable-next-line react-hooks/exhaustive-deps -- Preserve the pre-split dependency contract. [terminalShouldHandleMiddleClick] ) - const getPrimarySelectionMiddleClickPane = useCallback( - (target: EventTarget | null) => { - const clickedPane = findTerminalPaneForMiddleClick(target) - return clickedPane && clickedPane.terminal.modes.mouseTrackingMode === 'none' - ? clickedPane - : null - }, - [findTerminalPaneForMiddleClick] - ) const handlePrimarySelectionMiddleMouseDown = useCallback( (event: React.MouseEvent): void => { if (event.button !== 1 || !isPrimarySelectionEnabled()) { @@ -131,10 +122,12 @@ export function useTerminalPaneMobileActions(controller: TerminalPaneContextCont if (!targetPane) { return } - // Why: block Chromium's native middle-click paste and arm the shared - // suppression window unconditionally; only the paste-to-PTY below is - // gated on tracking mode, since a tracking TUI still needs the click - // forwarded as a mouse report and must not have propagation stopped. + // Why: arm the shared suppression window unconditionally — it, not + // preventDefault, is what swallows Chromium's native follow-up paste + // (fired on mouseup, not mousedown; see usePrimarySelectionPaste.ts). + // Only the paste-to-PTY below is gated on tracking mode, since a + // tracking TUI still needs the click forwarded as a mouse report and + // must not have propagation stopped. event.preventDefault() armPrimarySelectionNativePasteSuppression() if (targetPane.terminal.modes.mouseTrackingMode !== 'none') { @@ -263,7 +256,6 @@ export function useTerminalPaneMobileActions(controller: TerminalPaneContextCont restorePaneTerminalFit, restoreAllTerminalFits, terminalShouldHandleMiddleClick, - getPrimarySelectionMiddleClickPane, handlePrimarySelectionMiddleMouseDown, handlePrimarySelectionAuxClick, activatePaneTitleInteraction,