Skip to content

fix: prevent IME enter from sending chat#7845

Merged
zouyonghe merged 3 commits intoAstrBotDevs:masterfrom
zouyonghe:fix/chat-input-ime
Apr 27, 2026
Merged

fix: prevent IME enter from sending chat#7845
zouyonghe merged 3 commits intoAstrBotDevs:masterfrom
zouyonghe:fix/chat-input-ime

Conversation

@zouyonghe
Copy link
Copy Markdown
Member

@zouyonghe zouyonghe commented Apr 27, 2026

Summary

  • Prevent chat input Enter handling from sending messages while IME composition is active.
  • Track composition lifecycle on the textarea and keep existing Enter / Shift+Enter shortcut behavior unchanged.
  • Add a focused Node test for IME Enter detection.

Tests

  • node tests/imeInput.test.mjs
  • pnpm typecheck
  • pnpm build
  • uv run ruff format .
  • uv run ruff check .

Notes

  • pnpm lint currently fails on the baseline ESLint configuration because it cannot parse Vue, TypeScript, and MJS files.

Summary by Sourcery

Prevent chat messages from being sent when Enter is pressed during or immediately after IME composition in the chat input.

Bug Fixes:

  • Block Enter from sending chat messages while IME composition is active or just ended in the chat textarea.

Enhancements:

  • Track IME composition lifecycle for the chat input to preserve existing Enter and Shift+Enter shortcuts while avoiding accidental sends.
  • Extract IME-related Enter detection into a reusable utility helper.

Tests:

  • Add a dedicated Node-based test suite for IME Enter detection behavior.

@auto-assign auto-assign Bot requested review from Fridemn and Soulter April 27, 2026 09:38
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. feature:chatui The bug / feature is about astrbot's chatui, webchat labels Apr 27, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces IME (Input Method Editor) handling for the chat input component to prevent premature message submission during character composition. It adds a utility function isComposingEnter to detect active composition states, integrates this check into the ChatInput.vue keyboard handler via composition events, and includes unit tests for the new utility. Feedback suggests converting the utility to TypeScript for better type safety and omitting file extensions in imports to align with standard development practices.

Comment thread dashboard/src/utils/imeInput.mjs Outdated
Comment on lines +1 to +6
export function isComposingEnter(event, compositionActive) {
return (
event.key === "Enter" &&
(compositionActive || event.isComposing || event.keyCode === 229)
);
}
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.

medium

Since the project primarily uses TypeScript (as seen in ChatInput.vue), this utility should ideally be converted to a .ts file. This would allow for proper type annotations for the event and compositionActive parameters, ensuring type safety and better IDE support across the codebase. If you decide to keep it as a JavaScript file, consider adding JSDoc type hints to assist the TypeScript compiler.

import { useDisplay } from "vuetify";
import { useModuleI18n } from "@/i18n/composables";
import { useCustomizerStore } from "@/stores/customizer";
import { isComposingEnter } from "@/utils/imeInput.mjs";
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.

medium

In a TypeScript/Vite environment, it is standard practice to omit the file extension in import statements. This makes the code cleaner and more resilient to file type changes, such as converting the utility from .mjs to .ts.

import { isComposingEnter } from "@/utils/imeInput";

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • Consider also listening for compositioncancel (and possibly blur) on the textarea to ensure isComposing is always reset even if the IME session is aborted unexpectedly, so it doesn’t get stuck in the true state.
  • In isComposingEnter, you might explicitly type the event parameter (e.g., KeyboardEvent) and gate access to event.keyCode to clarify that this helper is only meant for keyboard events and to make future refactoring safer.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider also listening for `compositioncancel` (and possibly `blur`) on the textarea to ensure `isComposing` is always reset even if the IME session is aborted unexpectedly, so it doesn’t get stuck in the `true` state.
- In `isComposingEnter`, you might explicitly type the `event` parameter (e.g., `KeyboardEvent`) and gate access to `event.keyCode` to clarify that this helper is only meant for keyboard events and to make future refactoring safer.

## Individual Comments

### Comment 1
<location path="dashboard/src/components/chat/ChatInput.vue" line_range="115-119" />
<code_context>
         ref="inputField"
         v-model="localPrompt"
         @keydown="handleKeyDown"
+        @compositionstart="handleCompositionStart"
+        @compositionend="handleCompositionEnd"
         :disabled="disabled"
</code_context>
<issue_to_address>
**suggestion:** Consider resetting `isComposing` on blur or unmount to avoid a stuck composing state.

Because `isComposing` is only updated on `compositionstart`/`compositionend`, losing focus or unmounting mid-composition can leave it stuck at `true`, so `handleKeyDown` will keep early-returning on Enter. Adding a reset on blur or component teardown would guard against these cases.

Suggested implementation:

```
        @keydown="handleKeyDown"
        @compositionstart="handleCompositionStart"
        @compositionend="handleCompositionEnd"
        @blur="handleBlur"
        :disabled="disabled"

```

To fully implement the suggestion, you should also:

1. Add a blur handler that resets composing state, colocated with the existing composition handlers:

```ts
const handleCompositionStart = () => {
  isComposing.value = true;
};

const handleCompositionEnd = () => {
  isComposing.value = false;
};

const handleBlur = () => {
  // Reset composing state in case the user blurs mid-composition
  isComposing.value = false;
};
```

2. Ensure you reset `isComposing` on component teardown. If you're using the Composition API, extend your existing Vue imports to include `onBeforeUnmount` and add:

```ts
onBeforeUnmount(() => {
  isComposing.value = false;
});
```

3. If you already have a `vue` import like `import { ref, computed, ... } from "vue";`, add `onBeforeUnmount` to that import rather than creating a new one.

These changes ensure `handleKeyDown` will not be stuck early-returning when Enter is pressed after a blur or unmount that happens mid-composition.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread dashboard/src/components/chat/ChatInput.vue
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Apr 27, 2026
@zouyonghe
Copy link
Copy Markdown
Member Author

@sourcery-ai review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • The resetComposingState and clearCompositionState helpers both reset isComposing, which introduces a bit of duplication; consider consolidating these into a single function to simplify the composition state lifecycle.
  • In isComposingEnter, the 100ms threshold is a magic number; pulling this out into a named constant (and perhaps a brief comment) would make the intent of this timing window clearer and easier to tune in the future.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `resetComposingState` and `clearCompositionState` helpers both reset `isComposing`, which introduces a bit of duplication; consider consolidating these into a single function to simplify the composition state lifecycle.
- In `isComposingEnter`, the 100ms threshold is a magic number; pulling this out into a named constant (and perhaps a brief comment) would make the intent of this timing window clearer and easier to tune in the future.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@zouyonghe
Copy link
Copy Markdown
Member Author

@sourcery-ai review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@zouyonghe zouyonghe merged commit 67c7445 into AstrBotDevs:master Apr 27, 2026
21 checks passed
LIghtJUNction pushed a commit that referenced this pull request Apr 28, 2026
* fix: prevent IME enter from sending chat

* fix: prevent IME enter from sending chat

* refactor: clarify IME composition state handling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature:chatui The bug / feature is about astrbot's chatui, webchat size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant