From ea10994820e5c2d56cec4100a6c2eb59fe3b3493 Mon Sep 17 00:00:00 2001 From: mmarabel <166927047+mmarabel@users.noreply.github.com> Date: Thu, 22 Oct 2026 04:13:12 +0200 Subject: [PATCH] fix(terminal): link a framed multi-row URL when an overlay shows through (#21816) * fix(terminal): link a framed multi-row URL when an overlay shows through A TUI that draws a box-framed modal over existing output leaves the screen underneath visible beside the frame, so the text left of the URL differs from row to row. The hard-wrap detector required that text to be byte-identical across rows, so the join stopped at the first row whose gutter differed and the candidate collapsed back to a single row, opening a truncated URL. Anchor continuation rows on the frame's cell column and the run between the frame and the URL instead, which is the part that is actually shared. * refactor(terminal): drop the redundant scheme column guard The start row's fragment start column is re-derived per row, so guard that value where it is used instead of asserting it non-null. * fix(terminal): reject shared-table URL joins * test(terminal): isolate shifted URL frame guard * fix(terminal): keep joining framed URLs whose row prefix is unchanged Rows whose text left of the URL matches the start row are joined as before; only a changed prefix uses the overlay frame anchor and its table-cell guard. Restores nested-frame and overlay-over-frame wraps. * Verify framed overlay URL hover and cold clicks with real terminal buffers * Remove retired Portuguese agent-version translation * Preserve translation exports in queued-message editor tests * Run CI after resolving the upstream merge conflict --------- Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> Co-authored-by: Neil --- ...framed-overlay-terminal-http-links.test.ts | 109 ++++++++++++++++++ .../hard-wrapped-terminal-http-links.test.ts | 105 +++++++++++++++++ .../hard-wrapped-terminal-http-links.ts | 68 +++++++++-- 3 files changed, 272 insertions(+), 10 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/framed-overlay-terminal-http-links.test.ts diff --git a/src/renderer/src/components/terminal-pane/framed-overlay-terminal-http-links.test.ts b/src/renderer/src/components/terminal-pane/framed-overlay-terminal-http-links.test.ts new file mode 100644 index 00000000000..72bfdcd5892 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/framed-overlay-terminal-http-links.test.ts @@ -0,0 +1,109 @@ +import { Terminal } from '@xterm/headless' +import { Unicode11Addon } from '@xterm/addon-unicode11' +import type { ILink } from '@xterm/xterm' +import { describe, expect, it, vi } from 'vitest' +import { createWrappedTerminalHttpLinkProvider } from './wrapped-terminal-http-link-provider' +import { openHttpLinkAtBufferPosition } from './terminal-url-link-hit-testing' +import { registerHttpLinkStoreAccessor } from '@/lib/http-link-routing' + +const URL_TEXT = `https://example.com/docs/public-overlay?${'part%20value&'.repeat(7)}view=full#details` +const CONTENT_WIDTH = 42 + +function overlayRows(gutters: string[]) { + return Array.from( + { length: Math.ceil(URL_TEXT.length / CONTENT_WIDTH) }, + (_, index) => + `${gutters[index]}│ ${URL_TEXT.slice(index * CONTENT_WIDTH, (index + 1) * CONTENT_WIDTH).padEnd(CONTENT_WIDTH)} │` + ) +} + +async function terminalWithRows(rows: string[]) { + const terminal = new Terminal({ cols: 80, rows: 10, allowProposedApi: true }) + terminal.loadAddon(new Unicode11Addon()) + terminal.unicode.activeVersion = '11' + await new Promise((resolve) => terminal.write(rows.join('\r\n'), resolve)) + const provider = createWrappedTerminalHttpLinkProvider(terminal, { activate: vi.fn() }) + return { + terminal, + linksAt: (row: number) => + new Promise((resolve) => provider.provideLinks(row, resolve)) + } +} + +describe('framed overlay terminal HTTP links', () => { + it.each([ + { name: 'ASCII', gutters: [' ', 'todo ', ' in ', 'Final '] }, + { name: 'wide', gutters: [' ', '你你你', ' in ', 'Final '] }, + { name: 'surrogate', gutters: [' ', '😀😀😀', ' in ', 'Final '] }, + { name: 'combining', gutters: [' ', 'ééé ', ' in ', 'Final '] } + ])('hovers and cold-opens the full URL over a changing $name gutter', async ({ gutters }) => { + const { terminal, linksAt } = await terminalWithRows(overlayRows(gutters)) + const openUrl = vi.fn() + vi.stubGlobal('window', { api: { shell: { openUrl } } }) + registerHttpLinkStoreAccessor(() => ({ + settings: { openLinksInApp: false }, + setActiveWorktree: vi.fn(), + createBrowserTab: vi.fn() + })) + try { + const rowCount = Math.ceil(URL_TEXT.length / CONTENT_WIDTH) + const deps = { worktreeId: 'test', modifierHeld: true } + for (let y = 1; y <= rowCount; y++) { + const endX = 8 + Math.min(CONTENT_WIDTH, URL_TEXT.length - (y - 1) * CONTENT_WIDTH) + expect((await linksAt(y))?.[0], `row ${y}`).toMatchObject({ + text: URL_TEXT, + range: { start: { x: 9, y }, end: { x: endX, y } } + }) + for (const x of [1, 7, 8, endX + 1, 52]) { + expect(openHttpLinkAtBufferPosition(terminal.buffer.active, { x, y }, 80, deps)).toBe( + false + ) + } + expect(openHttpLinkAtBufferPosition(terminal.buffer.active, { x: 9, y }, 80, deps)).toBe( + true + ) + } + expect(openUrl.mock.calls).toEqual(Array.from({ length: rowCount }, () => [URL_TEXT])) + } finally { + terminal.dispose() + vi.unstubAllGlobals() + } + }) + + it.each(['\x07', '\x1b\\'])( + 'preserves explicit OSC 8 targets terminated with %j', + async (end) => { + const target = 'https://example.org/explicit-destination' + const rows = overlayRows([' ', 'todo ', ' in ', 'Final ']).map( + (row) => + `${row.slice(0, 8)}\x1b]8;;${target}${end}${row.slice(8, -2)}\x1b]8;;${end}${row.slice(-2)}` + ) + const { terminal, linksAt } = await terminalWithRows(rows) + try { + for (let y = 1; y <= rows.length; y++) { + expect(await linksAt(y)).toBeUndefined() + expect( + openHttpLinkAtBufferPosition(terminal.buffer.active, { x: 9, y }, 80, { + worktreeId: 'test', + modifierHeld: true + }) + ).toBe(false) + } + } finally { + terminal.dispose() + } + } + ) + + it('does not join URLs to subsequent cells of a table with changing labels', async () => { + const rows = overlayRows(['│ One ', '│ Two ', '│ Red ', '│ End ']) + const { terminal, linksAt } = await terminalWithRows(rows) + try { + for (let y = 1; y <= rows.length; y++) { + expect(await linksAt(y)).toBeUndefined() + } + } finally { + terminal.dispose() + } + }) +}) diff --git a/src/renderer/src/components/terminal-pane/hard-wrapped-terminal-http-links.test.ts b/src/renderer/src/components/terminal-pane/hard-wrapped-terminal-http-links.test.ts index f38f3bda8bc..8c82d2652e0 100644 --- a/src/renderer/src/components/terminal-pane/hard-wrapped-terminal-http-links.test.ts +++ b/src/renderer/src/components/terminal-pane/hard-wrapped-terminal-http-links.test.ts @@ -93,6 +93,111 @@ describe('hard-wrapped terminal HTTP candidate bounds', () => { expect(candidates[0]?.text).toBe('http://a/aba你你你你你tailzzzzzzz') }) + it('reconstructs a framed URL while the screen underneath shows through left of the frame', () => { + const inner = 'http://example.com/aaaa' + const rows = [ + ` |${inner}|`, + `todo |${'b'.repeat(inner.length)}|`, + ` in |${'c'.repeat(inner.length)}|`, + `Final |${'dddd'.padEnd(inner.length, ' ')}|` + ].map(bufferLineWithCellColumns) + + const candidates = buildHardWrappedHttpLogicalLineCandidates( + { getLine: (y) => rows[y] }, + rows.length + ) + + expect(candidates[0]?.text).toBe( + `${inner}${'b'.repeat(inner.length)}${'c'.repeat(inner.length)}dddd` + ) + }) + + it('reconstructs a URL in a nested frame whose left label repeats on every row', () => { + const inner = 'http://example.com/aaaa' + const rows = [ + `│ label │ ${inner} │`, + `│ label │ ${'b'.repeat(inner.length)} │`, + `│ label │ ${'c'.repeat(inner.length)} │`, + `│ label │ ${'dddd'.padEnd(inner.length, ' ')} │` + ].map(bufferLineWithCellColumns) + + const candidates = buildHardWrappedHttpLogicalLineCandidates( + { getLine: (y) => rows[y] }, + rows.length + ) + + expect(candidates[0]?.text).toBe( + `${inner}${'b'.repeat(inner.length)}${'c'.repeat(inner.length)}dddd` + ) + }) + + it('reconstructs an overlay URL when a box border underneath shows at a fixed column', () => { + const inner = 'http://example.com/aaaa' + const rows = [ + `│ │ ${inner} │`, + `│ │ ${'b'.repeat(inner.length)} │`, + `│ │ ${'c'.repeat(inner.length)} │`, + `│ │ ${'dddd'.padEnd(inner.length, ' ')} │` + ].map(bufferLineWithCellColumns) + + const candidates = buildHardWrappedHttpLogicalLineCandidates( + { getLine: (y) => rows[y] }, + rows.length + ) + + expect(candidates[0]?.text).toBe( + `${inner}${'b'.repeat(inner.length)}${'c'.repeat(inner.length)}dddd` + ) + }) + + it('does not join adjacent cells from a shared framed table', () => { + const rows = [ + '│ Label │ https://example.com/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa │', + '│ Other │ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb │', + '│ Third │ cccccccccccccccccccccccccccccccccccccccccccccccccc │' + ].map(bufferLineWithCellColumns) + + expect( + buildHardWrappedHttpLogicalLineCandidates({ getLine: (y) => rows[y] }, rows.length) + ).toEqual([]) + }) + + it('aligns continuation rows by the frame cell column when a gutter holds wide characters', () => { + const inner = 'http://example.com/aaaa' + const rows = [ + ` |${inner}|`, + `你你你|${'b'.repeat(inner.length)}|`, + ` in |${'c'.repeat(inner.length)}|`, + `Final |${'dddd'.padEnd(inner.length, ' ')}|` + ].map(bufferLineWithCellColumns) + + const candidates = buildHardWrappedHttpLogicalLineCandidates( + { getLine: (y) => rows[y] }, + rows.length + ) + + expect(candidates[0]?.text).toBe( + `${inner}${'b'.repeat(inner.length)}${'c'.repeat(inner.length)}dddd` + ) + }) + + it('stops joining when the frame moves to another column', () => { + const inner = 'http://example.com/aaaa' + const rows = [ + ` |${inner}|`, + `todo |${'b'.repeat(inner.length)}|`, + ` in |${'c'.repeat(inner.length)}|`, + `Final |${'dddd'.padEnd(inner.length - 1, ' ')}|` + ].map(bufferLineWithCellColumns) + + const candidates = buildHardWrappedHttpLogicalLineCandidates( + { getLine: (y) => rows[y] }, + rows.length + ) + + expect(candidates).toEqual([]) + }) + it('reconstructs a maximum-length wide-character URL beyond the ASCII row bound', () => { const scheme = 'http://' const continuation = '你'.repeat(TERMINAL_HTTP_URL_MAX_LENGTH - scheme.length) diff --git a/src/renderer/src/components/terminal-pane/hard-wrapped-terminal-http-links.ts b/src/renderer/src/components/terminal-pane/hard-wrapped-terminal-http-links.ts index 30b1d3a9f3c..3099272b02b 100644 --- a/src/renderer/src/components/terminal-pane/hard-wrapped-terminal-http-links.ts +++ b/src/renderer/src/components/terminal-pane/hard-wrapped-terminal-http-links.ts @@ -16,6 +16,39 @@ const MIN_HARD_WRAP_FILL_RATIO = 0.8 type TranslatedLine = ReturnType +function findFrameIndexBefore(text: string, beforeIndex: number): number { + for (let index = beforeIndex - 1; index >= 0; index--) { + if (VERTICAL_LAYOUT_FRAME_PATTERN.test(text[index]!)) { + return index + } + } + return -1 +} + +function hasFrameAtColumn(translated: TranslatedLine, frameColumn: number): boolean { + const frameIndex = translated.columns.indexOf(frameColumn) + return frameIndex !== -1 && VERTICAL_LAYOUT_FRAME_PATTERN.test(translated.text[frameIndex] ?? '') +} + +// Overlay rows share the frame column and inner padding; the uncovered gutter can differ. +function findOverlayFragmentStart( + translated: TranslatedLine, + frameColumn: number, + outerFrameColumn: number | null, + framedPrefix: string +): number { + // A second stable left frame under a changed prefix denotes adjacent table cells. + if (outerFrameColumn !== null && hasFrameAtColumn(translated, outerFrameColumn)) { + return -1 + } + const frameIndex = translated.columns.indexOf(frameColumn) + if (frameIndex === -1) { + return -1 + } + const prefixEnd = frameIndex + framedPrefix.length + return translated.text.slice(frameIndex, prefixEnd) === framedPrefix ? prefixEnd : -1 +} + function buildCandidateFromStart( buffer: { getLine(y: number): IBufferLine | undefined }, startY: number, @@ -34,12 +67,19 @@ function buildCandidateFromStart( } const translatedStart = cachedStart ?? translateLineWithColumns(startLine) translatedLines.set(startY, translatedStart) - const schemeColumn = translatedStart.columns[schemeIndex] - if (schemeColumn === undefined) { + const frameIndex = findFrameIndexBefore(translatedStart.text, schemeIndex) + if (frameIndex === -1) { return null } - - const continuationPrefix = translatedStart.text.slice(0, schemeIndex) + const frameColumn = translatedStart.columns[frameIndex] + if (frameColumn === undefined) { + return null + } + const outerFrameIndex = findFrameIndexBefore(translatedStart.text, frameIndex) + const outerFrameColumn = + outerFrameIndex === -1 ? null : (translatedStart.columns[outerFrameIndex] ?? null) + const rowPrefix = translatedStart.text.slice(0, schemeIndex) + const framedPrefix = translatedStart.text.slice(frameIndex, schemeIndex) let text = '' let rightFrameColumn: number | null = null let previousRowCanContinue = true @@ -59,15 +99,23 @@ function buildCandidateFromStart( ? translatedStart : (translatedLines.get(rowY) ?? translateLineWithColumns(line)) translatedLines.set(rowY, translated) - if (rowY > startY && translated.text.slice(0, schemeIndex) !== continuationPrefix) { + const fragmentStart = + rowY === startY || translated.text.slice(0, schemeIndex) === rowPrefix + ? schemeIndex + : findOverlayFragmentStart(translated, frameColumn, outerFrameColumn, framedPrefix) + if (fragmentStart === -1) { + break + } + const fragmentStartColumn = translated.columns[fragmentStart] + if (fragmentStartColumn === undefined) { break } - const fragment = translated.text.slice(schemeIndex).match(HTTP_FRAGMENT_PATTERN)?.[0] ?? '' + const fragment = translated.text.slice(fragmentStart).match(HTTP_FRAGMENT_PATTERN)?.[0] ?? '' if (!fragment || (rowY > startY && HTTP_SCHEME_START_PATTERN.test(fragment))) { break } - const fragmentEnd = schemeIndex + fragment.length + const fragmentEnd = fragmentStart + fragment.length const layoutSuffix = translated.text.slice(fragmentEnd) const rightFrameOffset = layoutSuffix.search(VERTICAL_LAYOUT_FRAME_PATTERN) const currentRightFrameIndex = rightFrameOffset === -1 ? -1 : fragmentEnd + rightFrameOffset @@ -90,15 +138,15 @@ function buildCandidateFromStart( y: rowY, text: fragment, sourceText: translated.text, - columns: translated.columns.slice(schemeIndex, fragmentEnd + 1), + columns: translated.columns.slice(fragmentStart, fragmentEnd + 1), startIndex: text.length, isWrapped: line.isWrapped, lineLength: line.length }) text += fragment - const contentWidth = currentRightFrameColumn - schemeColumn - const fragmentWidth = translated.columns[fragmentEnd]! - schemeColumn + const contentWidth = currentRightFrameColumn - fragmentStartColumn + const fragmentWidth = translated.columns[fragmentEnd]! - fragmentStartColumn const fillsRow = contentWidth > 0 && fragmentWidth / contentWidth >= MIN_HARD_WRAP_FILL_RATIO if (rowY === startY) { startRowFilled = fillsRow