Skip to content

fix(desktop): support IME composition in chat inputs#7539

Open
waffensam wants to merge 2 commits into
BasedHardware:mainfrom
waffensam:codex/desktop-ime-composition
Open

fix(desktop): support IME composition in chat inputs#7539
waffensam wants to merge 2 commits into
BasedHardware:mainfrom
waffensam:codex/desktop-ime-composition

Conversation

@waffensam
Copy link
Copy Markdown
Contributor

Bot fit:

  • For desktop: yes
  • Bug fix: yes

Summary:

  • Tracks AppKit marked-text state in the shared macOS chat editor.
  • Suppresses SwiftUI text overwrite while IME composition is active.
  • Keeps placeholders hidden during composition and lets Return confirm IME candidates.

Tests:

  • git diff --check origin/main..codex/desktop-ime-composition
  • xcrun swift build -c debug --package-path Desktop

Notes:

  • Build passes with existing upstream warnings only: unhandled GoogleService-Info-Dev.plist, Swift 6/deprecation warnings, and libwebp link target warning.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

Greptile Summary

This PR fixes IME (Input Method Editor) composition support in the macOS desktop chat inputs so that typing CJK characters via candidate selection no longer fights with SwiftUI's text bindings or accidentally submits mid-composition on Return.

  • OmiTextEditor.swift: Introduces an OmiNSTextView subclass that overrides setMarkedText, unmarkText, and insertText to publish composition state; guards textView.string = text in updateNSView with !textView.hasMarkedText() to prevent SwiftUI from clobbering the composing buffer; and short-circuits doCommandBy for Return while composition is active so the IME candidate is confirmed rather than the message submitted.
  • ChatInputView.swift / AskAIInputView.swift: Add a @State var hasMarkedText flag wired to the new onMarkedTextChange callback, using it to hide the placeholder text during composition — both files follow the same pattern consistently.

Confidence Score: 4/5

Safe to merge for the primary keyboard-Return IME fix; a secondary path (clicking the send arrow button mid-composition) is still unguarded and can produce an inconsistent input state.

The core logic preventing SwiftUI from overwriting the composing buffer and blocking Return from submitting during composition is correctly implemented and well-structured. The remaining gap is that the send button action and disabled predicate in both ChatInputView and AskAIInputView do not consult hasMarkedText, so a mouse click mid-composition sends intermediate text and leaves the text field in a state that temporarily repopulates after the IME session closes.

The send-button path in ChatInputView.swift (handleSubmit) and AskAIInputView.swift (the Button action closure) would benefit from a hasMarkedText guard.

Important Files Changed

Filename Overview
desktop/Desktop/Sources/Theme/OmiTextEditor.swift Core change: introduces OmiNSTextView subclass to hook setMarkedText/unmarkText/insertText, guards text binding writes and Return submission during composition. Logic is sound; minor edge case with send-button clicks mid-composition remains unguarded.
desktop/Desktop/Sources/MainWindow/Components/ChatInputView.swift Wires onMarkedTextChange callback and guards placeholder behind !hasMarkedText; send button and handleSubmit are not guarded against active composition, allowing a mouse-click send mid-IME.
desktop/Desktop/Sources/FloatingControlBar/AskAIInputView.swift Adds hasMarkedText state, wires callback, hides placeholder during composition. Same send-button gap as ChatInputView but slightly lower risk since it uses a local binding.

Sequence Diagram

sequenceDiagram
    participant User
    participant OmiNSTextView
    participant Coordinator
    participant SwiftUI View

    User->>OmiNSTextView: Begins IME composition
    OmiNSTextView->>OmiNSTextView: setMarkedText(_:selectedRange:replacementRange:)
    OmiNSTextView->>Coordinator: onMarkedTextStatusChange(true)
    Coordinator->>Coordinator: updateMarkedTextState(true)
    Coordinator-->>SwiftUI View: onMarkedTextChange(true) [async]
    SwiftUI View->>SwiftUI View: hasMarkedText = true (hide placeholder)

    User->>OmiNSTextView: Presses Return (confirm candidate)
    OmiNSTextView->>Coordinator: doCommandBy(insertNewline:)
    Coordinator->>Coordinator: "hasMarkedText() == true, return false"
    Note over OmiNSTextView: Default IME confirm behavior runs

    OmiNSTextView->>Coordinator: insertText (committed text)
    OmiNSTextView->>OmiNSTextView: publishMarkedTextStatus() false
    Coordinator->>Coordinator: updateMarkedTextState(false)
    Coordinator-->>SwiftUI View: onMarkedTextChange(false) [async]
    SwiftUI View->>SwiftUI View: hasMarkedText = false

    User->>OmiNSTextView: Presses Return (send message)
    OmiNSTextView->>Coordinator: doCommandBy(insertNewline:)
    Coordinator->>Coordinator: "hasMarkedText() == false, proceed"
    Coordinator->>SwiftUI View: onSubmit()
Loading

Comments Outside Diff (2)

  1. desktop/Desktop/Sources/MainWindow/Components/ChatInputView.swift, line 191-192 (link)

    P2 The send button's disabled state and handleSubmit do not check hasMarkedText, so a user can click the arrow button while the IME composition session is still active. This sends the intermediate composing text instead of the final candidate. Worse, handleSubmit then sets inputText = "", but the updateNSView guard (!textView.hasMarkedText()) prevents the text view from being cleared while composition is still in progress. When the user eventually confirms the IME candidate, textDidChange writes the now-committed text back into the (already-empty) binding, repopulating the input field with text that was supposed to have been sent and cleared.

  2. desktop/Desktop/Sources/FloatingControlBar/AskAIInputView.swift, line 81-85 (link)

    P2 Same send-button gap as in ChatInputView: the action closure and the disabled modifier both ignore hasMarkedText, so clicking the arrow while the IME session is open sends intermediate composing text. Adding a hasMarkedText guard here keeps both chat inputs consistent.

Reviews (1): Last reviewed commit: "fix(desktop): support IME composition in..." | Re-trigger Greptile

Comment on lines +274 to +281
func updateMarkedTextState(_ hasMarkedText: Bool) {
guard hasMarkedText != lastMarkedTextState else { return }
lastMarkedTextState = hasMarkedText
DispatchQueue.main.async { [weak self] in
guard self?.lastMarkedTextState == hasMarkedText else { return }
self?.onMarkedTextChange?(hasMarkedText)
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 updateMarkedTextState is called both from the OmiNSTextView override callbacks (via onMarkedTextStatusChange) and unconditionally at the end of every updateNSView. Since AppKit delegate methods and updateNSView all run on the main thread, the DispatchQueue.main.async adds a single-runloop-cycle delay to every state change. In practice the cancellation guard (self?.lastMarkedTextState == hasMarkedText) prevents stale deliveries, but the async hop means there is a brief window between setMarkedText and the next main-queue drain where lastMarkedTextState is already true while SwiftUI still sees hasMarkedText == false. Calling onMarkedTextChange directly on the main thread (without the async hop) would make the state transition atomic with respect to the AppKit text-change event.

Copy link
Copy Markdown
Collaborator

@kodjima33 kodjima33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks — IME composition fix looks good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants