Skip to content

Commit e29312b

Browse files
committed
Bugfix: ⌘A in ByteSeek-no-index mode no longer freezes the viewer
- `selectionAnnouncement`'s per-line loop ran `for (let i = start.line + 1; i < end.line; i++)`. In ByteSeek-no-index mode the ⌘A path sets `focus.line = Number.MAX_SAFE_INTEGER` (the sentinel that maps to `RangeEnd::Eof` at the IPC boundary), so the loop iterated 9e15 times and froze the viewer for many seconds. - Extracted the announcement logic to a pure `describeSelectionForAt(sel, getLineLength)` helper in `selection.svelte.ts`. New `MAX_ANNOUNCE_LINES = 10_000` cap: past that, returns `"Selected from line N to the end of the file"` without touching the line-length lookup at all. 10k lines is plenty for an actionable AT announcement; ⌘A on a typical log file lands well under. - The page wires the helper through `$derived`. Same announcement format as before; the only behavioural change is the cap. - 8 new tests cover: null selection, caret-only, single-line, multi-line, the `Number.MAX_SAFE_INTEGER` sentinel (asserts the lookup is never called), exact-cap boundary, reversed-selection symmetry, and missing-line-length graceful degradation.
1 parent 4464f76 commit e29312b

3 files changed

Lines changed: 120 additions & 22 deletions

File tree

apps/desktop/src/routes/viewer/+page.svelte

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import { handleNavigationKey, handleToggleKey } from './viewer-keyboard'
2929
import {
3030
createViewerSelection,
31+
describeSelectionForAt,
3132
estimateSelectionBytes,
3233
getLineSegmentBounds,
3334
normaliseSelection,
@@ -151,28 +152,9 @@
151152
* there's nothing selected (the live region stays silent). The format names the
152153
* selected line range and a UTF-16 character count for orientation.
153154
*/
154-
const selectionAnnouncement = $derived.by((): string => {
155-
const sel = selection.selection
156-
if (sel === null) return ''
157-
const { start, end } = normaliseSelection(sel)
158-
if (start.line === end.line && start.offset === end.offset) return ''
159-
160-
const totalChars = (() => {
161-
if (start.line === end.line) return end.offset - start.offset
162-
const startLineLen = scroll.lineCache.get(start.line)?.length ?? 0
163-
let chars = startLineLen - start.offset
164-
for (let i = start.line + 1; i < end.line; i++) {
165-
chars += scroll.lineCache.get(i)?.length ?? 0
166-
}
167-
chars += end.offset
168-
return chars
169-
})()
170-
171-
if (start.line === end.line) {
172-
return `Selected ${String(totalChars)} characters on line ${String(start.line + 1)}`
173-
}
174-
return `Selected lines ${String(start.line + 1)} to ${String(end.line + 1)}, ${String(totalChars)} characters`
175-
})
155+
const selectionAnnouncement = $derived(
156+
describeSelectionForAt(selection.selection, (line) => scroll.lineCache.get(line)?.length ?? null),
157+
)
176158
177159
/**
178160
* Converts a `Selection` to the `(anchor, focus)` `RangeEnd`s the IPC layer accepts.

apps/desktop/src/routes/viewer/selection.svelte.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,52 @@ export function makeSelectAll(totalLines: number, lastLineLength: number): Selec
138138
}
139139
}
140140

141+
/**
142+
* Maximum number of intermediate lines the AT (VoiceOver) announcement loop walks
143+
* before falling back to a generic "extends past visible content" message. Caps the
144+
* 9e15-line worst case from ⌘A in ByteSeek-no-index mode (where `focus.line` is set
145+
* to `Number.MAX_SAFE_INTEGER`).
146+
*/
147+
export const MAX_ANNOUNCE_LINES = 10_000
148+
149+
/**
150+
* Builds the live-region announcement string for the current selection. Pure: takes
151+
* a selection and a per-line length lookup, returns the string the screen reader will
152+
* speak. Empty string means "nothing to announce".
153+
*
154+
* Caps the line-span at `MAX_ANNOUNCE_LINES`; past that, returns a generic message
155+
* so the announcement work stays bounded (the alternative would freeze the UI on
156+
* ⌘A in ByteSeek-no-index mode where the focus line is `Number.MAX_SAFE_INTEGER`).
157+
*/
158+
export function describeSelectionForAt(sel: Selection | null, getLineLength: (line: number) => number | null): string {
159+
if (sel === null) return ''
160+
const { start, end } = normaliseSelection(sel)
161+
if (start.line === end.line && start.offset === end.offset) return ''
162+
163+
const lineSpan = end.line - start.line
164+
if (lineSpan > MAX_ANNOUNCE_LINES) {
165+
return `Selected from line ${String(start.line + 1)} to the end of the file`
166+
}
167+
168+
let totalChars: number
169+
if (start.line === end.line) {
170+
totalChars = end.offset - start.offset
171+
} else {
172+
const startLineLen = getLineLength(start.line) ?? 0
173+
let chars = startLineLen - start.offset
174+
for (let i = start.line + 1; i < end.line; i++) {
175+
chars += getLineLength(i) ?? 0
176+
}
177+
chars += end.offset
178+
totalChars = chars
179+
}
180+
181+
if (start.line === end.line) {
182+
return `Selected ${String(totalChars)} characters on line ${String(start.line + 1)}`
183+
}
184+
return `Selected lines ${String(start.line + 1)} to ${String(end.line + 1)}, ${String(totalChars)} characters`
185+
}
186+
141187
/**
142188
* Shift-click extension: returns a new selection that runs from the current selection's
143189
* anchor (or `point` if there's no current selection) to `point`. Caller-owned

apps/desktop/src/routes/viewer/selection.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@ import { describe, it, expect } from 'vitest'
22

33
import {
44
compareLineOffset,
5+
describeSelectionForAt,
56
estimateSelectionBytes,
67
extendSelection,
78
getLineSegmentBounds,
89
isEmpty,
910
isLineInRange,
1011
lineOffsetEquals,
1112
makeSelectAll,
13+
MAX_ANNOUNCE_LINES,
1214
normaliseSelection,
1315
type Selection,
1416
} from './selection.svelte'
@@ -324,3 +326,71 @@ describe('estimateSelectionBytes', () => {
324326
)
325327
})
326328
})
329+
330+
describe('describeSelectionForAt', () => {
331+
// Test-local lookups for the line-length argument.
332+
const allOnes = (): number | null => 1
333+
const empty = (): number | null => null
334+
335+
it('null selection returns empty string', () => {
336+
expect(describeSelectionForAt(null, empty)).toBe('')
337+
})
338+
339+
it('caret-only (start == end) returns empty string', () => {
340+
const sel: Selection = { anchor: { line: 3, offset: 4 }, focus: { line: 3, offset: 4 } }
341+
expect(describeSelectionForAt(sel, empty)).toBe('')
342+
})
343+
344+
it('single-line selection: announces character count and line number (1-indexed)', () => {
345+
const sel: Selection = { anchor: { line: 4, offset: 2 }, focus: { line: 4, offset: 7 } }
346+
expect(describeSelectionForAt(sel, allOnes)).toBe('Selected 5 characters on line 5')
347+
})
348+
349+
it('multi-line selection: announces line range and total chars', () => {
350+
// Lines 0..3, each "hello" (5 chars). Select from (0,2) to (3,3):
351+
// line 0 contributes "llo" (3), line 1 + 2 each contribute 5, line 3 contributes 3. Total 16.
352+
const sel: Selection = { anchor: { line: 0, offset: 2 }, focus: { line: 3, offset: 3 } }
353+
const getLen = (n: number) => (n >= 0 && n < 4 ? 5 : null)
354+
expect(describeSelectionForAt(sel, getLen)).toBe('Selected lines 1 to 4, 16 characters')
355+
})
356+
357+
it('ByteSeek-no-index ⌘A sentinel: line span > MAX_ANNOUNCE_LINES falls back to generic message', () => {
358+
// The ⌘A path sets focus.line = Number.MAX_SAFE_INTEGER when totalLines is null.
359+
// The pure function must not iterate 9e15 times.
360+
const sel: Selection = {
361+
anchor: { line: 0, offset: 0 },
362+
focus: { line: Number.MAX_SAFE_INTEGER, offset: 0 },
363+
}
364+
let calls = 0
365+
const counting = () => {
366+
calls++
367+
return 1
368+
}
369+
expect(describeSelectionForAt(sel, counting)).toBe('Selected from line 1 to the end of the file')
370+
// The fallback path must not invoke the line-length lookup at all.
371+
expect(calls).toBe(0)
372+
})
373+
374+
it('line span exactly at the cap (MAX_ANNOUNCE_LINES) still itemises', () => {
375+
const sel: Selection = {
376+
anchor: { line: 0, offset: 0 },
377+
focus: { line: MAX_ANNOUNCE_LINES, offset: 0 },
378+
}
379+
// Just verify we don't fall back; the exact char count isn't the point.
380+
const result = describeSelectionForAt(sel, allOnes)
381+
expect(result.startsWith('Selected lines')).toBe(true)
382+
})
383+
384+
it('reversed selection: same result as the normalised version', () => {
385+
const forward: Selection = { anchor: { line: 1, offset: 0 }, focus: { line: 3, offset: 2 } }
386+
const reversed: Selection = { anchor: { line: 3, offset: 2 }, focus: { line: 1, offset: 0 } }
387+
expect(describeSelectionForAt(forward, allOnes)).toBe(describeSelectionForAt(reversed, allOnes))
388+
})
389+
390+
it('missing line length in lookup contributes 0 to the count (degrades gracefully)', () => {
391+
const sel: Selection = { anchor: { line: 0, offset: 0 }, focus: { line: 2, offset: 0 } }
392+
const result = describeSelectionForAt(sel, empty)
393+
// line 0 contributes (0 - 0) = 0, intermediate line 1 contributes 0, line 2 contributes 0.
394+
expect(result).toBe('Selected lines 1 to 3, 0 characters')
395+
})
396+
})

0 commit comments

Comments
 (0)