mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 00:27:39 +00:00
fix(ime): stop a drifted composition flag from swallowing Escape
An Escape the IME owns always arrives marked, so an unmarked one means the document flag has drifted. compositionend is not guaranteed, and eleven call sites guard Escape above their key dispatch, so the drift left rename fields, search boxes and pickers with no way to cancel. Matches the rule xterm-bypass-policy.ts already applies to the terminal. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -195,6 +195,21 @@ describe('WorkspaceKanbanSearchField', () => {
|
||||
expect(onClose).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('still clears on an unmarked Escape after a composition that never ended', () => {
|
||||
// Why: compositionend is not guaranteed, and this field is the board's only
|
||||
// Escape handler, so trusting the document flag alone would strand the query.
|
||||
renderField({ query: '検索', matchCount: 1, totalCount: 12 })
|
||||
|
||||
act(() => {
|
||||
input().dispatchEvent(new CompositionEvent('compositionstart', { bubbles: true }))
|
||||
input().dispatchEvent(
|
||||
new KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true })
|
||||
)
|
||||
})
|
||||
|
||||
expect(onClear).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('keeps focus in the field after the clear button unmounts itself', () => {
|
||||
renderField({ query: 'orca', matchCount: 3, totalCount: 12 })
|
||||
act(() => {
|
||||
|
||||
@@ -86,6 +86,24 @@ describe('document composition tracking', () => {
|
||||
expect(isImeCompositionKeyDown(keyEvent({ isComposing: false, keyCode: 13 }))).toBe(true)
|
||||
})
|
||||
|
||||
it('still suppresses an Escape the IME actually marked', () => {
|
||||
dispatchOnBody('compositionstart')
|
||||
|
||||
expect(
|
||||
isImeCompositionKeyDown(keyEvent({ isComposing: true, key: 'Escape', keyCode: 27 }))
|
||||
).toBe(true)
|
||||
expect(isImeCompositionKeyDown(keyEvent({ key: 'Escape', keyCode: 229 }))).toBe(true)
|
||||
})
|
||||
|
||||
it('lets an unmarked Escape through so a drifted flag cannot strand the cancel path', () => {
|
||||
// No compositionend is guaranteed, so nothing else would clear this. Every
|
||||
// rename field, search box and picker in the app cancels on Escape.
|
||||
dispatchOnBody('compositionstart')
|
||||
|
||||
expect(isImeCompositionKeyDown(keyEvent({ key: 'Escape', keyCode: 27 }))).toBe(false)
|
||||
expect(isImeCompositionKeyDown(keyEvent({ key: 'Enter', keyCode: 13 }))).toBe(true)
|
||||
})
|
||||
|
||||
it('tracks composition anywhere in the document', () => {
|
||||
dispatchOnBody('compositionstart')
|
||||
expect(isDocumentImeCompositionActive()).toBe(true)
|
||||
|
||||
@@ -84,21 +84,30 @@ export function _setDocumentImeCompositionActiveForTests(active: boolean): void
|
||||
* Call this above the key dispatch, not inside each key branch, so every action
|
||||
* (send, history navigation, completion, cancel) is suppressed together.
|
||||
*
|
||||
* The 229 / 'Process' markers are unconditional here — there is deliberately no
|
||||
* opt-out argument. The only surface that needs a marked key to pass through is
|
||||
* the terminal, so it can reach xterm's CompositionHelper, and that decision
|
||||
* lives in xterm-bypass-policy.ts instead. Every caller of this predicate is
|
||||
* ordinary UI, so an opt-out would only ever be dead API.
|
||||
* The 229 / 'Process' markers are unconditional here, and deliberately stricter
|
||||
* than xterm-bypass-policy.ts, which lets a bare 229 keydown through on
|
||||
* macOS/Linux. That is not a contradiction: the terminal needs the keystroke
|
||||
* because xterm's CompositionHelper reads the committed glyph off the helper
|
||||
* textarea. Callers of this predicate receive committed text through `input`, so
|
||||
* a marked keydown carries nothing they could act on — its `key` is 'Process' or
|
||||
* 'Unidentified', never the glyph. Suppressing it here costs no input.
|
||||
*/
|
||||
export function isImeCompositionKeyDown(event: AnyKeyboardEvent): boolean {
|
||||
const nativeEvent = toNativeKeyboardEvent(event)
|
||||
if (!nativeEvent) {
|
||||
return isDocumentImeCompositionActive()
|
||||
}
|
||||
return (
|
||||
isDocumentImeCompositionActive() ||
|
||||
if (
|
||||
nativeEvent.isComposing === true ||
|
||||
nativeEvent.keyCode === 229 ||
|
||||
nativeEvent.key === 'Process'
|
||||
)
|
||||
) {
|
||||
return true
|
||||
}
|
||||
// Why: an Escape the IME owns is marked above, so an unmarked one means this
|
||||
// derived flag has drifted. compositionend is not guaranteed, so swallowing it
|
||||
// would strand every cancel path behind a flag nothing is going to clear. Same
|
||||
// rule as TERMINAL_IME_OWNED_KEYS in xterm-bypass-policy.ts, which omits Escape
|
||||
// for exactly this reason.
|
||||
return nativeEvent.key !== 'Escape' && isDocumentImeCompositionActive()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user