diff --git a/apps/desktop/e2e/accessibility-coverage.spec.ts b/apps/desktop/e2e/accessibility-coverage.spec.ts index 41ed8e69ac..8d55ab9c85 100644 --- a/apps/desktop/e2e/accessibility-coverage.spec.ts +++ b/apps/desktop/e2e/accessibility-coverage.spec.ts @@ -19,7 +19,7 @@ import { FAKE_HOLD_OPEN_PROMPT } from '@maka/runtime/test-only/fake-backend'; import type { CDPSession, Locator, Page } from '@playwright/test'; -import { awaitSendReady, expect, test, COMPOSER_INPUT } from './fixtures'; +import { awaitSendReady, ensureSidebarExpanded, expect, test, COMPOSER_INPUT } from './fixtures'; import { auditAxTree } from '../../../scripts/ax-tree-audit.mjs'; import { groupedNav } from '../src/renderer/settings/settings-nav'; @@ -52,27 +52,13 @@ async function tabTo(page: Page, target: Locator, label: string, limit = 30): Pr ).toBe(true); } -/** - * Walk to the skip link from the document start, taking the start back if a - * cold start moves it. - * - * Parking focus on `body` is not a one-shot the renderer respects: the composer - * restores its draft caret with `getSelection().addRange(...)`, and a range set - * inside a `contenteditable` focuses it — so once per cold start, tens of - * milliseconds after the park and with no `focus()` call to fence on, focus - * lands in the composer. A walk that starts there has to run out the tab ring - * and wrap around, which is over budget. The restore fires once, so re-park and - * walk again rather than widening the budget — the budget is the assertion. - */ async function enterMainFromSkipLink(page: Page): Promise { + await page.evaluate(() => { + document.body.tabIndex = -1; + document.body.focus(); + }); const skipLink = page.getByRole('link', { name: '跳到主要内容' }); - await expect(async () => { - await page.evaluate(() => { - document.body.tabIndex = -1; - document.body.focus(); - }); - await tabTo(page, skipLink, 'skip link', 10); - }).toPass({ timeout: 30_000 }); + await tabTo(page, skipLink, 'skip link', 10); await page.keyboard.press('Enter'); await expect(page.getByRole('main')).toBeFocused(); await page.evaluate(() => document.body.removeAttribute('tabindex')); @@ -322,3 +308,41 @@ test('composer and workbar entry points expose named actionable controls', async } } }); + +/** + * Restoring a draft is not a reason to move focus. The composer places the + * restored caret with a selection, and a selection inside a `contenteditable` + * focuses it whatever held focus before — so before the caret was held back, + * activating a session row took focus out from under the keyboard user who + * activated it. Asserted in a real browser because that is where the focus + * side effect lives; the unit harness models it and cannot observe it. + */ +test('activating a session row with an unsent draft keeps focus on the row', async ({ + window: page, +}) => { + const composer = page.locator(COMPOSER_INPUT); + const prompt = 'session for the draft focus contract'; + await composer.fill(prompt); + await awaitSendReady(page); + await composer.press('Enter'); + await expect(page.getByText(`Fake backend received: ${prompt}`)).toBeVisible({ + timeout: 30_000, + }); + + await composer.click(); + // Plain text, no Skill token: this contract is about the caret restore, and a + // token redraw writes the same selection for a reason of its own. + await page.keyboard.insertText('an unsent draft'); + await expect(composer).toHaveText('an unsent draft'); + + await ensureSidebarExpanded(page); + const sidebar = page.getByRole('navigation', { name: '任务列表' }); + await sidebar.getByRole('button', { name: '新任务', exact: true }).click(); + await expect(composer).toHaveText(''); + + const sessionRow = sidebar.locator('[data-session-id]').first(); + await sessionRow.click(); + await expect(composer).toHaveText('an unsent draft'); + await expect(composer).not.toBeFocused(); + await expect(sessionRow.locator(':focus')).toHaveCount(1); +}); diff --git a/packages/ui/src/__tests__/composer-draft-caret-focus.test.tsx b/packages/ui/src/__tests__/composer-draft-caret-focus.test.tsx new file mode 100644 index 0000000000..b9c76193a5 --- /dev/null +++ b/packages/ui/src/__tests__/composer-draft-caret-focus.test.tsx @@ -0,0 +1,265 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * Who owns the composer's caret, and who owns focus. + * + * A restored draft owes the caret the end of its content, or the next keystroke + * prepends to it. But the only way to place a caret is a selection, and a + * selection inside a `contenteditable` focuses that element — whoever held focus + * before — and moves the point sequential focus navigation resumes from. So the + * restore claimed focus nobody directed at it: on a cold start, past the skip + * link with no `focus()` call to explain it, so Tab from the document start + * began in the composer; and on a session swap, out from under the sidebar row + * the user had just activated. Both are pinned here. + * + * The caret is therefore owed rather than placed whenever the editor is not + * focused, and lands on its next real focus — the first moment the offset is the + * only thing being decided. A pointer press places the caret itself and drops + * the claim. + * + * linkedom carries no selection, no focus and no `Range` motion, so the harness + * models what the composer uses: `createRange` records where a caret was aimed, + * `getSelection` records the live selection and reproduces the focus a selection + * takes, and `focus()` sets `document.activeElement` and dispatches the `focusin` + * a browser would. It also lowercases `contentEditable` on the way into the DOM, because linkedom stores + * attribute names verbatim where HTML folds them — without that the composer's + * own `[contenteditable="true"]` lookup misses its editor here and nowhere else. + */ + +import assert from 'node:assert/strict'; +import { afterEach, test } from 'node:test'; +import { act } from 'react'; +import { createRoot } from 'react-dom/client'; +import { parseHTML } from 'linkedom'; +import { Composer } from '../composer.js'; +import { LocaleProvider } from '../locale-context.js'; + +const originalGlobals = { + document: globalThis.document, + matchMedia: globalThis.matchMedia, + requestAnimationFrame: globalThis.requestAnimationFrame, + cancelAnimationFrame: globalThis.cancelAnimationFrame, + window: globalThis.window, +}; +const originalActEnvironment = (globalThis as typeof globalThis & { + IS_REACT_ACT_ENVIRONMENT?: boolean; +}).IS_REACT_ACT_ENVIRONMENT; +const mountedRoots: ReturnType[] = []; +const restoreDom: (() => void)[] = []; + +afterEach(async () => { + for (const root of mountedRoots.splice(0)) await act(() => root.unmount()); + for (const restore of restoreDom.splice(0)) restore(); + Object.assign(globalThis, { + ...originalGlobals, + IS_REACT_ACT_ENVIRONMENT: originalActEnvironment, + }); +}); + +function computedStyle(): CSSStyleDeclaration { + return { + direction: 'ltr', + writingMode: 'horizontal-tb', + getPropertyValue: () => '', + } as unknown as CSSStyleDeclaration; +} + +/** Where a caret was aimed: `selectNodeContents` then `collapse` and no more. */ +interface AimedRange { + container: Node | null; + offset: number; + collapsed: boolean; +} + +function harness() { + const { document, window } = parseHTML('
'); + window.getComputedStyle = () => computedStyle(); + const setAttribute = window.Element.prototype.setAttribute; + window.Element.prototype.setAttribute = function normalized(name: string, value: string) { + return setAttribute.call(this, name === 'contentEditable' ? 'contenteditable' : name, value); + }; + restoreDom.push(() => { + window.Element.prototype.setAttribute = setAttribute; + }); + document.createRange = () => { + const range: AimedRange & { + selectNodeContents(node: Node): void; + collapse(toStart: boolean): void; + } = { + container: null, + offset: 0, + collapsed: false, + selectNodeContents(node) { + range.container = node; + range.offset = node.childNodes.length; + }, + collapse(toStart) { + range.offset = toStart ? 0 : range.offset; + range.collapsed = true; + }, + }; + return range as unknown as Range; + }; + let active: Element | null = null; + const selected: AimedRange[] = []; + const selection = { + get anchorNode(): Node | null { + return selected.at(-1)?.container ?? null; + }, + removeAllRanges() { + selected.length = 0; + }, + addRange(range: Range) { + const aimed = range as unknown as AimedRange; + selected.push(aimed); + // The whole point: a selection inside a `contenteditable` focuses it, + // whoever held focus before. Without this the harness would let a caret + // placed on a blurred editor look free. + const container = aimed.container as Element & { closest?: Element['closest'] }; + active = container?.closest?.('[contenteditable="true"]') ?? active; + }, + }; + document.getSelection = () => selection as unknown as Selection; + window.getSelection = () => selection as unknown as Selection; + Object.defineProperty(document, 'activeElement', { configurable: true, get: () => active }); + Object.assign(globalThis, { + document, + window, + matchMedia: () => ({ matches: false, addEventListener() {}, removeEventListener() {} }), + requestAnimationFrame: () => 1, + cancelAnimationFrame() {}, + IS_REACT_ACT_ENVIRONMENT: true, + }); + const container = document.querySelector('#root'); + assert.ok(container); + const root = createRoot(container); + mountedRoots.push(root); + return { + /** The live selection: what `removeAllRanges` cleared and `addRange` added. */ + selected: selected as readonly AimedRange[], + editable() { + const editable = document.querySelector('[contenteditable="true"]'); + assert.ok(editable, 'the composer rendered no editable node'); + return editable as unknown as HTMLElement; + }, + /** A focusable outside the composer — the sidebar row that swaps the draft. */ + outside() { + const existing = document.querySelector('#outside'); + if (existing) return existing as unknown as HTMLElement; + const button = document.createElement('button'); + button.id = 'outside'; + document.documentElement.appendChild(button); + return button as unknown as HTMLElement; + }, + focused: () => active, + /** Focus an element the way a browser does: activate it, then announce it. */ + async focus(element: HTMLElement) { + active = element as unknown as Element; + await act(() => { + element.dispatchEvent(new window.Event('focusin', { bubbles: true })); + }); + }, + async pointerDown(element: HTMLElement) { + await act(() => { + element.dispatchEvent(new window.Event('pointerdown', { bubbles: true })); + }); + }, + async render(props: Parameters[0]) { + await act(() => { + root.render( + + + , + ); + }); + }, + }; +} + +const base = { + onSend: () => undefined, + onStop: () => undefined, +}; + +/** A host that hands the named session back an unsent draft, as a cold start does. */ +function withDraft(key: string, draft: string) { + return { + ...base, + draftPersistence: { + read: (draftKey: string | undefined) => (draftKey === key ? draft : ''), + write: () => undefined, + }, + }; +} + +/** The end of the content, which is where every restored draft owes its caret. */ +function assertCaretAtEnd(selected: readonly AimedRange[], editable: HTMLElement): void { + const caret = selected.at(-1); + assert.ok(caret, 'the composer placed no caret'); + assert.equal(caret.collapsed, true, 'the caret must be a collapsed selection'); + assert.equal(caret.container, editable); + assert.equal(caret.offset, editable.childNodes.length); +} + +test('a draft restored while nothing holds focus places no selection', async () => { + const dom = harness(); + await dom.render({ ...withDraft('session-a', 'restored draft'), draftKey: 'session-a' }); + assert.equal(dom.editable().textContent, 'restored draft'); + assert.equal( + dom.selected.length, + 0, + 'the restored caret took a selection inside the contenteditable, which focuses it and moves ' + + 'the point Tab resumes from off the top of the document', + ); + assert.equal(dom.focused(), null, 'the restored caret focused the composer'); +}); + +test('the owed caret lands at the end of the draft on the next focus', async () => { + const dom = harness(); + await dom.render({ ...withDraft('session-a', 'restored draft'), draftKey: 'session-a' }); + await dom.focus(dom.editable()); + assertCaretAtEnd(dom.selected, dom.editable()); +}); + +test('a pointer press into the composer drops the owed caret', async () => { + const dom = harness(); + await dom.render({ ...withDraft('session-a', 'restored draft'), draftKey: 'session-a' }); + await dom.pointerDown(dom.editable()); + await dom.focus(dom.editable()); + assert.equal( + dom.selected.length, + 0, + 'a click places the caret where it lands; the owed caret must not overrule it', + ); +}); + +test('a session swap leaves focus on the row that caused it', async () => { + const dom = harness(); + const props = withDraft('session-b', 'other draft'); + await dom.render({ ...props, draftKey: 'session-a' }); + await dom.focus(dom.outside()); + await dom.render({ ...props, draftKey: 'session-b' }); + assert.equal(dom.editable().textContent, 'other draft'); + assert.equal( + dom.focused(), + dom.outside(), + 'the restored caret took focus out from under the row the user activated', + ); +}); diff --git a/packages/ui/src/composer.tsx b/packages/ui/src/composer.tsx index b17310da8a..fe19b9ea86 100644 --- a/packages/ui/src/composer.tsx +++ b/packages/ui/src/composer.tsx @@ -554,6 +554,8 @@ export const Composer = forwardRef< * identity so neither hook re-runs an effect when the draft changes. */ const caretToEndRef = useRef(false); + /** A caret-to-end owed to an editor that was not focused when it came due. */ + const caretPendingRef = useRef(false); const redrawPendingRef = useRef(false); const textPortRef = useRef(null); if (!textPortRef.current) { @@ -578,10 +580,27 @@ export const Composer = forwardRef< * say) then landed the caret at offset 0, so typing prepended to the restored * draft. Collapse to the end here when the editor holds no selection of its * own, which is what the retired `focusTextInputAtEnd` did unconditionally. + * + * Only on a focused editor, though. A selection inside a `contenteditable` is + * never only a caret: the browser focuses the element to carry it, whatever + * held focus before — measured in the shipping runtime, a selection placed + * here takes focus from a focused button exactly as it takes it from `body` — + * and sequential focus navigation then resumes from the selection rather than + * from the top of the document. So a restored draft claimed focus nobody + * directed at it: on a cold start, tens of milliseconds in, past the skip link + * and with no `focus()` call to explain it; and on a session swap, out from + * under the sidebar row the user had just activated. Hold the caret while the + * editor is not focused and land it on the editor's next real focus, which is + * the first moment the offset is the only thing being decided. */ function caretToContentEnd() { const editable = editableNode(); if (!editable) return; + if (document.activeElement !== editable) { + caretPendingRef.current = true; + return; + } + caretPendingRef.current = false; const selection = document.getSelection(); const range = document.createRange(); range.selectNodeContents(editable); @@ -596,6 +615,30 @@ export const Composer = forwardRef< if (!editable || (selection?.anchorNode && editable.contains(selection.anchorNode))) return; caretToContentEnd(); } + /** + * Settle a held caret when focus reaches the editor for real. On the component + * root, like the other native listeners here: `focusin` and `pointerdown` + * bubble, and a disabled composer renders no editable to look up at mount. + * + * A pointer press places the caret itself and is the more specific intent, so + * it drops the claim rather than being overruled by it. + */ + useEffect(() => { + const root = inputRootRef.current; + if (!root) return undefined; + const land = () => { + if (caretPendingRef.current) caretToContentEnd(); + }; + const drop = () => { + caretPendingRef.current = false; + }; + root.addEventListener('focusin', land); + root.addEventListener('pointerdown', drop); + return () => { + root.removeEventListener('focusin', land); + root.removeEventListener('pointerdown', drop); + }; + }, []); /** * The ＋ menu's Skills entry opens the same `/` menu the keyboard opens: it * types the trigger for the user. There is no second Skill surface to keep in @@ -727,13 +770,27 @@ export const Composer = forwardRef< * The redraw gets the same treatment for the same reason, and can land a * render later than the write that owed it: `insertToken` parks the selection * after the last chip it wrote, so the caret has to be collected again. + * + * A held caret is suspended across the redraw rather than left armed. The + * redraw drives `insertToken` through the document selection, and its first + * range focuses the editor — which would otherwise fire the focus lander onto + * the very range the redraw is holding, collapsing it to the end so the chip + * landed at the end and its source text stayed in the draft. The redraw ends + * by collecting the caret itself, so on success the claim is settled; on a + * pass that redrew nothing it is handed back untouched. */ useEffect(() => { let restoreCaret = caretToEndRef.current; caretToEndRef.current = false; - if (redrawPendingRef.current && redrawSkillTokens()) { - redrawPendingRef.current = false; - restoreCaret = true; + if (redrawPendingRef.current) { + const heldCaret = caretPendingRef.current; + caretPendingRef.current = false; + const redrew = redrawSkillTokens(); + caretPendingRef.current = redrew ? false : heldCaret; + if (redrew) { + redrawPendingRef.current = false; + restoreCaret = true; + } } if (restoreCaret) caretToContentEnd(); });