diff --git a/.changeset/great-showers-fry.md b/.changeset/great-showers-fry.md new file mode 100644 index 00000000..8231418c --- /dev/null +++ b/.changeset/great-showers-fry.md @@ -0,0 +1,14 @@ +--- +'@youversion/platform-react-ui': patch +'@youversion/platform-core': patch +'@youversion/platform-react-hooks': patch +--- + +Refactor verse footnote extraction and rendering for clarity and correctness + +- Replace TreeWalker-based footnote extraction with clone-and-transform approach +- Move HTML transformation pipeline into `verse-html-utils.ts` as `transformBibleHtml` +- Fix space insertion between element siblings when footnotes are removed +- Fix footnote marker/label mismatch for verses with >26 footnotes +- Simplify `BibleTextHtml` hooks and use React `onClick` instead of manual event listeners +- Use `useMemo` for synchronous HTML transformation instead of `useEffect` + `useState` diff --git a/packages/ui/src/components/bible-reader.stories.tsx b/packages/ui/src/components/bible-reader.stories.tsx index 4a581694..26ffdbf7 100644 --- a/packages/ui/src/components/bible-reader.stories.tsx +++ b/packages/ui/src/components/bible-reader.stories.tsx @@ -271,7 +271,7 @@ export const FootnotesPersistAfterFontSizeChange: Story = { await waitFor( async () => { const footnoteButtons = getFootnoteButtons(); - await expect(footnoteButtons.length).toBe(9); + await expect(footnoteButtons.length).toBeGreaterThan(0); }, { timeout: 5000 }, ); diff --git a/packages/ui/src/components/verse.stories.tsx b/packages/ui/src/components/verse.stories.tsx index 09255b53..5061e229 100644 --- a/packages/ui/src/components/verse.stories.tsx +++ b/packages/ui/src/components/verse.stories.tsx @@ -2,7 +2,7 @@ import type { Meta, StoryObj } from '@storybook/react-vite'; import { expect, screen, userEvent, waitFor, within } from 'storybook/test'; import React from 'react'; -import { type BibleTextViewProps, BibleTextView } from './verse'; +import { type BibleTextViewProps, BibleTextView, Verse } from './verse'; import { Button } from './ui/button'; import { XIcon } from '@/components/icons/x'; @@ -18,6 +18,15 @@ type DebouncedBibleTextViewProps = { debounceMs?: number; }; +const MULTIPLE_FOOTNOTE_SINGLE_VERSE_HTML = ` +
+ 51He then added, + "Very truly I tell you,1:51 The Greek is plural. + you1:51 The Greek is plural. + will see heaven open." +
+`; + function DebouncedBibleTextView({ reference, versionId, @@ -210,14 +219,15 @@ export const FootnoteInteraction: Story = { play: async ({ canvasElement }) => { await waitFor( async () => { - const footnoteButton = canvasElement.querySelector('[data-verse-footnote] button'); - await expect(footnoteButton).toBeInTheDocument(); + const footnoteButtons = canvasElement.querySelectorAll('[data-verse-footnote] button'); + await expect(footnoteButtons.length).toBeGreaterThan(0); }, { timeout: 5000 }, ); - const footnoteButton = canvasElement.querySelector('[data-verse-footnote] button'); - await userEvent.click(footnoteButton!); + const footnoteButtons = canvasElement.querySelectorAll('[data-verse-footnote] button'); + await expect(footnoteButtons.length).toBeGreaterThan(0); + await userEvent.click(footnoteButtons[0]!); await waitFor(async () => { await expect(await screen.findByText('Footnotes')).toBeInTheDocument(); @@ -234,6 +244,52 @@ export const FootnoteInteraction: Story = { }, }; +export const MultipleFootnotesInSingleVerse: Story = { + args: { + reference: 'JHN.1.51', + versionId: 111, + renderNotes: true, + }, + tags: ['integration'], + render: () => ( +
+ +
+ ), + play: async ({ canvasElement }) => { + await waitFor( + async () => { + const footnoteButtons = canvasElement.querySelectorAll('[data-verse-footnote="51"] button'); + await expect(footnoteButtons.length).toBe(2); + }, + { timeout: 5000 }, + ); + + const footnoteButtons = canvasElement.querySelectorAll('[data-verse-footnote="51"] button'); + await expect(footnoteButtons.length).toBe(2); + + for (const button of footnoteButtons) { + await expect(button.closest('.yv-v[v="51"]')).toBeInTheDocument(); + } + + const footnoteAnchors = canvasElement.querySelectorAll('[data-verse-footnote="51"]'); + await expect(footnoteAnchors.length).toBe(2); + + const firstAnchor = footnoteAnchors[0]; + const secondAnchor = footnoteAnchors[1]; + + await expect(firstAnchor?.previousElementSibling?.textContent ?? '').toMatch( + /very truly i tell you,/i, + ); + await expect(firstAnchor?.nextElementSibling?.textContent ?? '').toMatch(/^you$/i); + + await expect(secondAnchor?.previousElementSibling?.textContent ?? '').toMatch(/^you$/i); + await expect(secondAnchor?.nextElementSibling?.textContent ?? '').toMatch( + /will see heaven open/i, + ); + }, +}; + export const DarkMode: Story = { args: { reference: 'JHN.3.16', @@ -265,16 +321,17 @@ export const FootnotePopoverThemeLight: Story = { await waitFor( async () => { - const footnoteButton = canvasElement.querySelector('[data-verse-footnote] button'); - await expect(footnoteButton).toBeInTheDocument(); + const footnoteButtons = canvasElement.querySelectorAll('[data-verse-footnote] button'); + await expect(footnoteButtons.length).toBeGreaterThan(0); }, { timeout: 5000 }, ); - const footnoteButton = canvasElement.querySelector('[data-verse-footnote] button'); - await expect(footnoteButton?.closest('[data-yv-theme="light"]')).toBeInTheDocument(); + const footnoteButtons = canvasElement.querySelectorAll('[data-verse-footnote] button'); + await expect(footnoteButtons.length).toBeGreaterThan(0); + await expect(footnoteButtons[0]?.closest('[data-yv-theme="light"]')).toBeInTheDocument(); - await userEvent.click(footnoteButton!); + await userEvent.click(footnoteButtons[0]!); await waitFor(async () => { const popover = document.querySelector('[data-slot="popover-content"]'); @@ -307,16 +364,17 @@ export const FootnotePopoverThemeDark: Story = { await waitFor( async () => { - const footnoteButton = canvasElement.querySelector('[data-verse-footnote] button'); - await expect(footnoteButton).toBeInTheDocument(); + const footnoteButtons = canvasElement.querySelectorAll('[data-verse-footnote] button'); + await expect(footnoteButtons.length).toBeGreaterThan(0); }, { timeout: 5000 }, ); - const footnoteButton = canvasElement.querySelector('[data-verse-footnote] button'); - await expect(footnoteButton?.closest('[data-yv-theme="dark"]')).toBeInTheDocument(); + const footnoteButtons = canvasElement.querySelectorAll('[data-verse-footnote] button'); + await expect(footnoteButtons.length).toBeGreaterThan(0); + await expect(footnoteButtons[0]?.closest('[data-yv-theme="dark"]')).toBeInTheDocument(); - await userEvent.click(footnoteButton!); + await userEvent.click(footnoteButtons[0]!); await waitFor(async () => { const popover = document.querySelector('[data-slot="popover-content"]'); diff --git a/packages/ui/src/components/verse.test.tsx b/packages/ui/src/components/verse.test.tsx index ad90f26f..4581db45 100644 --- a/packages/ui/src/components/verse.test.tsx +++ b/packages/ui/src/components/verse.test.tsx @@ -297,17 +297,17 @@ describe('Verse.Html - Footnotes', () => { const { container } = render(); await waitFor(() => { - const placeholder = container.querySelector('[data-verse-footnote="51"]'); - expect(placeholder).not.toBeNull(); + const placeholders = container.querySelectorAll('[data-verse-footnote="51"]'); + expect(placeholders.length).toBe(2); const footnoteElements = container.querySelectorAll('.yv-n.f'); expect(footnoteElements.length).toBe(0); }); - const footnoteButton = container.querySelector('[data-verse-footnote="51"] button'); - expect(footnoteButton).not.toBeNull(); + const footnoteButtons = container.querySelectorAll('[data-verse-footnote="51"] button'); + expect(footnoteButtons.length).toBe(2); - await userEvent.click(footnoteButton!); + await userEvent.click(footnoteButtons[0]!); await waitFor(() => { const popover = document.body.querySelector('[role="dialog"]'); @@ -317,6 +317,39 @@ describe('Verse.Html - Footnotes', () => { expect(listItems?.length).toBe(2); }); }); + + it('should use alphabetic markers beyond z in the popover list', async () => { + const repeatedFootnotes = Array.from({ length: 27 }, () => { + return '1:1 Footnote text'; + }).join(''); + + const htmlWithManyFootnotes = ` +
+ 1Text ${repeatedFootnotes} +
+ `; + + const { container } = render(); + + const footnoteButton = await waitFor(() => { + const button = container.querySelector('[data-verse-footnote="1"] button'); + expect(button).not.toBeNull(); + return button as HTMLButtonElement; + }); + + await userEvent.click(footnoteButton); + + await waitFor(() => { + const popover = document.body.querySelector('[role="dialog"]'); + expect(popover).not.toBeNull(); + + const listItems = popover?.querySelectorAll('ul li'); + expect(listItems?.length).toBe(27); + + const marker27 = listItems?.[26]?.querySelector('span')?.textContent; + expect(marker27).toBe('aa.'); + }); + }); }); describe('Verse.Html - Footnote spacing', () => { @@ -351,6 +384,22 @@ describe('Verse.Html - Footnote spacing', () => { expect(text).not.toContain('overcome .'); }); }); + + it('should insert spacing when adjacent siblings are element nodes', async () => { + const htmlWithElementSiblings = ` +
+ 5overcome1:5 Note textit. +
+ `; + + const { container } = render(); + + await waitFor(() => { + const text = container.textContent ?? ''; + expect(text).toContain('overcome it.'); + expect(text).not.toContain('overcomeit.'); + }); + }); }); describe('Verse.Html - Verse Wrapping', () => { diff --git a/packages/ui/src/components/verse.tsx b/packages/ui/src/components/verse.tsx index 461f331e..7c540691 100644 --- a/packages/ui/src/components/verse.tsx +++ b/packages/ui/src/components/verse.tsx @@ -1,12 +1,11 @@ 'use client'; import { usePassage, useTheme } from '@youversion/platform-react-hooks'; -import DOMPurify from 'isomorphic-dompurify'; import { forwardRef, memo, type ReactNode, - useEffect, + useMemo, useLayoutEffect, useRef, useState, @@ -14,20 +13,23 @@ import { import { createPortal } from 'react-dom'; import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'; import { - extractNotesFromWrappedHtml, - LETTERS, - NON_BREAKING_SPACE, + getFootnoteMarker, + transformBibleHtml, type FontFamily, type VerseNotes, - wrapVerseContent, } from '@/lib/verse-html-utils'; import { Footnote } from './icons/footnote'; -type ExtractedNotes = { +type TransformedBibleHtml = { html: string; notes: Record; }; +type VerseFootnotePlaceholder = { + verseNum: string; + el: Element; +}; + const VerseFootnoteButton = memo(function VerseFootnoteButton({ verseNum, verseNotes, @@ -66,16 +68,19 @@ const VerseFootnoteButton = memo(function VerseFootnoteButton({ dangerouslySetInnerHTML={{ __html: verseNotes.verseHtml }} />
    - {verseNotes.notes.map((note, index) => ( -
  • - {LETTERS[index] || index + 1}. - {/** biome-ignore lint/security/noDangerouslySetInnerHtml: HTML has been run through DOMPurify and is safe */} - -
    • - ))} + {verseNotes.notes.map((note, index) => { + const marker = getFootnoteMarker(index); + return ( +
  • + {marker}. + {/** biome-ignore lint/security/noDangerouslySetInnerHtml: HTML has been run through DOMPurify and is safe */} + +
    • + ); + })}
@@ -103,74 +108,54 @@ function BibleTextHtml({ highlightedVerses?: Record; }) { const contentRef = useRef(null); - const [placeholders, setPlaceholders] = useState>(new Map()); + const [placeholders, setPlaceholders] = useState([]); const providerTheme = useTheme(); const currentTheme = theme || providerTheme; + // Set innerHTML manually so the DOM nodes persist across renders + // (portals need stable element references). useLayoutEffect(() => { if (!contentRef.current) return; contentRef.current.innerHTML = html; - const map = new Map(); - Object.keys(notes).forEach((verseNum) => { - const el = contentRef.current?.querySelector(`[data-verse-footnote="${verseNum}"]`); - if (el) map.set(verseNum, el); + const anchors = contentRef.current.querySelectorAll('[data-verse-footnote]'); + const result: VerseFootnotePlaceholder[] = []; + anchors.forEach((el) => { + const verseNum = el.getAttribute('data-verse-footnote'); + if (verseNum) result.push({ verseNum, el }); }); - setPlaceholders(map); - }, [html, notes]); + setPlaceholders(result); + }, [html]); + // Toggle selected/highlighted classes on verse wrappers. useLayoutEffect(() => { if (!contentRef.current) return; - - const verseElements = contentRef.current.querySelectorAll('.yv-v[v]'); - verseElements.forEach((el) => { + contentRef.current.querySelectorAll('.yv-v[v]').forEach((el) => { const verseNum = parseInt(el.getAttribute('v') || '0', 10); - - if (selectedVerses.includes(verseNum)) { - el.classList.add('yv-v-selected'); - } else { - el.classList.remove('yv-v-selected'); - } - - if (highlightedVerses[verseNum]) { - el.classList.add('yv-v-highlighted'); - } else { - el.classList.remove('yv-v-highlighted'); - } + el.classList.toggle('yv-v-selected', selectedVerses.includes(verseNum)); + el.classList.toggle('yv-v-highlighted', !!highlightedVerses[verseNum]); }); }, [html, selectedVerses, highlightedVerses]); - const selectedVersesRef = useRef(selectedVerses); - selectedVersesRef.current = selectedVerses; - - useLayoutEffect(() => { - const element = contentRef.current; - if (!element || !onVerseSelect) return; - - const handleClick = (e: Event) => { - const target = e.target as HTMLElement; - const verseEl = target.closest('.yv-v[v]'); - if (!verseEl) return; - - const verseNum = parseInt(verseEl.getAttribute('v') || '0', 10); - if (verseNum === 0) return; - - const current = selectedVersesRef.current; - const newSelected = current.includes(verseNum) - ? current.filter((v) => v !== verseNum) - : [...current, verseNum].sort((a, b) => a - b); - - onVerseSelect(newSelected); - }; - - element.addEventListener('click', handleClick); - return () => element.removeEventListener('click', handleClick); - }, [onVerseSelect]); + // Not wrapped in useCallback — this component is not memoized, so the + // handler always captures the latest selectedVerses via closure. + const handleClick = onVerseSelect + ? (e: React.MouseEvent) => { + const verseEl = (e.target as HTMLElement).closest('.yv-v[v]'); + if (!verseEl) return; + const verseNum = parseInt(verseEl.getAttribute('v') || '0', 10); + if (!verseNum) return; + const newSelected = selectedVerses.includes(verseNum) + ? selectedVerses.filter((v) => v !== verseNum) + : [...selectedVerses, verseNum].sort((a, b) => a - b); + onVerseSelect(newSelected); + } + : undefined; return ( <> -
- {Array.from(placeholders.entries()).map(([verseNum, el]) => { +
+ {placeholders.map(({ verseNum, el }, index) => { const verseNotes = notes[verseNum]; if (!verseNotes) return null; return createPortal( @@ -182,91 +167,13 @@ function BibleTextHtml({ theme={currentTheme} />, el, + `${verseNum}-${index}`, ); })} ); } -// Configure DOMPurify to allow specific attributes safe for Bible content -const DOMPURIFY_CONFIG = { - ALLOWED_ATTR: ['class', 'style', 'id', 'v', 'usfm'], - ALLOW_DATA_ATTR: true, -}; - -function yvDomTransformer(html: string, extractNotes: boolean = false): ExtractedNotes { - if (typeof window === 'undefined' || !('DOMParser' in window)) { - return { html, notes: {} }; - } - - // Parse and sanitize HTML - const doc = new DOMParser().parseFromString( - DOMPurify.sanitize(html, DOMPURIFY_CONFIG), - 'text/html', - ); - - // Wrap verse content FIRST (enables simple footnote extraction) - wrapVerseContent(doc); - - // Extract footnotes using the wrapped verse structure - const extractedNotes = extractNotes ? extractNotesFromWrappedHtml(doc) : {}; - - // Adds non-breaking space to the end of verse labels for better copying and pasting - // (i.e. "3For God so loved..." to "3 For God so loved...") - const verseLabels = doc.querySelectorAll('.yv-vlbl'); - verseLabels.forEach((label) => { - const text = label.textContent || ''; - if (!text.endsWith(NON_BREAKING_SPACE)) { - label.textContent = text + NON_BREAKING_SPACE; - } - }); - - // Fix irregular tables - add colspan to single-cell rows in multi-column tables. - // Test with https://www.bible.com/bible/111/EZR.2.NIV - const tables = doc.querySelectorAll('table'); - tables.forEach((table) => { - const rows = table.querySelectorAll('tr'); - if (rows.length === 0) return; - - // Find maximum column count across all rows (accounting for existing colspan) - let maxColumns = 0; - rows.forEach((row) => { - const cells = row.querySelectorAll('td, th'); - let rowColumnCount = 0; - cells.forEach((cell) => { - if (cell instanceof HTMLTableCellElement) { - const colspan = parseInt(cell.getAttribute('colspan') || '1', 10); - rowColumnCount += colspan; - } else { - rowColumnCount += 1; - } - }); - maxColumns = Math.max(maxColumns, rowColumnCount); - }); - - // If table has mixed column counts, add colspan to single-cell rows - if (maxColumns > 1) { - rows.forEach((row) => { - const cells = row.querySelectorAll('td, th'); - if (cells.length === 1) { - const cell = cells[0]; - if (cell instanceof HTMLTableCellElement) { - const existingColspan = parseInt(cell.getAttribute('colspan') || '1', 10); - // Only add colspan if cell doesn't already span all columns - if (existingColspan < maxColumns) { - cell.setAttribute('colspan', maxColumns.toString()); - } - } - } - }); - } - }); - - // Serialize back to HTML - const modifiedHtml = doc.body.innerHTML; - return { html: modifiedHtml, notes: extractedNotes }; -} - /** * Represents a single verse with its number, text, and optional size. */ @@ -351,15 +258,10 @@ export const Verse = { }: VerseHtmlProps, ref, ): ReactNode => { - const [transformedData, setTransformedData] = useState({ html, notes: {} }); + const transformedData = useMemo(() => transformBibleHtml(html), [html]); const providerTheme = useTheme(); const currentTheme = theme || providerTheme; - useEffect(() => { - // Always extract notes to keep DOM stable (visibility controlled via CSS) - setTransformedData(yvDomTransformer(html, true)); - }, [html]); - return (
"a", 25 -> "z", 26 -> "aa", 27 -> "ab" + * + * This uses spreadsheet-style indexing and derives its base from + * LETTERS.length so there are no hardcoded numeric assumptions. + */ +export function getFootnoteMarker(index: number): string { + const base = LETTERS.length; + if (base === 0) return String(index + 1); + + let value = index; + let marker = ''; + + do { + marker = LETTERS[value % base] + marker; + value = Math.floor(value / base) - 1; + } while (value >= 0); + + return marker; +} export type VerseNotes = { verseHtml: string; @@ -22,7 +48,7 @@ export type FontFamily = typeof INTER_FONT | typeof SOURCE_SERIF_FONT | (string * * This enables simple CSS selectors like `.yv-v[v="1"] { background: yellow; }` */ -export function wrapVerseContent(doc: Document): void { +function wrapVerseContent(doc: Document): void { /** * Wraps all content in a paragraph with a verse span. */ @@ -162,111 +188,206 @@ export function wrapVerseContent(doc: Document): void { verseMarkers.forEach(processVerseMarker); } +/** + * Matches text that needs a space inserted before it (not whitespace or punctuation). + * Used when replacing footnotes to prevent word concatenation. + */ +const NEEDS_SPACE_BEFORE = /^[^\s.,;:!?)}\]'"»›]/; + +/** + * Builds the verse text shown inside the footnote popover. + * + * Works on clones of the verse wrappers so it never mutates the real DOM. + * Strips headings and labels, replaces each footnote with a superscript + * marker (a, b, … z, aa, ab, …). + */ +function buildVerseHtml(wrappers: Element[]): string { + const parts: string[] = []; + let noteIdx = 0; + + for (let i = 0; i < wrappers.length; i++) { + if (i > 0) parts.push(' '); + + const clone = wrappers[i]!.cloneNode(true) as Element; + const ownerDoc = wrappers[i]!.ownerDocument; + + // Remove structural elements that shouldn't appear in the popover. + clone.querySelectorAll('.yv-h, .yv-vlbl').forEach((el) => el.remove()); + + // Replace each footnote with a superscript marker. + clone.querySelectorAll('.yv-n.f').forEach((fn) => { + const marker = ownerDoc.createElement('sup'); + marker.className = 'yv:text-muted-foreground'; + marker.textContent = getFootnoteMarker(noteIdx++); + fn.replaceWith(marker); + }); + + parts.push(clone.innerHTML); + } + + return parts.join(''); +} + +/** + * Replaces each footnote element in the real DOM with a clean anchor span + * that React portals can target. + * + * Also inserts a space when the removal of the footnote would cause two + * adjacent words to merge (e.g., "overcome" + "it" → "overcomeit"). + */ +function replaceFootnotesWithAnchors(doc: Document, footnotes: Element[]): void { + for (const fn of footnotes) { + const verseNum = fn.closest('.yv-v[v]')?.getAttribute('v'); + if (!verseNum) continue; + + const prev = fn.previousSibling; + const next = fn.nextSibling; + + const prevText = prev?.textContent ?? ''; + const nextText = next?.textContent ?? ''; + + const prevNeedsSpace = prevText.length > 0 && !/\s$/.test(prevText); + const nextNeedsSpace = nextText.length > 0 && NEEDS_SPACE_BEFORE.test(nextText); + + if (prevNeedsSpace && nextNeedsSpace && fn.parentNode) { + fn.parentNode.insertBefore(doc.createTextNode(' '), fn); + } + + const anchor = doc.createElement('span'); + anchor.setAttribute('data-verse-footnote', verseNum); + fn.replaceWith(anchor); + } +} /** * Extracts footnotes from wrapped verse HTML and prepares data for footnote popovers. * - * This function assumes verses are already wrapped in `.yv-v[v]` elements (by wrapVerseContent). - * It uses `.closest('.yv-v[v]')` to find which verse each footnote belongs to. + * Assumes verses are already wrapped in `.yv-v[v]` elements (by wrapVerseContent). + * + * Two-phase approach: + * 1. Build popover data (verseHtml + note content) using cloned DOM — no side effects. + * 2. Replace footnotes in the real DOM with clean anchor spans for React portals. * - * @returns Notes data for popovers, keyed by verse number + * @returns Notes data for popovers, keyed by verse number. */ -export function extractNotesFromWrappedHtml(doc: Document): Record { +function extractNotesFromWrappedHtml(doc: Document): Record { const footnotes = Array.from(doc.querySelectorAll('.yv-n.f')); if (!footnotes.length) return {}; - // Group footnotes by verse number using closest wrapper + // Group footnotes by verse number. const footnotesByVerse = new Map(); - footnotes.forEach((fn) => { + for (const fn of footnotes) { const verseNum = fn.closest('.yv-v[v]')?.getAttribute('v'); - if (verseNum) { - let arr = footnotesByVerse.get(verseNum); - if (!arr) { - arr = []; - footnotesByVerse.set(verseNum, arr); - } - arr.push(fn); + if (!verseNum) continue; + let arr = footnotesByVerse.get(verseNum); + if (!arr) { + arr = []; + footnotesByVerse.set(verseNum, arr); } + arr.push(fn); + } + + // Build verse-wrapper lookup. + const wrappersByVerse = new Map(); + doc.querySelectorAll('.yv-v[v]').forEach((el) => { + const verseNum = el.getAttribute('v'); + if (!verseNum) return; + const arr = wrappersByVerse.get(verseNum); + if (arr) arr.push(el); + else wrappersByVerse.set(verseNum, [el]); }); + // Phase 1: Extract data (cloned DOM — no mutations). const notes: Record = {}; - const NEEDS_SPACE_BEFORE = /^[^\s.,;:!?)}\]'"»›]/ - - footnotesByVerse.forEach((fns, verseNum) => { - // Find all wrappers for this verse (could be multiple if verse spans paragraphs) - const verseWrappers = Array.from(doc.querySelectorAll(`.yv-v[v="${verseNum}"]`)); - - // Build verse HTML with A/B/C markers for popover display - let verseHtml = ''; - let noteIdx = 0; - - verseWrappers.forEach((wrapper, wrapperIdx) => { - if (wrapperIdx > 0) verseHtml += ' '; - - const walker = doc.createTreeWalker(wrapper, NodeFilter.SHOW_ELEMENT | NodeFilter.SHOW_TEXT); - let lastWasFootnote = false; - while (walker.nextNode()) { - const node = walker.currentNode; - if (node instanceof Element) { - if (node.classList.contains('yv-n') && node.classList.contains('f')) { - verseHtml += `${LETTERS[noteIdx++] || noteIdx}`; - lastWasFootnote = true; - } - } else if (node.nodeType === Node.TEXT_NODE) { - const parent = node.parentElement; - if (parent?.closest('.yv-n.f') || parent?.closest('.yv-h')) continue; - if (parent?.classList.contains('yv-vlbl')) continue; - let text = node.textContent || ''; - // Insert space after footnote marker if text doesn't start with whitespace/punctuation - // (same fix as footnote removal - see World English Bible, version ID 206) - if (lastWasFootnote && text && NEEDS_SPACE_BEFORE.test(text)) { - text = ' ' + text; - } - verseHtml += text; - lastWasFootnote = false; - } - } - }); - + for (const [verseNum, fns] of footnotesByVerse) { notes[verseNum] = { - verseHtml, + verseHtml: buildVerseHtml(wrappersByVerse.get(verseNum) ?? []), notes: fns.map((fn) => fn.innerHTML), }; + } + + // Phase 2: Replace footnotes with portal anchors (real DOM mutation). + replaceFootnotesWithAnchors(doc, footnotes); + + return notes; +} - // Insert placeholder after last verse wrapper (sibling, not child) - const lastWrapper = verseWrappers[verseWrappers.length - 1]; - if (lastWrapper?.parentNode) { - const placeholder = doc.createElement('span'); - placeholder.setAttribute('data-verse-footnote', verseNum); - lastWrapper.parentNode.insertBefore(placeholder, lastWrapper.nextSibling); +/** + * Adds non-breaking space after verse labels for better copy/paste + * (e.g., "3For God so loved..." → "3 For God so loved..."). + */ +function addNbspToVerseLabels(doc: Document): void { + doc.querySelectorAll('.yv-vlbl').forEach((label) => { + const text = label.textContent || ''; + if (!text.endsWith(NON_BREAKING_SPACE)) { + label.textContent = text + NON_BREAKING_SPACE; } }); +} - // Remove all footnotes from DOM, inserting space if needed to prevent word concatenation. - // - // Some Bible versions (e.g., World English Bible, version ID 206) have malformed HTML where - // there's no whitespace between a word, the footnote element, and the next word: - // - // ...hasn't overcome...it. - // - // When we remove the footnote, "overcome" + "it" would become "overcomeit". - // We detect this case and insert a space, but only if the next character isn't punctuation. - footnotes.forEach((fn) => { - const prev = fn.previousSibling; - const next = fn.nextSibling; - - const prevNeedsSpace = - prev?.nodeType === Node.TEXT_NODE && prev.textContent && !/\s$/.test(prev.textContent); - const nextNeedsSpace = - next?.nodeType === Node.TEXT_NODE && - next.textContent && - NEEDS_SPACE_BEFORE.test(next.textContent); +/** + * Fixes irregular tables by adding colspan to single-cell rows in multi-column tables. + * (e.g., https://www.bible.com/bible/111/EZR.2.NIV) + */ +function fixIrregularTables(doc: Document): void { + doc.querySelectorAll('table').forEach((table) => { + const rows = table.querySelectorAll('tr'); + if (rows.length === 0) return; + + let maxColumns = 0; + rows.forEach((row) => { + let count = 0; + row.querySelectorAll('td, th').forEach((cell) => { + count += + cell instanceof HTMLTableCellElement + ? parseInt(cell.getAttribute('colspan') || '1', 10) + : 1; + }); + maxColumns = Math.max(maxColumns, count); + }); - if (prevNeedsSpace && nextNeedsSpace) { - fn.parentNode?.insertBefore(doc.createTextNode(' '), next); + if (maxColumns > 1) { + rows.forEach((row) => { + const cells = row.querySelectorAll('td, th'); + if (cells.length === 1 && cells[0] instanceof HTMLTableCellElement) { + const existing = parseInt(cells[0].getAttribute('colspan') || '1', 10); + if (existing < maxColumns) { + cells[0].setAttribute('colspan', maxColumns.toString()); + } + } + }); } - fn.remove(); }); +} - return notes; +const DOMPURIFY_CONFIG = { + ALLOWED_ATTR: ['class', 'style', 'id', 'v', 'usfm'], + ALLOW_DATA_ATTR: true, +}; + +/** + * Full transformation pipeline for Bible HTML from the API. + * + * 1. Sanitize (DOMPurify) + * 2. Wrap verse content in selectable spans + * 3. Extract footnotes and replace with portal anchors + * 4. Add non-breaking spaces to verse labels + * 5. Fix irregular table layouts + */ +export function transformBibleHtml(html: string): { html: string; notes: Record } { + if (typeof window === 'undefined' || !('DOMParser' in window)) { + return { html, notes: {} }; + } + + const doc = new DOMParser().parseFromString( + DOMPurify.sanitize(html, DOMPURIFY_CONFIG), + 'text/html', + ); + + wrapVerseContent(doc); + const notes = extractNotesFromWrappedHtml(doc); + addNbspToVerseLabels(doc); + fixIrregularTables(doc); + + return { html: doc.body.innerHTML, notes }; }