feat(web): preview opendocument, epub, rich text, and notebooks#21469

🟡 Medium files/extendedDocumentPreview.ts:354

When data["text/html"] is an empty array or string and text/plain contains content, this branch returns an empty HTML output that line 361 filters out, so the plain-text fallback is silently lost. Check the decoded HTML for non-whitespace content before preferring it.

Fixed in 2d3bbe6303: notebook HTML is preferred only when the decoded content is nonempty. The existing unit suite covers empty arrays, empty strings, and whitespace with plain-text fallbacks; the actual app renders the saved fallback outputs.

🟠 High files/extendedDocumentPreview.ts:371

Valid RTF files declaring a non-1252 code page render their text incorrectly; for example, \ansicpg1251 text containing byte 0xC0 is previewed as À instead of Cyrillic А. openRtf decodes both the entire stream and \'hh escapes as windows-1252 without reading \ansicpgN; use the declared code page for both decodings.

Fixed in 2d3bbe6303: RTF byte runs and hexadecimal escapes decode through the declared ansicpg code page, retaining byte offsets for binary destinations. The existing unit suite covers raw/escaped Windows-1251 and multibyte Shift-JIS; an actual RTF attachment now renders Cyrillic correctly.

🟠 High files/extendedDocumentPreview.ts:53

OpenDocument files that bind the standard ODF namespaces to prefixes such as t, o, or d are rejected as empty because find only matches the literal names table:table, office:text, and draw:page. XML element identity is determined by namespace URI and local name, not prefix; resolve namespace declarations and match expanded names instead.

Fixed in 2d3bbe6303: scoped namespace bindings normalize standard ODF element and attribute identities, including default namespaces and xlink. The existing suite covers alternate prefixes and default text namespaces, and actual ODT/ODS attachments render correctly.

🟠 High files/extendedDocumentPreview.ts:45

A valid EPUB chapter whose manifest href contains & is silently dropped because the raw attribute value is used to look up the archive entry (and the book fails if that is its only chapter). With processEntities: false, fast-xml-parser preserves attribute references, and only text() decodes them; decode built-in and numeric references in attribute values without enabling DTD entity expansion.

Fixed in 2d3bbe6303: built-in and numeric references decode in XML attributes without enabling DTD entity expansion. The existing suite reads a manifest path with three encoded ampersands; the actual EPUB attachment opens its ampersand-named chapter.

🟠 High files/extendedDocumentPreview.ts:277

Rendering 500 ODP slides that reference the same 4 MiB image can exhaust the renderer's memory: Promise.all starts every odfHtml render concurrently, and each imageData call copies and base64-encodes the image before boundedMarkup checks the aggregate limit. Render pages sequentially or with bounded concurrency, and reserve the output budget before encoding (or cache shared images).

No longer relevant as of 2d3bbe63031fa191c37713fe5f743570afe127f5

Fixed in 2d3bbe6303: pages render sequentially and reserve the base64 output budget before encoding images. The existing suite opens 500 slides referencing a 4 MiB image and verifies rejection before the third base64 allocation.

🟠 High files/extendedDocumentPreview.ts:60

ODS cell values silently lose explicit spaces, tabs, and line breaks, so text such as Account<text:s/>Name is previewed as AccountName. text() only handles #text nodes, unlike odfHtml(); handle text:s, text:tab, and text:line-break there, honoring text:c for repeated spaces.

Fixed in 2d3bbe6303: ODS text preserves bounded repeated spaces, tabs, and line breaks. The existing suite and actual spreadsheet attachment retain these characters in the cell value.

🟠 High files/extendedDocumentPreview.ts:215

An EPUB with many empty allowed elements can produce hundreds of MiB of HTML without hitting the 16 MiB limit, exhausting memory when openEpub retains and posts the pages. epubHtml budgets text and image payloads but not generated tags, and odfHtml has the same omission; charge opening and closing tags to the shared markup budget in both renderers.

Fixed in fb7cdc8be1afa75fd4b0e063896d2c8bfbbe2d6e. EPUB and ODF opening/closing tags and explicit ODF whitespace count toward the shared 16 MiB markup budget; image tags retain their existing reserved overhead. A tag-only EPUB reproduces the missing limit before the fix, and now rejects in both the focused check and the real client.

🟠 High files/extendedDocumentPreview.ts:467

An unknown skipped destination containing \u drops the first visible byte after the group: {'{'}\*unknown\u945?{'}'}Hello renders ello instead of Hello. \u sets fallback, but emitByte returns while state.skip is true before consuming the fallback ?, and restoring the group does not reset the counter. Consume fallback bytes before returning for skipped state.

Fixed in fb7cdc8be1afa75fd4b0e063896d2c8bfbbe2d6e. Both byte and escaped-character emission consume Unicode fallback before skipping a hidden destination. Literal, hex and escaped fallback checks reproduce the failure before the fix and pass afterward; the real app now preserves Hello.

