diff --git a/apps/readest-app/.claude/memory/MEMORY.md b/apps/readest-app/.claude/memory/MEMORY.md index 526ecbb0..ad55ee70 100644 --- a/apps/readest-app/.claude/memory/MEMORY.md +++ b/apps/readest-app/.claude/memory/MEMORY.md @@ -63,6 +63,7 @@ - [Design system → DESIGN.md](feedback_design_system_doc.md) — codify recurring UI/UX rules in `apps/readest-app/DESIGN.md`; never `pl/pr/ml/mr/text-left/text-right` (RTL); §5 boxed list anatomy has uniform `min-h-14` rows and chromeless controls ## Reader UI Fixes +- [Overlayer splitRange by text nodes](overlayer-splitrange-textnodes.md) — highlight SVG missed bullet-list text when range also touched a `

`: `#splitRangeByParagraph`'s `'p,h1-h4'` selector dropped `li` (3rd whack-a-mole after f087826/920676b); fix = walk text nodes + `img,svg` in overlayer.js, never block-tag selectors; jsdom test stubs `Range.prototype.getClientRects` - [Android image callout freeze](android-image-callout-freeze.md) — long-press `` fires WebView native callout that collides with app touch handlers → whole-app freeze; `-webkit-touch-callout: none` doesn't inherit so put `.no-context-menu` on an ancestor of the image (`.no-context-menu img` rule in globals.css); seen on book covers (#4345) + image preview/zoom (#4420, `ImageViewer.tsx`) - [ProgressBar focus-ring line (#4397)](progressbar-focus-ring-4397.md) — decorative `.progressinfo` footer was `tabIndex={-1}` → Android long-press focused it → stray content-width focus-ring line at the bottom every page; fix = drop tabIndex (role='presentation' must not be focusable); ffmpeg-the-video debugging + live-browser `:focus-visible` confirmation - [Table dark-mode tint regression (#4419)](table-dark-mode-tint-4419.md) — `blockquote, table *` color-mix tint in `getColorStyles` must stay gated on `overrideColor` (gate added #2377, removed #4055, re-broke → #4419); safe now that #4392 light-bg rewriters handle #4028 zebra legibility; SAME rule paints vertical-TOC `.space`/▉ spacer cells (▉ U+2589 = blank glyph, contours=0) → "spacing changes" symptom; both fixed by the gate diff --git a/apps/readest-app/.claude/memory/overlayer-splitrange-textnodes.md b/apps/readest-app/.claude/memory/overlayer-splitrange-textnodes.md new file mode 100644 index 00000000..fb535371 --- /dev/null +++ b/apps/readest-app/.claude/memory/overlayer-splitrange-textnodes.md @@ -0,0 +1,16 @@ +--- +name: overlayer-splitrange-textnodes +description: "Overlayer highlight rects — split range by TEXT NODES, not a block-tag selector; li/blockquote text was silently dropped from SVG when highlight also touched a

" +metadata: + node_type: memory + type: project + originSessionId: 58d4b1a1-8823-4474-9966-c96727692e3f +--- + +Bug class (3rd occurrence): `Overlayer` in `packages/foliate-js/overlayer.js` splits a range into sub-ranges before `getClientRects()` so fully-contained block elements don't contribute border boxes (over-highlighting blank space — fork commit f087826 #10). The split was `querySelectorAll('p, h1, h2, h3, h4')`; 920676b added headings after the same hole; June 2026 the hole reappeared for `

  • `: a highlight spanning paragraphs + a bullet list drew NO rects over the list (li text fell into no sub-range), while a highlight entirely inside the list worked via the `splitRanges.length === 0 → [range]` fallback. + +**Fix:** `#splitRange` walks TEXT NODES (TreeWalker, `FILTER_REJECT` on non-intersecting elements prunes subtrees) plus replaced elements (`img, svg`), clipping first/last text nodes to the range boundaries. Text-node line rects never include block border boxes, cover every block type, and can't double-paint (`li > p` nesting). Shared `#getRects(range)` used by both `add()` and `redraw()`. + +**Why:** any hard-coded block-tag selector is whack-a-mole (li, blockquote, dd, td, div-paragraphs…) and adding container tags double-paints nested matched tags. + +**How to apply:** when highlight/overlay rects miss some content or over-paint blank space, check `#splitRange` in overlayer.js first. Test at `src/__tests__/document/overlayer-highlight-blocks.test.ts` — jsdom has no `Range.getClientRects`; stub the prototype to record `range.toString()` per call and assert covered text. Dev-web picked up the symlinked foliate-js edit on page reload (no server restart needed for next dev, unlike [[paginator-gutter-bleed-asymmetry-4394]]'s note). diff --git a/apps/readest-app/src/__tests__/document/overlayer-highlight-blocks.test.ts b/apps/readest-app/src/__tests__/document/overlayer-highlight-blocks.test.ts new file mode 100644 index 00000000..8f9e713c --- /dev/null +++ b/apps/readest-app/src/__tests__/document/overlayer-highlight-blocks.test.ts @@ -0,0 +1,211 @@ +// Regression test for: highlights spanning a bullet list are not drawn over +// the list items. +// +// Overlayer splits a range into sub-ranges before collecting client rects so +// that fully-contained block elements don't contribute their border boxes +// (which over-highlight blank space between lines/paragraphs). The split was +// keyed on a hard-coded selector ('p, h1, h2, h3, h4'), so any text living in +// other block containers — li, blockquote, dd, td, ... — fell into no +// sub-range and was silently dropped from the drawn SVG rects whenever the +// range also touched a matching paragraph/heading. +// +// The fix splits by text nodes (plus replaced elements like img/svg), which +// covers every block type and still never yields block border boxes. +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { Overlayer } from 'foliate-js/overlayer.js'; + +interface RectLike { + left: number; + top: number; + right: number; + bottom: number; + width: number; + height: number; +} + +// Records what content each getClientRects() call covered, so assertions can +// check which parts of the document contribute rects to the overlay. +let covered: string[] = []; + +const describeRange = (range: Range): string => { + const text = range.toString(); + const frag = range.cloneContents(); + return frag.querySelector('img, svg') ? `${text}` : text; +}; + +function stubGetClientRects(this: Range): DOMRectList { + covered.push(describeRange(this)); + const i = covered.length; + const rect: RectLike = { + left: 0, + top: i * 20, + right: 100, + bottom: i * 20 + 10, + width: 100, + height: 10, + }; + return [rect] as unknown as DOMRectList; +} + +const realGetClientRects = Object.getOwnPropertyDescriptor(Range.prototype, 'getClientRects'); + +const drawSpy = () => { + const drawn: RectLike[] = []; + const draw = (rects: RectLike[]) => { + drawn.push(...rects); + return document.createElementNS('http://www.w3.org/2000/svg', 'g'); + }; + return { drawn, draw }; +}; + +const byId = (id: string) => { + const el = document.getElementById(id); + if (!el) throw new Error(`missing #${id}`); + return el; +}; + +const textNode = (id: string) => { + const node = byId(id).firstChild; + if (!node || node.nodeType !== Node.TEXT_NODE) throw new Error(`#${id} has no text node`); + return node as Text; +}; + +describe('Overlayer rect splitting across block types', () => { + beforeEach(() => { + covered = []; + Object.defineProperty(Range.prototype, 'getClientRects', { + value: stubGetClientRects, + configurable: true, + writable: true, + }); + }); + + afterEach(() => { + document.body.innerHTML = ''; + if (realGetClientRects) { + Object.defineProperty(Range.prototype, 'getClientRects', realGetClientRects); + } else { + delete (Range.prototype as { getClientRects?: unknown }).getClientRects; + } + }); + + it('draws rects for list items when a highlight spans paragraphs and a bullet list', () => { + document.body.innerHTML = ` +
    +

    The final role of taxes is to encourage certain behaviors.

    +

    To summarize, taxes have four functions:

    +
      +
    • Make people accept the currency.
      • +
    • Redistribute income.
      • +
    • Reduce consumption to fight inflation.
      • +
    • Deter undesirable behaviors.
      • +
    +
    `; + const range = document.createRange(); + range.setStart(textNode('p1'), 0); + range.setEnd(textNode('li4'), textNode('li4').length); + + const { drawn, draw } = drawSpy(); + const overlayer = new Overlayer(document); + overlayer.add('annotation', range, draw); + + const text = covered.join(''); + expect(text).toContain('To summarize, taxes have four functions:'); + expect(text).toContain('Make people accept the currency.'); + expect(text).toContain('Redistribute income.'); + expect(text).toContain('Reduce consumption to fight inflation.'); + expect(text).toContain('Deter undesirable behaviors.'); + expect(drawn.length).toBeGreaterThan(0); + + // The range must stay split per block: a single getClientRects call over + // the whole range would include block border boxes and over-highlight the + // blank space between paragraphs (the reason the split exists). + for (const chunk of covered) { + expect(chunk.includes('To summarize') && chunk.includes('Make people')).toBe(false); + } + }); + + it('does not double-draw text of paragraphs nested inside list items', () => { + document.body.innerHTML = ` +
    +

    Intro paragraph.

    +
      +

    • Paragraph inside a list item.

      • +
    +
    `; + const range = document.createRange(); + range.setStart(textNode('p1'), 0); + range.setEnd(textNode('nested'), textNode('nested').length); + + const { draw } = drawSpy(); + const overlayer = new Overlayer(document); + overlayer.add('annotation', range, draw); + + const hits = covered.filter((t) => t.includes('Paragraph inside a list item.')); + expect(hits).toHaveLength(1); + }); + + it('covers other text blocks such as blockquotes', () => { + document.body.innerHTML = ` +
    +

    Before the quote.

    +
    Quoted text in a blockquote.
    +

    After the quote.

    +
    `; + const range = document.createRange(); + range.setStart(textNode('p1'), 0); + range.setEnd(textNode('p2'), textNode('p2').length); + + const { draw } = drawSpy(); + const overlayer = new Overlayer(document); + overlayer.add('annotation', range, draw); + + const text = covered.join(''); + expect(text).toContain('Before the quote.'); + expect(text).toContain('Quoted text in a blockquote.'); + expect(text).toContain('After the quote.'); + }); + + it('clips the first and last blocks to the range boundaries', () => { + document.body.innerHTML = ` +
    +

    To summarize, taxes have four functions:

    +
      +
    • Make people accept the currency.
      • +
    +
    `; + const start = textNode('p1'); + const end = textNode('li1'); + const range = document.createRange(); + range.setStart(start, 'To summarize, taxes have '.length); + range.setEnd(end, 'Make people'.length); + + const { draw } = drawSpy(); + const overlayer = new Overlayer(document); + overlayer.add('annotation', range, draw); + + const chunks = covered.filter((t) => t.trim() !== ''); + expect(chunks[0]).toBe('four functions:'); + expect(chunks[chunks.length - 1]).toBe('Make people'); + }); + + it('still draws a rect for replaced elements like images inside the range', () => { + document.body.innerHTML = ` +
    +

    Before the image. After the image.

    +
    `; + const p = byId('p1'); + const range = document.createRange(); + range.selectNodeContents(p); + + const { draw } = drawSpy(); + const overlayer = new Overlayer(document); + overlayer.add('annotation', range, draw); + + expect(covered.some((t) => t.startsWith(''))).toBe(true); + const text = covered.join(''); + expect(text).toContain('Before the image.'); + expect(text).toContain('After the image.'); + }); +}); diff --git a/packages/foliate-js b/packages/foliate-js index a281b5db..1309db7e 160000 --- a/packages/foliate-js +++ b/packages/foliate-js @@ -1 +1 @@ -Subproject commit a281b5dbcbe55d73383efc77b7ebb003fbc21ca5 +Subproject commit 1309db7e8ce35d89f23e9da741b2faf3aa59b19c