From 0d6f7c4abab15962202de3c252497e9a37dd4dcd Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 15:16:26 +1100 Subject: [PATCH 1/8] fix(composer): move Up and Down across lines that only a wrapped link holds Links and mentions in the composer are uneditable inline tokens. When one wraps, a line holding only part of it, or only the token, has no caret position besides the token's edges. Native Up and Down with or without Shift therefore stall there in WebKit and jump to the start or end of the document in Chromium, both entering such a line and leaving it. Before a native line move, crossTokenLine reads each token's client rects to find the lines it holds alone and moves in document terms instead. Entering such a line stops at the token edge it holds. A middle line is crossed to the far edge. Leaving from an edge reaches the token's other edge or steps onto the neighbouring line, measured past a hard break, a typed newline or the paragraph boundary. Shift keeps the anchor. It runs from the React keydown handler after the host, so an open completion list still claims Up and Down, and from the control-character caret-key path. Every other line move stays native, with one adjustment. ProseMirror places a caret after a token between the token's element and the following text. Once WebKit has drawn the caret there beside a wrapped token, Down stalls. The DOM caret now moves to the start of that text first, which is the same document position. Known limitations: leaving a token-only line lands at the near end of the neighbouring line rather than the caret's column. A caret at a soft wrap is measured on the line it leaves, so a move there can go one line too far. The new Chromium and WebKit journey fails without each part of the fix. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src/features/messages/EditableInput.tsx | 131 ++++++++++++++++++- tests/browser/composer-links.spec.mjs | 163 ++++++++++++++++++++++++ 2 files changed, 289 insertions(+), 5 deletions(-) diff --git a/src/features/messages/EditableInput.tsx b/src/features/messages/EditableInput.tsx index 28ebc740ab..40f63909a2 100644 --- a/src/features/messages/EditableInput.tsx +++ b/src/features/messages/EditableInput.tsx @@ -150,6 +150,103 @@ function keyboardKey( CARET_KEYS.get(event.code) ?? CARET_KEYS.get(event.keyCode) ?? event.key ); } +/** Moves the head a line up or down where the browser cannot. A line that + * holds nothing but a token, or part of a wrapped one, offers no caret + * position besides the token's edge, so native Up and Down stall there in + * WebKit and jump to a document edge in Chromium, entering such a line and + * leaving it alike. Such a line is entered at the token edge it holds, or + * crossed to the far edge when it holds neither, and left for the nearest + * position on the neighbouring line. The token's client rects give its + * lines, so a line scrolled out of the composer still counts. A caret at a + * soft wrap is measured on the line it leaves: a move may go one line too + * far but never stalls. Every other line move stays native, from a DOM + * caret the browser can move. */ +function crossTokenLine( + editor: EditorView, + tokens: ReadonlyMap, + up: boolean, + extend: boolean, +) { + const { selection, doc } = editor.state; + if (!(selection instanceof TextSelection) || !tokens.size) return false; + const origin = extend ? selection.head : up ? selection.from : selection.to; + const caret = editor.coordsAtPos(origin, up ? -1 : 1); + const level = (line: { top: number; bottom: number }) => + (line.top + line.bottom) / 2; + const holds = (line?: { top: number; bottom: number }) => + !!line && line.top <= level(caret) && level(caret) <= line.bottom; + // The position beside a token that is drawn on the neighbouring line when + // the token starts or ends its own: past a line break (a hard break or a + // typed newline), into the adjacent textblock, or else the token edge. + const outside = (pos: number, dir: -1 | 1) => { + const $pos = doc.resolve(pos); + if (!(dir < 0 ? $pos.nodeBefore : $pos.nodeAfter)) + return Selection.findFrom(doc.resolve(pos + dir), dir, true)?.head; + const next = pos + dir; + return doc.textBetween(Math.min(pos, next), Math.max(pos, next)) === "\n" + ? next + : pos; + }; + for (const [element, host] of tokens) { + const from = host.position(); + if (from === undefined) continue; + const to = from + 1; + const lines = [...element.getClientRects()]; + const last = lines.length - 1; + const before = outside(from, -1); + const after = outside(to, 1); + const above = + before === undefined ? undefined : editor.coordsAtPos(before, -1); + const below = + after === undefined ? undefined : editor.coordsAtPos(after, 1); + for (const [index, line] of lines.entries()) { + const alone = + (index > 0 || !above || level(above) < line.top) && + (index < last || !below || level(below) > line.bottom); + if (!alone) continue; + const previous = up + ? (lines[index + 1] ?? below) + : (lines[index - 1] ?? above); + let head: number | undefined; + // Entering: stop at the edge this line holds, or cross a middle line. + if (holds(previous)) + head = up ? (index === last ? to : from) : index === 0 ? from : to; + // Leaving from the edge: cross the rest of the token, or step off it. + else if ( + holds(line) && + ((index === 0 && origin === from) || (index === last && origin === to)) + ) + head = up ? (index > 0 ? from : before) : index < last ? to : after; + if (head === undefined || (up ? head >= origin : head <= origin)) + continue; + editor.dispatch( + editor.state.tr + .setSelection( + TextSelection.create(doc, extend ? selection.anchor : head, head), + ) + .scrollIntoView(), + ); + return true; + } + } + // After a token, the DOM caret sits between its element and the text that + // follows. Once WebKit has drawn the caret there beside a wrapped token, + // Down stalls; from the text's start, the same position, it moves. + const native = editor.dom.ownerDocument.getSelection(); + const next = native?.focusNode?.childNodes[native.focusOffset]; + const token = native?.focusNode?.childNodes[native.focusOffset - 1]; + if ( + native?.anchorNode && + next?.nodeType === Node.TEXT_NODE && + token instanceof HTMLElement && + tokens.has(token) + ) { + if (native.isCollapsed) native.collapse(next, 0); + else + native.setBaseAndExtent(native.anchorNode, native.anchorOffset, next, 0); + } + return false; +} export type EditableInputProps = Omit< HTMLAttributes, "onChange" | "onInput" @@ -1097,11 +1194,12 @@ export function EditableInput({ /** Moves the caret for a caret key the host reported as a control character. * Left unhandled, such a keydown types the character instead of moving, so * the move happens here: in document terms across an inline leaf, where - * WebKit offers extra caret positions around uneditable nodes, and otherwise - * through the browser's own caret motion with the platform's modifier - * meanings (word and line ends sideways, paragraph and document ends - * vertically). Page keys scroll without moving the caret, and Home and End - * do so too on Apple platforms unless extending the selection. */ + * WebKit offers extra caret positions around uneditable nodes, or a line + * that only a token holds, and otherwise through the browser's own caret + * motion with the platform's modifier meanings (word and line ends + * sideways, paragraph and document ends vertically). Page keys scroll + * without moving the caret, and Home and End do so too on Apple platforms + * unless extending the selection. */ const moveCaret = (key: string, event: globalThis.KeyboardEvent) => { const mac = isApplePlatform(navigator.platform); const boundary = mac ? event.metaKey : event.ctrlKey; @@ -1135,6 +1233,13 @@ export function EditableInput({ return true; } } + if ( + (key === "ArrowUp" || key === "ArrowDown") && + !event.altKey && + !boundary && + crossTokenLine(editor, hosts.current, key === "ArrowUp", extend) + ) + return true; const selection = editor.dom.ownerDocument.getSelection(); if (!selection || typeof selection.modify !== "function") return true; const alter = extend ? "extend" : "move"; @@ -1926,6 +2031,22 @@ export function EditableInput({ event.preventDefault(); ref.current?.insertLineBreak(); } + // After the host, which claims Up and Down for a completion list. + if ( + !event.defaultPrevented && + view.current && + (event.key === "ArrowUp" || event.key === "ArrowDown") && + !event.altKey && + !event.metaKey && + !event.ctrlKey && + crossTokenLine( + view.current, + hosts.current, + event.key === "ArrowUp", + event.shiftKey, + ) + ) + event.preventDefault(); }} onCompositionStart={(event) => { composing.current = true; diff --git a/tests/browser/composer-links.spec.mjs b/tests/browser/composer-links.spec.mjs index 6499c0344f..9938262038 100644 --- a/tests/browser/composer-links.spec.mjs +++ b/tests/browser/composer-links.spec.mjs @@ -627,3 +627,166 @@ test("editable composer renders links and mentions while preserving source and n await server.close(); } }); + +test("Up and Down move one line at a time through links that wrap", async ({ + page, +}) => { + // A line that holds only part of a wrapped link has no caret position of its + // own: left to the browser, Up and Down stall there in WebKit and jump to the + // document edge in Chromium. Layout decides where a link wraps, so this needs + // a real browser at a width where both links wrap. + await page.setViewportSize({ width: 480, height: 900 }); + await page.addInitScript(() => { + localStorage.setItem("buzz-remember-mentioned-agents.v1", "off"); + }); + const server = await createServer({ + root: fileURLToPath(new URL("../../", import.meta.url)), + configFile: false, + envFile: false, + plugins: [react()], + logLevel: "error", + server: { host: "127.0.0.1", port: 0 }, + }); + await server.listen(); + try { + await page.goto( + `http://127.0.0.1:${server.httpServer.address().port}/tests/fixtures/link-messages.html`, + ); + const input = page.getByRole("textbox", { + name: "Message #design", + exact: true, + }); + await page.evaluate(() => document.fonts.ready); + const title = + "Add fully custom Auth0 login screens with staging and production rollout tooling"; + const link = (label, pull) => + `[${label}](https://github.com/block/buzz/pull/${pull})`; + // The first link starts its paragraph, so its first line holds only link + // text. The second wraps twice, so its middle line does too. + const first = link(title, 1); + const second = link(`${title} plus a long tail that wraps again`, 2); + const [lead, firstTail, secondTail] = [ + "Words before ", + ": wraps once.", + " after it.", + ]; + const text = [ + "Plain words above the links.", + `${first}${firstTail}`, + `${lead}${second}${secondTail}`, + // Enough lines that the composer scrolls the links out of view. + ...Array.from({ length: 5 }, (_, line) => `Plain line ${line} below.`), + ].join("\n"); + await input.fill(""); + await input.fill(text); + const links = input.locator('[data-link-kind="github"]'); + await expect(links).toHaveCount(2); + const lines = await input + .locator("[data-source]") + .evaluateAll((elements) => + elements.map((element) => element.getClientRects().length), + ); + expect(lines[0]).toBeGreaterThanOrEqual(2); + expect(lines[1]).toBeGreaterThanOrEqual(3); + expect( + await input.evaluate((el) => el.scrollHeight > el.clientHeight), + ).toBe(true); + + const firstFrom = text.indexOf(first); + const firstTo = firstFrom + first.length; + const secondFrom = text.indexOf(second); + const secondTo = secondFrom + second.length; + const caret = () => + input.evaluate((el) => + el.selectionDirection === "backward" + ? { head: el.selectionStart, anchor: el.selectionEnd } + : { head: el.selectionEnd, anchor: el.selectionStart }, + ); + // WebKit settles a caret beside a link when it draws it, and a press + // before then can move where a person's press would stall. + const rendered = () => + page.evaluate( + () => + new Promise((resolve) => + requestAnimationFrame(() => requestAnimationFrame(resolve)), + ), + ); + // Presses `key` until the head reaches `stop`, returning where each press + // lands. A press that leaves the head in place fails the poll. + const walk = async (key, start, stop) => { + await input.evaluate((el, at) => el.setSelectionRange(at, at), start); + await rendered(); + const heads = []; + let head = start; + while (head !== stop && heads.length < 12) { + const previous = head; + await page.keyboard.press(key); + await expect + .poll(async () => (head = (await caret()).head)) + .not.toBe(previous); + expect(Math.sign(head - previous)).toBe(key.endsWith("Up") ? -1 : 1); + expect((await caret()).anchor).toBe( + key.startsWith("Shift+") ? start : head, + ); + heads.push(head); + await rendered(); + } + return heads; + }; + for (const shift of ["", "Shift+"]) { + const down = await walk(`${shift}ArrowDown`, 0, secondTo); + // Onto the first link's own line at its start, then across the link. + expect(down.slice(0, 2)).toEqual([firstFrom, firstTo]); + // One line down: the words before the second link. + expect(down[2]).toBeGreaterThanOrEqual(secondFrom - lead.length); + expect(down[2]).toBeLessThanOrEqual(secondFrom); + // Across the second link's middle line. + expect(down.slice(3)).toEqual([secondTo]); + + const up = await walk(`${shift}ArrowUp`, text.length, firstFrom - 1); + const [afterSecond, beforeSecond, afterFirst, ...rest] = up.slice(-5); + // The line holding the end of the second link and the words after it. + expect(afterSecond).toBeGreaterThanOrEqual(secondTo); + expect(afterSecond).toBeLessThanOrEqual(secondTo + secondTail.length); + // Across the second link's middle line, then up to the first link's end + // line, onto its own first line, and off it to the line above. + expect(beforeSecond).toBe(secondFrom); + expect(afterFirst).toBeGreaterThanOrEqual(firstTo); + expect(afterFirst).toBeLessThanOrEqual(firstTo + firstTail.length); + expect(rest).toEqual([firstFrom, firstFrom - 1]); + } + + // A typed line break after the first link leaves its end line to it too. + await input.evaluate((el, at) => el.setSelectionRange(at, at), firstTo); + await input.press("Shift+Enter"); + await expect(input).toHaveJSProperty( + "value", + `${text.slice(0, firstTo)}\n${text.slice(firstTo)}`, + ); + for (const shift of ["", "Shift+"]) + expect( + await walk( + `${shift}ArrowUp`, + firstTo + 1 + firstTail.length, + firstFrom, + ), + ).toEqual([firstTo, firstFrom]); + + // WebKit can report a caret key as a control character; that path moves + // across a link line the same way. + await input.evaluate((el, at) => el.setSelectionRange(at, at), firstFrom); + await input.evaluate((el) => + el.dispatchEvent( + new KeyboardEvent("keydown", { + key: "\u001F", + code: "ArrowDown", + bubbles: true, + cancelable: true, + }), + ), + ); + expect((await caret()).head).toBe(firstTo); + } finally { + await server.close(); + } +}); From 8fa21dd2cd88cb13b4b03888ec0e6484faf643e7 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 15:55:04 +1100 Subject: [PATCH 2/8] fix(composer): treat a bidi-split link line as one line for Up and Down Bidi reordering can split a link into several client rects on one line, as WebKit reports them when the link's Hebrew runs on into the text after it. crossTokenLine took each rect for a line of its own, so the first fragment looked like a line the link held alone: Down from the line above stopped at the link's start and the next Down only stepped to its end, and Up from that line's text did the same in reverse. A rect on its predecessor's line is now folded into it. Folding alone is not enough. WebKit's native Down onto a column over the split link puts the DOM caret inside the token: the press does nothing and the next one skips a line. A line the link is split across is therefore entered at its edge like a line it holds alone, but only from the neighbouring line, and it is still left natively, which works from either edge in both engines. The DOM caret step before a native move now runs only for Down in WebKit. Without it Down stalls beside a wrapped link in macOS and Linux WebKit alike, so it checks the WebKit vendor rather than macWebKit(); Chromium and Up pass without it. It uses Selection.extend for a range. Both call sites now share crossTokenLine's own check for plain or Shift Up and Down, so Ctrl+Up on a Mac and Meta+Up elsewhere no longer reach it on the control-character path alone. The Mac's Ctrl+N and Ctrl+P stay native. moveHead now serves both crossTokenLine and the sideways inline-leaf step in moveCaret. Known gap, unchanged from before: native Up or Down onto a split link still stalls in WebKit when the link neither starts nor ends its line on the side the caret comes from, as no line break shows which line is the neighbour. The journey's new bidi case fails without the fold in macOS and Linux WebKit, with folding alone (the stall) and with the previous code (the sideways press). Chromium reports one rect and passes throughout. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src/features/messages/EditableInput.tsx | 119 +++++++++++++----------- tests/browser/composer-links.spec.mjs | 27 ++++++ 2 files changed, 90 insertions(+), 56 deletions(-) diff --git a/src/features/messages/EditableInput.tsx b/src/features/messages/EditableInput.tsx index 40f63909a2..8411decacc 100644 --- a/src/features/messages/EditableInput.tsx +++ b/src/features/messages/EditableInput.tsx @@ -150,31 +150,58 @@ function keyboardKey( CARET_KEYS.get(event.code) ?? CARET_KEYS.get(event.keyCode) ?? event.key ); } -/** Moves the head a line up or down where the browser cannot. A line that - * holds nothing but a token, or part of a wrapped one, offers no caret - * position besides the token's edge, so native Up and Down stall there in - * WebKit and jump to a document edge in Chromium, entering such a line and +/** Moves the selection head to a document position, keeping the anchor when + * extending, and scrolls it into view. */ +function moveHead(editor: EditorView, head: number, extend: boolean) { + const { tr, selection, doc } = editor.state; + editor.dispatch( + tr + .setSelection( + TextSelection.create(doc, extend ? selection.anchor : head, head), + ) + .scrollIntoView(), + ); +} +/** Moves the head for plain or Shift Up and Down where the browser cannot. A + * line that holds nothing but a token, or part of a wrapped one, offers no + * caret position besides the token's edge, so native Up and Down stall there + * in WebKit and jump to a document edge in Chromium, entering such a line and * leaving it alike. Such a line is entered at the token edge it holds, or * crossed to the far edge when it holds neither, and left for the nearest * position on the neighbouring line. The token's client rects give its * lines, so a line scrolled out of the composer still counts. A caret at a * soft wrap is measured on the line it leaves: a move may go one line too * far but never stalls. Every other line move stays native, from a DOM - * caret the browser can move. */ + * caret the browser can move. The Mac's emacs-style Ctrl+N and Ctrl+P line + * moves stay native too, and can still stall on such a line: they are rare + * in a composer and would need a platform key binding here. */ function crossTokenLine( editor: EditorView, tokens: ReadonlyMap, - up: boolean, - extend: boolean, + key: string, + event: Pick< + globalThis.KeyboardEvent, + "altKey" | "ctrlKey" | "metaKey" | "shiftKey" + >, ) { + const up = key === "ArrowUp"; + const extend = event.shiftKey; const { selection, doc } = editor.state; - if (!(selection instanceof TextSelection) || !tokens.size) return false; + if ( + (!up && key !== "ArrowDown") || + event.altKey || + event.ctrlKey || + event.metaKey || + !(selection instanceof TextSelection) || + !tokens.size + ) + return false; const origin = extend ? selection.head : up ? selection.from : selection.to; const caret = editor.coordsAtPos(origin, up ? -1 : 1); const level = (line: { top: number; bottom: number }) => (line.top + line.bottom) / 2; - const holds = (line?: { top: number; bottom: number }) => - !!line && line.top <= level(caret) && level(caret) <= line.bottom; + const holds = (line?: { top: number; bottom: number }, point = caret) => + !!line && line.top <= level(point) && level(point) <= line.bottom; // The position beside a token that is drawn on the neighbouring line when // the token starts or ends its own: past a line break (a hard break or a // typed newline), into the adjacent textblock, or else the token edge. @@ -191,7 +218,10 @@ function crossTokenLine( const from = host.position(); if (from === undefined) continue; const to = from + 1; - const lines = [...element.getClientRects()]; + // Bidi reordering can split a token into several rects on one line, as + // WebKit reports them, so a rect on its predecessor's line is folded in. + const rects = [...element.getClientRects()]; + const lines = rects.filter((rect, index) => !holds(rects[index - 1], rect)); const last = lines.length - 1; const before = outside(from, -1); const after = outside(to, 1); @@ -203,47 +233,48 @@ function crossTokenLine( const alone = (index > 0 || !above || level(above) < line.top) && (index < last || !below || level(below) > line.bottom); - if (!alone) continue; + // WebKit's native move onto a line the token is split across can put + // the caret inside the token, so such a line is entered here too. + const split = rects.filter((rect) => holds(line, rect)).length > 1; + if (!alone && !split) continue; const previous = up ? (lines[index + 1] ?? below) : (lines[index - 1] ?? above); let head: number | undefined; // Entering: stop at the edge this line holds, or cross a middle line. - if (holds(previous)) + if (holds(previous) && !holds(line)) head = up ? (index === last ? to : from) : index === 0 ? from : to; // Leaving from the edge: cross the rest of the token, or step off it. else if ( + alone && holds(line) && ((index === 0 && origin === from) || (index === last && origin === to)) ) head = up ? (index > 0 ? from : before) : index < last ? to : after; if (head === undefined || (up ? head >= origin : head <= origin)) continue; - editor.dispatch( - editor.state.tr - .setSelection( - TextSelection.create(doc, extend ? selection.anchor : head, head), - ) - .scrollIntoView(), - ); + moveHead(editor, head, extend); return true; } } // After a token, the DOM caret sits between its element and the text that - // follows. Once WebKit has drawn the caret there beside a wrapped token, - // Down stalls; from the text's start, the same position, it moves. + // follows. Once WebKit, on any platform, has drawn the caret there beside a + // wrapped token, Down stalls; from the text's start, the same position, it + // moves. The step is harmless beside a token that does not wrap, so it is + // not limited to wrapped ones. Up and Chromium need no help. const native = editor.dom.ownerDocument.getSelection(); const next = native?.focusNode?.childNodes[native.focusOffset]; const token = native?.focusNode?.childNodes[native.focusOffset - 1]; if ( - native?.anchorNode && + !up && + navigator.vendor === "Apple Computer, Inc." && + native && next?.nodeType === Node.TEXT_NODE && token instanceof HTMLElement && tokens.has(token) ) { if (native.isCollapsed) native.collapse(next, 0); - else - native.setBaseAndExtent(native.anchorNode, native.anchorOffset, next, 0); + else native.extend(next, 0); } return false; } @@ -1206,7 +1237,7 @@ export function EditableInput({ const extend = event.shiftKey; const sideways = key === "ArrowLeft" || key === "ArrowRight"; if (sideways && !event.altKey && !boundary) { - const { $head, anchor, empty } = editor.state.selection; + const { $head, empty } = editor.state.selection; const backward = key === "ArrowLeft"; const leaf = $head.textOffset ? null @@ -1216,30 +1247,15 @@ export function EditableInput({ // Only an inline leaf is stepped over in document terms. A block beside // the head, as after select-all, is left to the browser's caret motion. if ((empty || extend) && leaf?.isInline && !leaf.isText) { - const head = backward - ? $head.pos - leaf.nodeSize - : $head.pos + leaf.nodeSize; - editor.dispatch( - editor.state.tr - .setSelection( - TextSelection.create( - editor.state.doc, - extend ? anchor : head, - head, - ), - ) - .scrollIntoView(), + moveHead( + editor, + backward ? $head.pos - leaf.nodeSize : $head.pos + leaf.nodeSize, + extend, ); return true; } } - if ( - (key === "ArrowUp" || key === "ArrowDown") && - !event.altKey && - !boundary && - crossTokenLine(editor, hosts.current, key === "ArrowUp", extend) - ) - return true; + if (crossTokenLine(editor, hosts.current, key, event)) return true; const selection = editor.dom.ownerDocument.getSelection(); if (!selection || typeof selection.modify !== "function") return true; const alter = extend ? "extend" : "move"; @@ -2035,16 +2051,7 @@ export function EditableInput({ if ( !event.defaultPrevented && view.current && - (event.key === "ArrowUp" || event.key === "ArrowDown") && - !event.altKey && - !event.metaKey && - !event.ctrlKey && - crossTokenLine( - view.current, - hosts.current, - event.key === "ArrowUp", - event.shiftKey, - ) + crossTokenLine(view.current, hosts.current, event.key, event) ) event.preventDefault(); }} diff --git a/tests/browser/composer-links.spec.mjs b/tests/browser/composer-links.spec.mjs index 9938262038..4ddc840b00 100644 --- a/tests/browser/composer-links.spec.mjs +++ b/tests/browser/composer-links.spec.mjs @@ -786,6 +786,33 @@ test("Up and Down move one line at a time through links that wrap", async ({ ), ); expect((await caret()).head).toBe(firstTo); + + // WebKit splits this link into two rects on one line, reordering its + // Hebrew with the text after it. That is still one line, and native Down + // onto the link stalls there, so each press must still move one line. + const bidi = [ + "First line.", + `${link("PR שלום עולם", 1)} שלום עולם`, + "Last line.", + ]; + await input.fill(""); + await input.fill(bidi.join("\n")); + await expect(links).toHaveCount(1); + const starts = [0, bidi[0].length + 1, bidi[0].length + bidi[1].length + 2]; + const line = (head) => starts.findLastIndex((start) => head >= start); + for (const shift of ["", "Shift+"]) + for (const [key, start, expected] of [ + ["ArrowDown", 0, [1, 2]], + ["ArrowUp", bidi.join("\n").length, [1, 0]], + ]) { + await input.evaluate((el, at) => el.setSelectionRange(at, at), start); + await rendered(); + for (const index of expected) { + await page.keyboard.press(`${shift}${key}`); + await expect.poll(async () => line((await caret()).head)).toBe(index); + await rendered(); + } + } } finally { await server.close(); } From 2bcd59e86b4455cb2c0975984cb899b9a6e2071d Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 16:08:19 +1100 Subject: [PATCH 3/8] refactor(composer): share the WebKit check and pin the bidi Shift anchor Follow-ups from review of the bidi-split link fix; behaviour is unchanged. mac-webkit.ts now exports webKit(), the vendor check for WebKit on any platform, and macWebKit() builds on it. The DOM caret step before a native Down uses it instead of repeating the vendor literal. The crossTokenLine comment on split lines now says they are left natively, which works from either edge, and why a caret already on such a line is not entering: text beside the token can put the previous line's measure on that very line. The journey's bidi loop now checks the anchor after each press, held at the start with Shift and following the head without, as walk does for the other cases. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src/features/messages/EditableInput.tsx | 9 ++++++--- src/features/messages/mac-webkit.ts | 7 ++++--- tests/browser/composer-links.spec.mjs | 2 ++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/features/messages/EditableInput.tsx b/src/features/messages/EditableInput.tsx index 8411decacc..93ca85e4e3 100644 --- a/src/features/messages/EditableInput.tsx +++ b/src/features/messages/EditableInput.tsx @@ -81,7 +81,7 @@ import { type ComposerInputElement, } from "./composer-dom"; import { isApplePlatform } from "../shortcuts/format"; -import { macWebKit } from "./mac-webkit"; +import { macWebKit, webKit } from "./mac-webkit"; import { composerLinkUrl } from "./composer-link"; import { messageLinkParts } from "./message-link-parts"; import { updatePlainLinks, type PlainLink } from "./composer-link-edit"; @@ -234,7 +234,10 @@ function crossTokenLine( (index > 0 || !above || level(above) < line.top) && (index < last || !below || level(below) > line.bottom); // WebKit's native move onto a line the token is split across can put - // the caret inside the token, so such a line is entered here too. + // the caret inside the token, so such a line is entered here too. It is + // left natively, which works from either edge. Text beside the token can + // put `previous` on this very line, so a caret already on it is not + // entering. const split = rects.filter((rect) => holds(line, rect)).length > 1; if (!alone && !split) continue; const previous = up @@ -267,7 +270,7 @@ function crossTokenLine( const token = native?.focusNode?.childNodes[native.focusOffset - 1]; if ( !up && - navigator.vendor === "Apple Computer, Inc." && + webKit() && native && next?.nodeType === Node.TEXT_NODE && token instanceof HTMLElement && diff --git a/src/features/messages/mac-webkit.ts b/src/features/messages/mac-webkit.ts index 66c436a821..26cf10252b 100644 --- a/src/features/messages/mac-webkit.ts +++ b/src/features/messages/mac-webkit.ts @@ -1,6 +1,7 @@ +/** WebKit on any platform: Safari, WKWebView and WebKitGTK. */ +export const webKit = () => navigator.vendor === "Apple Computer, Inc."; + /** Desktop Mac WebKit: Safari and Tauri's WKWebView. Desktop-mode iPad also * reports MacIntel, but keeps its touch policies. */ export const macWebKit = () => - navigator.platform === "MacIntel" && - !navigator.maxTouchPoints && - navigator.vendor === "Apple Computer, Inc."; + navigator.platform === "MacIntel" && !navigator.maxTouchPoints && webKit(); diff --git a/tests/browser/composer-links.spec.mjs b/tests/browser/composer-links.spec.mjs index 4ddc840b00..c418d9a0eb 100644 --- a/tests/browser/composer-links.spec.mjs +++ b/tests/browser/composer-links.spec.mjs @@ -810,6 +810,8 @@ test("Up and Down move one line at a time through links that wrap", async ({ for (const index of expected) { await page.keyboard.press(`${shift}${key}`); await expect.poll(async () => line((await caret()).head)).toBe(index); + const { head, anchor } = await caret(); + expect(anchor).toBe(shift ? start : head); await rendered(); } } From 992b66df678256bafb599a7cc5f18fb93fabaefb Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 17:03:54 +1100 Subject: [PATCH 4/8] fix(composer): keep the caret's column when Up or Down steps off a link Stepping off a link that starts its paragraph, Up landed at the end of the line above, just before the line break. Down from a wrapped link that ends its paragraph landed just after the break. Either way a Shift selection grew by nothing but the break, so the press looked like it did nothing and the next press moved the line. In the PR's demo draft, Shift+Up from the end visited 465, 335, 334, 175, 174, 29, 28, 0. crossTokenLine now steps off to the position on the neighbouring line nearest the caret's column, as a browser does between lines of text. It walks that line's positions with coordsAtPos from the near end instead of hit-testing a point, so a line scrolled out of the composer still counts. Each position is measured where a caret there is drawn, so a soft wrap belongs to the line after it. From a link that starts its line, that is the start of the line above, or the end of a link whose tail starts that line. The demo draft now visits 465, 335, 309, 175, 153, 29, 0, one line per press, in WebKit and Chromium alike. The journey's walk now fails a press that crosses only a line break, the first Up walk climbs to the start of the draft, and a new case covers paragraphs that start or end with a wrapped link: Down lands under the link's end, and Up from a paragraph-leading link lands at the previous link's end. Without the fix the walk fails in both engines, first on the existing Up walk and, run alone, on the new case's Down walk. Unchanged: entering a line that only a link holds still stops at the edge it holds, so Shift+Down from the end of the line above a paragraph-leading link, or Shift+Up from the start of the line below a paragraph-ending one, still adds only the break. That line has no caret position at the caret's column. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src/features/messages/EditableInput.tsx | 51 +++++++++++++++---- tests/browser/composer-links.spec.mjs | 67 +++++++++++++++++++++++-- 2 files changed, 105 insertions(+), 13 deletions(-) diff --git a/src/features/messages/EditableInput.tsx b/src/features/messages/EditableInput.tsx index 93ca85e4e3..461c1cb812 100644 --- a/src/features/messages/EditableInput.tsx +++ b/src/features/messages/EditableInput.tsx @@ -167,14 +167,14 @@ function moveHead(editor: EditorView, head: number, extend: boolean) { * caret position besides the token's edge, so native Up and Down stall there * in WebKit and jump to a document edge in Chromium, entering such a line and * leaving it alike. Such a line is entered at the token edge it holds, or - * crossed to the far edge when it holds neither, and left for the nearest - * position on the neighbouring line. The token's client rects give its - * lines, so a line scrolled out of the composer still counts. A caret at a - * soft wrap is measured on the line it leaves: a move may go one line too - * far but never stalls. Every other line move stays native, from a DOM - * caret the browser can move. The Mac's emacs-style Ctrl+N and Ctrl+P line - * moves stay native too, and can still stall on such a line: they are rare - * in a composer and would need a platform key binding here. */ + * crossed to the far edge when it holds neither, and left for the position + * on the neighbouring line nearest the caret's column. The token's client + * rects give its lines, so a line scrolled out of the composer still counts. + * A caret at a soft wrap is measured on the line it leaves: a move may go one + * line too far but never stalls. Every other line move stays native, from a + * DOM caret the browser can move. The Mac's emacs-style Ctrl+N and Ctrl+P + * line moves stay native too, and can still stall on such a line: they are + * rare in a composer and would need a platform key binding here. */ function crossTokenLine( editor: EditorView, tokens: ReadonlyMap, @@ -214,6 +214,33 @@ function crossTokenLine( ? next : pos; }; + // Stepping off a token keeps the caret's column, as a browser does between + // lines: from `edge`, the near end of the neighbouring `line`, the position + // on that line drawn nearest the caret. Landing at the near end instead adds + // only the line break to a Shift selection, which looks like no move at all. + // Positions are measured rather than hit-tested, so a line scrolled out of + // the composer still counts. Each is measured where a caret there is drawn, + // so a soft wrap belongs to the line after it. + const column = ( + edge: number | undefined, + line?: { top: number; bottom: number }, + ) => { + if (edge === undefined) return edge; + const $edge = doc.resolve(edge); + let head = edge; + let gap = Number.POSITIVE_INFINITY; + for ( + let pos = edge; + pos >= $edge.start() && pos <= $edge.end(); + pos += up ? -1 : 1 + ) { + const point = editor.coordsAtPos(pos, 1); + if (!holds(line, point)) break; + const distance = Math.abs(point.left - caret.left); + if (distance < gap) [head, gap] = [pos, distance]; + } + return head; + }; for (const [element, host] of tokens) { const from = host.position(); if (from === undefined) continue; @@ -253,7 +280,13 @@ function crossTokenLine( holds(line) && ((index === 0 && origin === from) || (index === last && origin === to)) ) - head = up ? (index > 0 ? from : before) : index < last ? to : after; + head = up + ? index > 0 + ? from + : column(before, above) + : index < last + ? to + : column(after, below); if (head === undefined || (up ? head >= origin : head <= origin)) continue; moveHead(editor, head, extend); diff --git a/tests/browser/composer-links.spec.mjs b/tests/browser/composer-links.spec.mjs index c418d9a0eb..01a04f1617 100644 --- a/tests/browser/composer-links.spec.mjs +++ b/tests/browser/composer-links.spec.mjs @@ -712,8 +712,12 @@ test("Up and Down move one line at a time through links that wrap", async ({ ), ); // Presses `key` until the head reaches `stop`, returning where each press - // lands. A press that leaves the head in place fails the poll. + // lands. A press that leaves the head in place fails the poll, and so does + // one that crosses nothing but a line break: the caret jumps to the far + // end of a line, and a Shift selection grows by the break alone, which + // looks like no move at all. const walk = async (key, start, stop) => { + const value = await input.evaluate((el) => el.value); await input.evaluate((el, at) => el.setSelectionRange(at, at), start); await rendered(); const heads = []; @@ -725,6 +729,9 @@ test("Up and Down move one line at a time through links that wrap", async ({ .poll(async () => (head = (await caret()).head)) .not.toBe(previous); expect(Math.sign(head - previous)).toBe(key.endsWith("Up") ? -1 : 1); + expect( + value.slice(Math.min(previous, head), Math.max(previous, head)), + ).toMatch(/[^\n]/); expect((await caret()).anchor).toBe( key.startsWith("Shift+") ? start : head, ); @@ -743,17 +750,18 @@ test("Up and Down move one line at a time through links that wrap", async ({ // Across the second link's middle line. expect(down.slice(3)).toEqual([secondTo]); - const up = await walk(`${shift}ArrowUp`, text.length, firstFrom - 1); + const up = await walk(`${shift}ArrowUp`, text.length, 0); const [afterSecond, beforeSecond, afterFirst, ...rest] = up.slice(-5); // The line holding the end of the second link and the words after it. expect(afterSecond).toBeGreaterThanOrEqual(secondTo); expect(afterSecond).toBeLessThanOrEqual(secondTo + secondTail.length); // Across the second link's middle line, then up to the first link's end - // line, onto its own first line, and off it to the line above. + // line, onto its own first line, and off it to the line above at the + // caret's column, its start. expect(beforeSecond).toBe(secondFrom); expect(afterFirst).toBeGreaterThanOrEqual(firstTo); expect(afterFirst).toBeLessThanOrEqual(firstTo + firstTail.length); - expect(rest).toEqual([firstFrom, firstFrom - 1]); + expect(rest).toEqual([firstFrom, 0]); } // A typed line break after the first link leaves its end line to it too. @@ -787,6 +795,57 @@ test("Up and Down move one line at a time through links that wrap", async ({ ); expect((await caret()).head).toBe(firstTo); + // Stepping off a link that starts or ends its paragraph keeps the caret's + // column on the neighbouring line, even where that line starts with the + // tail of another link. + const edges = [ + "Plain words above.", + `${link(title, 3)} ends here.`, + link(title, 4), + "Plain words below, longer than the link's last line.", + ]; + const draft = edges.join("\n"); + await input.fill(""); + await input.fill(draft); + await expect(links).toHaveCount(2); + const thirdFrom = draft.indexOf(link(title, 3)); + const thirdTo = thirdFrom + link(title, 3).length; + const fourthFrom = draft.indexOf(link(title, 4)); + const fourthTo = fourthFrom + link(title, 4).length; + for (const shift of ["", "Shift+"]) { + const down = await walk(`${shift}ArrowDown`, 0, draft.length); + expect(down.slice(0, 4)).toEqual([ + thirdFrom, + thirdTo, + fourthFrom, + fourthTo, + ]); + // Off the fourth link's last line to the line below, under its end. + const [column, ...end] = down.slice(4); + expect(column).toBeGreaterThan(fourthTo + 1); + expect(end).toEqual([draft.length]); + const { x, edge, em } = await input.evaluate((el, at) => { + el.setSelectionRange(at, at); + const fourth = [...el.querySelectorAll("[data-source]")].at(-1); + return { + x: getSelection().getRangeAt(0).getClientRects()[0].left, + edge: [...fourth.getClientRects()].at(-1).right, + em: Number.parseFloat(getComputedStyle(el).fontSize), + }; + }, column); + // The nearest position is within half a glyph, so half an em. + expect(Math.abs(x - edge)).toBeLessThanOrEqual(em / 2); + // Off the fourth link's first line to the third link's end, which is + // where the line above starts. + expect(await walk(`${shift}ArrowUp`, draft.length, 0)).toEqual([ + fourthTo, + fourthFrom, + thirdTo, + thirdFrom, + 0, + ]); + } + // WebKit splits this link into two rects on one line, reordering its // Hebrew with the text after it. That is still one line, and native Down // onto the link stalls there, so each press must still move one line. From 6b4495b5da974febd7d3af8b5071f32c567a5e7a Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 17:31:43 +1100 Subject: [PATCH 5/8] fix(composer): step off a link onto character boundaries only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stepping off a link with Up or Down measured every UTF-16 position on the neighbouring line for the one nearest the caret's column. A position inside a surrogate pair or a grapheme cluster is drawn at its glyph, so it could tie with or beat a real boundary. Up from a link that starts its paragraph, below `😀😀😀 words above`, put the head between the first emoji's surrogates in Chromium and WebKit, and Backspace after Shift+Up left a lone `\ud83d` in the draft. In WebKit, Down onto a line of 👍🏽, ZWJ families or decomposed é landed inside a cluster too. The walk now visits grapheme boundaries only, segmenting the textblock's text with Intl.Segmenter. Each inline leaf reads as one character, so the text's offsets are the textblock's positions. The journey gains a case with an emoji line above a paragraph-leading link and a line of 👍🏽 below it. Up and Shift+Up land at the start, Backspace then leaves the link intact, and Down lands between clusters. It fails without the fix in both engines: in Chromium on Up, which visits offset 1, and in WebKit on Down, mid-👍🏽. The journey also asserts that the third and fourth links wrap, which the expected moves depend on, and pins today's Shift+Down from the end of the line above a paragraph-leading link at the link's start. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src/features/messages/EditableInput.tsx | 22 ++++++++--- tests/browser/composer-links.spec.mjs | 52 ++++++++++++++++++++++--- 2 files changed, 63 insertions(+), 11 deletions(-) diff --git a/src/features/messages/EditableInput.tsx b/src/features/messages/EditableInput.tsx index 461c1cb812..1e1b2e23e4 100644 --- a/src/features/messages/EditableInput.tsx +++ b/src/features/messages/EditableInput.tsx @@ -110,6 +110,7 @@ export type EditorDecoration = { const CONTROL_CHARACTERS = /[\p{Cc}\uF700-\uF747]/gu; /** The control characters typed text holds: tab, newline and carriage return. */ const TYPED_CONTROLS = /[\t\n\r]/u; +const graphemes = new Intl.Segmenter(undefined, { granularity: "grapheme" }); /** True when a native text insertion holds only control characters. Text that * mixes them with real characters is not control text and passes untouched. */ function controlCharacterText(text: string | null | undefined) { @@ -220,20 +221,29 @@ function crossTokenLine( // only the line break to a Shift selection, which looks like no move at all. // Positions are measured rather than hit-tested, so a line scrolled out of // the composer still counts. Each is measured where a caret there is drawn, - // so a soft wrap belongs to the line after it. + // so a soft wrap belongs to the line after it. Only grapheme boundaries + // count: a position inside an emoji is drawn at its glyph, and a Shift + // selection ending there splits the character. Each inline leaf reads as + // one character, so the text's offsets are the textblock's positions. const column = ( edge: number | undefined, line?: { top: number; bottom: number }, ) => { if (edge === undefined) return edge; const $edge = doc.resolve(edge); + const { parent } = $edge; + const text = parent.textBetween( + 0, + parent.content.size, + undefined, + "\uFFFC", + ); + const positions = [...graphemes.segment(text), { index: text.length }] + .map(({ index }) => $edge.start() + index) + .filter((pos) => (up ? pos <= edge : pos >= edge)); let head = edge; let gap = Number.POSITIVE_INFINITY; - for ( - let pos = edge; - pos >= $edge.start() && pos <= $edge.end(); - pos += up ? -1 : 1 - ) { + for (const pos of up ? positions.reverse() : positions) { const point = editor.coordsAtPos(pos, 1); if (!holds(line, point)) break; const distance = Math.abs(point.left - caret.left); diff --git a/tests/browser/composer-links.spec.mjs b/tests/browser/composer-links.spec.mjs index 01a04f1617..03179cd98b 100644 --- a/tests/browser/composer-links.spec.mjs +++ b/tests/browser/composer-links.spec.mjs @@ -681,11 +681,14 @@ test("Up and Down move one line at a time through links that wrap", async ({ await input.fill(text); const links = input.locator('[data-link-kind="github"]'); await expect(links).toHaveCount(2); - const lines = await input - .locator("[data-source]") - .evaluateAll((elements) => - elements.map((element) => element.getClientRects().length), - ); + // How many rects each link draws, more than one where it wraps. + const rects = () => + input + .locator("[data-source]") + .evaluateAll((elements) => + elements.map((element) => element.getClientRects().length), + ); + const lines = await rects(); expect(lines[0]).toBeGreaterThanOrEqual(2); expect(lines[1]).toBeGreaterThanOrEqual(3); expect( @@ -808,6 +811,7 @@ test("Up and Down move one line at a time through links that wrap", async ({ await input.fill(""); await input.fill(draft); await expect(links).toHaveCount(2); + for (const count of await rects()) expect(count).toBeGreaterThanOrEqual(2); const thirdFrom = draft.indexOf(link(title, 3)); const thirdTo = thirdFrom + link(title, 3).length; const fourthFrom = draft.indexOf(link(title, 4)); @@ -846,6 +850,44 @@ test("Up and Down move one line at a time through links that wrap", async ({ ]); } + // Unchanged: entering a line that only a link holds stops at the edge it + // holds, which has no position at the caret's column, so Shift+Down from + // the end of the line above adds only the break. + await input.evaluate( + (el, at) => el.setSelectionRange(at, at), + edges[0].length, + ); + await rendered(); + await page.keyboard.press("Shift+ArrowDown"); + await expect + .poll(caret) + .toEqual({ head: thirdFrom, anchor: edges[0].length }); + + // The nearest position on the neighbouring line is a character boundary, + // never inside an emoji or a cluster, where a caret is drawn at the glyph + // and a Shift selection would split the character. + const thumb = "👍🏽"; + const emoji = ["😀😀😀 words above", link(title, 1), thumb.repeat(20)]; + const emojiDraft = emoji.join("\n"); + await input.fill(""); + await input.fill(emojiDraft); + await expect(links).toHaveCount(1); + expect((await rects())[0]).toBeGreaterThanOrEqual(2); + const emojiFrom = emoji[0].length + 1; + const emojiTo = emojiFrom + emoji[1].length; + for (const shift of ["", "Shift+"]) { + const [below] = await walk( + `${shift}ArrowDown`, + emojiTo, + emojiDraft.length, + ); + expect((below - emojiTo - 1) % thumb.length).toBe(0); + expect(await walk(`${shift}ArrowUp`, emojiFrom, 0)).toEqual([0]); + } + // Deleting that Shift selection leaves no half of the first emoji behind. + await page.keyboard.press("Backspace"); + await expect(input).toHaveJSProperty("value", emojiDraft.slice(emojiFrom)); + // WebKit splits this link into two rects on one line, reordering its // Hebrew with the text after it. That is still one line, and native Down // onto the link stalls there, so each press must still move one line. From 124c7006ffd690dd49ead495d37b437830f09bac Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 17:59:09 +1100 Subject: [PATCH 6/8] fix(composer): select a whole link when Shift enters its own line Entering a line that only a link holds stops at the edge it holds. From just past the line break beside the link, a Shift selection therefore grew by that break alone, which looks like no move at all. With a link pasted on its own line between 'Please review' and 'Thanks', Shift+Up from the start of 'Thanks' visited the link's end, then its start, then 0, so the first press looked dead. Shift+Down from the end of the line above a paragraph-leading link did the same. crossTokenLine now takes the whole link in that case: Shift+Up moves to the link's start and Shift+Down to its end, and the next press goes on from there. Plain Up and Down still stop at the near edge, a real caret position, and Shift from anywhere else on the neighbouring line still stops there too, since the selection then grows by more than the break. The column walk now reads each inline leaf as a newline rather than U+FFFC. A combining mark, variation selector, skin-tone modifier or ZWJ typed after a link joined U+FFFC in one grapheme, dropping the position after the link from the walk; nothing after a newline joins it. Each leaf is still one character, so offsets map 1:1 to positions. Only a bare carriage return directly before a leaf joins a newline, and typed text never holds one: inserted text normalises CR LF. A synced draft could still hold one, and the walk would then skip that one candidate. It still never lands mid-character. The journey replaces the pin on the old Shift+Down and checks plain and Shift moves from just past the break in both directions. It also adds the 'Please review' / link / 'Thanks' draft, where every Shift+Up and Shift+Down press must cross more than a line break. Before the fix, each new walk fails in Chromium and WebKit on a break-only move. The leaf change has no browser case: the two candidates sit on either side of a zero-width mark, so it is checked in Node only. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src/features/messages/EditableInput.tsx | 27 +++++++------ tests/browser/composer-links.spec.mjs | 50 +++++++++++++++++++------ 2 files changed, 54 insertions(+), 23 deletions(-) diff --git a/src/features/messages/EditableInput.tsx b/src/features/messages/EditableInput.tsx index 1e1b2e23e4..eab100ce7e 100644 --- a/src/features/messages/EditableInput.tsx +++ b/src/features/messages/EditableInput.tsx @@ -168,9 +168,10 @@ function moveHead(editor: EditorView, head: number, extend: boolean) { * caret position besides the token's edge, so native Up and Down stall there * in WebKit and jump to a document edge in Chromium, entering such a line and * leaving it alike. Such a line is entered at the token edge it holds, or - * crossed to the far edge when it holds neither, and left for the position - * on the neighbouring line nearest the caret's column. The token's client - * rects give its lines, so a line scrolled out of the composer still counts. + * crossed to the far edge when it holds neither or a Shift selection would + * otherwise grow by the line break alone, and left for the position on the + * neighbouring line nearest the caret's column. The token's client rects give + * its lines, so a line scrolled out of the composer still counts. * A caret at a soft wrap is measured on the line it leaves: a move may go one * line too far but never stalls. Every other line move stays native, from a * DOM caret the browser can move. The Mac's emacs-style Ctrl+N and Ctrl+P @@ -223,8 +224,9 @@ function crossTokenLine( // the composer still counts. Each is measured where a caret there is drawn, // so a soft wrap belongs to the line after it. Only grapheme boundaries // count: a position inside an emoji is drawn at its glyph, and a Shift - // selection ending there splits the character. Each inline leaf reads as - // one character, so the text's offsets are the textblock's positions. + // selection ending there splits the character. Each inline leaf reads as a + // newline, one character that nothing after it can join, so the text's + // offsets are the textblock's positions and the edge after a token counts. const column = ( edge: number | undefined, line?: { top: number; bottom: number }, @@ -232,12 +234,7 @@ function crossTokenLine( if (edge === undefined) return edge; const $edge = doc.resolve(edge); const { parent } = $edge; - const text = parent.textBetween( - 0, - parent.content.size, - undefined, - "\uFFFC", - ); + const text = parent.textBetween(0, parent.content.size, undefined, "\n"); const positions = [...graphemes.segment(text), { index: text.length }] .map(({ index }) => $edge.start() + index) .filter((pos) => (up ? pos <= edge : pos >= edge)); @@ -282,8 +279,14 @@ function crossTokenLine( : (lines[index - 1] ?? above); let head: number | undefined; // Entering: stop at the edge this line holds, or cross a middle line. - if (holds(previous) && !holds(line)) + // From just past the line break beside the token, a Shift selection + // would grow by the break alone, which looks like no move at all, so it + // takes the whole token instead. + if (holds(previous) && !holds(line)) { head = up ? (index === last ? to : from) : index === 0 ? from : to; + if (extend && head !== origin && origin === (up ? after : before)) + head = up ? from : to; + } // Leaving from the edge: cross the rest of the token, or step off it. else if ( alone && diff --git a/tests/browser/composer-links.spec.mjs b/tests/browser/composer-links.spec.mjs index 03179cd98b..8008c1a772 100644 --- a/tests/browser/composer-links.spec.mjs +++ b/tests/browser/composer-links.spec.mjs @@ -850,18 +850,46 @@ test("Up and Down move one line at a time through links that wrap", async ({ ]); } - // Unchanged: entering a line that only a link holds stops at the edge it - // holds, which has no position at the caret's column, so Shift+Down from - // the end of the line above adds only the break. - await input.evaluate( - (el, at) => el.setSelectionRange(at, at), - edges[0].length, + // Entering a line that only a link holds stops at the edge it holds, which + // has no position at the caret's column. From just past the line break + // beside the link, a Shift selection would grow by the break alone, so it + // takes the whole link instead, and the next press goes on from there. + for (const [key, start, near, shifted] of [ + [ + "ArrowDown", + edges[0].length, + thirdFrom, + [thirdTo, fourthFrom, fourthTo], + ], + ["ArrowUp", fourthTo + 1, fourthTo, [fourthFrom, thirdTo, thirdFrom, 0]], + ]) { + await input.evaluate((el, at) => el.setSelectionRange(at, at), start); + await rendered(); + await page.keyboard.press(key); + await expect.poll(caret).toEqual({ head: near, anchor: near }); + const heads = await walk( + `Shift+${key}`, + start, + key === "ArrowUp" ? 0 : draft.length, + ); + expect(heads.slice(0, shifted.length)).toEqual(shifted); + } + + // So does a link pasted on its own line between two lines of text. + const own = ["Please review", link(title, 5), "Thanks"]; + const ownDraft = own.join("\n"); + await input.fill(""); + await input.fill(ownDraft); + await expect(links).toHaveCount(1); + const ownFrom = own[0].length + 1; + const ownTo = ownFrom + own[1].length; + expect(await walk("Shift+ArrowUp", ownTo + 1, 0)).toEqual([ownFrom, 0]); + const [ownDown] = await walk( + "Shift+ArrowDown", + own[0].length, + ownDraft.length, ); - await rendered(); - await page.keyboard.press("Shift+ArrowDown"); - await expect - .poll(caret) - .toEqual({ head: thirdFrom, anchor: edges[0].length }); + expect(ownDown).toBe(ownTo); // The nearest position on the neighbouring line is a character boundary, // never inside an emoji or a cluster, where a caret is drawn at the glyph From fff8e8786aca85e8a4c47f6db25af879825b96e9 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 18:45:35 +1100 Subject: [PATCH 7/8] fix(composer): measure a caret past a newline on its own line Shift+Enter and a text/plain paste put newline characters in one paragraph, where the journey's fill() made a paragraph per line. crossTokenLine measured the Up caret and the line above a link with side -1, and just past a newline that measures the newline itself, drawn at the end of the line above. A caret at the start of a line was taken to be one line higher, so in such a draft, in Chromium and WebKit alike: - Up from the start of the line below a link-only line skipped that line onto the link's end, and Up from just past the break below the link skipped its end for its start. - With an empty line above the link, Down from the line above skipped the empty line onto the link, and Down from the empty line hit the original bug: Chromium jumped to the end and WebKit stalled. - The PR's demo draft, pasted rather than filled, visited 465, 335, 175, 29, 0 on Shift+Up, skipping the words before each link. A position directly after a typed newline or a hard break is now measured with side 1, on its own line, for the caret and for the lines above and below a link. Newline drafts now move as paragraph drafts do, and the demo draft visits 465, 335, 309, 175, 153, 29, 0 either way. Undoing the whole-link Shift press left the break alone selected. After Shift+Up from just past the break below a wrapped link, Shift+Down crossed the rest of the link to its end, one break short of the anchor: the selection looked collapsed and typing joined the link to the next line. Shift+Down then Shift+Up from the line above did the same. Crossing a token toward an anchor just past the break beside it now goes to the anchor. For Up that cross happens entering the link's first line from its last, so the check follows both branches. The journey adds drafts built by a text/plain paste: 'Please review', the link, 'X line' and 'Thanks', and the same with an empty line either side of the link. From either end of every line, a plain press either way must move exactly one visual line, and Shift walks must land on exact heads. walk still fails a press that crosses only a line break, except onto or off an empty line, which holds nothing else. Undoing the whole-link press is checked on the filled own-line draft and on both pasted drafts. With every assertion soft, the journey fails 18 of them at 124c7006f in both engines, covering every new check that the old code breaks. Dropping only the Up caret fix fails the 7 Up checks, only the line-above fix the 6 Down checks, and only the undo rule the 6 reverse presses. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src/features/messages/EditableInput.tsx | 33 +++++-- tests/browser/composer-links.spec.mjs | 123 ++++++++++++++++++++++-- 2 files changed, 139 insertions(+), 17 deletions(-) diff --git a/src/features/messages/EditableInput.tsx b/src/features/messages/EditableInput.tsx index eab100ce7e..3fd77fdc22 100644 --- a/src/features/messages/EditableInput.tsx +++ b/src/features/messages/EditableInput.tsx @@ -199,22 +199,28 @@ function crossTokenLine( ) return false; const origin = extend ? selection.head : up ? selection.from : selection.to; - const caret = editor.coordsAtPos(origin, up ? -1 : 1); + // A line break, a hard break or a typed newline, directly before `pos`. + const broken = (pos: number) => + doc.textBetween(Math.max(pos - 1, 0), pos) === "\n"; + // Where a caret at `pos` is drawn. Side -1 measures the character before, + // which keeps a token's end on the token's line, but past a line break that + // is the break itself, drawn at the end of the line above. + const at = (pos: number, side: -1 | 1) => + editor.coordsAtPos(pos, broken(pos) ? 1 : side); + const caret = at(origin, up ? -1 : 1); const level = (line: { top: number; bottom: number }) => (line.top + line.bottom) / 2; const holds = (line?: { top: number; bottom: number }, point = caret) => !!line && line.top <= level(point) && level(point) <= line.bottom; // The position beside a token that is drawn on the neighbouring line when - // the token starts or ends its own: past a line break (a hard break or a - // typed newline), into the adjacent textblock, or else the token edge. + // the token starts or ends its own: past a line break, into the adjacent + // textblock, or else the token edge. const outside = (pos: number, dir: -1 | 1) => { const $pos = doc.resolve(pos); if (!(dir < 0 ? $pos.nodeBefore : $pos.nodeAfter)) return Selection.findFrom(doc.resolve(pos + dir), dir, true)?.head; const next = pos + dir; - return doc.textBetween(Math.min(pos, next), Math.max(pos, next)) === "\n" - ? next - : pos; + return broken(dir < 0 ? pos : next) ? next : pos; }; // Stepping off a token keeps the caret's column, as a browser does between // lines: from `edge`, the near end of the neighbouring `line`, the position @@ -259,10 +265,8 @@ function crossTokenLine( const last = lines.length - 1; const before = outside(from, -1); const after = outside(to, 1); - const above = - before === undefined ? undefined : editor.coordsAtPos(before, -1); - const below = - after === undefined ? undefined : editor.coordsAtPos(after, 1); + const above = before === undefined ? undefined : at(before, -1); + const below = after === undefined ? undefined : at(after, 1); for (const [index, line] of lines.entries()) { const alone = (index > 0 || !above || level(above) < line.top) && @@ -300,6 +304,15 @@ function crossTokenLine( : index < last ? to : column(after, below); + // Undoing a Shift press that took the whole token from just past the + // line break beside it goes back there: the token's far edge would leave + // the break alone selected. + if ( + extend && + head === (up ? from : to) && + selection.anchor === (up ? before : after) + ) + head = selection.anchor; if (head === undefined || (up ? head >= origin : head <= origin)) continue; moveHead(editor, head, extend); diff --git a/tests/browser/composer-links.spec.mjs b/tests/browser/composer-links.spec.mjs index 8008c1a772..e84317935f 100644 --- a/tests/browser/composer-links.spec.mjs +++ b/tests/browser/composer-links.spec.mjs @@ -714,11 +714,15 @@ test("Up and Down move one line at a time through links that wrap", async ({ requestAnimationFrame(() => requestAnimationFrame(resolve)), ), ); + // An empty line holds nothing but its break, so a press onto or off one + // can cross the break alone. + const empty = (value, pos) => + [value[pos - 1], value[pos]].every((char) => (char ?? "\n") === "\n"); // Presses `key` until the head reaches `stop`, returning where each press // lands. A press that leaves the head in place fails the poll, and so does - // one that crosses nothing but a line break: the caret jumps to the far - // end of a line, and a Shift selection grows by the break alone, which - // looks like no move at all. + // one that crosses nothing but a line break, unless onto or off an empty + // line: the caret jumps to the far end of a line, and a Shift selection + // grows by the break alone, which looks like no move at all. const walk = async (key, start, stop) => { const value = await input.evaluate((el) => el.value); await input.evaluate((el, at) => el.setSelectionRange(at, at), start); @@ -732,9 +736,10 @@ test("Up and Down move one line at a time through links that wrap", async ({ .poll(async () => (head = (await caret()).head)) .not.toBe(previous); expect(Math.sign(head - previous)).toBe(key.endsWith("Up") ? -1 : 1); - expect( - value.slice(Math.min(previous, head), Math.max(previous, head)), - ).toMatch(/[^\n]/); + if (!empty(value, previous) && !empty(value, head)) + expect( + value.slice(Math.min(previous, head), Math.max(previous, head)), + ).toMatch(/[^\n]/); expect((await caret()).anchor).toBe( key.startsWith("Shift+") ? start : head, ); @@ -875,12 +880,13 @@ test("Up and Down move one line at a time through links that wrap", async ({ expect(heads.slice(0, shifted.length)).toEqual(shifted); } - // So does a link pasted on its own line between two lines of text. + // So does a link on its own line between two lines of text. const own = ["Please review", link(title, 5), "Thanks"]; const ownDraft = own.join("\n"); await input.fill(""); await input.fill(ownDraft); await expect(links).toHaveCount(1); + expect((await rects())[0]).toBeGreaterThanOrEqual(2); const ownFrom = own[0].length + 1; const ownTo = ownFrom + own[1].length; expect(await walk("Shift+ArrowUp", ownTo + 1, 0)).toEqual([ownFrom, 0]); @@ -890,6 +896,109 @@ test("Up and Down move one line at a time through links that wrap", async ({ ownDraft.length, ); expect(ownDown).toBe(ownTo); + // Undoing such a press goes back to just past the break, rather than + // leave the break alone selected. + const undo = async (from, to) => { + for (const [key, back, start, edge] of [ + ["Shift+ArrowUp", "Shift+ArrowDown", to + 1, from], + ["Shift+ArrowDown", "Shift+ArrowUp", from - 1, to], + ]) { + await input.evaluate((el, at) => el.setSelectionRange(at, at), start); + await rendered(); + await page.keyboard.press(key); + await expect.poll(caret).toEqual({ head: edge, anchor: start }); + await rendered(); + await page.keyboard.press(back); + await expect.poll(caret).toEqual({ head: start, anchor: start }); + } + }; + await undo(ownFrom, ownTo); + + // fill() makes a paragraph of each line, but a text/plain paste, like + // Shift+Enter, puts newline characters in one paragraph. A newline is + // drawn at the end of its line, apart from a caret just past it. From + // either end of each line of such a draft, a plain press moves exactly + // one visual line. + const pasted = link(title, 6); + const paste = async (lines) => { + const value = lines.join("\n"); + await input.fill(""); + await input.evaluate((el, value) => { + const clipboardData = new DataTransfer(); + clipboardData.setData("text/plain", value); + el.dispatchEvent( + new ClipboardEvent("paste", { + bubbles: true, + cancelable: true, + clipboardData, + }), + ); + }, value); + await expect(input).toHaveJSProperty("value", value); + await expect(input.locator(":scope > p")).toHaveCount(1); + const [count] = await rects(); + expect(count).toBeGreaterThanOrEqual(2); + const from = value.indexOf(pasted); + const to = from + pasted.length; + // The link's start is on its first visual line and its end on its last. + const visual = (head) => + value.slice(0, head).split("\n").length - + 1 + + (head > from ? count - 1 : 0); + const ends = new Set([0, value.length]); + for (const { index } of value.matchAll(/\n/g)) + ends.add(index).add(index + 1); + for (const at of ends) + for (const [key, step] of [ + ["ArrowUp", -1], + ["ArrowDown", 1], + ]) { + const target = visual(at) + step; + if (target < 0 || target > visual(value.length)) continue; + await input.evaluate((el, at) => el.setSelectionRange(at, at), at); + await rendered(); + await page.keyboard.press(key); + await expect + .poll(async () => visual((await caret()).head), { + message: `${key} from ${at}`, + }) + .toBe(target); + } + return { value, from, to }; + }; + { + const lines = ["Please review", pasted, "X line", "Thanks"]; + const { value, from, to } = await paste(lines); + const thanks = value.length - lines.at(-1).length; + expect(await walk("Shift+ArrowUp", thanks, 0)).toEqual([to + 1, from, 0]); + const down = await walk("Shift+ArrowDown", 0, value.length); + expect(down.slice(0, 2)).toEqual([from, to]); + await undo(from, to); + } + { + // An empty line beside the link is a line of its own. + const { value, from, to } = await paste([ + "Please review", + "", + pasted, + "", + "Thanks", + ]); + expect(await walk("Shift+ArrowDown", 0, value.length)).toEqual([ + from - 1, + to, + to + 1, + to + 2, + value.length, + ]); + expect(await walk("Shift+ArrowUp", value.length, 0)).toEqual([ + to + 1, + from, + from - 1, + 0, + ]); + await undo(from, to); + } // The nearest position on the neighbouring line is a character boundary, // never inside an emoji or a cluster, where a caret is drawn at the glyph From 46b0cc63f25cf230c2a01ca03380522a3cf2c6a9 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 19:48:55 +1100 Subject: [PATCH 8/8] fix(composer): settle a Shift Up or Down by the selection it leaves Two Shift-only rules, keyed on where a press started, let a press and its reverse carry the head past the anchor. 124c7006f took a whole link when Shift entered its line from just past the line break beside it, and fff8e8786 sent the reverse press back to the anchor. In Chromium and WebKit, with pasted and filled drafts alike: - 'Please review' / blank / link / blank / 'Thanks': Shift+Down from the link's end, then Shift+Up, selected the whole link backwards instead of collapsing. From the link's start, the mirror did the same. - A link that fits on its own line: Shift+Down from the end of the line above, then Shift+Up, selected 'review' backwards, so typing would replace it. - Two links on consecutive lines: Shift+Down from the end of the first selected only the line break between them, and Shift+Up back then selected the first link whole. - 'Please review' / link / blank / 'Thanks': three Shift+Down presses from the end of the first line, then three Shift+Up, visited 135, 13 and 0 on the way back. One press undid two, and the next overshot the start. Both rules are gone. Once the geometry has picked a head, a Shift press settles it by the selection it leaves: - Toward the anchor, the head may reach the anchor but never pass it. It collapses there rather than leave a lone line break selected: that looks collapsed, and typing would replace the break and join two lines. - Away from the anchor, growing by a lone break onto a token's near edge takes the token, since the break alone looks like no move at all. That also covers stepping off one link onto the next. The two never apply to the same press. Collapsing applies only toward the anchor: from a collapsed caret, a lone break onto an empty line is the whole move, as that line holds nothing else. Plain presses are unchanged. Separately, Chromium draws no rect at a textblock start when the block's text begins with a newline. Measured with side -1 there, the line above a link read as top 0, so Down from an empty first line jumped to the end of the document. A textblock start is now measured with side 1, like a position just past a line break. The journey now checks a Shift press and the opposite press from either end of every line and link of each pasted draft, and of the filled own-line draft in place of `undo`. The anchor must stay put, the head must never pass it, and no lone break may be left selected. New pasted drafts cover a link that fits on one line, two links on consecutive lines, an empty line below the link only (three presses down and back), and an empty first and last line. With every assertion soft, the journey fails 13 assertions at fff8e8786 in Chromium and 11 in WebKit, and none now. Dropping one part at a time, in both engines unless noted: - the anchor clamp: 1 (the one-line link case); - taking the token: 17; - collapsing a lone break: 12; - the block-start side: 3, in Chromium only. Known limitation: press-and-back returns exactly to the start everywhere but on a link that fits on its own line. Shift+Down from its start then Shift+Up, or Shift+Up from its end then Shift+Down, leaves the link selected. Entering that line stops at the edge chosen by direction, not by the caret's column. The head still never passes the anchor. The PR's demo draft still gives 465, 335, 309, 175, 153, 29, 0, 0 on Shift+Up from the end, filled or pasted, in both engines. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src/features/messages/EditableInput.tsx | 54 +++--- tests/browser/composer-links.spec.mjs | 218 +++++++++++++++++------- 2 files changed, 187 insertions(+), 85 deletions(-) diff --git a/src/features/messages/EditableInput.tsx b/src/features/messages/EditableInput.tsx index 3fd77fdc22..fd3f3e4610 100644 --- a/src/features/messages/EditableInput.tsx +++ b/src/features/messages/EditableInput.tsx @@ -168,10 +168,9 @@ function moveHead(editor: EditorView, head: number, extend: boolean) { * caret position besides the token's edge, so native Up and Down stall there * in WebKit and jump to a document edge in Chromium, entering such a line and * leaving it alike. Such a line is entered at the token edge it holds, or - * crossed to the far edge when it holds neither or a Shift selection would - * otherwise grow by the line break alone, and left for the position on the - * neighbouring line nearest the caret's column. The token's client rects give - * its lines, so a line scrolled out of the composer still counts. + * crossed to the far edge when it holds neither, and left for the position on + * the neighbouring line nearest the caret's column. The token's client rects + * give its lines, so a line scrolled out of the composer still counts. * A caret at a soft wrap is measured on the line it leaves: a move may go one * line too far but never stalls. Every other line move stays native, from a * DOM caret the browser can move. The Mac's emacs-style Ctrl+N and Ctrl+P @@ -204,9 +203,13 @@ function crossTokenLine( doc.textBetween(Math.max(pos - 1, 0), pos) === "\n"; // Where a caret at `pos` is drawn. Side -1 measures the character before, // which keeps a token's end on the token's line, but past a line break that - // is the break itself, drawn at the end of the line above. + // is the break itself, drawn at the end of the line above. A textblock start + // has nothing before it, and Chromium draws no rect before a leading newline. const at = (pos: number, side: -1 | 1) => - editor.coordsAtPos(pos, broken(pos) ? 1 : side); + editor.coordsAtPos( + pos, + broken(pos) || !doc.resolve(pos).parentOffset ? 1 : side, + ); const caret = at(origin, up ? -1 : 1); const level = (line: { top: number; bottom: number }) => (line.top + line.bottom) / 2; @@ -254,6 +257,26 @@ function crossTokenLine( } return head; }; + // A Shift press then settles the head by the selection it leaves. Toward + // the anchor, it may reach the anchor but never pass it, and it collapses + // there rather than leave a lone line break selected, which looks collapsed + // and which typing would replace, joining two lines. Away from the anchor, + // growing by a lone break onto a token's near edge looks like no move at + // all, so it takes the token. Collapsing only applies toward the anchor: from + // a collapsed caret, a lone break onto an empty line is the whole move, as + // that line holds nothing else. + const { anchor } = selection; + const lone = (start: number, end: number) => + doc.textBetween(Math.min(start, end), Math.max(start, end), "\n") === "\n"; + const shifted = (head: number) => { + if (up ? origin > anchor : origin < anchor) + return (up ? head <= anchor : head >= anchor) || lone(anchor, head) + ? anchor + : head; + const from = up ? head - 1 : head; + const token = [...tokens.values()].some((host) => host.position() === from); + return token && lone(origin, head) ? (up ? from : from + 1) : head; + }; for (const [element, host] of tokens) { const from = host.position(); if (from === undefined) continue; @@ -283,14 +306,8 @@ function crossTokenLine( : (lines[index - 1] ?? above); let head: number | undefined; // Entering: stop at the edge this line holds, or cross a middle line. - // From just past the line break beside the token, a Shift selection - // would grow by the break alone, which looks like no move at all, so it - // takes the whole token instead. - if (holds(previous) && !holds(line)) { + if (holds(previous) && !holds(line)) head = up ? (index === last ? to : from) : index === 0 ? from : to; - if (extend && head !== origin && origin === (up ? after : before)) - head = up ? from : to; - } // Leaving from the edge: cross the rest of the token, or step off it. else if ( alone && @@ -304,18 +321,9 @@ function crossTokenLine( : index < last ? to : column(after, below); - // Undoing a Shift press that took the whole token from just past the - // line break beside it goes back there: the token's far edge would leave - // the break alone selected. - if ( - extend && - head === (up ? from : to) && - selection.anchor === (up ? before : after) - ) - head = selection.anchor; if (head === undefined || (up ? head >= origin : head <= origin)) continue; - moveHead(editor, head, extend); + moveHead(editor, extend ? shifted(head) : head, extend); return true; } } diff --git a/tests/browser/composer-links.spec.mjs b/tests/browser/composer-links.spec.mjs index e84317935f..20b17264d8 100644 --- a/tests/browser/composer-links.spec.mjs +++ b/tests/browser/composer-links.spec.mjs @@ -718,14 +718,23 @@ test("Up and Down move one line at a time through links that wrap", async ({ // can cross the break alone. const empty = (value, pos) => [value[pos - 1], value[pos]].every((char) => (char ?? "\n") === "\n"); - // Presses `key` until the head reaches `stop`, returning where each press + // Presses `key` from a head at `start`, with a Shift selection's anchor + // at `anchor`, until the head reaches `stop`, returning where each press // lands. A press that leaves the head in place fails the poll, and so does // one that crosses nothing but a line break, unless onto or off an empty // line: the caret jumps to the far end of a line, and a Shift selection // grows by the break alone, which looks like no move at all. - const walk = async (key, start, stop) => { + const walk = async (key, start, stop, anchor = start) => { const value = await input.evaluate((el) => el.value); - await input.evaluate((el, at) => el.setSelectionRange(at, at), start); + await input.evaluate( + (el, [anchor, head]) => + el.setSelectionRange( + Math.min(anchor, head), + Math.max(anchor, head), + head < anchor ? "backward" : "forward", + ), + [anchor, start], + ); await rendered(); const heads = []; let head = start; @@ -741,7 +750,7 @@ test("Up and Down move one line at a time through links that wrap", async ({ value.slice(Math.min(previous, head), Math.max(previous, head)), ).toMatch(/[^\n]/); expect((await caret()).anchor).toBe( - key.startsWith("Shift+") ? start : head, + key.startsWith("Shift+") ? anchor : head, ); heads.push(head); await rendered(); @@ -896,29 +905,99 @@ test("Up and Down move one line at a time through links that wrap", async ({ ownDraft.length, ); expect(ownDown).toBe(ownTo); - // Undoing such a press goes back to just past the break, rather than - // leave the break alone selected. - const undo = async (from, to) => { - for (const [key, back, start, edge] of [ - ["Shift+ArrowUp", "Shift+ArrowDown", to + 1, from], - ["Shift+ArrowDown", "Shift+ArrowUp", from - 1, to], - ]) { - await input.evaluate((el, at) => el.setSelectionRange(at, at), start); - await rendered(); - await page.keyboard.press(key); - await expect.poll(caret).toEqual({ head: edge, anchor: start }); - await rendered(); - await page.keyboard.press(back); - await expect.poll(caret).toEqual({ head: start, anchor: start }); - } + + // The draft's links, and the visual line of a head: the newlines before + // it, plus the lines each link before it wraps onto. A head past a link's + // start is at or past its end, on its last line. + const layout = async () => { + const value = await input.evaluate((el) => el.value); + const counts = await rects(); + const links = [...value.matchAll(/\[[^\]]*\]\([^)]*\)/g)].map( + (match, index) => ({ + from: match.index, + to: match.index + match[0].length, + count: counts[index], + }), + ); + const visual = (head) => + value.slice(0, head).split("\n").length - + 1 + + links.reduce( + (lines, { from, count }) => lines + (head > from ? count - 1 : 0), + 0, + ); + return { value, links, visual }; + }; + // Presses `key` and returns the caret once its head leaves `head`. + const press = async (key, head, message) => { + await page.keyboard.press(key); + let now; + await expect + .poll( + async () => { + now = await caret(); + return now.head; + }, + { message }, + ) + .not.toBe(head); + return now; + }; + // From either end of each line and link, a plain press moves exactly one + // visual line. A Shift press and then the opposite one keep the anchor, + // never carry the head past it, and never leave a lone line break + // selected. Most return to the start, but entering a line that only a + // link holds stops at an edge chosen by direction, not by the caret's + // column, so the second press can stop short of it. + const moves = async ({ value, links, visual }) => { + const ends = new Set([0, value.length]); + for (const { index } of value.matchAll(/\n/g)) + ends.add(index).add(index + 1); + for (const { from, to } of links) ends.add(from).add(to); + for (const at of ends) + for (const [key, back, step] of [ + ["ArrowUp", "ArrowDown", -1], + ["ArrowDown", "ArrowUp", 1], + ]) { + const target = visual(at) + step; + if (target < 0 || target > visual(value.length)) continue; + const message = `${key} from ${at}`; + await input.evaluate((el, at) => el.setSelectionRange(at, at), at); + await rendered(); + await page.keyboard.press(key); + await expect + .poll(async () => visual((await caret()).head), { message }) + .toBe(target); + await input.evaluate((el, at) => el.setSelectionRange(at, at), at); + await rendered(); + const there = await press(`Shift+${key}`, at, `Shift+${message}`); + expect(there.anchor, `Shift+${message}`).toBe(at); + if (!empty(value, at) && !empty(value, there.head)) + expect( + value.slice(Math.min(at, there.head), Math.max(at, there.head)), + `Shift+${message}`, + ).toMatch(/[^\n]/); + await rendered(); + const backMessage = `Shift+${key} then Shift+${back} from ${at}`; + const { head, anchor } = await press( + `Shift+${back}`, + there.head, + backMessage, + ); + expect(anchor, backMessage).toBe(at); + expect(Math.sign(head - at), backMessage).not.toBe(-step); + if (head !== at) + expect( + value.slice(Math.min(at, head), Math.max(at, head)), + backMessage, + ).toMatch(/[^\n]/); + } }; - await undo(ownFrom, ownTo); + await moves(await layout()); // fill() makes a paragraph of each line, but a text/plain paste, like // Shift+Enter, puts newline characters in one paragraph. A newline is - // drawn at the end of its line, apart from a caret just past it. From - // either end of each line of such a draft, a plain press moves exactly - // one visual line. + // drawn at the end of its line, apart from a caret just past it. const pasted = link(title, 6); const paste = async (lines) => { const value = lines.join("\n"); @@ -936,54 +1015,30 @@ test("Up and Down move one line at a time through links that wrap", async ({ }, value); await expect(input).toHaveJSProperty("value", value); await expect(input.locator(":scope > p")).toHaveCount(1); - const [count] = await rects(); - expect(count).toBeGreaterThanOrEqual(2); - const from = value.indexOf(pasted); - const to = from + pasted.length; - // The link's start is on its first visual line and its end on its last. - const visual = (head) => - value.slice(0, head).split("\n").length - - 1 + - (head > from ? count - 1 : 0); - const ends = new Set([0, value.length]); - for (const { index } of value.matchAll(/\n/g)) - ends.add(index).add(index + 1); - for (const at of ends) - for (const [key, step] of [ - ["ArrowUp", -1], - ["ArrowDown", 1], - ]) { - const target = visual(at) + step; - if (target < 0 || target > visual(value.length)) continue; - await input.evaluate((el, at) => el.setSelectionRange(at, at), at); - await rendered(); - await page.keyboard.press(key); - await expect - .poll(async () => visual((await caret()).head), { - message: `${key} from ${at}`, - }) - .toBe(target); - } - return { value, from, to }; + const draft = await layout(); + // Links titled `title` wrap, and shorter ones do not. + for (const { from, to, count } of draft.links) + expect(count > 1).toBe(value.slice(from, to).includes(title)); + await moves(draft); + return draft; }; { const lines = ["Please review", pasted, "X line", "Thanks"]; - const { value, from, to } = await paste(lines); + const { + value, + links: [{ from, to }], + } = await paste(lines); const thanks = value.length - lines.at(-1).length; expect(await walk("Shift+ArrowUp", thanks, 0)).toEqual([to + 1, from, 0]); const down = await walk("Shift+ArrowDown", 0, value.length); expect(down.slice(0, 2)).toEqual([from, to]); - await undo(from, to); } { // An empty line beside the link is a line of its own. - const { value, from, to } = await paste([ - "Please review", - "", - pasted, - "", - "Thanks", - ]); + const { + value, + links: [{ from, to }], + } = await paste(["Please review", "", pasted, "", "Thanks"]); expect(await walk("Shift+ArrowDown", 0, value.length)).toEqual([ from - 1, to, @@ -997,8 +1052,47 @@ test("Up and Down move one line at a time through links that wrap", async ({ from - 1, 0, ]); - await undo(from, to); } + { + // With an empty line below the link alone, three Shift presses down + // from the line above and three back retrace each other, collapsing at + // the start rather than undoing two presses at once. + const { + links: [{ from, to }], + } = await paste(["Please review", pasted, "", "Thanks"]); + expect(await walk("Shift+ArrowDown", from - 1, to + 2)).toEqual([ + to, + to + 1, + to + 2, + ]); + expect(await walk("Shift+ArrowUp", to + 2, from - 1, from - 1)).toEqual([ + to + 1, + to, + from - 1, + ]); + } + // A link short enough not to wrap still holds its line alone. + await paste(["Please review", link("PR 7", 7), "Thanks"]); + { + // On consecutive lines, each Shift press across two links takes a whole + // one, rather than the line break between them. + const { + value, + links: [first, second], + } = await paste(["PRs:", link(title, 8), link(title, 9), "Thanks"]); + expect( + (await walk("Shift+ArrowDown", 0, value.length)).slice(0, 3), + ).toEqual([first.from, first.to, second.to]); + expect(await walk("Shift+ArrowUp", value.length, 0)).toEqual([ + second.to, + second.from, + first.from, + 0, + ]); + } + // A draft that starts with an empty line: Chromium draws no rect before + // its leading newline. + await paste(["", pasted, ""]); // The nearest position on the neighbouring line is a character boundary, // never inside an emoji or a cluster, where a caret is drawn at the glyph