From 9b1c6724b3b55101350a30fea5ed1cd905fd7b13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20C=C3=B4ng=20Thu=E1=BA=ADn=20Huy?= Date: Sat, 24 Oct 2026 11:35:30 +0700 Subject: [PATCH 1/2] Fix stale props causing corrupted text during IME batched input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vietnamese IMEs (Unikey, EVKey) send backspace + accented char in a single stdin chunk. OpenTUI splits this into separate key events, but React batches state updates, so handleDeletionKeys read stale value/cursorPosition props while insertTextAtCursor already read synchronous refs. Result: 'chào' rendered as 'chaà' — backspace deleted from the pre-batch string, never removing the base character. All keyboard handlers (deletion, enter, navigation, mouse, cursor movement) now read from valueRef.current/cursorPositionRef.current and update refs before calling onChange, matching the pattern insertTextAtCursor already used. Removes value/cursorPosition from useCallback dependency arrays. --- .../__tests__/multiline-input.test.tsx | 142 ++++++++++++++++++ cli/src/components/multiline-input.tsx | 139 +++++++++++------ 2 files changed, 239 insertions(+), 42 deletions(-) diff --git a/cli/src/components/__tests__/multiline-input.test.tsx b/cli/src/components/__tests__/multiline-input.test.tsx index 7fcf7eaa17..6f994713fc 100644 --- a/cli/src/components/__tests__/multiline-input.test.tsx +++ b/cli/src/components/__tests__/multiline-input.test.tsx @@ -1130,3 +1130,145 @@ describe('MultilineInput - newline keyboard shortcuts', () => { expect(isAltModifier({ option: false })).toBe(false) }) }) + +/** + * Tests for Vietnamese IME compose-and-replace input handling. + * + * Vietnamese IMEs (Unikey, EVKey) replace the base char with the accented + * version by sending two events per diacritic keystroke: + * 1. backspace ('\x7f') — delete the base char + * 2. composed char ('à') — insert the accented version + * + * These arrive in a single stdin chunk and fire as separate key events within + * one React batch. The handlers must read from synchronous refs (not stale + * props) so that event N+1 sees the result of event N. + * + * Without ref-based reads, backspace operates on the stale prop value (before + * the batch started), so the base char is never removed — producing "chaà" + * instead of "chà". + */ +describe('MultilineInput - Vietnamese IME compose-and-replace', () => { + /** + * Simulates the ref-based state pattern from multiline-input.tsx. + * onChange callbacks are deferred (batched), refs are updated synchronously. + */ + function createBatchedState(initial: string, initialCursor: number) { + let state = { text: initial, cursorPosition: initialCursor } + const valueRef = { current: initial } + const cursorRef = { current: initialCursor } + const pendingChanges: Array<{ text: string; cursorPosition: number }> = [] + + const onChange = (v: { text: string; cursorPosition: number }) => { + pendingChanges.push(v) + // Simulate React batching: state does NOT update until flush + } + + const flush = () => { + const last = pendingChanges[pendingChanges.length - 1] + if (last) state = last + pendingChanges.length = 0 + // Simulate React re-render: refs refreshed from props + valueRef.current = state.text + cursorRef.current = state.cursorPosition + } + + // Mirrors insertTextAtCursor: reads refs, updates refs before onChange + const insertText = (text: string) => { + const v = valueRef.current + const c = cursorRef.current + const newText = v.slice(0, c) + text + v.slice(c) + valueRef.current = newText + cursorRef.current = c + text.length + onChange({ text: newText, cursorPosition: c + text.length }) + } + + // Mirrors handleDeletionKeys backspace (fixed version): + // reads refs, updates refs before onChange + const backspace = () => { + const v = valueRef.current + const c = cursorRef.current + if (c > 0) { + const newText = v.slice(0, c - 1) + v.slice(c) + valueRef.current = newText + cursorRef.current = c - 1 + onChange({ text: newText, cursorPosition: c - 1 }) + } + } + + // Simulates the OLD buggy backspace: reads from a stale snapshot (props) + // instead of refs. Only used to verify the regression. + const staleBackspace = (staleText: string, staleCursor: number) => { + if (staleCursor > 0) { + const newText = staleText.slice(0, staleCursor - 1) + staleText.slice(staleCursor) + onChange({ text: newText, cursorPosition: staleCursor - 1 }) + // Note: valueRef and cursorRef are NOT updated (old code path) + } + } + + const getState = () => state + return { insertText, backspace, staleBackspace, flush, getState, valueRef, cursorRef } + } + + test('IME DEL+à after "cha" produces "chà" (ref-based backspace)', () => { + const s = createBatchedState('', 0) + + for (const ch of 'cha') s.insertText(ch) + + // IME compose-and-replace: DEL, then 'à' — both in one batch + s.backspace() + s.insertText('à') + + s.flush() + + expect(s.getState().text).toBe('chà') + expect(s.getState().cursorPosition).toBe('chà'.length) + }) + + test('IME DEL+à after "xin cha" produces "xin chà"', () => { + const s = createBatchedState('', 0) + + for (const ch of 'xin cha') s.insertText(ch) + + s.backspace() + s.insertText('à') + + s.flush() + + expect(s.getState().text).toBe('xin chà') + }) + + test('stale backspace produces "chaà" (regression guard)', () => { + const s = createBatchedState('', 0) + + for (const ch of 'cha') s.insertText(ch) + + // Simulate OLD bug: backspace reads stale props (text='cha', cursor=3) + // instead of refs. After the batch, refs are 'cha'/3 (no prior event). + s.staleBackspace('cha', 3) + // refs NOT updated — still 'cha'/3 + s.insertText('à') + + s.flush() + + // Stale backspace deleted from 'cha' → 'ch', but valueRef still 'cha', + // so 'à' inserted at cursor 3 → 'chaà' + expect(s.getState().text).toBe('chaà') + }) + + test('multiple IME diacritics across batches', () => { + const s = createBatchedState('', 0) + + // Type "cha" then accent to "chà" + for (const ch of 'cha') s.insertText(ch) + s.backspace() + s.insertText('à') + s.flush() + + // Continue: "o" → "chào" + s.insertText('o') + s.flush() + + expect(s.getState().text).toBe('chào') + expect(s.getState().cursorPosition).toBe('chào'.length) + }) +}) diff --git a/cli/src/components/multiline-input.tsx b/cli/src/components/multiline-input.tsx index 532a600854..0d3cf85532 100644 --- a/cli/src/components/multiline-input.tsx +++ b/cli/src/components/multiline-input.tsx @@ -253,17 +253,18 @@ export const MultilineInput = forwardRef< if (stickyColumnRef.current != null) { return stickyColumnRef.current } + const cursorPos = cursorPositionRef.current const lineIndex = lineStarts.findLastIndex( - (lineStart) => lineStart <= cursorPosition, + (lineStart) => lineStart <= cursorPos, ) const column = lineIndex === -1 ? 0 - : cursorPosition - lineStarts[lineIndex] + (cursorIsChar ? -1 : 0) + : cursorPos - lineStarts[lineIndex] + (cursorIsChar ? -1 : 0) stickyColumnRef.current = Math.max(0, column) return stickyColumnRef.current }, - [cursorPosition], + [], ) // Update last activity on value or cursor changes @@ -309,7 +310,7 @@ export const MultilineInput = forwardRef< ? Math.max( 0, lineInfo.lineStartCols.findLastIndex( - (lineStart) => lineStart <= cursorPosition, + (lineStart) => lineStart <= cursorPositionRef.current, ), ) : 0 @@ -338,12 +339,13 @@ export const MultilineInput = forwardRef< if (!selection) return null // Convert from render positions to original text positions - const start = renderPositionToOriginal(value, Math.min(selection.start, selection.end)) - const end = renderPositionToOriginal(value, Math.max(selection.start, selection.end)) + const currentValue = valueRef.current + const start = renderPositionToOriginal(currentValue, Math.min(selection.start, selection.end)) + const end = renderPositionToOriginal(currentValue, Math.max(selection.start, selection.end)) if (start === end) return null return { start, end } - }, [value]) + }, []) // Helper to clear the current selection const clearSelection = useCallback(() => { @@ -356,16 +358,19 @@ export const MultilineInput = forwardRef< const selection = getSelectionRange() if (!selection) return null - const newValue = value.slice(0, selection.start) + value.slice(selection.end) + const currentValue = valueRef.current + const newValue = currentValue.slice(0, selection.start) + currentValue.slice(selection.end) clearSelection() return { newValue, newCursor: selection.start } - }, [value, getSelectionRange, clearSelection]) + }, [getSelectionRange, clearSelection]) // Helper to handle selection deletion and call onChange if selection existed // Returns true if selection was deleted, false otherwise const handleSelectionDeletion = useCallback((): boolean => { const deleted = deleteSelection() if (deleted) { + valueRef.current = deleted.newValue + cursorPositionRef.current = deleted.newCursor onChange({ text: deleted.newValue, cursorPosition: deleted.newCursor, @@ -432,15 +437,18 @@ export const MultilineInput = forwardRef< const moveCursor = useCallback( (nextPosition: number) => { - const clamped = clamp(nextPosition, 0, value.length) - if (clamped === cursorPosition) return + const currentValue = valueRef.current + const currentCursor = cursorPositionRef.current + const clamped = clamp(nextPosition, 0, currentValue.length) + if (clamped === currentCursor) return + cursorPositionRef.current = clamped onChange({ - text: value, + text: currentValue, cursorPosition: clamped, lastEditDueToNav: false, }) }, - [cursorPosition, onChange, value], + [onChange], ) // Handle mouse clicks to position cursor @@ -454,6 +462,9 @@ export const MultilineInput = forwardRef< const scrollBox = scrollBoxRef.current if (!scrollBox) return + const currentValue = valueRef.current + const currentCursor = cursorPositionRef.current + const lineStarts = lineInfo?.lineStartCols ?? [0] const viewport = (scrollBox as any).viewport @@ -473,7 +484,7 @@ export const MultilineInput = forwardRef< // Get the character range for this line const lineStartChar = lineStarts[lineIndex] - const lineEndChar = lineStarts[lineIndex + 1] ?? value.length + const lineEndChar = lineStarts[lineIndex + 1] ?? currentValue.length // Convert click x to character position, accounting for tabs const clickCol = Math.max(0, Math.floor(event.x - viewportLeft)) @@ -482,7 +493,7 @@ export const MultilineInput = forwardRef< let charIndex = lineStartChar while (charIndex < lineEndChar && visualCol < clickCol) { - const char = value[charIndex] + const char = currentValue[charIndex] if (char === '\t') { visualCol += TAB_WIDTH } else if (char === '\n') { @@ -494,18 +505,19 @@ export const MultilineInput = forwardRef< } // Clamp to valid range - const newCursorPosition = Math.min(charIndex, value.length) + const newCursorPosition = Math.min(charIndex, currentValue.length) // Update cursor position if changed - if (newCursorPosition !== cursorPosition) { + if (newCursorPosition !== currentCursor) { + cursorPositionRef.current = newCursorPosition onChange({ - text: value, + text: currentValue, cursorPosition: newCursorPosition, lastEditDueToNav: false, }) } }, - [focused, lineInfo, value, cursorPosition, onChange], + [focused, lineInfo, onChange], ) const isPlaceholder = value.length === 0 && placeholder.length > 0 @@ -576,6 +588,9 @@ export const MultilineInput = forwardRef< // Only handle Enter and Ctrl+J here if (!isEnterKey && !isCtrlJ) return false + const currentValue = valueRef.current + const currentCursor = cursorPositionRef.current + const isAltLikeModifier = isAltModifier(key) // Kitty-protocol keyboards encode plain Enter as an escape sequence // (e.g. \x1b[13u) but report modifiers reliably, so the escape-prefix @@ -588,7 +603,7 @@ export const MultilineInput = forwardRef< key.sequence.length > 0 && key.sequence.charCodeAt(0) === 0x1b const hasBackslashBeforeCursor = - cursorPosition > 0 && value[cursorPosition - 1] === '\\' + currentCursor > 0 && currentValue[currentCursor - 1] === '\\' // Plain Enter: no modifiers, sequence is '\r' (macOS) or '\n' (Linux) const isPlainEnter = @@ -618,12 +633,14 @@ export const MultilineInput = forwardRef< // For backslash+Enter, remove the backslash and insert newline if (isBackslashEnter) { const newValue = - value.slice(0, cursorPosition - 1) + + currentValue.slice(0, currentCursor - 1) + '\n' + - value.slice(cursorPosition) + currentValue.slice(currentCursor) + valueRef.current = newValue + cursorPositionRef.current = currentCursor onChange({ text: newValue, - cursorPosition, + cursorPosition: currentCursor, lastEditDueToNav: false, }) return true @@ -631,10 +648,12 @@ export const MultilineInput = forwardRef< // For other newline shortcuts (Shift+Enter, Option+Enter, Ctrl+J), just insert newline const newValue = - value.slice(0, cursorPosition) + '\n' + value.slice(cursorPosition) + currentValue.slice(0, currentCursor) + '\n' + currentValue.slice(currentCursor) + valueRef.current = newValue + cursorPositionRef.current = currentCursor + 1 onChange({ text: newValue, - cursorPosition: cursorPosition + 1, + cursorPosition: currentCursor + 1, lastEditDueToNav: false, }) return true @@ -648,7 +667,7 @@ export const MultilineInput = forwardRef< return false }, - [value, cursorPosition, onChange, onSubmit], + [onChange, onSubmit], ) // Handle deletion keys (backspace, delete, ctrl+h, ctrl+d, word/line deletion) @@ -656,6 +675,8 @@ export const MultilineInput = forwardRef< (key: KeyEvent): boolean => { const lowerKeyName = (key.name ?? '').toLowerCase() const isAltLikeModifier = isAltModifier(key) + const value = valueRef.current + const cursorPosition = cursorPositionRef.current const lineStart = findLineStart(value, cursorPosition) const lineEnd = findLineEnd(value, cursorPosition) const wordStart = findPreviousWordBoundary(value, cursorPosition) @@ -670,6 +691,8 @@ export const MultilineInput = forwardRef< if (cursorPosition > visualLineStart) { const newValue = value.slice(0, visualLineStart) + value.slice(cursorPosition) + valueRef.current = newValue + cursorPositionRef.current = visualLineStart onChange({ text: newValue, cursorPosition: visualLineStart, @@ -678,6 +701,8 @@ export const MultilineInput = forwardRef< } else if (cursorPosition > 0) { const newValue = value.slice(0, cursorPosition - 1) + value.slice(cursorPosition) + valueRef.current = newValue + cursorPositionRef.current = cursorPosition - 1 onChange({ text: newValue, cursorPosition: cursorPosition - 1, @@ -696,6 +721,8 @@ export const MultilineInput = forwardRef< if (handleSelectionDeletion()) return true const newValue = value.slice(0, wordStart) + value.slice(cursorPosition) + valueRef.current = newValue + cursorPositionRef.current = wordStart onChange({ text: newValue, cursorPosition: wordStart, @@ -733,6 +760,8 @@ export const MultilineInput = forwardRef< } if (newValue !== originalValue) { + valueRef.current = newValue + cursorPositionRef.current = nextCursor onChange({ text: newValue, cursorPosition: nextCursor, @@ -747,6 +776,8 @@ export const MultilineInput = forwardRef< preventKeyDefault(key) if (handleSelectionDeletion()) return true const newValue = value.slice(0, cursorPosition) + value.slice(wordEnd) + valueRef.current = newValue + cursorPositionRef.current = cursorPosition onChange({ text: newValue, cursorPosition, @@ -760,6 +791,8 @@ export const MultilineInput = forwardRef< preventKeyDefault(key) if (handleSelectionDeletion()) return true const newValue = value.slice(0, cursorPosition) + value.slice(lineEnd) + valueRef.current = newValue + cursorPositionRef.current = cursorPosition onChange({ text: newValue, cursorPosition, lastEditDueToNav: false }) return true } @@ -771,6 +804,8 @@ export const MultilineInput = forwardRef< if (cursorPosition > 0) { const newValue = value.slice(0, cursorPosition - 1) + value.slice(cursorPosition) + valueRef.current = newValue + cursorPositionRef.current = cursorPosition - 1 onChange({ text: newValue, cursorPosition: cursorPosition - 1, @@ -787,6 +822,8 @@ export const MultilineInput = forwardRef< if (cursorPosition < value.length) { const newValue = value.slice(0, cursorPosition) + value.slice(cursorPosition + 1) + valueRef.current = newValue + cursorPositionRef.current = cursorPosition onChange({ text: newValue, cursorPosition, @@ -803,6 +840,8 @@ export const MultilineInput = forwardRef< if (cursorPosition > 0) { const newValue = value.slice(0, cursorPosition - 1) + value.slice(cursorPosition) + valueRef.current = newValue + cursorPositionRef.current = cursorPosition - 1 onChange({ text: newValue, cursorPosition: cursorPosition - 1, @@ -819,6 +858,8 @@ export const MultilineInput = forwardRef< if (cursorPosition < value.length) { const newValue = value.slice(0, cursorPosition) + value.slice(cursorPosition + 1) + valueRef.current = newValue + cursorPositionRef.current = cursorPosition onChange({ text: newValue, cursorPosition, @@ -830,7 +871,7 @@ export const MultilineInput = forwardRef< return false }, - [value, cursorPosition, onChange, lineInfo, cursorRow, handleSelectionDeletion], + [onChange, lineInfo, cursorRow, handleSelectionDeletion], ) // Handle navigation keys (arrows, home, end, word navigation, emacs bindings) @@ -838,6 +879,8 @@ export const MultilineInput = forwardRef< (key: KeyEvent): boolean => { const lowerKeyName = (key.name ?? '').toLowerCase() const isAltLikeModifier = isAltModifier(key) + const value = valueRef.current + const cursorPosition = cursorPositionRef.current const logicalLineStart = findLineStart(value, cursorPosition) const logicalLineEnd = findLineEnd(value, cursorPosition) const wordStart = findPreviousWordBoundary(value, cursorPosition) @@ -867,6 +910,7 @@ export const MultilineInput = forwardRef< (key.name === 'left' || lowerKeyName === 'b') ) { preventKeyDefault(key) + cursorPositionRef.current = wordStart onChange({ text: value, cursorPosition: wordStart, @@ -881,6 +925,7 @@ export const MultilineInput = forwardRef< (key.name === 'right' || lowerKeyName === 'f') ) { preventKeyDefault(key) + cursorPositionRef.current = wordEnd onChange({ text: value, cursorPosition: wordEnd, @@ -896,6 +941,7 @@ export const MultilineInput = forwardRef< (key.name === 'home' && !key.ctrl && !key.meta) ) { preventKeyDefault(key) + cursorPositionRef.current = visualLineStart onChange({ text: value, cursorPosition: visualLineStart, @@ -911,6 +957,7 @@ export const MultilineInput = forwardRef< (key.name === 'end' && !key.ctrl && !key.meta) ) { preventKeyDefault(key) + cursorPositionRef.current = visualLineEnd onChange({ text: value, cursorPosition: visualLineEnd, @@ -925,6 +972,7 @@ export const MultilineInput = forwardRef< (key.ctrl && key.name === 'home') ) { preventKeyDefault(key) + cursorPositionRef.current = 0 onChange({ text: value, cursorPosition: 0, lastEditDueToNav: false }) return true } @@ -935,6 +983,7 @@ export const MultilineInput = forwardRef< (key.ctrl && key.name === 'end') ) { preventKeyDefault(key) + cursorPositionRef.current = value.length onChange({ text: value, cursorPosition: value.length, @@ -946,6 +995,7 @@ export const MultilineInput = forwardRef< // Ctrl+B: Backward char (Emacs) if (key.ctrl && lowerKeyName === 'b' && !key.meta && !key.option) { preventKeyDefault(key) + cursorPositionRef.current = cursorPosition - 1 onChange({ text: value, cursorPosition: cursorPosition - 1, @@ -957,6 +1007,7 @@ export const MultilineInput = forwardRef< // Ctrl+F: Forward char (Emacs) if (key.ctrl && lowerKeyName === 'f' && !key.meta && !key.option) { preventKeyDefault(key) + cursorPositionRef.current = Math.min(value.length, cursorPosition + 1) onChange({ text: value, cursorPosition: Math.min(value.length, cursorPosition + 1), @@ -983,15 +1034,17 @@ export const MultilineInput = forwardRef< if (key.name === 'up' && !key.ctrl && !key.meta && !key.option) { preventKeyDefault(key) const desiredIndex = getOrSetStickyColumn(lineStarts, !shouldHighlight) + const nextCursor = calculateNewCursorPosition({ + cursorPosition, + lineStarts, + cursorIsChar: !shouldHighlight, + direction: 'up', + desiredIndex, + }) + cursorPositionRef.current = nextCursor onChange({ text: value, - cursorPosition: calculateNewCursorPosition({ - cursorPosition, - lineStarts, - cursorIsChar: !shouldHighlight, - direction: 'up', - desiredIndex, - }), + cursorPosition: nextCursor, lastEditDueToNav: false, }) return true @@ -1001,15 +1054,17 @@ export const MultilineInput = forwardRef< if (key.name === 'down' && !key.ctrl && !key.meta && !key.option) { preventKeyDefault(key) const desiredIndex = getOrSetStickyColumn(lineStarts, !shouldHighlight) + const nextCursor = calculateNewCursorPosition({ + cursorPosition, + lineStarts, + cursorIsChar: !shouldHighlight, + direction: 'down', + desiredIndex, + }) + cursorPositionRef.current = nextCursor onChange({ text: value, - cursorPosition: calculateNewCursorPosition({ - cursorPosition, - lineStarts, - cursorIsChar: !shouldHighlight, - direction: 'down', - desiredIndex, - }), + cursorPosition: nextCursor, lastEditDueToNav: false, }) return true @@ -1017,7 +1072,7 @@ export const MultilineInput = forwardRef< return false }, - [value, cursorPosition, onChange, moveCursor, shouldHighlight, getOrSetStickyColumn], + [onChange, moveCursor, shouldHighlight, getOrSetStickyColumn], ) // Handle character input (regular chars, tab, and IME/multi-byte input) From c703b801e6ae33299abf5df2021f4b6a3e57c6a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20C=C3=B4ng=20Thu=E1=BA=ADn=20Huy?= Date: Sun, 25 Oct 2026 14:44:08 +0700 Subject: [PATCH 2/2] Replace reimplementation tests with integration tests driving real handlers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback: the four Vietnamese IME tests used a hand-rolled reimplementation of the ref/batch pattern instead of exercising the actual MultilineInput code paths. They would pass even if the fix were wrong or reverted. New multiline-input.ime.test.tsx uses createTestRenderer + mockInput (the same harness as queue-editing.test.tsx) to mount MultilineInput and drive real key events through handleDeletionKeys and insertTextAtCursor. Verified the tests fail when the ref update in the backspace handler is removed (produces 'chaà' as expected). Removed the four reimplementation tests from multiline-input.test.tsx. --- .../__tests__/multiline-input.ime.test.tsx | 156 ++++++++++++++++++ .../__tests__/multiline-input.test.tsx | 142 ---------------- 2 files changed, 156 insertions(+), 142 deletions(-) create mode 100644 cli/src/components/__tests__/multiline-input.ime.test.tsx diff --git a/cli/src/components/__tests__/multiline-input.ime.test.tsx b/cli/src/components/__tests__/multiline-input.ime.test.tsx new file mode 100644 index 0000000000..40859646c7 --- /dev/null +++ b/cli/src/components/__tests__/multiline-input.ime.test.tsx @@ -0,0 +1,156 @@ +import { afterEach, beforeAll, describe, expect, test } from 'bun:test' +import { createTestRenderer } from '@opentui/core/testing' +import { createRoot, flushSync } from '@opentui/react' +import React from 'react' + +import { MultilineInput } from '../multiline-input' +import { initializeThemeStore } from '../../hooks/use-theme' + +import type { InputValue } from '../../types/store' + +let cleanupRenderer: (() => void) | undefined + +beforeAll(() => { + initializeThemeStore() +}) + +afterEach(() => { + cleanupRenderer?.() + cleanupRenderer = undefined +}) + +/** + * Mounts MultilineInput with controlled state and returns helpers to + * drive real key events through the component's actual handlers. + */ +async function mountInput(initialValue = '', initialCursor = 0) { + const changes: InputValue[] = [] + + const Harness = () => { + const [value, setValue] = React.useState(initialValue) + const [cursorPosition, setCursorPosition] = React.useState(initialCursor) + + const handleChange = React.useCallback((v: InputValue) => { + changes.push(v) + setValue(v.text) + setCursorPosition(v.cursorPosition) + }, []) + + return ( + {}} + onPaste={() => {}} + focused + maxHeight={5} + /> + ) + } + + const setup = await createTestRenderer({ width: 80, height: 6 }) + const root = createRoot(setup.renderer) + cleanupRenderer = () => { + flushSync(() => root.unmount()) + setup.renderer.destroy() + } + + flushSync(() => root.render()) + await setup.renderOnce() + + const settle = async () => { + await setup.renderOnce() + await new Promise((resolve) => setTimeout(resolve, 20)) + await setup.renderOnce() + } + await settle() + + const currentValue = () => { + const last = changes[changes.length - 1] + return last ? last.text : initialValue + } + + return { + setup, + changes, + currentValue, + async press(act: () => void) { + act() + await settle() + }, + async typeText(text: string) { + await setup.mockInput.typeText(text) + await settle() + }, + async pressBackspace() { + setup.mockInput.pressBackspace() + await settle() + }, + /** Emit multiple key inputs with no delay between them, then settle once. */ + async pressRapid(keys: () => void) { + keys() + await settle() + }, + } +} + +describe('MultilineInput IME integration', () => { + test('typing plain text produces correct onChange output', async () => { + const input = await mountInput() + + await input.typeText('chao') + + expect(input.currentValue()).toBe('chao') + expect(input.changes.length).toBeGreaterThan(0) + }) + + test('backspace deletes the last character', async () => { + const input = await mountInput('chao', 4) + + await input.pressBackspace() + + expect(input.currentValue()).toBe('cha') + }) + + test('typing accented char after backspace produces correct text', async () => { + const input = await mountInput('cha', 3) + + // IME compose-and-replace: backspace deletes base char, then accented char + input.setup.mockInput.pressBackspace() + input.setup.mockInput.typeText('à') + await new Promise((resolve) => setTimeout(resolve, 20)) + await input.setup.renderOnce() + + expect(input.currentValue()).toBe('chà') + }) + + test('rapid backspace + accented char in one batch produces correct text', async () => { + const input = await mountInput('cha', 3) + + // Emit backspace and accented char with no delay, settle once. + // Simulates a single stdin chunk from the IME. + input.setup.mockInput.pressKeys(['\x7f', 'à'], 0) + await new Promise((resolve) => setTimeout(resolve, 20)) + await input.setup.renderOnce() + + expect(input.currentValue()).toBe('chà') + }) + + test('full Vietnamese phrase: xin chào', async () => { + const input = await mountInput() + + await input.typeText('xin cha') + + // IME replaces 'a' with 'à' + input.setup.mockInput.pressKeys(['\x7f', 'à'], 0) + await new Promise((resolve) => setTimeout(resolve, 20)) + await input.setup.renderOnce() + + expect(input.currentValue()).toBe('xin chà') + + await input.typeText('o') + + expect(input.currentValue()).toBe('xin chào') + }) +}) diff --git a/cli/src/components/__tests__/multiline-input.test.tsx b/cli/src/components/__tests__/multiline-input.test.tsx index 6f994713fc..7fcf7eaa17 100644 --- a/cli/src/components/__tests__/multiline-input.test.tsx +++ b/cli/src/components/__tests__/multiline-input.test.tsx @@ -1130,145 +1130,3 @@ describe('MultilineInput - newline keyboard shortcuts', () => { expect(isAltModifier({ option: false })).toBe(false) }) }) - -/** - * Tests for Vietnamese IME compose-and-replace input handling. - * - * Vietnamese IMEs (Unikey, EVKey) replace the base char with the accented - * version by sending two events per diacritic keystroke: - * 1. backspace ('\x7f') — delete the base char - * 2. composed char ('à') — insert the accented version - * - * These arrive in a single stdin chunk and fire as separate key events within - * one React batch. The handlers must read from synchronous refs (not stale - * props) so that event N+1 sees the result of event N. - * - * Without ref-based reads, backspace operates on the stale prop value (before - * the batch started), so the base char is never removed — producing "chaà" - * instead of "chà". - */ -describe('MultilineInput - Vietnamese IME compose-and-replace', () => { - /** - * Simulates the ref-based state pattern from multiline-input.tsx. - * onChange callbacks are deferred (batched), refs are updated synchronously. - */ - function createBatchedState(initial: string, initialCursor: number) { - let state = { text: initial, cursorPosition: initialCursor } - const valueRef = { current: initial } - const cursorRef = { current: initialCursor } - const pendingChanges: Array<{ text: string; cursorPosition: number }> = [] - - const onChange = (v: { text: string; cursorPosition: number }) => { - pendingChanges.push(v) - // Simulate React batching: state does NOT update until flush - } - - const flush = () => { - const last = pendingChanges[pendingChanges.length - 1] - if (last) state = last - pendingChanges.length = 0 - // Simulate React re-render: refs refreshed from props - valueRef.current = state.text - cursorRef.current = state.cursorPosition - } - - // Mirrors insertTextAtCursor: reads refs, updates refs before onChange - const insertText = (text: string) => { - const v = valueRef.current - const c = cursorRef.current - const newText = v.slice(0, c) + text + v.slice(c) - valueRef.current = newText - cursorRef.current = c + text.length - onChange({ text: newText, cursorPosition: c + text.length }) - } - - // Mirrors handleDeletionKeys backspace (fixed version): - // reads refs, updates refs before onChange - const backspace = () => { - const v = valueRef.current - const c = cursorRef.current - if (c > 0) { - const newText = v.slice(0, c - 1) + v.slice(c) - valueRef.current = newText - cursorRef.current = c - 1 - onChange({ text: newText, cursorPosition: c - 1 }) - } - } - - // Simulates the OLD buggy backspace: reads from a stale snapshot (props) - // instead of refs. Only used to verify the regression. - const staleBackspace = (staleText: string, staleCursor: number) => { - if (staleCursor > 0) { - const newText = staleText.slice(0, staleCursor - 1) + staleText.slice(staleCursor) - onChange({ text: newText, cursorPosition: staleCursor - 1 }) - // Note: valueRef and cursorRef are NOT updated (old code path) - } - } - - const getState = () => state - return { insertText, backspace, staleBackspace, flush, getState, valueRef, cursorRef } - } - - test('IME DEL+à after "cha" produces "chà" (ref-based backspace)', () => { - const s = createBatchedState('', 0) - - for (const ch of 'cha') s.insertText(ch) - - // IME compose-and-replace: DEL, then 'à' — both in one batch - s.backspace() - s.insertText('à') - - s.flush() - - expect(s.getState().text).toBe('chà') - expect(s.getState().cursorPosition).toBe('chà'.length) - }) - - test('IME DEL+à after "xin cha" produces "xin chà"', () => { - const s = createBatchedState('', 0) - - for (const ch of 'xin cha') s.insertText(ch) - - s.backspace() - s.insertText('à') - - s.flush() - - expect(s.getState().text).toBe('xin chà') - }) - - test('stale backspace produces "chaà" (regression guard)', () => { - const s = createBatchedState('', 0) - - for (const ch of 'cha') s.insertText(ch) - - // Simulate OLD bug: backspace reads stale props (text='cha', cursor=3) - // instead of refs. After the batch, refs are 'cha'/3 (no prior event). - s.staleBackspace('cha', 3) - // refs NOT updated — still 'cha'/3 - s.insertText('à') - - s.flush() - - // Stale backspace deleted from 'cha' → 'ch', but valueRef still 'cha', - // so 'à' inserted at cursor 3 → 'chaà' - expect(s.getState().text).toBe('chaà') - }) - - test('multiple IME diacritics across batches', () => { - const s = createBatchedState('', 0) - - // Type "cha" then accent to "chà" - for (const ch of 'cha') s.insertText(ch) - s.backspace() - s.insertText('à') - s.flush() - - // Continue: "o" → "chào" - s.insertText('o') - s.flush() - - expect(s.getState().text).toBe('chào') - expect(s.getState().cursorPosition).toBe('chào'.length) - }) -})