Skip to content

fix(desktop): prevent Enter from submitting during IME composition - #37487

Closed
satotakumi wants to merge 1 commit into
NousResearch:mainfrom
satotakumi:fix/desktop-composer-ime-composition
Closed

fix(desktop): prevent Enter from submitting during IME composition#37487
satotakumi wants to merge 1 commit into
NousResearch:mainfrom
satotakumi:fix/desktop-composer-ime-composition

Conversation

@satotakumi

@satotakumi satotakumi commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Prevents the desktop app's Enter-to-submit branches from firing while an IME composition is active. Users typing in Japanese, Chinese, Korean, Vietnamese, or any other compositional input method were seeing their messages sent mid-conversion instead of committing the candidate.

The fix is a single !event.nativeEvent.isComposing guard added to every Enter handler in the desktop renderer that previously had none.

This is the right approach because:

  • event.nativeEvent.isComposing is the standard Chromium signal for active IME composition, exposed as a property on every KeyboardEvent regardless of IME implementation.
  • Referring to the native event (rather than the React synthetic isComposing) is safer across Electron's bundled Chromium versions.
  • The five affected sites share an identical one-line pattern, so bundling them into one PR keeps reviewable context together and avoids four future duplicate issues.

Related Issue

Fixes #37483

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

Added !event.nativeEvent.isComposing to the following Enter handlers:

  • apps/desktop/src/app/chat/composer/index.tsx — main composer submit (L592) and / @ trigger popover (L573)
  • apps/desktop/src/components/assistant-ui/thread.tsx — message edit submit (L1239) and trigger popover (L1213)
  • apps/desktop/src/components/desktop-onboarding-overlay.tsx — API key input (L486) and auth code input (L554)
  • apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx — session rename input (L227)

Each file is a one-line change; total diff is 4 files, 7 insertions, 7 deletions.

How to Test

Reproduction (before fix):

  1. Launch Hermes Desktop on macOS.
  2. Enable a Japanese IME (macOS "Japanese Input", ATOK, or Google Japanese Input).
  3. Type きょうは (pre-conversion kana) in the chat composer.
  4. The candidate list appears (e.g. "今日", "京は").
  5. Press Enter to commit the candidate.
  6. Observe: the commit fires and the message is sent immediately.

Verification (after fix):

  1. Repeat the same reproduction steps.
  2. Observe: Enter commits the selected candidate. The composer retains the committed text. No message is sent.
  3. A subsequent Enter (or a click on the send button) sends the message as expected — Enter-to-send behaviour is preserved once composition has ended.

Additional verification targets:

  • The / and @ trigger popovers still close on Enter/Tab and commit on click when composing is not active.
  • The message edit composer still submits on Enter after typing a revised message.
  • The session rename dialog still submits on Enter after typing a new title.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass — N/A (no Desktop-specific Python tests exist; Desktop is pure TypeScript/Electron)
  • I've added tests for my changes — Not added. jsdom does not model KeyboardEvent.isComposing reliably (it's a native Chromium property set by the IME subsystem), so a jsdom unit test here would not catch the real-world failure. Manual IME verification was preferred; I'm happy to add a jsdom test if maintainers want one.
  • I've tested on my platform: macOS 26.5 (darwin-arm64, Apple Silicon) with the built-in Japanese IME

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

N/A (this is a bug fix, not a new skill).

Screenshots / Logs

Not applicable. This is a pure input-handling fix — no visual change. The difference is observable only with an IME active and is verified by the behavioural steps in "How to Test" above.

Additional reviewer notes

  • Cross-platform note: isComposing is a standard KeyboardEvent property in Chromium, WebKit, and Firefox — Windows, Linux, and macOS all behave identically here. The fix doesn't introduce any platform-specific code.
  • Tab key: the trigger popover branches now also skip Tab during composition. This is a safety measure for symmetry; Tab is not a key users press while composing, but adding the guard keeps the Enter/Tab branches uniform.
  • Lower-priority sites: the onboarding overlay (API key / auth code input) and session rename are rarely hit by IME users in practice, but share the same pattern and are patched for completeness to prevent duplicate future issues.

Check event.nativeEvent.isComposing in Enter-to-submit branches so CJK (Japanese, Chinese, Korean) and other compositional input methods commit the candidate instead of firing the send handler.

Applied to all five sites in the desktop renderer that currently handle Enter with no composition guard:

- chat composer main submit and trigger popover (apps/desktop/src/app/chat/composer/index.tsx)
- message edit composer submit and trigger popover (apps/desktop/src/components/assistant-ui/thread.tsx)
- onboarding API key and auth code inputs (apps/desktop/src/components/desktop-onboarding-overlay.tsx)
- session rename input (apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx)

Fixes NousResearch#37483

@combatsheep combatsheep left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed the IME-composition guards across composer, edit, onboarding, and rename flows. The native check is applied consistently at every Enter/Tab submit path and the scope matches the reported bug. Approving.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have labels Jun 2, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Clean IME composition guard fix. Adds !event.nativeEvent.isComposing checks to Enter/Tab key handlers across 4 files (composer, session-actions, thread, onboarding). Standard pattern used throughout the web ecosystem. +7/-7 lines — minimal, targeted, correct.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused IME fix. The primary chat-composer behavior is already covered on current main, but the same bug class remains in refactored sibling paths.

Problems

  • The original thread.tsx and desktop-onboarding-overlay.tsx targets no longer exist. Their live replacements still submit without a composition guard: apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx:545,573, apps/desktop/src/components/onboarding/index.tsx:647,660, and apps/desktop/src/components/onboarding/flow.tsx:85.
  • The composer hunk is now redundant: apps/desktop/src/app/chat/composer/index.tsx:370 already returns for either composingRef.current or event.nativeEvent.isComposing via 40420a619.

Suggested changes

  • Port the remaining guards to the current paths, including the newer local-key input at components/onboarding/index.tsx:660.
  • Add focused coverage that composing Enter does not invoke each ported submit action.

Automated hermes-sweeper review.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for this fix! It was salvaged into #86760 (cherry-picked onto current main with your authorship preserved in the commit history) and is now merged. Closing since the work has landed.

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

Labels

P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Desktop app: Enter key sends message during IME composition (Japanese, Chinese, Korean, etc.)

5 participants