🟠 High files/extendedDocumentPreview.ts:566

RTF text controls such as \endash, \emdash, \bullet, and the quote controls are omitted from the preview, so 10\endash 20 renders as 1020 instead of 10–20. The dispatch handles \tab but has no cases for these character controls; emit their Unicode equivalents so the displayed text is preserved.

Fixed in ce05226f61: RTF dash, bullet, and quote controls emit their Unicode characters through the existing skip/fallback handling. The regression covers Unicode fallback and hidden destinations; the real report renders 10–20 and its quotes/bullet.

🟠 High files/extendedDocumentPreview.ts:279

ODS cell values include text from office:annotation subtrees, so a comment is appended to the displayed value or replaces office:value when the cell has no paragraph text. find(cell, "text:p") recurses into annotations; skip those subtrees when collecting cell paragraphs.

Fixed in ce05226f61: cell paragraph collection skips annotation subtrees, including nested annotations, while retaining attribute-value fallback. The worker/client case and real Forecast sheet show North and 150 without either comment.

🟡 Medium files/extendedDocumentPreview.ts:203

EPUB line breaks render as two breaks, adding blank lines wherever the source contains <br/>. epubHtml serializes br with an end tag, which HTML treats as another br start tag; serialize void elements without closing tags.

Fixed in ce05226f61: EPUB br/hr elements serialize without closing tags and still consume the markup budget. Worker and attachment-viewer regressions pass, and the real handbook now displays two source breaks as two breaks.

🟠 High files/extendedDocumentPreview.ts:565

RTF table cells are concatenated in the preview, so Name\cell Value\cell\row renders as NameValue and adjacent numeric values become indistinguishable. \cell and \row are not handled here, so their terminators are discarded; emit separators for them so cell contents remain distinct.

Fixed in ce05226f61: RTF cell terminators emit tabs and row terminators emit line breaks, respecting hidden destinations. The worker/client regression and real report keep adjacent numeric values and rows separate.

🟠 High files/extendedDocumentPreview.ts:118

EPUB/ODF text inside CDATA is decoded a second time, so <p><![CDATA[&]]></p> renders as & instead of the literal & and corrupts content such as code examples. parseXml stores CDATA in the same #text node as ordinary text, so text() cannot distinguish it; preserve CDATA separately and avoid entity-decoding its content.

The CDATA handling at parseXml/text is unchanged from 88a53624fe, before this four-finding pass. The literal & example already stays &; preserving entity-like CDATA such as & is a separate, pre-existing content-fidelity follow-up outside the scoped image-pixel fix.

🟠 High files/extendedDocumentPreview.ts:420

Notebook HTML outputs can render data-URI images that bypass the 16,777,216-pixel limit, so oversized images reach the browser even though the image/png output branch rejects them. This path returns text/html without checking embedded image dimensions, and the downstream sanitizer only checks URL syntax; validate embedded raster images with imageFits before returning the HTML.

Fixed in 77b5ddbc5b. The worker checks embedded HTML data images with the same bounded header/pixel check, including HTML-encoded URLs, and uses saved plain text when an image exceeds the limit. Both plain and entity-encoded HTML paths are covered in the existing browser test; all 152 scoped cases and frozen install/check/knip/typecheck passed through testbox.

External image Minor · Functional Correctness · Quick win

Do not count trailing repeated empty cells and rows as spreadsheet data.

LibreOffice often writes trailing filler. One example is <table:table-row table:number-rows-repeated="1048570"><table:table-cell table:number-columns-repeated="1024"/></table:table-row>. Another is a final table:number-columns-repeated empty cell in each row. Both are common when a column or row has a style.

The current loop handles this filler as data:

• Line 322 sets truncated = true for the empty run.
• Lines 323-327 pad each row with empty strings up to 50 columns.
• Lines 330-336 append empty rows until 500 rows exist.

The result is wrong for a small styled sheet. The viewer shows "Limited to 500 rows and 50 columns". It reports rowCount 500 and columns 50, and it renders mostly empty grid cells.

Use this correction:

• Set the column truncation flag only for non-empty values.
• Remove trailing empty cells from each row.
• Hold back empty rows. Emit them only when a later row has content.

✅ Confirmed as addressed by @maria-rcks

fixed in 0bf166fb3f: trailing empty cells are trimmed and empty rows are deferred until later content, while real overflow still reports truncation. unit and real-worker/client checks cover styled filler, internal gaps, and the exact 500-row boundary.

@maria-rcks Thanks for fixing this in 0bf166fb3f. The reported changes address the styled filler issue and preserve truncation for real overflow. The reported checks cover internal gaps and the exact 500-row boundary.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Stability & Availability · Quick win

Reject HTML fallback responses and identify the asset kind.

The web deployment rewrites unmatched paths to /index.html. If a bundled asset URL is missing or resolves against the wrong deployment path, fetch(url) can return HTML with status 200. The current check accepts that body and passes HTML bytes to PDF.js. The generic "Unsupported PDF font." message is also shown by PdfViewer for cMap and wasm asset failures.

✅ Confirmed as addressed by @maria-rcks

fixed in 0bf166fb3f: the factory rejects HTML fallback responses before reading bytes and identifies the selected asset directory in errors. browser checks cover fonts, cMaps, and wasm with rejected HTML, valid binary bytes, and unsupported filenames.

@maria-rcks Thanks for confirming the fix in 0bf166fb3f. The reported changes address both HTML fallback rejection and asset-directory errors. The reported browser checks cover all three asset kinds.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Functional Correctness · Quick win

Reset OfficeViewer when the attachment refreshes.

The worker effect runs only when src, kind, or sizeBytes changes. Ordinary workspace-file URLs do not include a file revision, so a refresh can keep the same src after the file changes. The existing worker result, page, and sheet-page cache then remain active for the new file.

Include the refresh revision in the wrapper key.

workspace refresh already remounts AttachmentFilePreview through FilePreviewPanel's document key, including fileRefreshRevision and workspaceMutationId. its reauthorization effect also clears remoteUrl before issuing another URL, and the existing same-URL workspace Word remount regression passed; adding another revision prop would duplicate this reset.

@maria-rcks You’re right. FilePreviewPanel already keys the workspace document’s AttachmentFilePreview with both workspaceMutationId and fileRefreshRevision. A refresh therefore remounts OfficeViewer and resets its worker, page state, and sheet-page cache, even when src stays unchanged.

I also confirmed that reauthorization clears remoteUrl. The additional revision prop is unnecessary. I withdraw this finding.

✅ Review thread resolved.

_{You are interacting with an AI system.}

🟡 Medium files/extendedDocumentPreview.ts:241

ODT/ODP paragraphs lose the spacing represented by text:s and text:tab: these branches emit collapsible whitespace, so HTML's default white-space: normal reduces it to a single space. Wrap these runs in an element with white-space: pre-wrap so the rendered pages preserve the document's spacing.

fixed in f671c7052a: explicit ODF whitespace uses bounded spans whose safe marker survives sanitization and selects pre-wrap in the viewer stylesheet. real-worker/browser checks verify rendered ODT and ODP spaces and tabs, and the real ODT attachment preserves the spacing.

External image Minor · Functional Correctness · Quick win

Strip the {uri} form before you derive the EPUB tag name.

parseXml only maps namespaces listed in namespacePrefixes. Any other namespace becomes "{" + uri + "}" + local (Line 82). SVG (http://www.w3.org/2000/svg) and MathML (http://www.w3.org/1998/Math/MathML) are not in that list. An inline SVG with a default namespace is common in EPUB covers. Such an element gets a name like {http://www.w3.org/2000/svg}svg.

On Line 225, node.name.split(":").at(-1) splits on the colon in http:. The result is //www.w3.org/2000/svg}svg. The blocklist on Line 226 therefore does not match svg, math, or a nested {…svg}style or {…svg}script. The function then walks their children and emits all text nodes as escaped text. The preview can show SVG <style> CSS, <script> source, <title>/<desc> text, and flattened MathML as visible chapter text. The output is escaped, so this is not an injection. It is wrong visible content, and it defeats the intended omission.

✅ Confirmed as addressed by @maria-rcks

Fixed in d8e4b64ea1: the {uri} prefix is stripped before the tag check, so namespaced svg and math are skipped.

@maria-rcks Thanks for fixing the namespace handling in d8e4b64ea1. Stripping {uri} before the tag check addresses this finding.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}

🟠 High files/NotebookPreview.tsx:71

The seventeenth visible output remains blank when a cell has 17 small outputs: slice(0, 16) never mounts it, and once the first 16 shrink from their placeholders, there may be no scrolling or intersection change to trigger another selection. Keep non-iframe outputs renderable independently and provide a way to reveal deferred HTML outputs without exceeding the iframe bound.

Fixed in d8e4b64ea1: text and image outputs always render now (images lazy-load); only html iframe outputs go through the 16-slot bound. A 17th html iframe visible at the same moment stays deferred by design and mounts once the visible set changes.

Non-HTML outputs are fixed, but the HTML case remains: 17 simultaneously intersecting HTML outputs still selects the first 16 indefinitely unless intersection state changes. The finding requires a deterministic way to reveal the deferred iframe while preserving the cap.

🟡 Medium files/PdfViewer.tsx:226

Clicking Fit width while already in fit-width mode leaves a stale zoomAnchor, so a later panel resize scrolls back to the position captured by that click instead of preserving the reader’s current position. The equal-width early return does not consume the anchor; clear it on that path.

Fixed in d8e4b64ea1: the anchor is consumed on every pass, including the equal-width return.