From 42d539373e4b1a0bbe9c02bc13c9e489c73634e1 Mon Sep 17 00:00:00 2001 From: Yash Singh Date: Wed, 11 Nov 2026 10:53:53 -0600 Subject: [PATCH 1/7] fix(web): improve Excel spreadsheet previews --- apps/web/package.json | 2 +- .../AttachmentFilePreview.browser.test.tsx | 87 ++++++++-- .../components/files/OpenXmlOfficeViewer.tsx | 14 +- .../components/files/office/Spreadsheet.tsx | 8 +- apps/web/src/components/files/office/fonts.ts | 8 + .../components/files/office/sheet-fonts.ts | 58 +++++++ .../components/files/office/sheet-frame.ts | 77 +++++++-- .../components/files/office/sheet-protocol.ts | 3 +- .../files/office/spreadsheet.worker.ts | 31 +++- .../web/src/components/files/officeArchive.ts | 155 ++++++++++++++++-- .../components/files/officePreviewProtocol.ts | 10 ++ pnpm-lock.yaml | 10 +- third-party-licenses.config.json | 2 +- 13 files changed, 408 insertions(+), 57 deletions(-) create mode 100644 apps/web/src/components/files/office/sheet-fonts.ts diff --git a/apps/web/package.json b/apps/web/package.json index e42f4e3bcc61..d93a5e837f58 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -16,7 +16,7 @@ "@betteroffice/docx": "0.4.3", "@betteroffice/fonts": "0.4.0", "@betteroffice/pptx": "0.2.0", - "@betteroffice/xlsx": "0.3.0", + "@betteroffice/xlsx": "0.5.1", "@clerk/clerk-js": "catalog:", "@clerk/electron": "catalog:", "@clerk/react": "catalog:", diff --git a/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx b/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx index 2835e4ab678b..7dc469f583b1 100644 --- a/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx +++ b/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx @@ -1047,15 +1047,15 @@ it.each([ const ns = "http://schemas.openxmlformats.org/spreadsheetml/2006/main"; archive.file( "xl/workbook.xml", - '', + '', ); archive.file( "xl/_rels/workbook.xml.rels", - ``, + ``, ); archive.file( "xl/worksheets/sheet1.xml", - `<${prefix}worksheet xmlns${prefix ? ":x" : ""}="${ns}"><${prefix}dimension ref="XFD1048576"/><${prefix}cols><${prefix}col min="${OFFICE_PREVIEW_MAX_COLUMNS - 1}" max="${OFFICE_PREVIEW_MAX_COLUMNS + 10}" width="20"/><${prefix}col min="${OFFICE_PREVIEW_MAX_COLUMNS + 1}" max="${OFFICE_PREVIEW_MAX_COLUMNS + 30}" hidden="1"/><${prefix}sheetData><${prefix}row ht="42" customHeight="1" hidden="1"><${prefix}c r="A50" t="inlineStr"><${prefix}is><${prefix}t>${"value".repeat(1000)}<${prefix}mergeCells><${prefix}mergeCell ref="XFD1048576"/>` + `<${prefix}worksheet xmlns${prefix ? ":x" : ""}="${ns}"><${prefix}dimension ref="XFD1048576"/><${prefix}sheetViews><${prefix}sheetView showGridLines="0"/><${prefix}sheetFormatPr defaultColWidth="30"/><${prefix}cols><${prefix}col min="${OFFICE_PREVIEW_MAX_COLUMNS - 1}" max="${OFFICE_PREVIEW_MAX_COLUMNS + 10}" width="20"/><${prefix}col min="${OFFICE_PREVIEW_MAX_COLUMNS + 1}" max="${OFFICE_PREVIEW_MAX_COLUMNS + 30}" hidden="1"/><${prefix}sheetData><${prefix}row ht="42" customHeight="1" hidden="1"><${prefix}c r="A50" t="inlineStr"><${prefix}is><${prefix}t>${"value".repeat(1000)}<${prefix}mergeCells><${prefix}mergeCell ref="XFD1048576"/>` .replace(/<((?:x:)?(?:col|dimension|mergeCell))\b([^>]*)\/>/g, prefix ? "<$1$2>" : "$&") .replace(/[\s\S]*?<\/sheetData>/, (data) => (empty ? "" : data)) .replace(/="([^"]*)"/g, (attribute, value: string) => @@ -1079,6 +1079,8 @@ it.each([ }); expect(result.type).toBe("xlsx"); if (result.type !== "xlsx") throw new Error("Expected bounded workbook"); + // The engine counts the chart sheet before it. + expect(result.view?.hiddenGridlines).toEqual([1]); const bounded = await JSZip.loadAsync(result.bytes); const sheet = new DOMParser().parseFromString( await bounded.file("xl/worksheets/sheet1.xml")!.async("string"), @@ -1095,10 +1097,16 @@ it.each([ } expect(sheet.getElementsByTagNameNS(ns, "dimension")).toHaveLength(0); expect(sheet.getElementsByTagNameNS(ns, "mergeCell")).toHaveLength(0); - const columns = sheet.getElementsByTagNameNS(ns, "col"); - expect(columns).toHaveLength(1); - expect(columns[0]!.getAttribute("min")).toBe(String(OFFICE_PREVIEW_MAX_COLUMNS - 1)); - expect(columns[0]!.getAttribute("max")).toBe(String(OFFICE_PREVIEW_MAX_COLUMNS)); + const columns = [...sheet.getElementsByTagNameNS(ns, "col")].map((column) => + ["min", "max", "width"].map((name) => column.getAttribute(name)), + ); + // The engine ignores `defaultColWidth`, so the columns it sizes are spelled out. + expect(columns).toEqual([ + [String(OFFICE_PREVIEW_MAX_COLUMNS - 1), String(OFFICE_PREVIEW_MAX_COLUMNS), "20"], + // Hidden columns keep their span past the limit, so they stay hidden. + [String(OFFICE_PREVIEW_MAX_COLUMNS + 1), String(OFFICE_PREVIEW_MAX_COLUMNS + 30), null], + ["1", String(OFFICE_PREVIEW_MAX_COLUMNS - 2), "30"], + ]); } finally { worker.terminate(); } @@ -1711,7 +1719,8 @@ it.each(["repeated binary", "XML relationship alias", "duplicate relationship ID }, ); -it.each(["XFD200", "XFD1048576", "AX500", "AZ9999999"])( +// XFD300 is a styled blank past the column limit, which loses nothing. +it.each(["XFD200", "XFD300", "XFD1048576", "AX500", "AZ9999999"])( "bounds spreadsheet cells at %s before padding", async (ref) => { const { default: JSZip } = await import("jszip"); @@ -1727,7 +1736,7 @@ it.each(["XFD200", "XFD1048576", "AX500", "AZ9999999"])( ); archive.file( "xl/custom.xml", - `79`, + `7${ref === "XFD300" ? `` : `9`}`, ); const bytes = await archive.generateAsync({ type: "arraybuffer" }); const worker = new Worker(new URL("./officePreview.worker.ts", import.meta.url), { @@ -1757,7 +1766,7 @@ it.each(["XFD200", "XFD1048576", "AX500", "AZ9999999"])( ); expect(sheet.querySelector('c[r="A1"] v')?.textContent).toBe("7"); expect(sheet.querySelectorAll("row").length).toBeLessThanOrEqual(500); - expect(result.truncated).toBe(ref !== "AX500"); + expect(result.truncated).toBe(ref === "XFD200"); if (ref === "AX500") expect(sheet.querySelector('c[r="AX500"] v')?.textContent).toBe("9"); else expect(sheet.querySelector('c[r="' + ref + '"]')).toBeNull(); } @@ -1767,6 +1776,55 @@ it.each(["XFD200", "XFD1048576", "AX500", "AZ9999999"])( }, ); +it("shows the number formats and underlines a workbook's own styles define", async () => { + const { default: JSZip } = await import("jszip"); + const archive = new JSZip(); + archive.file( + "xl/workbook.xml", + '', + ); + archive.file( + "xl/_rels/workbook.xml.rels", + '', + ); + archive.file( + "xl/styles.xml", + ``, + ); + archive.file( + "xl/worksheets/sheet1.xml", + '10044986Total', + ); + const { prepareOffice } = await import("./officeArchive"); + const prepared = await prepareOffice( + await archive.generateAsync({ type: "arraybuffer" }), + "xlsx", + ); + const styles = await ( + await JSZip.loadAsync(prepared.bytes) + ) + .file("xl/styles.xml")! + .async("string"); + expect(styles).toContain(""); + const worker = new Worker(new URL("./office/spreadsheet.worker.ts", import.meta.url), { + type: "module", + }); + try { + const { createWorkerClient } = await import("./office/worker-rpc"); + const client = createWorkerClient(worker); + const bytes = prepared.bytes; + await client.call("open", { bytes, view: prepared.view! }, { transfer: [bytes] }); + const cells = await client.call("cells", { + sheet: 0, + range: { top: 0, bottom: 0, left: 0, right: 1 }, + }); + // Ids under 164 that a workbook defines keep its formats, not Excel's built-in ones. + expect(cells[0]).toEqual(["￥100.00", "01-Mar-2023"]); + } finally { + worker.terminate(); + } +}); + it("keeps spreadsheet dates on their calendar day west of UTC", async () => { const { default: JSZip } = await import("jszip"); const archive = new JSZip(); @@ -1825,7 +1883,14 @@ it("keeps spreadsheet dates on their calendar day west of UTC", async () => { }); const { createWorkerClient } = await import("./office/worker-rpc"); const client = createWorkerClient(worker); - await client.call("open", { bytes }, { transfer: [bytes] }); + await client.call( + "open", + { + bytes, + view: { hiddenGridlines: [], themeFonts: { major: "Calibri Light", minor: "Calibri" } }, + }, + { transfer: [bytes] }, + ); const cells = await client.call("cells", { sheet: 0, range: { top: 0, bottom: 0, left: 0, right: 0 }, diff --git a/apps/web/src/components/files/OpenXmlOfficeViewer.tsx b/apps/web/src/components/files/OpenXmlOfficeViewer.tsx index 8802e9fd2913..7e7268cdf03c 100644 --- a/apps/web/src/components/files/OpenXmlOfficeViewer.tsx +++ b/apps/web/src/components/files/OpenXmlOfficeViewer.tsx @@ -104,8 +104,7 @@ export default function OfficeViewer(props: { ); if (!prepared) return ; - const Viewer = - props.kind === "docx" ? WordDocument : props.kind === "xlsx" ? Spreadsheet : Presentation; + const Viewer = props.kind === "docx" ? WordDocument : Presentation; return (
{details ? ( @@ -122,7 +121,16 @@ export default function OfficeViewer(props: { ) : null} }> - + {prepared.view ? ( + + ) : ( + + )}
); diff --git a/apps/web/src/components/files/office/Spreadsheet.tsx b/apps/web/src/components/files/office/Spreadsheet.tsx index 7bea259b9f07..d8784031c813 100644 --- a/apps/web/src/components/files/office/Spreadsheet.tsx +++ b/apps/web/src/components/files/office/Spreadsheet.tsx @@ -14,6 +14,7 @@ import { type CellAddr, } from "@betteroffice/xlsx/headless"; import { FileSurfaceLoading, FileSurfaceNotice } from "../fileSurfaceChrome"; +import type { WorkbookView } from "../officePreviewProtocol"; import { createWorkerClient, SupersededError } from "./worker-rpc"; import type { SheetLayout, SheetMethods, SheetFrameRequest, SheetTab } from "./sheet-protocol"; import { @@ -45,10 +46,11 @@ type Opened = { }; export default function Spreadsheet(props: { bytes: ArrayBuffer; + view: WorkbookView; onDetails: (text: string) => void; onError: (cause: unknown) => void; }) { - const { onError, bytes: source, onDetails } = props; + const { onError, bytes: source, onDetails, view } = props; const [opened, setOpened] = useState(); const [error, setError] = useState(""); const picking = useRef(null); @@ -66,7 +68,7 @@ export default function Spreadsheet(props: { worker.addEventListener("messageerror", fail); const bytes = source.slice(0); void engine - .call("open", { bytes }, { transfer: [bytes], signal: controller.signal }) + .call("open", { bytes, view }, { transfer: [bytes], signal: controller.signal }) .then((value) => { if (!value) throw new Error("This workbook has no visible sheets."); setOpened({ ...value, engine }); @@ -78,7 +80,7 @@ export default function Spreadsheet(props: { picking.current?.abort(); engine.close(); }; - }, [source, onDetails, onError]); + }, [source, view, onDetails, onError]); const show = (sheet: number, target?: CellAddr) => { if (!opened) return; picking.current?.abort(); diff --git a/apps/web/src/components/files/office/fonts.ts b/apps/web/src/components/files/office/fonts.ts index cdac25d4bbe1..13a83294a532 100644 --- a/apps/web/src/components/files/office/fonts.ts +++ b/apps/web/src/components/files/office/fonts.ts @@ -42,6 +42,14 @@ export function presentationFace(family: string, bold: boolean, italic: boolean) ); } +/** The face Excel's cells draw a family with when it is missing: its metric-compatible or bundled face, if any. */ +export function sheetFace(family: string, bold: boolean, italic: boolean) { + return ( + shippedFace(resolveMetricCompatFace(family, bold, italic)) ?? + shippedFace(resolveBundledFamilyFace(family, bold, italic)) + ); +} + function shippedFace(face: BundledFontFace | undefined) { return face && (!face.script || shipped(face.script)) ? face : undefined; } diff --git a/apps/web/src/components/files/office/sheet-fonts.ts b/apps/web/src/components/files/office/sheet-fonts.ts new file mode 100644 index 000000000000..f4b4f67f1f4b --- /dev/null +++ b/apps/web/src/components/files/office/sheet-fonts.ts @@ -0,0 +1,58 @@ +import type { DisplayList } from "@betteroffice/xlsx"; +import { loadFace, sheetFace } from "./fonts"; + +/** + * The metric-compatible faces a workbook's cells draw with, such as Carlito for Calibri, added to the worker's font set + * under the family name the painter asks for, so text widths, wrapping and overflow match Excel on machines without + * Office fonts. A family the app ships no face for stays with the browser's own fallback. + */ +export function sheetFonts() { + // SAFETY: a worker's global scope is a FontFaceSource, whose `fonts` is a FontFaceSet (CSS Font Loading, exposed to + // workers). The window's types declare it on documents only. Without one, cells keep the browser's fallback. + const fonts = "fonts" in self ? (self.fonts as FontFaceSet) : undefined; + const loads = new Map>(); + + const load = (family: string, bold: boolean, italic: boolean) => { + const key = `${family}\u0000${bold}\u0000${italic}`; + const existing = loads.get(key); + + if (existing) return existing; + + const face = sheetFace(family, bold, italic); + const loading = + face && fonts + ? loadFace(face) + // A family without the requested style gets the face it has, which the browser then slants or emboldens. + .then((bytes) => + new FontFace(family, bytes, { + weight: face.weight === 700 ? "700" : "400", + style: face.style === "italic" ? "italic" : "normal", + }).load(), + ) + .then((loaded) => void fonts.add(loaded)) + .catch(() => undefined) + : Promise.resolve(); + + loads.set(key, loading); + + return loading; + }; + + /** Loads the faces a frame's cell text uses before it paints. */ + return (lists: readonly DisplayList[]) => + Promise.all( + lists.flatMap((list) => + list.commands.flatMap((command) => { + // Chart text arrives as a CSS family list whose first family is the one to load. + const family = + command.op === "text" + ? (/^"([^"]+)"/.exec(command.fontFamily ?? "")?.[1] ?? command.fontFamily) + : undefined; + + return command.op === "text" && family + ? [load(family, !!command.bold, !!command.italic)] + : []; + }), + ), + ); +} diff --git a/apps/web/src/components/files/office/sheet-frame.ts b/apps/web/src/components/files/office/sheet-frame.ts index 67789b6b79fd..3b7868bf2d3f 100644 --- a/apps/web/src/components/files/office/sheet-frame.ts +++ b/apps/web/src/components/files/office/sheet-frame.ts @@ -50,7 +50,7 @@ export function drawFrame( frozen: { readonly rows: number; readonly cols: number }, wider?: SheetViewport, ): Frame | undefined { - const around = wider && attempt(() => workbook.displayList(wider)); + const around = wider && untruncated(workbook, wider); if (wider && around) return { @@ -59,15 +59,15 @@ export function drawFrame( viewport: wider, }; - const whole = attempt(() => workbook.displayList(viewport)); + const whole = untruncated(workbook, viewport); if (whole) return { list: whole, tiles: [{ list: whole, x: 0, y: 0, clipX: 0, clipY: 0 }], viewport }; // The engine refuses a viewport with too many cells, as a wide pane of narrow columns has. One row and one column of // the frame give its tracks, and so where its frozen panes end. - const columns = attempt(() => workbook.displayList({ ...viewport, height: 1 }).grid); - const rows = attempt(() => workbook.displayList({ ...viewport, width: 1 }).grid); + const columns = untruncated(workbook, { ...viewport, height: 1 })?.grid; + const rows = untruncated(workbook, { ...viewport, width: 1 })?.grid; if (!columns || !rows) return undefined; @@ -83,6 +83,54 @@ export function drawFrame( return tiled(workbook, viewport, frozenSize(grid, frozen), grid, 2); } +/** The engine's screen gridlines: one-pixel solid lines in its gridline grey, unlike any border it draws. */ +const gridline = (command: DisplayList["commands"][number]) => + command.op === "line" && command.color === "#d4d4d4" && command.width === 1 && !command.style; + +/** A frame without the gridlines the engine draws on every sheet, for a sheet whose view hides them. */ +export function withoutGridlines(frame: Frame): Frame { + return { + ...frame, + tiles: frame.tiles.map((tile) => ({ + ...tile, + list: { ...tile.list, commands: tile.list.commands.filter((command) => !gridline(command)) }, + })), + }; +} + +/** + * A frame whose chart text names real fonts. The engine leaves the theme's `+mj-lt` and `+mn-lt` in chart text and the + * painter sets chart families as CSS as they are, so an unresolved or missing family would keep the previous font. + */ +export function chartFonts( + frame: Frame, + theme: { readonly major: string; readonly minor: string }, +): Frame { + const family = (name: string) => { + const resolved = name.startsWith("+mj-") + ? theme.major + : name.startsWith("+mn-") + ? theme.minor + : name; + return `"${resolved.replaceAll('"', "")}", sans-serif`; + }; + + return { + ...frame, + tiles: frame.tiles.map((tile) => ({ + ...tile, + list: { + ...tile.list, + commands: tile.list.commands.map((command) => + command.op === "text" && command.chart + ? { ...command, fontFamily: family(command.fontFamily ?? theme.minor) } + : command, + ), + }, + })), + }; +} + /** * Draws a frame's tiles with the cells' top-left corner at `origin`, in device pixels, each tile from its seam on, so a * tile covers the panes it repeats. @@ -175,14 +223,12 @@ function tiled( const right = column.end === viewport.width ? column.end : column.end + seamOverlap; const bottom = row.end === viewport.height ? row.end : row.end + seamOverlap; - const list = attempt(() => - workbook.displayList({ - x: viewport.x + x, - y: viewport.y + y, - width: right - x, - height: bottom - y, - }), - ); + const list = untruncated(workbook, { + x: viewport.x + x, + y: viewport.y + y, + width: right - x, + height: bottom - y, + }); if (list) tiles.push({ list, x, y, clipX: column.start, clipY: row.start }); @@ -205,6 +251,13 @@ function tiled( return tiled(workbook, viewport, panes, grid, split + 1); } +/** A viewport's display list, or undefined where the engine refuses it or cuts it short at its cell limit. */ +function untruncated(workbook: WorkbookHandle, viewport: SheetViewport) { + const list = attempt(() => workbook.displayList(viewport)); + + return list?.truncated ? undefined : list; +} + /** Even spans of a side, the first holding the frozen panes whole; a seam inside them is dropped. */ function segments(size: number, panes: number, split: number) { const starts = Array.from({ length: split }, (_, part) => diff --git a/apps/web/src/components/files/office/sheet-protocol.ts b/apps/web/src/components/files/office/sheet-protocol.ts index 6f41fc8c3cfe..fd66ef9f7405 100644 --- a/apps/web/src/components/files/office/sheet-protocol.ts +++ b/apps/web/src/components/files/office/sheet-protocol.ts @@ -1,6 +1,7 @@ // Adapted from opencode (MIT). Copyright (c) 2025 opencode. See LICENSE.opencode. import type { SheetInfo } from "@betteroffice/xlsx"; import type { CellAddr, CellRange, GridMeta, HyperlinkRegion } from "@betteroffice/xlsx/headless"; +import type { WorkbookView } from "../officePreviewProtocol"; import type { Span } from "./sheet-cells"; import type { Method } from "./worker-rpc"; @@ -70,7 +71,7 @@ export type SheetMethods = { * saved on a hidden sheet. Undefined when no sheet is visible. */ readonly open: Method< - { readonly bytes: ArrayBuffer }, + { readonly bytes: ArrayBuffer; readonly view: WorkbookView }, { readonly tabs: readonly SheetTab[]; readonly layout: SheetLayout } | undefined >; /** Makes a sheet the active one; rejects, leaving the sheet before it active, when it cannot open. */ diff --git a/apps/web/src/components/files/office/spreadsheet.worker.ts b/apps/web/src/components/files/office/spreadsheet.worker.ts index eeb6b41478fd..466e29b29bff 100644 --- a/apps/web/src/components/files/office/spreadsheet.worker.ts +++ b/apps/web/src/components/files/office/spreadsheet.worker.ts @@ -10,7 +10,9 @@ import { } from "@betteroffice/xlsx"; import type { CellRange } from "@betteroffice/xlsx/headless"; import { anchorRange, attempt, lastCell, parseRange, parseSpans } from "./sheet-cells"; -import { drawFrame, paintFrame } from "./sheet-frame"; +import type { WorkbookView } from "../officePreviewProtocol"; +import { chartFonts, drawFrame, paintFrame, withoutGridlines } from "./sheet-frame"; +import { sheetFonts } from "./sheet-fonts"; import { frozenSize } from "./sheet-paint"; import type { SheetLayout, SheetMethods, SheetTab } from "./sheet-protocol"; import { serveWorker, Transfer } from "./worker-rpc"; @@ -26,6 +28,9 @@ type Opened = { readonly ends: Map; /** The one canvas every frame paints on and hands over as a bitmap. */ readonly surface: OffscreenCanvas; + /** The sheets whose view hides gridlines, which the engine draws on every sheet. */ + readonly hiddenGridlines: ReadonlySet; + readonly themeFonts: WorkbookView["themeFonts"]; }; /** The workbook this worker opened, once it has. */ @@ -33,6 +38,8 @@ type State = { opened: Opened | undefined }; const state: State = { opened: undefined }; +const loadFonts = sheetFonts(); + serveWorker({ open: async (input) => { if (state.opened) throw new Error("The worker already holds a workbook"); @@ -42,7 +49,14 @@ serveWorker({ const workbook = openWorkbook(new Uint8Array(input.bytes)); const info = workbook.sheetInfo(); - state.opened = { workbook, info, ends: new Map(), surface: new OffscreenCanvas(1, 1) }; + state.opened = { + workbook, + info, + ends: new Map(), + surface: new OffscreenCanvas(1, 1), + hiddenGridlines: new Set(input.view.hiddenGridlines), + themeFonts: input.view.themeFonts, + }; const tabs = visibleTabs(workbook, info); // A workbook saved on a hidden sheet opens on the first visible one, as Excel does. @@ -64,13 +78,20 @@ serveWorker({ throw new Error(`Sheet ${input.sheet} could not open`); }, - frame: (input) => { + frame: async (input) => { const opened = active(input.sheet); const frozen = { rows: opened.info.frozenRows, cols: opened.info.frozenCols }; const viewport = { x: input.x, y: input.y, width: input.width, height: input.height }; - const drawn = drawFrame(opened.workbook, viewport, frozen, input.wider); + const frame = drawFrame(opened.workbook, viewport, frozen, input.wider); + + if (!frame) return undefined; + + const drawn = chartFonts( + opened.hiddenGridlines.has(input.sheet) ? withoutGridlines(frame) : frame, + opened.themeFonts, + ); - if (!drawn) return undefined; + await loadFonts(drawn.tiles.map((tile) => tile.list)); const surface = opened.surface; const width = Math.ceil(input.offset.x + drawn.viewport.width * input.ratio); diff --git a/apps/web/src/components/files/officeArchive.ts b/apps/web/src/components/files/officeArchive.ts index 1d9526621324..3477480a4f61 100644 --- a/apps/web/src/components/files/officeArchive.ts +++ b/apps/web/src/components/files/officeArchive.ts @@ -215,10 +215,20 @@ function boundedWorksheet( const rowTags = new Map(); const metadataAttributes = new Map< string, - { ref: string | undefined; min: string | undefined; max: string | undefined } + { + ref: string | undefined; + min: string | undefined; + max: string | undefined; + hidden: string | undefined; + } >(); let dataStart = 0; + let dataTag = 0; + let dataPrefix = ""; let dataEnd = 0; + let defaultWidth: number | undefined; + let gridlines: boolean | undefined; + const sizedColumns: { min: number; max: number }[] = []; let emptyData = false; let depth = 0; let row = 0; @@ -267,17 +277,38 @@ function boundedWorksheet( ref: attribute("ref"), min: attribute("min"), max: attribute("max"), + hidden: attribute("hidden"), }, ); + // The engine draws gridlines on every sheet, so the first view's choice travels beside the bytes. + if (depth === 3 && tag.local === "sheetView" && gridlines === undefined) { + const shown = attribute("showGridLines"); + gridlines = shown !== "0" && shown !== "false"; + } + if (depth === 2 && tag.local === "sheetFormatPr") { + const width = Number(attribute("defaultColWidth") ?? Number.NaN); + if (width >= 0 && width <= 255) defaultWidth = width; + } + if (depth === 3 && tag.local === "col") { + const hidden = attribute("hidden"); + const min = Number(attribute("min") ?? 1); + const max = Number(attribute("max") ?? min); + if ( + (attribute("width") !== undefined || hidden === "1" || hidden === "true") && + Number.isInteger(min) && + Number.isInteger(max) + ) + sizedColumns.push({ min, max }); + } if (depth === 2 && tag.local === "sheetData") { dataStart = parser.position; + dataTag = xml.lastIndexOf("<", parser.position - 1); + dataPrefix = tag.prefix ? `${tag.prefix}:` : ""; emptyData = tag.isSelfClosing; } + // A dimension past the limits loses nothing by itself; the cells past them say whether content is lost. if (depth === 2 && tag.local === "dimension") { - for (const ref of (attribute("ref") ?? "").split(":")) { - const { col, line } = coordinate(ref); - if (col > OFFICE_PREVIEW_MAX_COLUMNS || line > OFFICE_PREVIEW_MAX_ROWS) truncated = true; - } + for (const ref of (attribute("ref") ?? "").split(":")) coordinate(ref); } else if (depth === 3 && tag.local === "row") { explicitRow = attribute("r") !== undefined; row = attribute("r") === undefined ? row + 1 : Number(attribute("r")); @@ -298,7 +329,8 @@ function boundedWorksheet( if (row < 1 || position.line !== row) throw new Error("This workbook contains inconsistent cell rows."); if (row > OFFICE_PREVIEW_MAX_ROWS || column > OFFICE_PREVIEW_MAX_COLUMNS) { - truncated = true; + // An empty element is a styled blank, which shows nothing. + if (!tag.isSelfClosing) truncated = true; return; } maxRow = Math.max(maxRow, row); @@ -403,7 +435,12 @@ function boundedWorksheet( } if (/<(?:\w+:)?col\b/.test(opening)) { const min = Math.max(1, Number(attributes.min ?? 1)); - const max = Math.min(OFFICE_PREVIEW_MAX_COLUMNS, Number(attributes.max ?? 16384)); + // Hidden columns keep their full span, or the columns past the limit would show again. + const hidden = attributes.hidden === "1" || attributes.hidden === "true"; + const max = Math.min( + hidden ? 16384 : OFFICE_PREVIEW_MAX_COLUMNS, + Number(attributes.max ?? 16384), + ); if (!Number.isInteger(min) || !Number.isInteger(max) || min > max) return ""; replace("min", String(min)); replace("max", String(max)); @@ -411,10 +448,27 @@ function boundedWorksheet( return tag; }, ); + // The engine sizes columns without a width at Excel's 8.43 characters and ignores `defaultColWidth`, so the + // columns it covers get that width spelled out. A second `` is valid: the schema allows any number. + let defaultColumns = ""; + if (defaultWidth !== undefined) { + let next = 1; + for (const span of sizedColumns.toSorted((a, b) => a.min - b.min)) { + if (span.min > next) + defaultColumns += `<${dataPrefix}col min="${next}" max="${Math.min(span.min - 1, OFFICE_PREVIEW_MAX_COLUMNS)}" width="${defaultWidth}"/>`; + next = Math.max(next, span.max + 1); + if (next > OFFICE_PREVIEW_MAX_COLUMNS) break; + } + if (next <= OFFICE_PREVIEW_MAX_COLUMNS) + defaultColumns += `<${dataPrefix}col min="${next}" max="${OFFICE_PREVIEW_MAX_COLUMNS}" width="${defaultWidth}"/>`; + if (defaultColumns) defaultColumns = `<${dataPrefix}cols>${defaultColumns}`; + } + const head = (part: string) => + metadata(part.slice(0, dataTag) + defaultColumns + part.slice(dataTag)); return { xml: emptyData - ? metadata(xml) - : `${metadata(xml.slice(0, dataStart))}${[...rows] + ? head(xml) + : `${head(xml.slice(0, dataStart))}${[...rows] .sort(([a], [b]) => a - b) .map( ([line, values]) => @@ -425,12 +479,49 @@ function boundedWorksheet( ) .join("")}${metadata(xml.slice(dataEnd))}`, truncated, + gridlines: gridlines ?? true, }; } +/** + * A stylesheet as Excel reads it, in the terms the engine understands. The engine takes a number format id under 164 as + * Excel's built-in one, though workbooks define their own there (Excel writes its locale formats 5 to 8 that way), so + * those move to free ids. It also reads `` as a boolean, which loses every named underline but the bare one. + */ +function engineStyles(xml: string) { + const name = (local: string) => `<((?:[\\w.-]+:)?)${local}\\b`; + const block = (local: string) => + new RegExp(`${name(local)}[^>]*>[\\s\\S]*?`, "g"); + const id = /(numFmtId\s*=\s*)(["'])(\d+)\2/g; + const moved = new Map(); + const formats = xml.match(block("numFmts"))?.[0] ?? ""; + let next = Math.max(163, ...[...formats.matchAll(id)].map((match) => Number(match[3]))) + 1; + const renumber = (part: string, assign: boolean) => + part.replace(id, (attribute: string, key: string, quote: string, value: string) => { + if (assign && Number(value) < 164 && !moved.has(value)) moved.set(value, next++); + const to = moved.get(value); + return to === undefined ? attribute : `${key}${quote}${to}${quote}`; + }); + return xml + .replace(block("numFmts"), (part) => renumber(part, true)) + .replace(block("cellStyleXfs"), (part) => renumber(part, false)) + .replace(block("cellXfs"), (part) => renumber(part, false)) + .replace( + new RegExp( + `${name("u")}\\s+val\\s*=\\s*(["'])(?:single|double|singleAccounting|doubleAccounting)\\2\\s*/>`, + "g", + ), + "<$1u/>", + ); +} + async function boundWorkbook(archive: JSZip) { const paths = new Map(); + // Chart sheets and other non-grid sheets hold no cells to bound. + const otherSheets = new Set(); let sharedPath = "xl/sharedStrings.xml"; + let stylesPath = "xl/styles.xml"; + let themePath = "xl/theme/theme1.xml"; const relationships = xmlParser(); relationships.on("opentag", (tag) => { if (tag.local !== "Relationship") return; @@ -440,22 +531,44 @@ async function boundWorkbook(archive: JSZip) { "https://docx.invalid/xl/workbook.xml", ).pathname.slice(1); if (type?.endsWith("/worksheet")) paths.set(tag.attributes.Id?.value ?? "", path); + else if (/\/(?:chartsheet|dialogsheet|xlMacrosheet|xlIntlMacrosheet)$/.test(type ?? "")) + otherSheets.add(tag.attributes.Id?.value ?? ""); if (type?.endsWith("/sharedStrings")) sharedPath = path; + if (type?.endsWith("/styles")) stylesPath = path; + if (type?.endsWith("/theme")) themePath = path; }); relationships .write((await archive.file("xl/_rels/workbook.xml.rels")?.async("string")) ?? "") .close(); const sheetPaths: string[] = []; + // Every sheet in workbook order, as the engine indexes them; chart sheets have no worksheet path. + const sheetOrder: (string | undefined)[] = []; const workbook = xmlParser(); workbook.on("opentag", (tag) => { if (tag.local !== "sheet") return; const id = Object.values(tag.attributes).find((attribute) => attribute.local === "id")?.value; + if (otherSheets.has(id ?? "")) { + sheetOrder.push(undefined); + return; + } const path = paths.get(id ?? ""); if (!path || sheetPaths.length === 100) throw new Error("This workbook has invalid or too many sheets."); sheetPaths.push(path); + sheetOrder.push(path); }); workbook.write((await archive.file("xl/workbook.xml")?.async("string")) ?? "").close(); + const theme = (await archive.file(themePath)?.async("string")) ?? ""; + const typeface = (font: string, fallback: string) => + new RegExp( + `<(?:[\\w.-]+:)?${font}\\b[^>]*>\\s*<(?:[\\w.-]+:)?latin\\b[^>]*\\btypeface\\s*=\\s*["']([^"']+)["']`, + ).exec(theme)?.[1] ?? fallback; + const themeFonts = { + major: typeface("majorFont", "Calibri Light"), + minor: typeface("minorFont", "Calibri"), + }; + const styles = await archive.file(stylesPath)?.async("string"); + if (styles) archive.file(stylesPath, engineStyles(styles)); const strings: string[] = []; const shared = await archive.file(sharedPath)?.async("string"); if (shared) { @@ -478,15 +591,20 @@ async function boundWorkbook(archive: JSZip) { parser.write(shared).close(); } const budget = { remaining: 1024 * 1024, cells: 0 }; - const limited = new Map(); + const bounded = new Map(); for (const path of new Set(sheetPaths)) { const file = archive.file(path); if (!file) throw new Error("This workbook has a missing worksheet."); const sheet = boundedWorksheet(await file.async("string"), strings, budget); archive.file(path, sheet.xml); - limited.set(path, sheet.truncated); + bounded.set(path, sheet); } - return sheetPaths.map((path) => limited.get(path) ?? false); + return { + sheets: sheetOrder.map( + (path) => (path && bounded.get(path)) || { truncated: false, gridlines: true }, + ), + themeFonts, + }; } const WORD_RELATIONSHIP = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/"; @@ -1116,8 +1234,8 @@ export async function boundedArchive(input: Extract sheet.truncated) || false, + view: result.workbook && { + hiddenGridlines: result.workbook.sheets.flatMap((sheet, index) => + sheet.gridlines ? [] : [index], + ), + themeFonts: result.workbook.themeFonts, + }, }; } diff --git a/apps/web/src/components/files/officePreviewProtocol.ts b/apps/web/src/components/files/officePreviewProtocol.ts index 51f3b8f4cd19..bd35181dd2f2 100644 --- a/apps/web/src/components/files/officePreviewProtocol.ts +++ b/apps/web/src/components/files/officePreviewProtocol.ts @@ -4,6 +4,14 @@ export const OFFICE_PREVIEW_MAX_BYTES = 50 * 1024 * 1024; export const OFFICE_PREVIEW_MAX_ROWS = 250_000; export const OFFICE_PREVIEW_MAX_COLUMNS = 1024; +/** What a workbook's views ask of the preview that its engine does not read. */ +export type WorkbookView = { + /** The sheets, by index in workbook order, whose view hides gridlines. */ + readonly hiddenGridlines: readonly number[]; + /** The theme's heading and body fonts, which chart text names as `+mj-lt` and `+mn-lt`. */ + readonly themeFonts: { readonly major: string; readonly minor: string }; +}; + export type OfficeWorkerRequest = | { readonly type: "open"; readonly kind: OfficePreviewKind; readonly bytes: ArrayBuffer } | { readonly type: "sheet-page"; readonly sheet: number; readonly index: number }; @@ -26,6 +34,8 @@ export type OfficeWorkerResult = readonly type: "docx" | "xlsx" | "pptx"; readonly bytes: ArrayBuffer; readonly truncated: boolean; + /** For a workbook; undefined for other files. */ + readonly view: WorkbookView | undefined; } | { readonly type: "ods"; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6451f3f97cb2..a0e46ef10ade 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1021,8 +1021,8 @@ importers: specifier: 0.2.0 version: 0.2.0 '@betteroffice/xlsx': - specifier: 0.3.0 - version: 0.3.0 + specifier: 0.5.1 + version: 0.5.1 '@clerk/clerk-js': specifier: 6.42.3 version: 6.42.3(react-dom@19.3.0(react@19.3.0))(react@19.3.0) @@ -3263,8 +3263,8 @@ packages: '@betteroffice/pptx@0.2.0': resolution: {integrity: sha512-pNPQ58tlA+pVibERbLdQNskvP7a0GIMf4jhkCU/gPp8043Ichf6gB3PSb4ra9sfqoIaWqOaZKCWyptovA2+Q0A==} - '@betteroffice/xlsx@0.3.0': - resolution: {integrity: sha512-K2bHtdk/epAxsmvxJlIWBRndjP3ltQU0Oh3p4gcZd2n5/YgXglo3SmMYX4nQe8WEm/yvz91p+aIasKyXXQP9Wg==} + '@betteroffice/xlsx@0.5.1': + resolution: {integrity: sha512-UHOnzcMML6Ud9D1M4d8FZKmw3AAg2/DttE/sVxM858SgiIpOtzGOqlh62RM9/8y/JW8bR8t+wN69EX8UJvR8EQ==} '@blazediff/core@1.10.0': resolution: {integrity: sha512-AOQff0zgR7cGsZL+4E7hVkmujoPUpm0J9xzWGWZj5wCjd3gmxESXAPfKyuzs93VdpQNFhHlBhfOjrcZ+XTERtQ==} @@ -13464,7 +13464,7 @@ snapshots: '@betteroffice/pptx@0.2.0': {} - '@betteroffice/xlsx@0.3.0': {} + '@betteroffice/xlsx@0.5.1': {} '@blazediff/core@1.10.0': {} diff --git a/third-party-licenses.config.json b/third-party-licenses.config.json index 0ce031a30f35..91a4a8249e89 100644 --- a/third-party-licenses.config.json +++ b/third-party-licenses.config.json @@ -261,7 +261,7 @@ }, { "name": "@betteroffice/xlsx", - "version": "0.3.0", + "version": "0.5.1", "license": "Apache-2.0", "sourceUrl": "https://github.com/openooxml/betteroffice", "generatedNotice": { From a94718a1fca393a198cda2385d5f18f0d6199629 Mon Sep 17 00:00:00 2001 From: Yash Singh Date: Wed, 11 Nov 2026 11:02:34 -0600 Subject: [PATCH 2/7] fix(web): keep preview number format IDs within engine limits --- .../files/AttachmentFilePreview.browser.test.tsx | 2 +- apps/web/src/components/files/officeArchive.ts | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx b/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx index 7dc469f583b1..208b4a97ed0d 100644 --- a/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx +++ b/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx @@ -1789,7 +1789,7 @@ it("shows the number formats and underlines a workbook's own styles define", asy ); archive.file( "xl/styles.xml", - ``, + ``, ); archive.file( "xl/worksheets/sheet1.xml", diff --git a/apps/web/src/components/files/officeArchive.ts b/apps/web/src/components/files/officeArchive.ts index 3477480a4f61..3202d5112c90 100644 --- a/apps/web/src/components/files/officeArchive.ts +++ b/apps/web/src/components/files/officeArchive.ts @@ -495,10 +495,15 @@ function engineStyles(xml: string) { const id = /(numFmtId\s*=\s*)(["'])(\d+)\2/g; const moved = new Map(); const formats = xml.match(block("numFmts"))?.[0] ?? ""; - let next = Math.max(163, ...[...formats.matchAll(id)].map((match) => Number(match[3]))) + 1; + const used = new Set([...formats.matchAll(id)].map((match) => Number(match[3]))); + let next = 164; const renumber = (part: string, assign: boolean) => part.replace(id, (attribute: string, key: string, quote: string, value: string) => { - if (assign && Number(value) < 164 && !moved.has(value)) moved.set(value, next++); + if (assign && Number(value) < 164 && !moved.has(value)) { + while (used.has(next)) next++; + if (next > 65535) throw new Error("This workbook has too many custom number formats."); + moved.set(value, next++); + } const to = moved.get(value); return to === undefined ? attribute : `${key}${quote}${to}${quote}`; }); From 7622eea4f7a267e9badf3e5167b3ba404b16fa2a Mon Sep 17 00:00:00 2001 From: Yash Singh Date: Wed, 11 Nov 2026 11:09:40 -0600 Subject: [PATCH 3/7] fix(web): report truncated styled spreadsheet cells --- .../components/files/AttachmentFilePreview.browser.test.tsx | 3 +-- apps/web/src/components/files/officeArchive.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx b/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx index 208b4a97ed0d..2235e2b78607 100644 --- a/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx +++ b/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx @@ -1719,7 +1719,6 @@ it.each(["repeated binary", "XML relationship alias", "duplicate relationship ID }, ); -// XFD300 is a styled blank past the column limit, which loses nothing. it.each(["XFD200", "XFD300", "XFD1048576", "AX500", "AZ9999999"])( "bounds spreadsheet cells at %s before padding", async (ref) => { @@ -1766,7 +1765,7 @@ it.each(["XFD200", "XFD300", "XFD1048576", "AX500", "AZ9999999"])( ); expect(sheet.querySelector('c[r="A1"] v')?.textContent).toBe("7"); expect(sheet.querySelectorAll("row").length).toBeLessThanOrEqual(500); - expect(result.truncated).toBe(ref === "XFD200"); + expect(result.truncated).toBe(ref === "XFD200" || ref === "XFD300"); if (ref === "AX500") expect(sheet.querySelector('c[r="AX500"] v')?.textContent).toBe("9"); else expect(sheet.querySelector('c[r="' + ref + '"]')).toBeNull(); } diff --git a/apps/web/src/components/files/officeArchive.ts b/apps/web/src/components/files/officeArchive.ts index 3202d5112c90..eff859d8dc56 100644 --- a/apps/web/src/components/files/officeArchive.ts +++ b/apps/web/src/components/files/officeArchive.ts @@ -329,8 +329,7 @@ function boundedWorksheet( if (row < 1 || position.line !== row) throw new Error("This workbook contains inconsistent cell rows."); if (row > OFFICE_PREVIEW_MAX_ROWS || column > OFFICE_PREVIEW_MAX_COLUMNS) { - // An empty element is a styled blank, which shows nothing. - if (!tag.isSelfClosing) truncated = true; + truncated = true; return; } maxRow = Math.max(maxRow, row); From 93e5869b7f651c1a2327395a3944e3c2d04951a9 Mon Sep 17 00:00:00 2001 From: Yash Singh Date: Wed, 11 Nov 2026 11:15:49 -0600 Subject: [PATCH 4/7] fix(web): parse spreadsheet style blocks without repeated scans --- .../AttachmentFilePreview.browser.test.tsx | 4 ++ .../web/src/components/files/officeArchive.ts | 43 ++++++++++++++++--- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx b/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx index 2235e2b78607..9cc642657615 100644 --- a/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx +++ b/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx @@ -1822,6 +1822,10 @@ it("shows the number formats and underlines a workbook's own styles define", asy } finally { worker.terminate(); } + archive.file("xl/styles.xml", ""); + await expect( + prepareOffice(await archive.generateAsync({ type: "arraybuffer" }), "xlsx"), + ).rejects.toThrow("duplicate stylesheet blocks"); }); it("keeps spreadsheet dates on their calendar day west of UTC", async () => { diff --git a/apps/web/src/components/files/officeArchive.ts b/apps/web/src/components/files/officeArchive.ts index eff859d8dc56..030ef79aa876 100644 --- a/apps/web/src/components/files/officeArchive.ts +++ b/apps/web/src/components/files/officeArchive.ts @@ -489,11 +489,31 @@ function boundedWorksheet( */ function engineStyles(xml: string) { const name = (local: string) => `<((?:[\\w.-]+:)?)${local}\\b`; - const block = (local: string) => - new RegExp(`${name(local)}[^>]*>[\\s\\S]*?`, "g"); + const blocks = new Map(); + const parser = xmlParser(); + let depth = 0; + parser.on("opentag", (tag) => { + if (++depth > 64) throw new Error("This workbook contains excessive XML nesting."); + if (depth !== 2 || !["numFmts", "cellStyleXfs", "cellXfs"].includes(tag.local)) return; + if (blocks.has(tag.local)) + throw new Error("This workbook contains duplicate stylesheet blocks."); + blocks.set(tag.local, { + start: xml.lastIndexOf("<", parser.position - 1), + end: parser.position, + }); + }); + parser.on("closetag", (tag) => { + if (depth === 2) { + const block = blocks.get(tag.local); + if (block) block.end = parser.position; + } + depth--; + }); + parser.write(xml).close(); const id = /(numFmtId\s*=\s*)(["'])(\d+)\2/g; const moved = new Map(); - const formats = xml.match(block("numFmts"))?.[0] ?? ""; + const formatBlock = blocks.get("numFmts"); + const formats = formatBlock ? xml.slice(formatBlock.start, formatBlock.end) : ""; const used = new Set([...formats.matchAll(id)].map((match) => Number(match[3]))); let next = 164; const renumber = (part: string, assign: boolean) => @@ -506,10 +526,19 @@ function engineStyles(xml: string) { const to = moved.get(value); return to === undefined ? attribute : `${key}${quote}${to}${quote}`; }); - return xml - .replace(block("numFmts"), (part) => renumber(part, true)) - .replace(block("cellStyleXfs"), (part) => renumber(part, false)) - .replace(block("cellXfs"), (part) => renumber(part, false)) + const numberedFormats = renumber(formats, true); + const parts: string[] = []; + let offset = 0; + for (const [local, block] of blocks) { + parts.push( + xml.slice(offset, block.start), + local === "numFmts" ? numberedFormats : renumber(xml.slice(block.start, block.end), false), + ); + offset = block.end; + } + parts.push(xml.slice(offset)); + return parts + .join("") .replace( new RegExp( `${name("u")}\\s+val\\s*=\\s*(["'])(?:single|double|singleAccounting|doubleAccounting)\\2\\s*/>`, From 0e88e8fc1acebca80394fa12b656a9e91174cdf7 Mon Sep 17 00:00:00 2001 From: Yash Singh Date: Wed, 11 Nov 2026 11:46:08 -0600 Subject: [PATCH 5/7] fix(web): normalize equivalent spreadsheet style values --- .../AttachmentFilePreview.browser.test.tsx | 105 +++++++++--------- .../web/src/components/files/officeArchive.ts | 11 +- 2 files changed, 60 insertions(+), 56 deletions(-) diff --git a/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx b/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx index 9cc642657615..059e730663bf 100644 --- a/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx +++ b/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx @@ -1775,58 +1775,61 @@ it.each(["XFD200", "XFD300", "XFD1048576", "AX500", "AZ9999999"])( }, ); -it("shows the number formats and underlines a workbook's own styles define", async () => { - const { default: JSZip } = await import("jszip"); - const archive = new JSZip(); - archive.file( - "xl/workbook.xml", - '', - ); - archive.file( - "xl/_rels/workbook.xml.rels", - '', - ); - archive.file( - "xl/styles.xml", - ``, - ); - archive.file( - "xl/worksheets/sheet1.xml", - '10044986Total', - ); - const { prepareOffice } = await import("./officeArchive"); - const prepared = await prepareOffice( - await archive.generateAsync({ type: "arraybuffer" }), - "xlsx", - ); - const styles = await ( - await JSZip.loadAsync(prepared.bytes) - ) - .file("xl/styles.xml")! - .async("string"); - expect(styles).toContain(""); - const worker = new Worker(new URL("./office/spreadsheet.worker.ts", import.meta.url), { - type: "module", - }); - try { - const { createWorkerClient } = await import("./office/worker-rpc"); - const client = createWorkerClient(worker); - const bytes = prepared.bytes; - await client.call("open", { bytes, view: prepared.view! }, { transfer: [bytes] }); - const cells = await client.call("cells", { - sheet: 0, - range: { top: 0, bottom: 0, left: 0, right: 1 }, +it.each(['', ''])( + "shows workbook formats and underline %s", + async (underline) => { + const { default: JSZip } = await import("jszip"); + const archive = new JSZip(); + archive.file( + "xl/workbook.xml", + '', + ); + archive.file( + "xl/_rels/workbook.xml.rels", + '', + ); + archive.file( + "xl/styles.xml", + `${underline}`, + ); + archive.file( + "xl/worksheets/sheet1.xml", + '10044986Total', + ); + const { prepareOffice } = await import("./officeArchive"); + const prepared = await prepareOffice( + await archive.generateAsync({ type: "arraybuffer" }), + "xlsx", + ); + const styles = await ( + await JSZip.loadAsync(prepared.bytes) + ) + .file("xl/styles.xml")! + .async("string"); + expect(styles).toContain(""); + const worker = new Worker(new URL("./office/spreadsheet.worker.ts", import.meta.url), { + type: "module", }); - // Ids under 164 that a workbook defines keep its formats, not Excel's built-in ones. - expect(cells[0]).toEqual(["￥100.00", "01-Mar-2023"]); - } finally { - worker.terminate(); - } - archive.file("xl/styles.xml", ""); - await expect( - prepareOffice(await archive.generateAsync({ type: "arraybuffer" }), "xlsx"), - ).rejects.toThrow("duplicate stylesheet blocks"); -}); + try { + const { createWorkerClient } = await import("./office/worker-rpc"); + const client = createWorkerClient(worker); + const bytes = prepared.bytes; + await client.call("open", { bytes, view: prepared.view! }, { transfer: [bytes] }); + const cells = await client.call("cells", { + sheet: 0, + range: { top: 0, bottom: 0, left: 0, right: 1 }, + }); + // Ids under 164 that a workbook defines keep its formats, not Excel's built-in ones. + expect(cells[0]).toEqual(["￥100.00", "01-Mar-2023"]); + } finally { + worker.terminate(); + } + archive.file("xl/styles.xml", ""); + await expect( + prepareOffice(await archive.generateAsync({ type: "arraybuffer" }), "xlsx"), + ).rejects.toThrow("duplicate stylesheet blocks"); + }, +); it("keeps spreadsheet dates on their calendar day west of UTC", async () => { const { default: JSZip } = await import("jszip"); diff --git a/apps/web/src/components/files/officeArchive.ts b/apps/web/src/components/files/officeArchive.ts index 030ef79aa876..d33d7a046165 100644 --- a/apps/web/src/components/files/officeArchive.ts +++ b/apps/web/src/components/files/officeArchive.ts @@ -511,19 +511,20 @@ function engineStyles(xml: string) { }); parser.write(xml).close(); const id = /(numFmtId\s*=\s*)(["'])(\d+)\2/g; - const moved = new Map(); + const moved = new Map(); const formatBlock = blocks.get("numFmts"); const formats = formatBlock ? xml.slice(formatBlock.start, formatBlock.end) : ""; const used = new Set([...formats.matchAll(id)].map((match) => Number(match[3]))); let next = 164; const renumber = (part: string, assign: boolean) => part.replace(id, (attribute: string, key: string, quote: string, value: string) => { - if (assign && Number(value) < 164 && !moved.has(value)) { + const number = Number(value); + if (assign && number < 164 && !moved.has(number)) { while (used.has(next)) next++; if (next > 65535) throw new Error("This workbook has too many custom number formats."); - moved.set(value, next++); + moved.set(number, next++); } - const to = moved.get(value); + const to = moved.get(number); return to === undefined ? attribute : `${key}${quote}${to}${quote}`; }); const numberedFormats = renumber(formats, true); @@ -541,7 +542,7 @@ function engineStyles(xml: string) { .join("") .replace( new RegExp( - `${name("u")}\\s+val\\s*=\\s*(["'])(?:single|double|singleAccounting|doubleAccounting)\\2\\s*/>`, + `${name("u")}\\s+val\\s*=\\s*(["'])(?:single|double|singleAccounting|doubleAccounting)\\2\\s*(?:/>|>\\s*)`, "g", ), "<$1u/>", From da105735dadcabc0a9a1fad2a65b72e657c0b02c Mon Sep 17 00:00:00 2001 From: Yash Singh Date: Wed, 11 Nov 2026 11:50:04 -0600 Subject: [PATCH 6/7] fix(web): declare namespaces on generated spreadsheet columns --- .../AttachmentFilePreview.browser.test.tsx | 153 ++++++++++-------- .../web/src/components/files/officeArchive.ts | 8 +- 2 files changed, 89 insertions(+), 72 deletions(-) diff --git a/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx b/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx index 059e730663bf..cc36083864aa 100644 --- a/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx +++ b/apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx @@ -1038,79 +1038,90 @@ it.each(["utf-16le", "utf-16be", "windows-1252"])( ); it.each([ - ["", "worksheets/./sheet1.xml", false], - ["x:", "../xl/worksheets/sheet1.xml", false], - ["", "/xl/worksheets/sheet1.xml", true], -])("preserves %sworksheet metadata with relationship target %s", async (prefix, target, empty) => { - const { default: JSZip } = await import("jszip"); - const archive = new JSZip(); - const ns = "http://schemas.openxmlformats.org/spreadsheetml/2006/main"; - archive.file( - "xl/workbook.xml", - '', - ); - archive.file( - "xl/_rels/workbook.xml.rels", - ``, - ); - archive.file( - "xl/worksheets/sheet1.xml", - `<${prefix}worksheet xmlns${prefix ? ":x" : ""}="${ns}"><${prefix}dimension ref="XFD1048576"/><${prefix}sheetViews><${prefix}sheetView showGridLines="0"/><${prefix}sheetFormatPr defaultColWidth="30"/><${prefix}cols><${prefix}col min="${OFFICE_PREVIEW_MAX_COLUMNS - 1}" max="${OFFICE_PREVIEW_MAX_COLUMNS + 10}" width="20"/><${prefix}col min="${OFFICE_PREVIEW_MAX_COLUMNS + 1}" max="${OFFICE_PREVIEW_MAX_COLUMNS + 30}" hidden="1"/><${prefix}sheetData><${prefix}row ht="42" customHeight="1" hidden="1"><${prefix}c r="A50" t="inlineStr"><${prefix}is><${prefix}t>${"value".repeat(1000)}<${prefix}mergeCells><${prefix}mergeCell ref="XFD1048576"/>` - .replace(/<((?:x:)?(?:col|dimension|mergeCell))\b([^>]*)\/>/g, prefix ? "<$1$2>" : "$&") - .replace(/[\s\S]*?<\/sheetData>/, (data) => (empty ? "" : data)) - .replace(/="([^"]*)"/g, (attribute, value: string) => - prefix || empty - ? ` = '${value.replace(/\d/g, (digit) => `&#${digit.charCodeAt(0)};`)}'` - : attribute, - ), - ); - const bytes = await archive.generateAsync({ type: "arraybuffer" }); - const worker = new Worker(new URL("./officePreview.worker.ts", import.meta.url), { - type: "module", - }); - try { - const result = await new Promise((resolve) => { - worker.addEventListener( - "message", - (event: MessageEvent) => resolve(event.data), - { once: true }, - ); - worker.postMessage({ type: "open", kind: "xlsx", bytes }, [bytes]); - }); - expect(result.type).toBe("xlsx"); - if (result.type !== "xlsx") throw new Error("Expected bounded workbook"); - // The engine counts the chart sheet before it. - expect(result.view?.hiddenGridlines).toEqual([1]); - const bounded = await JSZip.loadAsync(result.bytes); - const sheet = new DOMParser().parseFromString( - await bounded.file("xl/worksheets/sheet1.xml")!.async("string"), - "application/xml", + ["", "worksheets/./sheet1.xml", false, false], + ["x:", "../xl/worksheets/sheet1.xml", false, false], + ["", "/xl/worksheets/sheet1.xml", true, false], + ["", "worksheets/sheet1.xml", false, true], + ["", "worksheets/sheet1.xml", true, true], +])( + "preserves %sworksheet metadata with relationship target %s", + async (prefix, target, empty, localPrefix) => { + const { default: JSZip } = await import("jszip"); + const archive = new JSZip(); + const ns = "http://schemas.openxmlformats.org/spreadsheetml/2006/main"; + archive.file( + "xl/workbook.xml", + '', ); - expect(sheet.querySelector("parsererror")).toBeNull(); - const row = sheet.getElementsByTagNameNS(ns, "row")[0]; - if (empty) expect(row).toBeUndefined(); - else { - expect(row!.getAttribute("ht")).toBe("42"); - expect(row!.getAttribute("hidden")).toBe("1"); - expect(row!.getElementsByTagNameNS(ns, "c")[0]!.getAttribute("r")).toBe("A50"); - expect(row!.getElementsByTagNameNS(ns, "t")[0]!.textContent).toHaveLength(4096); + archive.file( + "xl/_rels/workbook.xml.rels", + ``, + ); + archive.file( + "xl/worksheets/sheet1.xml", + `<${prefix}worksheet xmlns${prefix ? ":x" : ""}="${ns}"><${prefix}dimension ref="XFD1048576"/><${prefix}sheetViews><${prefix}sheetView showGridLines="0"/><${prefix}sheetFormatPr defaultColWidth="30"/><${prefix}cols><${prefix}col min="${OFFICE_PREVIEW_MAX_COLUMNS - 1}" max="${OFFICE_PREVIEW_MAX_COLUMNS + 10}" width="20"/><${prefix}col min="${OFFICE_PREVIEW_MAX_COLUMNS + 1}" max="${OFFICE_PREVIEW_MAX_COLUMNS + 30}" hidden="1"/><${prefix}sheetData><${prefix}row ht="42" customHeight="1" hidden="1"><${prefix}c r="A50" t="inlineStr"><${prefix}is><${prefix}t>${"value".repeat(1000)}<${prefix}mergeCells><${prefix}mergeCell ref="XFD1048576"/>` + .replace( + /<((?:x:)?(?:col|dimension|mergeCell))\b([^>]*)\/>/g, + prefix ? "<$1$2>" : "$&", + ) + .replace(/[\s\S]*?<\/sheetData>/, (data) => (empty ? "" : data)) + .replace(/<(\/?)sheetData\b/g, (tag, closing: string) => + localPrefix ? `<${closing}x:sheetData${closing ? "" : ` xmlns:x="${ns}"`}` : tag, + ) + .replace(/="([^"]*)"/g, (attribute, value: string) => + prefix || empty + ? ` = '${value.replace(/\d/g, (digit) => `&#${digit.charCodeAt(0)};`)}'` + : attribute, + ), + ); + const bytes = await archive.generateAsync({ type: "arraybuffer" }); + const worker = new Worker(new URL("./officePreview.worker.ts", import.meta.url), { + type: "module", + }); + try { + const result = await new Promise((resolve) => { + worker.addEventListener( + "message", + (event: MessageEvent) => resolve(event.data), + { once: true }, + ); + worker.postMessage({ type: "open", kind: "xlsx", bytes }, [bytes]); + }); + expect(result.type).toBe("xlsx"); + if (result.type !== "xlsx") throw new Error("Expected bounded workbook"); + // The engine counts the chart sheet before it. + expect(result.view?.hiddenGridlines).toEqual([1]); + const bounded = await JSZip.loadAsync(result.bytes); + const sheet = new DOMParser().parseFromString( + await bounded.file("xl/worksheets/sheet1.xml")!.async("string"), + "application/xml", + ); + expect(sheet.querySelector("parsererror")).toBeNull(); + const row = sheet.getElementsByTagNameNS(ns, "row")[0]; + if (empty) expect(row).toBeUndefined(); + else { + expect(row!.getAttribute("ht")).toBe("42"); + expect(row!.getAttribute("hidden")).toBe("1"); + expect(row!.getElementsByTagNameNS(ns, "c")[0]!.getAttribute("r")).toBe("A50"); + expect(row!.getElementsByTagNameNS(ns, "t")[0]!.textContent).toHaveLength(4096); + } + expect(sheet.getElementsByTagNameNS(ns, "dimension")).toHaveLength(0); + expect(sheet.getElementsByTagNameNS(ns, "mergeCell")).toHaveLength(0); + const columns = [...sheet.getElementsByTagNameNS(ns, "col")].map((column) => + ["min", "max", "width"].map((name) => column.getAttribute(name)), + ); + // The engine ignores `defaultColWidth`, so the columns it sizes are spelled out. + expect(columns).toEqual([ + [String(OFFICE_PREVIEW_MAX_COLUMNS - 1), String(OFFICE_PREVIEW_MAX_COLUMNS), "20"], + // Hidden columns keep their span past the limit, so they stay hidden. + [String(OFFICE_PREVIEW_MAX_COLUMNS + 1), String(OFFICE_PREVIEW_MAX_COLUMNS + 30), null], + ["1", String(OFFICE_PREVIEW_MAX_COLUMNS - 2), "30"], + ]); + } finally { + worker.terminate(); } - expect(sheet.getElementsByTagNameNS(ns, "dimension")).toHaveLength(0); - expect(sheet.getElementsByTagNameNS(ns, "mergeCell")).toHaveLength(0); - const columns = [...sheet.getElementsByTagNameNS(ns, "col")].map((column) => - ["min", "max", "width"].map((name) => column.getAttribute(name)), - ); - // The engine ignores `defaultColWidth`, so the columns it sizes are spelled out. - expect(columns).toEqual([ - [String(OFFICE_PREVIEW_MAX_COLUMNS - 1), String(OFFICE_PREVIEW_MAX_COLUMNS), "20"], - // Hidden columns keep their span past the limit, so they stay hidden. - [String(OFFICE_PREVIEW_MAX_COLUMNS + 1), String(OFFICE_PREVIEW_MAX_COLUMNS + 30), null], - ["1", String(OFFICE_PREVIEW_MAX_COLUMNS - 2), "30"], - ]); - } finally { - worker.terminate(); - } -}); + }, +); it("reports the final slide at maximum scroll after page visibility changes", async () => { const { OfficePages } = await import("./office/OfficePages"); diff --git a/apps/web/src/components/files/officeArchive.ts b/apps/web/src/components/files/officeArchive.ts index d33d7a046165..68369b1f8980 100644 --- a/apps/web/src/components/files/officeArchive.ts +++ b/apps/web/src/components/files/officeArchive.ts @@ -225,6 +225,7 @@ function boundedWorksheet( let dataStart = 0; let dataTag = 0; let dataPrefix = ""; + let dataNamespace = ""; let dataEnd = 0; let defaultWidth: number | undefined; let gridlines: boolean | undefined; @@ -304,6 +305,10 @@ function boundedWorksheet( dataStart = parser.position; dataTag = xml.lastIndexOf("<", parser.position - 1); dataPrefix = tag.prefix ? `${tag.prefix}:` : ""; + dataNamespace = tag.uri + .replaceAll("&", "&") + .replaceAll('"', """) + .replaceAll("<", "<"); emptyData = tag.isSelfClosing; } // A dimension past the limits loses nothing by itself; the cells past them say whether content is lost. @@ -460,7 +465,8 @@ function boundedWorksheet( } if (next <= OFFICE_PREVIEW_MAX_COLUMNS) defaultColumns += `<${dataPrefix}col min="${next}" max="${OFFICE_PREVIEW_MAX_COLUMNS}" width="${defaultWidth}"/>`; - if (defaultColumns) defaultColumns = `<${dataPrefix}cols>${defaultColumns}`; + if (defaultColumns) + defaultColumns = `<${dataPrefix}cols xmlns${dataPrefix ? `:${dataPrefix.slice(0, -1)}` : ""}="${dataNamespace}">${defaultColumns}`; } const head = (part: string) => metadata(part.slice(0, dataTag) + defaultColumns + part.slice(dataTag)); From 191e230a45c8a5165248ca67ec1b5691ab7b53f8 Mon Sep 17 00:00:00 2001 From: Yash Singh Date: Wed, 11 Nov 2026 13:40:15 -0600 Subject: [PATCH 7/7] fix(web): retain bundled fonts for missing spreadsheet styles --- .../components/files/office/fonts.browser.test.ts | 13 ++++++++++++- apps/web/src/components/files/office/fonts.ts | 5 ++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/apps/web/src/components/files/office/fonts.browser.test.ts b/apps/web/src/components/files/office/fonts.browser.test.ts index b9f67db4d7b6..65a56a86fd18 100644 --- a/apps/web/src/components/files/office/fonts.browser.test.ts +++ b/apps/web/src/components/files/office/fonts.browser.test.ts @@ -1,6 +1,6 @@ import { BUNDLED_FONTS } from "@betteroffice/fonts"; import { describe, expect, it } from "vite-plus/test"; -import { wordFonts } from "./fonts"; +import { sheetFace, wordFonts } from "./fonts"; const notoSansJp = BUNDLED_FONTS.find((face) => face.file === "NotoSansJP-Regular.otf")!; const fonts = wordFonts(); @@ -16,3 +16,14 @@ describe("wordFonts", () => { expect(bytes?.byteLength).toBe(notoSansJp.byteLength); }); }); + +it.each([false, true])( + "uses the available Comic Relief face for italic spreadsheet text (bold: %s)", + (bold) => { + expect(sheetFace("Comic Relief", bold, true)).toMatchObject({ + family: "Comic Relief", + weight: bold ? 700 : 400, + style: "normal", + }); + }, +); diff --git a/apps/web/src/components/files/office/fonts.ts b/apps/web/src/components/files/office/fonts.ts index 00c86e425ab8..34742174f475 100644 --- a/apps/web/src/components/files/office/fonts.ts +++ b/apps/web/src/components/files/office/fonts.ts @@ -38,7 +38,10 @@ export function presentationFace(family: string, bold: boolean, italic: boolean) /** The face Excel's cells draw a family with when it is missing: its metric-compatible or bundled face, if any. */ export function sheetFace(family: string, bold: boolean, italic: boolean) { return ( - resolveMetricCompatFace(family, bold, italic) ?? resolveBundledFamilyFace(family, bold, italic) + resolveMetricCompatFace(family, bold, italic) ?? + resolveBundledFamilyFace(family, bold, italic) ?? + resolveBundledFamilyFace(family, bold, false) ?? + resolveBundledFamilyFace(family, false, false) ); }