Skip to content

Fix desktop IME composition submit handling - #38794

Closed
wbcoder0 wants to merge 1 commit into
NousResearch:mainfrom
wbcoder0:fix/desktop-ime-enter
Closed

Fix desktop IME composition submit handling#38794
wbcoder0 wants to merge 1 commit into
NousResearch:mainfrom
wbcoder0:fix/desktop-ime-enter

Conversation

@wbcoder0

@wbcoder0 wbcoder0 commented Jun 4, 2026

Copy link
Copy Markdown

What changed

  • Add a shared desktop composer IME detection helper.
  • Prevent Enter from submitting while the main composer or edit composer is still composing IME text.
  • Sync finalized contentEditable text on compositionend.
  • Add focused tests for IME composition signals, including the process-key 229 fallback.

Why

Chinese/Japanese/Korean IMEs use Enter to confirm candidate text. The desktop composer could treat that Enter as a submit action, sending the prompt before the user had finalized the intended text.

Validation

  • npm -w apps/desktop run test:ui -- src/app/chat/composer/ime.test.ts src/app/chat/composer/slash-nav-dom-repro.test.tsx
  • npm -w apps/desktop run type-check

@wbcoder0
wbcoder0 marked this pull request as ready for review June 4, 2026 06:57
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jun 4, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of the IME-composition-Enter fix cluster for #37483. There are several competing open PRs implementing the same ime.ts (isComposing + legacy keyCode 229 + Process-key 229) guard in the desktop composer: #37996, #37757, #38785 (and earlier #37487/#38135/#38247). Maintainers should pick one canonical PR and close the rest.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for covering both composition signals and the edit-composer path.

Problems

  • The PR currently conflicts because its edit-composer change targets the former apps/desktop/src/components/assistant-ui/thread.tsx; current main moved that component to apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx (commits 7ff6908a5 and fa7bce078).
  • The remaining live defect is in apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx:527-577: unmodified Enter calls submitEdit() without a composition guard, and the editor at :627-652 has no composition handlers.
  • ime.test.ts checks the helper only; it does not cover that edit-composer submit path.

Suggested changes

  • Port the edit-composer guard and composition-end synchronization to thread/user-edit-composer.tsx, and add a DOM regression test proving an IME Enter does not submit or select a trigger item.
  • Preserve current main's existing main-composer behavior at apps/desktop/src/app/chat/composer/index.tsx:364-372 and :727-741; apply the 229 fallback to that current implementation rather than transplanting the stale snapshot.

Automated hermes-sweeper review.

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

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants