fix(insertion): IME-safe accept so Japanese/CJK suggestions land on Tab#668
Merged
Conversation
…d on Tab With a composing input method active (Japanese kana, Chinese pinyin, Korean hangul), the synthetic Unicode keystroke that commits an accepted suggestion is re-absorbed into composition by the IME instead of landing as literal text. The session then desyncs against the live field and the suggestion clears and regenerates, which Japanese users reported as Tab consuming the suggestion word by word while inserting nothing. Detect composing input sources via Text Input Sources (TIS), cached and refreshed on the input-source-changed distributed notification, with the classification kept in a pure helper. While one is active, route the commit through the pasteboard and trigger the paste by AXPress on the host's real Edit > Paste menu item, located language-independently by its Cmd-V key equivalent and cached per process. No key event exists on that path, so the IME can neither swallow nor re-interpret the commit. Synthetic Cmd-V remains as a fallback for hosts without a Paste menu item, posted with a session event source at the annotated session tap so its Command flag survives (the HID tap merges flags with the live hardware state, and during a Tab accept only Tab is physically down; observed on-device as 12/12 pastes traversing the tap chain with nothing landing). The user's clipboard is snapshotted and restored around the paste, and the observer tap now also recognizes synthetic events by identity so a raced suppression token cannot let our own fallback Cmd-V classify as a shortcut mutation and tear down the session it is committing.
Make KeyboardInputSourceMonitor.refresh() private (the notification path is the only caller), apply the short poll messaging timeout to every element the paste-menu walk messages so a stalled host cannot hold the main actor for the multi-second AX default, and make the missing-modifiers rejection in isCommandVMenuItem an explicit guard instead of relying on nil == 0.
4fec121 to
2ec2fd9
Compare
Owner
Author
|
Addressed all three review findings in 2ec2fd9: refresh() is now private, the paste-menu walk applies the 50ms poll timeout to every element it messages (menu bar, top-level items, menus, and items), and the missing-modifiers case is an explicit guard. Local validation on the rebased branch: xcodegen no drift, swiftlint clean, build-for-testing succeeded, full unit bundle 982 tests / 0 failures. |
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
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.
Summary
With a composing input method active (Japanese kana, Chinese pinyin, Korean hangul), the synthetic Unicode keystroke that commits an accepted suggestion is re-absorbed into the IME's composition instead of landing as literal text, so Tab consumed the suggestion word by word while inserting nothing and the session reset on the next AX poll. This detects composing input sources via Text Input Sources and commits accepts through the pasteboard instead, triggered by an AXPress on the host's real Edit > Paste menu item (located language-independently by its Cmd-V key equivalent, cached per process), so no key event exists for the input method to swallow.
Validation
kAXSelectedTextAttributewrite (Chromium reports.successbut silently no-ops) and synthetic Cmd-V at the HID tap (the Command flag is merged away against the physically held Tab; 12/12 pastes traversed the tap chain with nothing landing).xcodebuild build-for-testing ... CODE_SIGNING_ALLOWED=NOthentest-without-buildingforCompositionInputModeClassifierTests+SuggestionSessionReconcilerTests: ** TEST SUCCEEDED **, 80 tests, 0 failures.swiftlint lint --quieton all changed files: exit 0, no findings.Linked issues
None filed; reported by a Japanese user on Reddit against v0.5.1-beta (suggestions intermittent, Tab accepts vanish without inserting). Happy to link a tracking issue if we open one.
Risk / rollout notes
com.apple.inputmethod.Roman) is treated as composing, so unknown third-party IMEs route through the paste path rather than the swallowable keystroke path. Latin keyboard users see zero behavior change.cotabbyinferencerevision withsetComputeLogprob(Cut inference and AX hot-path waste: P-core threads, gated logprob, halved KV, geometry cache #667) that is not yet pushed to the package's GitHub main, so the fresh-resolvePackageDependenciesin CI fails on main and on this PR alike until the package commit is pushed.Greptile Summary
Introduces an IME-safe suggestion acceptance path for Japanese, Chinese, Korean, and other composing input methods, fixing a bug where Tab-accepted completions were silently re-absorbed by the input method instead of landing as text.
KeyboardInputSourceMonitorsubscribes tokTISNotifySelectedKeyboardInputSourceChangedand caches the result ofCompositionInputModeClassifier, which conservatively treats any non-keyboard-layout, non-Roman-eisu source as composing — so unknown third-party IMEs (ATOK, Sogou) default to the safe path rather than the broken keystroke path.SuggestionInserter.insertnow checks the provider and routes composing sessions throughinsertViaPaste, which prefers pressing the host's realEdit > Pastemenu item viaAXPress(language-independently located by Cmd-V key equivalent, cached per PID) over a synthetic Cmd-V, since no key event exists for the IME to intercept.keyDownhandler now also checksisSynthetic(event)as a backstop, preventing the synthetic Cmd-V from being misclassified as a shortcut mutation when a real keystroke races to consume the countdown token.Confidence Score: 5/5
Safe to merge. Latin keyboard users follow the unchanged code path; the composing-IME path falls back gracefully through menu press → synthetic Cmd-V → keystroke, restoring the clipboard on every failure branch.
The classification rule is pure and fully unit-tested. The AX menu walk is bounded (depth-1 only, early exit on the first Cmd-V item) and every element handle receives a 50 ms messaging timeout before being queried, capping per-accept latency. Clipboard snapshot/restore is guarded against overlapping accepts and user copies. The suppression backstop in the observer tap correctly prevents the synthetic Cmd-V from being misidentified as a user shortcut. No correctness issues were found across all changed files.
No files require special attention.
Important Files Changed
Sequence Diagram
sequenceDiagram participant User as User (Tab key) participant IM as InputMonitor (accept tap) participant SI as SuggestionInserter participant KIM as KeyboardInputSourceMonitor participant AX as AXHelper participant App as Host App User->>IM: Tab keyDown IM->>SI: insert(suggestion) SI->>KIM: isComposingIMEActiveProvider() KIM-->>SI: true (Japanese IME active) SI->>SI: insertViaPaste(text) SI->>SI: snapshotPasteboard() SI->>SI: pasteboard.setString(text) SI->>AX: pasteMenuItem(forApplicationPID:) Note over AX: Walk menuBar→topLevelItem→menu→item Note over AX: Match Cmd-V by key char+modifiers AX-->>SI: "AXUIElement (Edit > Paste)" SI->>App: AXUIElementPerformAction(kAXPressAction) App->>App: Execute paste command (no key event) App-->>SI: .success Note over SI: Cache menu item per pid SI->>SI: Schedule clipboard restore (300ms) SI-->>IM: true (inserted) Note over SI,App: 300ms later SI->>SI: restorePasteboard (if changeCount unchanged)Comments Outside Diff (1)
Cotabby/Services/Suggestion/SuggestionInserter.swift, line 252-261 (link)DispatchWorkItemrestore closure weak-self bookkeepingThe closure calls
self?.clearPendingPasteboardRestore()via a weak capture. Ifselfis released before the 300 ms fires,pendingPasteboardRestoreandsavedClipboardForRestoreare never nilled, leaving stale in-flight bookkeeping. The clipboard itself is still restored correctly (savedis captured by value). In practiceSuggestionInserterlives for the app's lifetime so this is currently harmless, but worth noting for maintenance.Reviews (2): Last reviewed commit: "Address Greptile review on #668" | Re-trigger Greptile