From 321f8c14c82222d757bf55239f606cf2399f9e60 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 24 Mar 2026 22:56:55 -0700 Subject: [PATCH] fix(browser): keep IME Enter on composition path (#2108) * test: cover browser IME Enter composition routing * fix(browser): keep IME Enter on composition path --- Sources/AppDelegate.swift | 21 +++++ cmuxTests/BrowserConfigTests.swift | 138 +++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index ff17ab70..d86e8c48 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -1504,12 +1504,31 @@ func browserOmnibarShouldSubmitOnReturn(flags: NSEvent.ModifierFlags) -> Bool { return normalizedFlags == [] || normalizedFlags == [.shift] } +func browserResponderHasMarkedText(_ responder: NSResponder?) -> Bool { + guard let responder else { return false } + + // During IME composition, Return/Enter belongs to the text system so the + // candidate list can commit or confirm the marked text. + if let textInputClient = responder as? NSTextInputClient { + return textInputClient.hasMarkedText() + } + + if let textField = responder as? NSTextField, + let editor = textField.currentEditor() as? NSTextView { + return editor.hasMarkedText() + } + + return false +} + func shouldDispatchBrowserReturnViaFirstResponderKeyDown( keyCode: UInt16, firstResponderIsBrowser: Bool, + firstResponderHasMarkedText: Bool = false, flags: NSEvent.ModifierFlags ) -> Bool { guard firstResponderIsBrowser else { return false } + guard !firstResponderHasMarkedText else { return false } guard keyCode == 36 || keyCode == 76 else { return false } // Keep browser Return forwarding narrow: only plain/Shift Return should be // treated as submit-intent. Command-modified Return is reserved for app shortcuts @@ -12399,6 +12418,7 @@ private extension NSWindow { let firstResponderWebView = self.firstResponder.flatMap { Self.cmuxOwningWebView(for: $0, in: self, event: event) } + let firstResponderHasMarkedText = browserResponderHasMarkedText(self.firstResponder) if let ghosttyView = firstResponderGhosttyView { // If the IME is composing and the key has no Cmd modifier, don't intercept — // let it flow through normal AppKit event dispatch so the input method can @@ -12442,6 +12462,7 @@ private extension NSWindow { if shouldDispatchBrowserReturnViaFirstResponderKeyDown( keyCode: event.keyCode, firstResponderIsBrowser: firstResponderWebView != nil, + firstResponderHasMarkedText: firstResponderHasMarkedText, flags: event.modifierFlags ) { // Forwarding keyDown can re-enter performKeyEquivalent in WebKit/AppKit internals. diff --git a/cmuxTests/BrowserConfigTests.swift b/cmuxTests/BrowserConfigTests.swift index ae60de6b..a618ade1 100644 --- a/cmuxTests/BrowserConfigTests.swift +++ b/cmuxTests/BrowserConfigTests.swift @@ -56,6 +56,21 @@ func installCmuxUnitTestInspectorOverride() { cmuxUnitTestInspectorOverrideInstalled = true } +private final class BrowserMarkedTextProbeTextView: NSTextView { + var hasMarkedTextForTesting = false + private(set) var keyDownEvents: [NSEvent] = [] + + override var acceptsFirstResponder: Bool { true } + + override func hasMarkedText() -> Bool { + hasMarkedTextForTesting + } + + override func keyDown(with event: NSEvent) { + keyDownEvents.append(event) + } +} + final class CmuxWebViewKeyEquivalentTests: XCTestCase { private final class ActionSpy: NSObject { private(set) var invoked: Bool = false @@ -2455,6 +2470,107 @@ final class BrowserOmnibarCommandNavigationTests: XCTestCase { } +final class BrowserIMEKeyDownRoutingTests: XCTestCase { + @MainActor + func testWindowPerformKeyEquivalentDoesNotForwardReturnDuringMarkedTextComposition() { + _ = NSApplication.shared + AppDelegate.installWindowResponderSwizzlesForTesting() + + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 640, height: 420), + styleMask: [.titled, .closable], + backing: .buffered, + defer: false + ) + let container = NSView(frame: window.contentRect(forFrameRect: window.frame)) + window.contentView = container + + let webView = CmuxWebView(frame: container.bounds, configuration: WKWebViewConfiguration()) + webView.autoresizingMask = [.width, .height] + container.addSubview(webView) + + let responder = BrowserMarkedTextProbeTextView(frame: NSRect(x: 0, y: 0, width: 32, height: 20)) + responder.hasMarkedTextForTesting = true + webView.addSubview(responder) + + window.makeKeyAndOrderFront(nil) + defer { window.orderOut(nil) } + + XCTAssertTrue(window.makeFirstResponder(responder)) + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "\r", + charactersIgnoringModifiers: "\r", + isARepeat: false, + keyCode: 36 + ) else { + XCTFail("Failed to construct Return event") + return + } + + let consumed = window.performKeyEquivalent(with: event) + + XCTAssertFalse(consumed, "Return should stay in the IME path while marked text is active") + XCTAssertTrue(responder.hasMarkedText(), "Marked text should still be active until the input method commits it") + XCTAssertEqual(responder.keyDownEvents.count, 0, "Return should not be force-forwarded to the browser responder during IME composition") + } + + @MainActor + func testWindowPerformKeyEquivalentDoesNotForwardKeypadEnterDuringMarkedTextComposition() { + _ = NSApplication.shared + AppDelegate.installWindowResponderSwizzlesForTesting() + + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 640, height: 420), + styleMask: [.titled, .closable], + backing: .buffered, + defer: false + ) + let container = NSView(frame: window.contentRect(forFrameRect: window.frame)) + window.contentView = container + + let webView = CmuxWebView(frame: container.bounds, configuration: WKWebViewConfiguration()) + webView.autoresizingMask = [.width, .height] + container.addSubview(webView) + + let responder = BrowserMarkedTextProbeTextView(frame: NSRect(x: 0, y: 0, width: 32, height: 20)) + responder.hasMarkedTextForTesting = true + webView.addSubview(responder) + + window.makeKeyAndOrderFront(nil) + defer { window.orderOut(nil) } + + XCTAssertTrue(window.makeFirstResponder(responder)) + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "\r", + charactersIgnoringModifiers: "\r", + isARepeat: false, + keyCode: 76 + ) else { + XCTFail("Failed to construct keypad Enter event") + return + } + + let consumed = window.performKeyEquivalent(with: event) + + XCTAssertFalse(consumed, "Keypad Enter should stay in the IME path while marked text is active") + XCTAssertTrue(responder.hasMarkedText(), "Marked text should still be active until the input method commits it") + XCTAssertEqual(responder.keyDownEvents.count, 0, "Keypad Enter should not be force-forwarded to the browser responder during IME composition") + } +} + + final class BrowserReturnKeyDownRoutingTests: XCTestCase { func testRoutesForReturnWhenBrowserFirstResponder() { XCTAssertTrue( @@ -2496,6 +2612,28 @@ final class BrowserReturnKeyDownRoutingTests: XCTestCase { ) } + func testDoesNotRouteReturnWhenBrowserFirstResponderHasMarkedText() { + XCTAssertFalse( + shouldDispatchBrowserReturnViaFirstResponderKeyDown( + keyCode: 36, + firstResponderIsBrowser: true, + firstResponderHasMarkedText: true, + flags: [] + ) + ) + } + + func testDoesNotRouteKeypadEnterWhenBrowserFirstResponderHasMarkedText() { + XCTAssertFalse( + shouldDispatchBrowserReturnViaFirstResponderKeyDown( + keyCode: 76, + firstResponderIsBrowser: true, + firstResponderHasMarkedText: true, + flags: [] + ) + ) + } + func testRoutesForShiftReturnWhenBrowserFirstResponder() { XCTAssertTrue( shouldDispatchBrowserReturnViaFirstResponderKeyDown(