diff --git a/packages/documents.js/src/bin-dispatch.ts b/packages/documents.js/src/bin-dispatch.ts index 89291757f..dc4a67aab 100644 --- a/packages/documents.js/src/bin-dispatch.ts +++ b/packages/documents.js/src/bin-dispatch.ts @@ -26,13 +26,15 @@ const RUNNERS: Readonly> = { }; function detectPackageManager(userAgent: string | undefined): PackageManager { - const ua = userAgent ?? ""; - if (ua.startsWith("yarn/")) { + // No npm_config_user_agent at all (Deno never sets it; running the bin via bare `node` sets nothing) falls back to npm exactly like every other unrecognised value below -- handled as its own branch, rather than defaulting `userAgent` to an empty string first, so there is no fallback string literal whose own value is unobservable (every one of the startsWith checks below is false for it) and therefore untestable. + if (userAgent === undefined) return "npm"; + + if (userAgent.startsWith("yarn/")) { // Yarn classic (1.x) has no `dlx` subcommand -- it is Yarn Berry (2+) only -- so classic is treated as npm and runs through npx rather than a command that fails. - return ua.startsWith("yarn/1.") ? "npm" : "yarn"; + return userAgent.startsWith("yarn/1.") ? "npm" : "yarn"; } - if (ua.startsWith("pnpm/")) return "pnpm"; - if (ua.startsWith("bun/")) return "bun"; + if (userAgent.startsWith("pnpm/")) return "pnpm"; + if (userAgent.startsWith("bun/")) return "bun"; // npm, and any agent that doesn't identify itself (Deno doesn't set this env var at all; running the bin via bare `node` sets nothing), falls back to npx. return "npm"; } diff --git a/packages/documents.js/src/bin.test.ts b/packages/documents.js/src/bin.test.ts new file mode 100644 index 000000000..792dd1a5d --- /dev/null +++ b/packages/documents.js/src/bin.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it, vi } from "vitest"; + +// bin.ts is a real executable entry point: importing it runs its top-level code immediately, which spawns a child process and calls process.exit. Every test here mocks node:child_process's spawnSync and stubs process.exit/argv/env before a fresh dynamic import, then restores them. + +interface SpawnSyncCall { + readonly command: string; + readonly args: readonly string[]; + readonly options: unknown; +} + +async function runBin( + argv: readonly string[], + userAgent: string | undefined, + status: number | null, +): Promise<{ readonly call: SpawnSyncCall; readonly exitCode: unknown }> { + vi.resetModules(); + let call: SpawnSyncCall | undefined; + vi.doMock("node:child_process", () => ({ + spawnSync: (command: string, args: readonly string[], options: unknown) => { + call = { command, args, options }; + return { status }; + }, + })); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => { + return undefined as never; + }); + const originalArgv = process.argv; + const originalUserAgent = process.env.npm_config_user_agent; + process.argv = ["node", "documents.js", ...argv]; + if (userAgent === undefined) { + delete process.env.npm_config_user_agent; + } else { + process.env.npm_config_user_agent = userAgent; + } + try { + await import("./bin"); + } finally { + process.argv = originalArgv; + if (originalUserAgent === undefined) { + delete process.env.npm_config_user_agent; + } else { + process.env.npm_config_user_agent = originalUserAgent; + } + } + if (call === undefined) { + throw new Error("expected spawnSync to have been called"); + } + const exitCode = exitSpy.mock.calls[0]?.[0]; + exitSpy.mockRestore(); + vi.doUnmock("node:child_process"); + return { call, exitCode }; +} + +describe("bin", () => { + it("strips the node/script argv[0..1] before resolving dispatch, not the full argv", () => { + return runBin(["mcp"], "npm/10.2.4 node/v20", 0).then(({ call }) => { + // Without process.argv.slice(2), argv[0] would be "node" (not "mcp"), never triggering the mcp dispatch path -- this only resolves to document-mcp because the strip happened. + expect(call.command).toBe("npx"); + expect(call.args).toEqual(["-y", "document-mcp"]); + }); + }); + + it("spawns with the exact resolved args array and { stdio: 'inherit' } options", () => { + return runBin(["convert", "a.docx"], "npm/10.2.4 node/v20", 0).then( + ({ call }) => { + expect(call.args).toEqual(["-y", "document-cli", "convert", "a.docx"]); + expect(call.options).toEqual({ stdio: "inherit" }); + }, + ); + }); + + it("exits with the spawned process's own non-zero status, not always the same code", () => { + return runBin([], "npm/10.2.4 node/v20", 2).then(({ exitCode }) => { + expect(exitCode).toBe(2); + }); + }); + + it("exits with 1 when spawnSync reports no status at all (e.g. killed by a signal)", () => { + return runBin([], "npm/10.2.4 node/v20", null).then(({ exitCode }) => { + expect(exitCode).toBe(1); + }); + }); +}); diff --git a/packages/documents.js/src/codecs/read.test.ts b/packages/documents.js/src/codecs/read.test.ts new file mode 100644 index 000000000..469a99f98 --- /dev/null +++ b/packages/documents.js/src/codecs/read.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it, vi } from "vitest"; +import { CONTENT_READERS, readDocumentLayout } from "./read"; +import { encodeMarkdownText } from "../markdown/text"; + +describe("CONTENT_READERS.markdown", () => { + it("forwards the images resolver through to readMarkdownContent", () => { + const resolver = vi.fn(() => undefined); + CONTENT_READERS.markdown(encodeMarkdownText("![alt](img.png)"), { + images: resolver, + }); + expect(resolver).toHaveBeenCalledWith("img.png", expect.anything()); + }); + + it("forwards the abort signal through to readMarkdownContent, which checks it before parsing", () => { + const controller = new AbortController(); + controller.abort(); + let caught: unknown; + try { + CONTENT_READERS.markdown(encodeMarkdownText("hi"), { + signal: controller.signal, + }); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(DOMException); + expect((caught as DOMException).name).toBe("AbortError"); + }); +}); + +describe("CONTENT_READERS.rtf", () => { + it("forwards the abort signal through to readRtfContent, which checks it before tokenizing", () => { + const controller = new AbortController(); + controller.abort(); + let caught: unknown; + try { + CONTENT_READERS.rtf(new TextEncoder().encode("{\\rtf1 hi}"), { + signal: controller.signal, + }); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(DOMException); + expect((caught as DOMException).name).toBe("AbortError"); + }); +}); + +describe("readDocumentLayout", () => { + it("forwards the signal option through to readPdf, which checks it before parsing", () => { + const controller = new AbortController(); + controller.abort(); + // A real "%PDF-" header but otherwise garbage bytes: readPdf checks the header first, then the abort signal, before it ever opens the document -- if the signal were not forwarded (an empty options object), this would fail trying to parse the document instead. + const bytes = new TextEncoder().encode("%PDF-1.4\n%garbage"); + let caught: unknown; + try { + readDocumentLayout(bytes, { signal: controller.signal }); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(DOMException); + expect((caught as DOMException).name).toBe("AbortError"); + }); +}); diff --git a/packages/documents.js/src/codecs/registry.test.ts b/packages/documents.js/src/codecs/registry.test.ts index 34a8d3756..d6d9c49a5 100644 --- a/packages/documents.js/src/codecs/registry.test.ts +++ b/packages/documents.js/src/codecs/registry.test.ts @@ -7,7 +7,7 @@ import type { import { PAGE_SIZE_LETTER } from "document-schema.js"; import { describe, expect, it } from "vitest"; import type { XlsContentDocument } from "xls-codec"; -import { odsToXlsx } from "../convert/convert"; +import { docxToPdf, odsToXlsx } from "../convert/convert"; import { readOdfFormulaContent } from "../odf/formula/read"; import { FRACTION_FORMULA, odfFormulaBytes } from "../test-support/odf"; import { minimalDocxBytes } from "../test-support/docx"; @@ -300,6 +300,18 @@ describe("DOCUMENT_FORMAT_CODECS: content read/write round trips", () => { expect(roundTripped).toEqual(expected); }); + it("xls: content.write refuses a non-spreadsheet ContentDocument by name", () => { + const codec = requireContentCodec("xls"); + const wordprocessing: ContentDocument = { + kind: "wordprocessing", + metadata: {}, + sections: [], + }; + expect(() => codec.write!(wordprocessing)).toThrow( + "DOCUMENT_FORMAT_CODECS.xls.content.write: expected a spreadsheet ContentDocument", + ); + }); + // Mirrors ppt-codec's own write.test.ts fixture shape. The writer's own scope is text-box slides only (see that package's README scope note); like pptx/odp above, a black-box substantive-text check is the right-scoped proof of wiring here rather than exact equality -- ppt-codec's own reader always reports PowerPoint's fixed default text insets (0.1in left/right, 0.05in top/bottom) regardless of what a shape actually carries, since it does not yet read a shape's own OfficeArtFOPT inset override (see read.ts's own DEFAULT_INSET_LEFT_RIGHT_PT/DEFAULT_INSET_TOP_BOTTOM_PT comment), a pre-existing, documented gap this registry wiring did not introduce. it("ppt: read -> write -> read carries the source slide text through", () => { const codec = requireContentCodec("ppt"); @@ -353,6 +365,16 @@ describe("DOCUMENT_FORMAT_CODECS: pdf has a layout codec, not a content codec", expect(DOCUMENT_FORMAT_CODECS.pdf.content).toBeUndefined(); expect(DOCUMENT_FORMAT_CODECS.pdf.layout).toBeDefined(); }); + + it("layout.write forwards the abort signal through to writePdf's own per-page check", () => { + const codec = DOCUMENT_FORMAT_CODECS.pdf.layout!; + const layout = codec.read(docxToPdf(minimalDocxBytes())); + const controller = new AbortController(); + controller.abort(); + expect(() => { + codec.write(layout, { signal: controller.signal }); + }).toThrow(DOMException); + }); }); describe("DOCUMENT_FORMAT_CODECS: xlsx has a content codec, no layout codec", () => { diff --git a/packages/documents.js/src/convert/bridges.test.ts b/packages/documents.js/src/convert/bridges.test.ts index 2264e83ae..e7618df72 100644 --- a/packages/documents.js/src/convert/bridges.test.ts +++ b/packages/documents.js/src/convert/bridges.test.ts @@ -616,6 +616,20 @@ describe("ods <-> xlsx: ods -> xlsx (one hop, the character-width-unit conversio const original = odsContentOf(richOdsBytes()); const originalSheet = original.sheets[0]!; + // The source ODS fixture's own header row and every cell's rendered displayText: buildRichFixturePackage (test-support/ods.ts) writes a distinct text:p run for every cell alongside its office:value, and none of it is exercised by any assertion below (those check only the CONVERTED xlsx side's `.value`) -- so a header cell silently losing its label, or a cell's displayText silently losing its rendered text, would go undetected without checking the source fixture directly. + expect(cellAt(originalSheet, 0, 0)?.displayText).toBe("Name"); + expect(cellAt(originalSheet, 0, 1)?.displayText).toBe("Amount"); + expect(cellAt(originalSheet, 0, 2)?.displayText).toBe("Active"); + expect(cellAt(originalSheet, 1, 0)?.displayText).toBe("Widget"); + expect(cellAt(originalSheet, 1, 1)?.displayText).toBe("42.5"); + expect(cellAt(originalSheet, 1, 2)?.displayText).toBe("TRUE"); + expect(cellAt(originalSheet, 2, 0)?.displayText).toBe("15%"); + expect(cellAt(originalSheet, 2, 1)?.displayText).toBe("$9.99"); + expect(cellAt(originalSheet, 2, 2)?.displayText).toBe("2026-01-15"); + expect(cellAt(originalSheet, 3, 0)?.displayText).toBe("14:30"); + expect(cellAt(originalSheet, 3, 1)?.displayText).toBe("85"); + expect(cellAt(originalSheet, 4, 0)?.displayText).toBe("Merged Cell"); + const xlsxBytes = odsToXlsx(richOdsBytes()); const xlsx = xlsxContentOf(xlsxBytes); const sheet = xlsx.sheets[0]!; @@ -624,6 +638,14 @@ describe("ods <-> xlsx: ods -> xlsx (one hop, the character-width-unit conversio kind: "string", value: "Name", }); + expect(cellAt(sheet, 0, 1)?.value).toEqual({ + kind: "string", + value: "Amount", + }); + expect(cellAt(sheet, 0, 2)?.value).toEqual({ + kind: "string", + value: "Active", + }); expect(cellAt(sheet, 1, 0)?.value).toEqual({ kind: "string", value: "Widget", diff --git a/packages/documents.js/src/convert/convert-fonts.test.ts b/packages/documents.js/src/convert/convert-fonts.test.ts index fcdc0fe1f..3025359ee 100644 --- a/packages/documents.js/src/convert/convert-fonts.test.ts +++ b/packages/documents.js/src/convert/convert-fonts.test.ts @@ -4,7 +4,10 @@ import type { FontSubstitution } from "pdf-codec"; import { createStandardFontMeasurer, loadMathFont, writePdf } from "pdf-codec"; const mathMetricsAt = (sizePt: number) => loadMathFont().metricsAt(sizePt); import { decodePackage as decodeOdfPackage } from "odf.js"; -import { encodePackage as encodeOoxmlPackage } from "ooxml.js"; +import { + decodePackage as decodeOoxmlPackage, + encodePackage as encodeOoxmlPackage, +} from "ooxml.js"; import { openDocx } from "../edit/docx/editor"; import { openPptx } from "../edit/pptx/editor"; import { buildDocumentBytes } from "./from-package"; @@ -146,6 +149,21 @@ describe("X -> PDF: caller-supplied faces", () => { }); expect(substitutions).toEqual([]); }); + + it("standardFontDocxBytes genuinely requests Arial, not merely a request no vendored substitute happens to claim", () => { + const content = readDocxContent( + decodeOoxmlPackage(standardFontDocxBytes()), + ); + if (content.kind !== "wordprocessing") { + throw new Error("expected a wordprocessing ContentDocument"); + } + const paragraph = content.sections[0]?.blocks[0]; + expect( + paragraph?.kind === "paragraph" + ? paragraph.runs[0]?.fontFamily + : undefined, + ).toBe("Arial"); + }); }); // The backward-compatibility guarantee this phase had to keep: wiring a FontRegistry into all six conversions must not change a single byte of output for a document that embeds no fonts and asks for no family a vendored substitute claims. Each reference below reproduces the exact pre-registry pipeline -- createStandardFontMeasurer() into the format's own layout engine, then writePdf with no `fonts` option at all -- so this is a genuine before/after byte comparison rather than a self-consistency check of the new code against itself. diff --git a/packages/documents.js/src/convert/document-fonts.test.ts b/packages/documents.js/src/convert/document-fonts.test.ts index 919587f6b..5357bd381 100644 --- a/packages/documents.js/src/convert/document-fonts.test.ts +++ b/packages/documents.js/src/convert/document-fonts.test.ts @@ -72,5 +72,6 @@ describe("extractSourceFontsForFormat", () => { ); } expect(caught.format).toBe("xlsx"); + expect(caught.name).toBe("UnsupportedFontSourceFormatError"); }); }); diff --git a/packages/documents.js/src/convert/from-pdf.test.ts b/packages/documents.js/src/convert/from-pdf.test.ts index eb766b4d3..18b97f38b 100644 --- a/packages/documents.js/src/convert/from-pdf.test.ts +++ b/packages/documents.js/src/convert/from-pdf.test.ts @@ -12,7 +12,12 @@ import { type Package as OoxmlPackage, } from "ooxml.js"; import { el, txt } from "ooxml.js/xml/fragment"; -import { readPdf } from "pdf-codec"; +import { + LAYOUT_FORMAT_VERSION, + readPdf, + type LayoutDocument, + writePdf, +} from "pdf-codec"; import { describe, expect, it } from "vitest"; import { docxToPdf, odsToXlsx } from "./convert"; import { readCsvContent } from "../csv/read"; @@ -142,6 +147,34 @@ describe("readDocumentMetadata", () => { expect(metadata.modifiedIso).toBe("2024-02-03T04:05:06Z"); expect(metadata.producer).toBeUndefined(); }); + + it("pdf: forwards the abort signal to readDocumentLayout, which checks it before parsing", () => { + const controller = new AbortController(); + controller.abort(); + const bytes = docxToPdf(minimalDocxBytes()); + let caught: unknown; + try { + readDocumentMetadata("pdf", bytes, { signal: controller.signal }); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(DOMException); + expect((caught as DOMException).name).toBe("AbortError"); + }); + + it("markdown: forwards the abort signal to the underlying CONTENT_READERS entry, which checks it before parsing", () => { + const controller = new AbortController(); + controller.abort(); + const bytes = encodeMarkdownText("hi"); + let caught: unknown; + try { + readDocumentMetadata("markdown", bytes, { signal: controller.signal }); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(DOMException); + expect((caught as DOMException).name).toBe("AbortError"); + }); }); // Each case proves readNativeDocumentTree(format, bytes) dispatches to exactly the same underlying reader every ergonomic conversion in this package already uses for that format, decomposed into tree form via assembleTree with no bridging, no cross-variant transform, and (for every format but pdf) no layout pass at all -- unlike ConversionResult.package/onDocument, which report whatever hop actually produced a REQUESTED conversion's output (see this file's own from-pdf.ts module comment, and ExaDev/documents.js#823, for why that can be a different, lossy shape). @@ -246,6 +279,64 @@ describe("readNativeDocumentTree", () => { expect(captured.pages).toBeDefined(); }); + // A PDF built directly through pdf-codec's own writePdf (bypassing every documents.js writer) with a bare, destination-less outline entry -- a minimalDocxBytes()-derived pdf carries no outline at all, so that fixture alone cannot distinguish readNativeDocumentTree actually calling stampPdfPackageTables from silently skipping it. This one can: stampPdfPackageTables only ever populates pkg.destinations when layout.outline (or layout.destinations) is non-empty, so its own presence here proves the call happened. + it("pdf: stamps the outline table onto the reported tree, not just the pages", () => { + const doc: LayoutDocument = { + formatVersion: LAYOUT_FORMAT_VERSION, + metadata: {}, + pages: [{ widthPt: 612, heightPt: 792, items: [] }], + images: {}, + outline: [{ title: "Chapter 1", children: [] }], + }; + const pdfBytes = writePdf(doc, { compress: false }); + const tree = readNativeDocumentTree("pdf", pdfBytes); + expect(tree.destinations?.["outline-1"]).toEqual({ + kind: "outline", + title: "Chapter 1", + }); + }); + + it("pdf: forwards the abort signal to readPdf, which checks it before parsing", () => { + const controller = new AbortController(); + controller.abort(); + const bytes = docxToPdf(minimalDocxBytes()); + let caught: unknown; + try { + readNativeDocumentTree("pdf", bytes, { signal: controller.signal }); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(DOMException); + expect((caught as DOMException).name).toBe("AbortError"); + }); + + it("markdown: forwards the abort signal to the underlying CONTENT_READERS entry, which checks it before parsing", () => { + const controller = new AbortController(); + controller.abort(); + const bytes = encodeMarkdownText("hi"); + let caught: unknown; + try { + readNativeDocumentTree("markdown", bytes, { signal: controller.signal }); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(DOMException); + expect((caught as DOMException).name).toBe("AbortError"); + }); + + it("markdown: forwards the images resolver to the underlying CONTENT_READERS entry", () => { + const resolver = (): undefined => undefined; + let called: readonly [string, unknown] | undefined; + const bytes = encodeMarkdownText("![alt](img.png)"); + readNativeDocumentTree("markdown", bytes, { + images: (src, context) => { + called = [src, context]; + resolver(); + }, + }); + expect(called?.[0]).toBe("img.png"); + }); + // The regression test for ExaDev/documents.js#823's Ask 1: a real xlsx workbook with cell values, a formula, a merged range, and a comment -- exactly the data the issue reports the OLD --dump-package path losing entirely once a cross-variant bridge (here, xlsx -> markdown, which shares no ContentDocument variant and so composes through a pdf pivot) is in the picture. buildXlsxPackageFromContent/OdsSheet have no write path for a comment (see ooxml.js's own documented cell-comment asymmetry, "read but do not write"), so the comment part is spliced onto the real xlsx package by hand, mirroring ooxml.js's own comments.test.ts synthetic-package convention -- every other fact (cells, the merge, the formula) comes from the real xlsx writer, unedited. const REL_COMMENTS = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/comments"; diff --git a/packages/documents.js/src/convert/local.test.ts b/packages/documents.js/src/convert/local.test.ts index 5715bdb29..0a2972d0e 100644 --- a/packages/documents.js/src/convert/local.test.ts +++ b/packages/documents.js/src/convert/local.test.ts @@ -420,6 +420,41 @@ describe("createLocalDocumentConverter: convert", () => { ); expect(result.document.format).toBe("pdf"); expect(pdfHeader(result.document.bytes)).toBe("%PDF-"); + // onDocument must actually reach odfToPdf -- this is the pair's own onDocument-forwarding contract (see this file's own top comment on the special-case odf -> pdf route), and the surest proof options genuinely reach the call rather than an empty object. + expect(result.package?.kind).toBe("formula"); + }); + + it("pdf: odf source forwards the abort signal to odfToPdf, which checks it before rendering", () => { + const converter = createLocalDocumentConverter(); + const controller = new AbortController(); + controller.abort(); + let caught: unknown; + try { + void converter.convert( + { + source: { format: "odf", bytes: odfFormulaBytes(FRACTION_FORMULA) }, + targetFormat: "pdf", + }, + { signal: controller.signal }, + ); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(DOMException); + expect((caught as DOMException).name).toBe("AbortError"); + }); + + // odf's own special-case route only ever applies to a pdf target -- every other odf conversion goes through the ordinary composition pathfinder, which reports odf unsupported as a source for anything but pdf (see composition.ts's own module doc). A source.format === "odf" check alone, without also requiring targetFormat === "pdf", would wrongly route this non-pdf request through odfToPdf and resolve successfully with mislabelled PDF bytes instead of rejecting. + it("rejects odf as a source for a non-pdf target, rather than silently routing it through odfToPdf", async () => { + const converter = createLocalDocumentConverter(); + const promise = converter.convert( + { + source: { format: "odf", bytes: odfFormulaBytes(FRACTION_FORMULA) }, + targetFormat: "markdown", + }, + { signal: new AbortController().signal }, + ); + await expect(promise).rejects.toBeInstanceOf(UnsupportedConversionError); }); it("converts xlsx to pdf", async () => { @@ -649,6 +684,9 @@ describe("createLocalDocumentConverter: convert", () => { ); await expect(promise).rejects.toBeInstanceOf(UnsupportedConversionError); await expect(promise).rejects.toThrow(/unsupported conversion/); + await promise.catch((error: unknown) => { + expect((error as Error).name).toBe("UnsupportedConversionError"); + }); }); it("collects a char/substituted diagnostic for a character outside WinAnsi", async () => { @@ -663,6 +701,13 @@ describe("createLocalDocumentConverter: convert", () => { expect(result.diagnostics.some((d) => d.code === "char/substituted")).toBe( true, ); + expect(result.diagnostics).toContainEqual({ + severity: "info", + code: "char/substituted", + message: + '"中" is not representable in a standard-14 font; substituted "?"', + pageIndex: 0, + }); }); it("collects PDF read diagnostics on the pdf->docx path", async () => { @@ -743,6 +788,30 @@ describe("createLocalDocumentConverter: fonts", () => { }); }); + it("names the requested weight and style in the substitution message for a bold italic run", async () => { + const editor = createDocx(); + editor.body.appendParagraph().appendRun({ + text: "Bold italic Calibri", + bold: true, + italic: true, + fontFamily: "Calibri", + }); + const converter = createLocalDocumentConverter(); + const result = await converter.convert( + { + source: { format: "docx", bytes: editor.toBytes() }, + targetFormat: "pdf", + }, + { signal: new AbortController().signal }, + ); + expect(result.diagnostics).toContainEqual({ + severity: "info", + code: "font/substituted", + message: + '"Calibri bold italic" is not available; substituted the metric-compatible "carlito"', + }); + }); + it("forwards the structured substitution to the caller own callback as well", async () => { const converter = createLocalDocumentConverter(); const substitutions: FontSubstitution[] = []; diff --git a/packages/documents.js/src/csv/read-write.test.ts b/packages/documents.js/src/csv/read-write.test.ts index 3322658d9..10aa35791 100644 --- a/packages/documents.js/src/csv/read-write.test.ts +++ b/packages/documents.js/src/csv/read-write.test.ts @@ -107,7 +107,10 @@ describe("readCsvContent", () => { it("maps an empty data field to the empty cell and pads a short record to the grid width with empty cells", () => { // Row 2 has one field where the grid is three wide: columns 1 and 2 are genuine empty cells, not holes. - const document = readCsvContent("a,b,c\n1,,3\nsolo\n"); + const events: CellTypeInference[] = []; + const document = readCsvContent("a,b,c\n1,,3\nsolo\n", { + onCellTypeInference: (event) => events.push(event), + }); if (document.kind !== "spreadsheet") { throw new Error("expected a spreadsheet ContentDocument"); } @@ -119,6 +122,10 @@ describe("readCsvContent", () => { expect(valueAt(2, 0)).toEqual({ kind: "string", value: "solo" }); expect(valueAt(2, 1)).toEqual({ kind: "empty" }); expect(valueAt(2, 2)).toEqual({ kind: "empty" }); + // The three empty fields (row 1 col 1, row 2 cols 1 and 2) never fire a type-inference event -- only the populated fields ("1" and "3", both plain numbers) do. + expect( + events.map((event) => `${String(event.row)},${String(event.column)}`), + ).toEqual(["1,0", "1,2"]); }); it("names the lone sheet Sheet1 and emits exactly one sheet, since a csv file is one table by construction", () => { @@ -315,5 +322,15 @@ describe("decodeCsvText / encodeCsvText", () => { expect(() => decodeCsvText(new Uint8Array([0xff, 0xfe, 0x00]))).toThrow( CsvInvalidUtf8Error, ); + let caught: unknown; + try { + decodeCsvText(new Uint8Array([0xff, 0xfe, 0x00])); + } catch (error) { + caught = error; + } + expect((caught as Error).name).toBe("CsvInvalidUtf8Error"); + expect((caught as Error).message).toBe( + "csv text must be well-formed UTF-8", + ); }); }); diff --git a/packages/documents.js/src/csv/read.ts b/packages/documents.js/src/csv/read.ts index 135af6585..ec6049da1 100644 --- a/packages/documents.js/src/csv/read.ts +++ b/packages/documents.js/src/csv/read.ts @@ -49,7 +49,8 @@ function dataCell( field: string, onCellTypeInference: CellTypeInferenceSink | undefined, ): ContentSheetCell { - const inference = field === "" ? undefined : inferCellValue(field); + // No separate empty-field check here: inferCellValue("") already returns undefined on its own (its own text.length === 0 guard), so a guard here would only restate that in a second place. + const inference = inferCellValue(field); if (inference !== undefined) { onCellTypeInference?.({ sheetIndex: 0, diff --git a/packages/documents.js/src/edit/docx/scaffold.test.ts b/packages/documents.js/src/edit/docx/scaffold.test.ts index 8ed0116e1..167453535 100644 --- a/packages/documents.js/src/edit/docx/scaffold.test.ts +++ b/packages/documents.js/src/edit/docx/scaffold.test.ts @@ -1,7 +1,28 @@ -import { decodePackage, encodePackage, rootElement } from "ooxml.js"; +import type { XmlElement } from "ooxml.js"; +import { attr, decodePackage, encodePackage, rootElement } from "ooxml.js"; import { describe, expect, it } from "vitest"; import { createEmptyDocxPackage } from "./scaffold"; +function elementChildren( + node: XmlElement | undefined, + tag: string, +): XmlElement[] { + if (node === undefined) { + return []; + } + return node.children.filter( + (c): c is XmlElement => c.type === "element" && c.tag === tag, + ); +} + +function elementChild(node: XmlElement | undefined, tag: string): XmlElement { + const found = elementChildren(node, tag)[0]; + if (found === undefined) { + throw new Error(`expected a <${tag}> child`); + } + return found; +} + describe("createEmptyDocxPackage", () => { it("has every part a minimal docx needs", () => { const pkg = createEmptyDocxPackage(); @@ -49,4 +70,138 @@ describe("createEmptyDocxPackage", () => { ); expect(normalStyle).toBeDefined(); }); + + it("every XML part starts with the standard version/encoding/standalone declaration", () => { + const pkg = createEmptyDocxPackage(); + for (const partName of [ + "[Content_Types].xml", + "_rels/.rels", + "word/document.xml", + "word/_rels/document.xml.rels", + "word/styles.xml", + ] as const) { + const part = pkg.parts[partName]; + if (part?.kind !== "xml") { + throw new Error(`expected ${partName} to be an xml part`); + } + expect(part.nodes[0]).toEqual({ + type: "declaration", + attributes: [ + { name: "version", value: "1.0" }, + { name: "encoding", value: "UTF-8" }, + { name: "standalone", value: "yes" }, + ], + }); + } + }); + + it("[Content_Types].xml declares the package namespace, the two Default extensions, and both part Overrides with their exact content types", () => { + const pkg = createEmptyDocxPackage(); + const root = rootElement(pkg.parts["[Content_Types].xml"]); + if (root === undefined) { + throw new Error("expected a root element"); + } + expect(root.tag).toBe("Types"); + expect(attr(root, "xmlns")).toBe( + "http://schemas.openxmlformats.org/package/2006/content-types", + ); + + const defaults = elementChildren(root, "Default"); + expect(defaults).toHaveLength(2); + expect(attr(defaults[0]!, "Extension")).toBe("rels"); + expect(attr(defaults[0]!, "ContentType")).toBe( + "application/vnd.openxmlformats-package.relationships+xml", + ); + expect(attr(defaults[1]!, "Extension")).toBe("xml"); + expect(attr(defaults[1]!, "ContentType")).toBe("application/xml"); + + const overrides = elementChildren(root, "Override"); + expect(overrides).toHaveLength(2); + expect(attr(overrides[0]!, "PartName")).toBe("/word/document.xml"); + expect(attr(overrides[0]!, "ContentType")).toBe( + "application/vnd.openxmlformats-officedocument.wordprocessingml.document.main+xml", + ); + expect(attr(overrides[1]!, "PartName")).toBe("/word/styles.xml"); + expect(attr(overrides[1]!, "ContentType")).toBe( + "application/vnd.openxmlformats-officedocument.wordprocessingml.styles+xml", + ); + }); + + it("_rels/.rels points rId1 at word/document.xml via the officeDocument relationship type", () => { + const pkg = createEmptyDocxPackage(); + const root = rootElement(pkg.parts["_rels/.rels"]); + if (root === undefined) { + throw new Error("expected a root element"); + } + expect(root.tag).toBe("Relationships"); + expect(attr(root, "xmlns")).toBe( + "http://schemas.openxmlformats.org/package/2006/relationships", + ); + const relationship = elementChild(root, "Relationship"); + expect(attr(relationship, "Id")).toBe("rId1"); + expect(attr(relationship, "Type")).toBe( + "http://schemas.openxmlformats.org/officeDocument/2006/relationships/officeDocument", + ); + expect(attr(relationship, "Target")).toBe("word/document.xml"); + }); + + it("word/_rels/document.xml.rels points rId1 at styles.xml via the styles relationship type", () => { + const pkg = createEmptyDocxPackage(); + const root = rootElement(pkg.parts["word/_rels/document.xml.rels"]); + if (root === undefined) { + throw new Error("expected a root element"); + } + expect(root.tag).toBe("Relationships"); + expect(attr(root, "xmlns")).toBe( + "http://schemas.openxmlformats.org/package/2006/relationships", + ); + const relationship = elementChild(root, "Relationship"); + expect(attr(relationship, "Id")).toBe("rId1"); + expect(attr(relationship, "Type")).toBe( + "http://schemas.openxmlformats.org/officeDocument/2006/relationships/styles", + ); + expect(attr(relationship, "Target")).toBe("styles.xml"); + }); + + it("word/document.xml declares the wordprocessingml namespace and a US-Letter w:sectPr with 1in margins and 0.5in header/footer", () => { + const pkg = createEmptyDocxPackage(); + const root = rootElement(pkg.parts["word/document.xml"]); + if (root === undefined) { + throw new Error("expected a root element"); + } + expect(attr(root, "xmlns:w")).toBe( + "http://schemas.openxmlformats.org/wordprocessingml/2006/main", + ); + const body = elementChild(root, "w:body"); + const sectPr = elementChild(body, "w:sectPr"); + const pgSz = elementChild(sectPr, "w:pgSz"); + expect(attr(pgSz, "w:w")).toBe("12240"); + expect(attr(pgSz, "w:h")).toBe("15840"); + const pgMar = elementChild(sectPr, "w:pgMar"); + expect(attr(pgMar, "w:top")).toBe("1440"); + expect(attr(pgMar, "w:right")).toBe("1440"); + expect(attr(pgMar, "w:bottom")).toBe("1440"); + expect(attr(pgMar, "w:left")).toBe("1440"); + expect(attr(pgMar, "w:header")).toBe("720"); + expect(attr(pgMar, "w:footer")).toBe("720"); + expect(attr(pgMar, "w:gutter")).toBe("0"); + }); + + it("word/styles.xml declares the wordprocessingml namespace and the Normal style's exact type/id/name", () => { + const pkg = createEmptyDocxPackage(); + const root = rootElement(pkg.parts["word/styles.xml"]); + if (root === undefined) { + throw new Error("expected a root element"); + } + expect(root.tag).toBe("w:styles"); + expect(attr(root, "xmlns:w")).toBe( + "http://schemas.openxmlformats.org/wordprocessingml/2006/main", + ); + const style = elementChild(root, "w:style"); + expect(attr(style, "w:type")).toBe("paragraph"); + expect(attr(style, "w:default")).toBe("1"); + expect(attr(style, "w:styleId")).toBe("Normal"); + const name = elementChild(style, "w:name"); + expect(attr(name, "w:val")).toBe("Normal"); + }); }); diff --git a/packages/documents.js/src/edit/docx/table.test.ts b/packages/documents.js/src/edit/docx/table.test.ts index 59e1f4092..a6c2bf2f5 100644 --- a/packages/documents.js/src/edit/docx/table.test.ts +++ b/packages/documents.js/src/edit/docx/table.test.ts @@ -70,6 +70,15 @@ describe("DocxTable cell access and mutation", () => { expect(cell.colSpan).toBeUndefined(); }); + it("setting colSpan again while one already exists replaces it rather than leaving a stale gridSpan behind", () => { + const tableElement = buildTable({ rows: 1, columns: 3 }); + const table = new DocxTable([tableElement], tableElement); + const cell = table.cell(0, 0); + cell.colSpan = 2; + cell.colSpan = 3; + expect(cell.colSpan).toBe(3); + }); + it("verticalMerge writes and reads w:tcPr/w:vMerge, distinguishing restart from continue", () => { const tableElement = buildTable({ rows: 1, columns: 1 }); const table = new DocxTable([tableElement], tableElement); @@ -161,6 +170,18 @@ describe("DocxTable cell access and mutation", () => { } expect(roundTrippedTable.rows[0]?.heightPt).toBeCloseTo(34, 5); }); + + it("heightPt can be updated to a new value and cleared back to undefined on a row that already has one", () => { + const tableElement = buildTable({ rows: 1, columns: 1 }); + const table = new DocxTable([tableElement], tableElement); + const row = table.rows()[0]!; + row.heightPt = 20; + expect(row.heightPt).toBeCloseTo(20, 5); + row.heightPt = 40; + expect(row.heightPt).toBeCloseTo(40, 5); + row.heightPt = undefined; + expect(row.heightPt).toBeUndefined(); + }); }); describe("DocxTableCell background", () => { @@ -196,6 +217,15 @@ describe("DocxTableCell background", () => { expect(cell.background).toBeUndefined(); }); + it("setting background again while one already exists replaces it rather than leaving a stale w:shd behind", () => { + const tableElement = buildTable({ rows: 1, columns: 1 }); + const table = new DocxTable([tableElement], tableElement); + const cell = table.cell(0, 0); + cell.background = { r: 1, g: 0, b: 0 }; + cell.background = { r: 0, g: 0, b: 1 }; + expect(cell.background).toEqual({ r: 0, g: 0, b: 1 }); + }); + it('resolves a w:val="solid" shading from w:color, not w:fill -- the real bug this getter once had, since it read w:fill unconditionally regardless of w:val', () => { const tableElement = buildTable({ rows: 1, columns: 1 }); const table = new DocxTable([tableElement], tableElement); @@ -219,6 +249,157 @@ describe("DocxTableCell background", () => { }); }); +describe("DocxTableCell.borders", () => { + // Same walk-to-w:tcPr helper as the background describe block above, duplicated locally since that one is scoped to its own describe callback. + function tcPrOf(tableElement: XmlNode, cell: DocxTableCell): XmlElement { + cell.colSpan = 1; + const tr = tableElement.type === "element" ? tableElement : undefined; + const row = + tr?.children.find((c) => c.type === "element" && c.tag === "w:tr") ?? + undefined; + const tc = + row?.type === "element" + ? row.children.find((c) => c.type === "element" && c.tag === "w:tc") + : undefined; + const tcPr = + tc?.type === "element" + ? tc.children.find((c) => c.type === "element" && c.tag === "w:tcPr") + : undefined; + if (tcPr?.type !== "element") { + throw new Error("expected w:tcPr"); + } + return tcPr; + } + + it("borders is undefined for a cell with no w:tcBorders, and round-trips all four edges through the setter", () => { + const tableElement = buildTable({ rows: 1, columns: 1 }); + const table = new DocxTable([tableElement], tableElement); + const cell = table.cell(0, 0); + expect(cell.borders).toBeUndefined(); + + cell.borders = { + top: { color: { r: 1, g: 0, b: 0 }, widthPt: 2, style: "dashed" }, + left: { color: { r: 0, g: 1, b: 0 }, widthPt: 1.5, style: "dotted" }, + bottom: { color: { r: 0, g: 0, b: 1 }, widthPt: 0.5, style: "double" }, + right: { color: { r: 0, g: 0, b: 0 }, widthPt: 1, style: "solid" }, + }; + + expect(cell.borders).toEqual({ + top: { color: { r: 1, g: 0, b: 0 }, widthPt: 2, style: "dashed" }, + left: { color: { r: 0, g: 1, b: 0 }, widthPt: 1.5, style: "dotted" }, + bottom: { color: { r: 0, g: 0, b: 1 }, widthPt: 0.5, style: "double" }, + right: { color: { r: 0, g: 0, b: 0 }, widthPt: 1, style: "solid" }, + }); + }); + + it("setting borders again while one already exists replaces it rather than leaving a stale w:tcBorders behind", () => { + const tableElement = buildTable({ rows: 1, columns: 1 }); + const table = new DocxTable([tableElement], tableElement); + const cell = table.cell(0, 0); + cell.borders = { + top: { color: { r: 1, g: 0, b: 0 }, widthPt: 2, style: "dashed" }, + }; + cell.borders = { + left: { color: { r: 0, g: 1, b: 0 }, widthPt: 1, style: "solid" }, + }; + expect(cell.borders).toEqual({ + left: { color: { r: 0, g: 1, b: 0 }, widthPt: 1, style: "solid" }, + }); + }); + + it("clearing borders (undefined) removes w:tcBorders entirely", () => { + const tableElement = buildTable({ rows: 1, columns: 1 }); + const table = new DocxTable([tableElement], tableElement); + const cell = table.cell(0, 0); + cell.borders = { + top: { color: { r: 0, g: 0, b: 0 }, widthPt: 1, style: "solid" }, + }; + expect(cell.borders).not.toBeUndefined(); + cell.borders = undefined; + expect(cell.borders).toBeUndefined(); + }); + + it("clearing borders on a cell with no w:tcPr at all is a no-op, not an error", () => { + const tableElement = buildTable({ rows: 1, columns: 1 }); + const table = new DocxTable([tableElement], tableElement); + const cell = table.cell(0, 0); + expect(() => { + cell.borders = undefined; + }).not.toThrow(); + expect(cell.borders).toBeUndefined(); + }); + + it('an edge whose w:val is "nil" or "none" is excluded from the read-back borders, and an edge with neither w:sz nor w:color falls back to a 1pt black solid border', () => { + const tableElement = buildTable({ rows: 1, columns: 1 }); + const table = new DocxTable([tableElement], tableElement); + const cell = table.cell(0, 0); + const tcPr = tcPrOf(tableElement, cell); + tcPr.children.push( + el("w:tcBorders", {}, [ + el("w:top", { "w:val": "nil" }), + el("w:left", { "w:val": "none" }), + el("w:bottom", { "w:val": "single" }), + ]), + ); + + const borders = cell.borders; + expect(borders?.top).toBeUndefined(); + expect(borders?.left).toBeUndefined(); + expect(borders?.bottom).toEqual({ + color: { r: 0, g: 0, b: 0 }, + widthPt: 1, + style: "solid", + }); + }); + + it('an edge whose w:color is "auto" (rather than absent) also falls back to black', () => { + const tableElement = buildTable({ rows: 1, columns: 1 }); + const table = new DocxTable([tableElement], tableElement); + const cell = table.cell(0, 0); + const tcPr = tcPrOf(tableElement, cell); + tcPr.children.push( + el("w:tcBorders", {}, [ + el("w:top", { "w:val": "single", "w:sz": "16", "w:color": "auto" }), + ]), + ); + + expect(cell.borders?.top).toEqual({ + color: { r: 0, g: 0, b: 0 }, + widthPt: 2, + style: "solid", + }); + }); + + it("borders is undefined when w:tcBorders is present but every edge is nil/none, since an empty resolved map is treated the same as no borders at all", () => { + const tableElement = buildTable({ rows: 1, columns: 1 }); + const table = new DocxTable([tableElement], tableElement); + const cell = table.cell(0, 0); + const tcPr = tcPrOf(tableElement, cell); + tcPr.children.push( + el("w:tcBorders", {}, [ + el("w:top", { "w:val": "nil" }), + el("w:left", { "w:val": "none" }), + ]), + ); + + expect(cell.borders).toBeUndefined(); + }); + + it("an unrecognised w:val reads back as the 'solid' default, mirroring ooxml.js read.js's own fallback for unrecognised vals", () => { + const tableElement = buildTable({ rows: 1, columns: 1 }); + const table = new DocxTable([tableElement], tableElement); + const cell = table.cell(0, 0); + const tcPr = tcPrOf(tableElement, cell); + tcPr.children.push( + el("w:tcBorders", {}, [ + el("w:top", { "w:val": "wave", "w:sz": "8", "w:color": "123456" }), + ]), + ); + + expect(cell.borders?.top?.style).toBe("solid"); + }); +}); + describe("DocxTableRow.mergeCellsHorizontally", () => { it("merges colSpan columns into one cell, removing the consumed w:tc elements and leaving w:tblGrid untouched", () => { const tableElement = buildTable({ rows: 1, columns: 4 }); diff --git a/packages/documents.js/src/edit/geometry.test.ts b/packages/documents.js/src/edit/geometry.test.ts new file mode 100644 index 000000000..9e914e7cf --- /dev/null +++ b/packages/documents.js/src/edit/geometry.test.ts @@ -0,0 +1,56 @@ +import type { Box } from "document-schema.js"; +import { describe, expect, it } from "vitest"; +import { el } from "../xml/fragment"; +import { applyOdfGeometry, buildTransformAttr } from "./geometry"; + +function attr(node: ReturnType, name: string): string | undefined { + return node.attributes.find((a) => a.name === name)?.value; +} + +const frame: Box = { xPt: 10, yPt: 20, widthPt: 100, heightPt: 50 }; + +describe("applyOdfGeometry", () => { + it("writes plain svg:x/svg:y and removes draw:transform when rotationDeg is undefined", () => { + const node = el("draw:frame", { + "draw:transform": "rotate(1) translate(2 3)", + }); + applyOdfGeometry(node, frame, undefined); + expect(attr(node, "svg:width")).toBe("100pt"); + expect(attr(node, "svg:height")).toBe("50pt"); + expect(attr(node, "svg:x")).toBe("10pt"); + expect(attr(node, "svg:y")).toBe("20pt"); + expect(attr(node, "draw:transform")).toBeUndefined(); + }); + + it("writes plain svg:x/svg:y and removes draw:transform when rotationDeg is exactly 0", () => { + const node = el("draw:frame", { + "draw:transform": "rotate(1) translate(2 3)", + }); + applyOdfGeometry(node, frame, 0); + expect(attr(node, "svg:x")).toBe("10pt"); + expect(attr(node, "svg:y")).toBe("20pt"); + expect(attr(node, "draw:transform")).toBeUndefined(); + }); + + it("writes draw:transform and removes svg:x/svg:y for a non-zero rotation", () => { + const node = el("draw:frame", { "svg:x": "10pt", "svg:y": "20pt" }); + applyOdfGeometry(node, frame, 90); + expect(attr(node, "svg:x")).toBeUndefined(); + expect(attr(node, "svg:y")).toBeUndefined(); + expect(attr(node, "draw:transform")).toBe(buildTransformAttr(frame, 90)); + }); +}); + +describe("buildTransformAttr", () => { + it("round-trips the frame's own centre through the rotate+translate composition", () => { + const transform = buildTransformAttr(frame, 90); + expect(transform).toMatch( + /^rotate\(-1\.5707963267948966\) translate\(-?\d+(\.\d+)?pt -?\d+(\.\d+)?pt\)$/, + ); + }); + + it("produces a zero translate when rotating an already-centred (origin) frame with zero angle", () => { + const centred: Box = { xPt: 0, yPt: 0, widthPt: 10, heightPt: 10 }; + expect(buildTransformAttr(centred, 0)).toBe("rotate(0) translate(0pt 0pt)"); + }); +}); diff --git a/packages/documents.js/src/edit/markdown/editor.test.ts b/packages/documents.js/src/edit/markdown/editor.test.ts index fdf871201..2d89a255b 100644 --- a/packages/documents.js/src/edit/markdown/editor.test.ts +++ b/packages/documents.js/src/edit/markdown/editor.test.ts @@ -1,8 +1,14 @@ +import type { ContentDocument } from "document-schema.js"; import { MarkdownDiagnosticCodes } from "markdown-codec"; import { describe, expect, it } from "vitest"; import { readMarkdownContent } from "../../markdown/read"; import { buildMarkdownText } from "../../markdown/write"; -import { createMarkdownEditor, openMarkdown } from "./editor"; +import { createMarkdownEditor, MarkdownEditor, openMarkdown } from "./editor"; + +// MarkdownEditor deliberately exposes no pageSize/margins getter of its own (mirroring every other live editor's constructor-only intake of these fields) -- reaching the private `document` field this way is the only way to prove createMarkdownEditor's own pageSize/margins options genuinely reach readMarkdownContent, since neither field is observable through toMarkdownText() (plain CommonMark/GFM text carries no page-geometry construct at all). +function underlyingDocument(editor: MarkdownEditor): ContentDocument { + return (editor as unknown as { document: ContentDocument }).document; +} describe("createMarkdownEditor", () => { it("produces a document whose toMarkdownText() matches what an empty readMarkdownContent round trip produces", () => { @@ -10,6 +16,54 @@ describe("createMarkdownEditor", () => { const expected = buildMarkdownText(readMarkdownContent("")); expect(editor.toMarkdownText()).toBe(expected); }); + + it("passes pageSize and margins through to the underlying document rather than the default geometry", () => { + const pageSize = { widthPt: 300, heightPt: 400 }; + const margins = { topPt: 10, rightPt: 20, bottomPt: 30, leftPt: 40 }; + const editor = createMarkdownEditor({ pageSize, margins }); + const document = underlyingDocument(editor); + if (document.kind !== "wordprocessing") { + throw new Error("expected a wordprocessing ContentDocument"); + } + expect(document.sections[0]?.pageSize).toEqual(pageSize); + expect(document.sections[0]?.margins).toEqual(margins); + }); + + it("defaults created/modified timestamps from systemClock when no clock is given", () => { + const before = Date.now(); + const editor = createMarkdownEditor(); + const after = Date.now(); + const document = underlyingDocument(editor); + const createdIso = document.metadata.createdIso; + expect(createdIso).toBeDefined(); + const createdMs = new Date(createdIso ?? "").getTime(); + expect(createdMs).toBeGreaterThanOrEqual(before); + expect(createdMs).toBeLessThanOrEqual(after); + }); +}); + +describe("MarkdownEditor constructor", () => { + it("throws for a non-wordprocessing ContentDocument", () => { + const presentation: ContentDocument = { + kind: "presentation", + metadata: {}, + slides: [], + }; + expect(() => new MarkdownEditor(presentation)).toThrow( + 'MarkdownEditor requires a wordprocessing ContentDocument, got "presentation"', + ); + }); + + it("throws for a wordprocessing document with no sections", () => { + const empty: ContentDocument = { + kind: "wordprocessing", + metadata: {}, + sections: [], + }; + expect(() => new MarkdownEditor(empty)).toThrow( + "markdown ContentDocument carries no sections", + ); + }); }); describe("openMarkdown / toMarkdownText round trip", () => { @@ -29,6 +83,9 @@ describe("openMarkdown / toMarkdownText round trip", () => { it("round-trips headings, bold/italic/strike, a hyperlink, a bullet list, and a table", () => { const editor = openMarkdown(fixture); + // Exactly four paragraph-kind blocks (heading, prose, two list items), NOT five: the fixture's own table must not be surfaced through paragraphs() alongside them. + expect(editor.paragraphs()).toHaveLength(4); + const [heading, prose] = editor.paragraphs(); expect(heading?.headingLevel).toBe(1); expect(heading?.text).toBe("Title"); diff --git a/packages/documents.js/src/edit/markdown/list.test.ts b/packages/documents.js/src/edit/markdown/list.test.ts index 7289bef1f..1a12877ff 100644 --- a/packages/documents.js/src/edit/markdown/list.test.ts +++ b/packages/documents.js/src/edit/markdown/list.test.ts @@ -36,6 +36,8 @@ describe("MarkdownList.appendItem", () => { const first = editor.body.startList({ type: "bullet" }); const second = editor.body.startList({ type: "bullet" }); expect(first.numId).not.toBe(second.numId); + // No `task` field supplied at all: the minted numId's own +task suffix (markdown-codec's own grammar, see list-id.ts) must be absent, not defaulted on. + expect(first.numId).not.toContain("+task"); const itemA = first.appendItem(0, { text: "A" }); const itemB = first.appendItem(0, { text: "B" }); expect(itemA.list?.numId).toBe(first.numId); diff --git a/packages/documents.js/src/edit/markdown/table.test.ts b/packages/documents.js/src/edit/markdown/table.test.ts index 91ae40952..7b85c766e 100644 --- a/packages/documents.js/src/edit/markdown/table.test.ts +++ b/packages/documents.js/src/edit/markdown/table.test.ts @@ -1,5 +1,7 @@ +import type { ContentBlock, ContentTableCell } from "document-schema.js"; import { describe, expect, it } from "vitest"; import { openMarkdown } from "./editor"; +import { buildTable, MarkdownTable, MarkdownTableCell } from "./table"; describe("MarkdownTable appendTable / appendRow / cell.text", () => { it("produces a real GFM table, re-parseable back into the same cell texts", () => { @@ -39,6 +41,29 @@ describe("MarkdownTable appendTable / appendRow / cell.text", () => { const paragraph = cell.appendParagraph({ text: "Second" }); expect(cell.paragraphs()).toHaveLength(2); expect(paragraph.text).toBe("Second"); + // Two paragraphs joined with a real newline, not concatenated bare -- the first is the cell's own untouched default (empty text), the second is "Second". + expect(cell.text).toBe("\nSecond"); + }); + + it("paragraphs()/text ignore a non-paragraph block sharing the cell, filtering strictly by kind", () => { + const node: ContentTableCell = { + blocks: [ + { kind: "paragraph", runs: [{ text: "First" }] }, + { kind: "pageBreak" }, + { kind: "paragraph", runs: [{ text: "Third" }] }, + ], + }; + const cell = new MarkdownTableCell(node); + expect(cell.paragraphs()).toHaveLength(2); + expect(cell.text).toBe("First\nThird"); + }); +}); + +describe("buildTable", () => { + it("divides the default table width evenly across the requested column count", () => { + const table = buildTable({ rows: 1, columns: 4 }); + expect(table.columnWidthsPt).toEqual([117, 117, 117, 117]); + expect(table.columnWidthsPt.reduce((sum, w) => sum + w, 0)).toBe(468); }); }); @@ -51,4 +76,17 @@ describe("MarkdownTable.remove", () => { expect(editor.tables()).toHaveLength(0); expect(() => table.rows()).toThrow(/removed/); }); + + it("does nothing to the container when its own node is no longer in it, rather than splicing the wrong element", () => { + // If the not-found guard were skipped, Array.prototype.splice(-1, 1) would silently remove the container's own LAST element instead of doing nothing. + const tableNode = buildTable({ rows: 1, columns: 1 }); + const other: ContentBlock = { kind: "paragraph", runs: [] }; + const container: ContentBlock[] = [other, tableNode]; + const table = new MarkdownTable(container, tableNode); + // Remove the table's own node from the container by some other means first, so remove()'s own indexOf lookup genuinely fails to find it. + container.splice(container.indexOf(tableNode), 1); + expect(container).toEqual([other]); + table.remove(); + expect(container).toEqual([other]); + }); }); diff --git a/packages/documents.js/src/edit/odg/page.test.ts b/packages/documents.js/src/edit/odg/page.test.ts new file mode 100644 index 000000000..ef595271f --- /dev/null +++ b/packages/documents.js/src/edit/odg/page.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { createOdg } from "./editor"; + +describe("OdgPage.remove", () => { + it("splices the page out of the drawing's own document so it no longer appears in pages()", () => { + const editor = createOdg(); + editor.addPage(); + editor.addPage(); + expect(editor.pages()).toHaveLength(2); + + const [first] = editor.pages(); + first?.remove(); + + expect(editor.pages()).toHaveLength(1); + }); + + it("marks the handle removed, so any further use throws rather than silently operating on a detached element", () => { + const editor = createOdg(); + const page = editor.addPage(); + page.remove(); + + expect(() => page.shapes()).toThrow( + "this OdgPage has been removed from the drawing and can no longer be used", + ); + expect(() => + page.addRect({ frame: { xPt: 0, yPt: 0, widthPt: 10, heightPt: 10 } }), + ).toThrow(/removed/); + }); +}); diff --git a/packages/documents.js/src/edit/odg/svg-path.ts b/packages/documents.js/src/edit/odg/svg-path.ts index 630911bb6..b3458d99a 100644 --- a/packages/documents.js/src/edit/odg/svg-path.ts +++ b/packages/documents.js/src/edit/odg/svg-path.ts @@ -2,7 +2,7 @@ import type { ContentPathPoint, ContentSubpath } from "document-schema.js"; // The write-side inverse of odf.js's own typed/shared/path.ts (parseOdfPathData/parseOdfViewBox): turns a ContentVector 'path' variant's own subpaths (already in the path's local coordinate space, sized to frame.widthPt x frame.heightPt -- see document-schema.js's content.ts, the exact same convention scaleOdfRawPoint/buildOdfSubpaths read INTO on the parse side) into a real svg:d + svg:viewBox attribute pair. Anchoring the viewBox at "0 0 {widthPt} {heightPt}" -- exactly the frame's own current size -- gives a 1:1 scale (buildOdfSubpaths' own scale factor is frame.widthPt/viewBox.width), so the numbers written into svg:d are the SAME numbers as the source ContentPathPoint values, with no rescaling arithmetic needed on write and none needed to recover them on a later reparse. -// A single numeric coordinate, formatted to satisfy BOTH grammars odf.js's own path.ts parses: svg:d's PATH_TOKEN_PATTERN (`-?(\d+\.\d+|\.\d+|\d+)([eE][-+]?\d+)?`) and svg:viewBox's stricter VIEW_BOX_PATTERN (`-?\d+(?:\.\d+)?`, no bare ".5" leading-dot form, no exponent). Always emitting at least one leading digit before any decimal point and never using exponential notation satisfies both at once, so one formatter serves both callers below. Rounds to a fixed sub-point precision first to strip IEEE-754 noise (e.g. 0.1 + 0.2) from leaking into the written string, and normalizes -0 to a plain "0" rather than "-0" (cosmetic, but "-0" reads as a stray negative sign to a human inspecting the XML). +// A single numeric coordinate, formatted to satisfy BOTH grammars odf.js's own path.ts parses: svg:d's PATH_TOKEN_PATTERN (`-?(\d+\.\d+|\.\d+|\d+)([eE][-+]?\d+)?`) and svg:viewBox's stricter VIEW_BOX_PATTERN (`-?\d+(?:\.\d+)?`, no bare ".5" leading-dot form, no exponent). Always emitting at least one leading digit before any decimal point and never using exponential notation satisfies both at once, so one formatter serves both callers below. Rounds to a fixed sub-point precision first to strip IEEE-754 noise (e.g. 0.1 + 0.2) from leaking into the written string. No separate zero/-0 special case is needed to get a plain "0" (never "-0") for a zero-valued coordinate: Number.prototype.toFixed already normalizes -0 to "0.000000" on its own, which the trailing-zero trim below then collapses to a bare "0" through the exact same path every other value takes. const PATH_NUMBER_DECIMALS = 6; const PATH_NUMBER_SCALE = 10 ** PATH_NUMBER_DECIMALS; @@ -11,9 +11,6 @@ export function formatPathNumber(value: number): string { throw new Error(`cannot format a non-finite path coordinate: ${value}`); } const rounded = Math.round(value * PATH_NUMBER_SCALE) / PATH_NUMBER_SCALE; - if (rounded === 0) { - return "0"; - } const fixed = rounded.toFixed(PATH_NUMBER_DECIMALS); return fixed.replace(/0+$/, "").replace(/\.$/, ""); } diff --git a/packages/documents.js/src/edit/odp/content.test.ts b/packages/documents.js/src/edit/odp/content.test.ts index c521ffade..283ab97d3 100644 --- a/packages/documents.js/src/edit/odp/content.test.ts +++ b/packages/documents.js/src/edit/odp/content.test.ts @@ -325,6 +325,7 @@ describe("buildOdpPackage", () => { ) { throw new Error("expected a drawing-kind embeddedObject block"); } + expect(drawingBlock.sourcePath).toBe("slides[0].shapes[1]"); expect( withoutRotation(drawingBlock.document.pages[0]?.vectors ?? []), ).toEqual(withoutRotation(VECTOR_FIXTURE)); diff --git a/packages/documents.js/src/edit/odp/formula.test.ts b/packages/documents.js/src/edit/odp/formula.test.ts index 9d3f89f4d..c4f7ec464 100644 --- a/packages/documents.js/src/edit/odp/formula.test.ts +++ b/packages/documents.js/src/edit/odp/formula.test.ts @@ -199,6 +199,7 @@ describe("buildOdpPackage: an embedded formula block", () => { throw new Error("expected a formula-kind embedded document"); } expect(signature(block.document.formula.mathml)).toBe("mfrac(mi(a),mi(b))"); + expect(block.sourcePath).toBe("slides[0].shapes[0]"); }); it("still writes the plain-text stand-in for a formula carrying no MathML at all", () => { diff --git a/packages/documents.js/src/edit/ods/print-settings.test.ts b/packages/documents.js/src/edit/ods/print-settings.test.ts new file mode 100644 index 000000000..b6da7cbe3 --- /dev/null +++ b/packages/documents.js/src/edit/ods/print-settings.test.ts @@ -0,0 +1,469 @@ +import type { ContentSheetPrintSettings } from "document-schema.js"; +import type { XmlElement } from "odf.js"; +import { findStyleElement } from "odf.js"; +import { attr } from "ooxml.js"; +import { describe, expect, it } from "vitest"; +import { readOdsContent } from "../../odf/ods/read"; +import { setAttr } from "../../xml/edit"; +import { createOds, type OdsEditor } from "./editor"; +import { + readSheetPrintSettings, + writeSheetPrintSettings, +} from "./print-settings"; + +function directChild(parent: XmlElement, tag: string): XmlElement | undefined { + return parent.children.find( + (c): c is XmlElement => c.type === "element" && c.tag === tag, + ); +} + +function findTableElement(editor: OdsEditor): XmlElement { + const contentPart = editor.toPackage().parts["content.xml"]; + const root = + contentPart?.kind === "xml" + ? contentPart.nodes.find((n): n is XmlElement => n.type === "element") + : undefined; + const body = + root === undefined ? undefined : directChild(root, "office:body"); + const spreadsheet = + body === undefined ? undefined : directChild(body, "office:spreadsheet"); + const table = + spreadsheet === undefined + ? undefined + : directChild(spreadsheet, "table:table"); + if (table === undefined) { + throw new Error("expected a table:table element"); + } + return table; +} + +// The style:page-layout-properties element the most recently written printSettings minted -- style:page-layout is always appended (never reused, see print-settings.ts's own top-of-file note), so the LAST one in styles.xml/office:automatic-styles is always the current sheet's. +function currentPageLayoutProperties(editor: OdsEditor): XmlElement { + const stylesPart = editor.toPackage().parts["styles.xml"]; + const root = + stylesPart?.kind === "xml" + ? stylesPart.nodes.find((n): n is XmlElement => n.type === "element") + : undefined; + const automaticStyles = + root === undefined + ? undefined + : directChild(root, "office:automatic-styles"); + const pageLayouts = + automaticStyles === undefined + ? [] + : automaticStyles.children.filter( + (c): c is XmlElement => + c.type === "element" && c.tag === "style:page-layout", + ); + const last = pageLayouts.at(-1); + const properties = + last === undefined + ? undefined + : directChild(last, "style:page-layout-properties"); + if (properties === undefined) { + throw new Error("expected a style:page-layout-properties element"); + } + return properties; +} + +const BASE: ContentSheetPrintSettings = { + pageSize: { widthPt: 612, heightPt: 792 }, + margins: { topPt: 72, rightPt: 90, bottomPt: 72, leftPt: 54 }, + gridlines: false, + headers: false, + pageOrder: "downThenOver", +}; + +describe("OdsSheet.printSettings: pageSize/margins/gridlines/headers/pageOrder", () => { + it("round-trips pageSize, margins, and pageOrder=downThenOver with neither gridlines nor headers, writing no style:print attribute at all", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = BASE; + + expect(sheet.printSettings).toEqual(BASE); + const properties = currentPageLayoutProperties(editor); + expect(attr(properties, "fo:page-width")).toBe("612pt"); + expect(attr(properties, "fo:page-height")).toBe("792pt"); + expect(attr(properties, "fo:margin-top")).toBe("72pt"); + expect(attr(properties, "fo:margin-right")).toBe("90pt"); + expect(attr(properties, "fo:margin-bottom")).toBe("72pt"); + expect(attr(properties, "fo:margin-left")).toBe("54pt"); + expect(attr(properties, "style:print")).toBeUndefined(); + expect(attr(properties, "style:print-page-order")).toBe("ttb"); + }); + + it("round-trips pageOrder=overThenDown as style:print-page-order=ltr", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = { ...BASE, pageOrder: "overThenDown" }; + + expect(sheet.printSettings.pageOrder).toBe("overThenDown"); + expect( + attr(currentPageLayoutProperties(editor), "style:print-page-order"), + ).toBe("ltr"); + }); + + it('gridlines alone writes style:print="grid" and reads back gridlines=true, headers=false', () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = { ...BASE, gridlines: true }; + + expect(sheet.printSettings.gridlines).toBe(true); + expect(sheet.printSettings.headers).toBe(false); + expect(attr(currentPageLayoutProperties(editor), "style:print")).toBe( + "grid", + ); + }); + + it('headers alone writes style:print="headers" and reads back gridlines=false, headers=true', () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = { ...BASE, headers: true }; + + expect(sheet.printSettings.gridlines).toBe(false); + expect(sheet.printSettings.headers).toBe(true); + expect(attr(currentPageLayoutProperties(editor), "style:print")).toBe( + "headers", + ); + }); + + it('both gridlines and headers write style:print="grid headers" and both read back true', () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = { ...BASE, gridlines: true, headers: true }; + + expect(sheet.printSettings.gridlines).toBe(true); + expect(sheet.printSettings.headers).toBe(true); + expect(attr(currentPageLayoutProperties(editor), "style:print")).toBe( + "grid headers", + ); + }); + + it("falls back to PAGE_SIZE_A4/DEFAULT_MARGINS/downThenOver when the sheet's own style chain never resolves a page layout at all", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + // A freshly-created sheet has no table:style-name at all yet -- readSheetPrintSettings must fall back rather than throw. + const settings = sheet.printSettings; + expect(settings.pageSize).toEqual({ widthPt: 595.28, heightPt: 841.89 }); + expect(settings.margins).toEqual({ + topPt: 56.69291338582677, + rightPt: 56.69291338582677, + bottomPt: 56.69291338582677, + leftPt: 56.69291338582677, + }); + expect(settings.pageOrder).toBe("downThenOver"); + expect(settings.gridlines).toBe(false); + expect(settings.headers).toBe(false); + }); +}); + +describe("OdsSheet.printSettings: printRange", () => { + it("round-trips a printRange as SheetName-prefixed table:print-ranges", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = { + ...BASE, + printRange: { startRow: 1, startColumn: 2, endRow: 9, endColumn: 4 }, + }; + + expect(sheet.printSettings.printRange).toEqual({ + startRow: 1, + startColumn: 2, + endRow: 9, + endColumn: 4, + }); + const table = findTableElement(editor); + expect(attr(table, "table:print-ranges")).toBe("Sheet1.C2:Sheet1.E10"); + }); + + it("has no printRange when the field is omitted", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = BASE; + expect(sheet.printSettings.printRange).toBeUndefined(); + }); + + it("parses a bare (no SheetName prefix) reference in table:print-ranges the same as a prefixed one", () => { + const editor = createOds(); + const table = findTableElement(editor); + setAttr(table, "table:print-ranges", "A1:C3"); + const settings = readSheetPrintSettings(editor.toPackage(), table); + expect(settings.printRange).toEqual({ + startRow: 0, + startColumn: 0, + endRow: 2, + endColumn: 2, + }); + }); + + it("table:print-ranges with no colon separator, or an unparseable cell reference, yields no printRange", () => { + const editor = createOds(); + const table = findTableElement(editor); + + setAttr(table, "table:print-ranges", "Sheet1.A1"); + expect( + readSheetPrintSettings(editor.toPackage(), table).printRange, + ).toBeUndefined(); + + setAttr(table, "table:print-ranges", "not-a-cell:C3"); + expect( + readSheetPrintSettings(editor.toPackage(), table).printRange, + ).toBeUndefined(); + }); + + it("only the first of several space-separated table:print-ranges is read", () => { + const editor = createOds(); + const table = findTableElement(editor); + setAttr( + table, + "table:print-ranges", + "Sheet1.A1:Sheet1.B2 Sheet1.D4:Sheet1.E5", + ); + const settings = readSheetPrintSettings(editor.toPackage(), table); + expect(settings.printRange).toEqual({ + startRow: 0, + startColumn: 0, + endRow: 1, + endColumn: 1, + }); + }); +}); + +describe("OdsSheet.printSettings: scalePercent/fitToPages", () => { + it("round-trips scalePercent", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = { ...BASE, scalePercent: 150 }; + expect(sheet.printSettings.scalePercent).toBe(150); + expect(attr(currentPageLayoutProperties(editor), "style:scale-to")).toBe( + "150%", + ); + }); + + it("has no scalePercent when the field is omitted", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = BASE; + expect(sheet.printSettings.scalePercent).toBeUndefined(); + }); + + it("an unparseable style:scale-to value yields no scalePercent", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = BASE; + const properties = currentPageLayoutProperties(editor); + setAttr(properties, "style:scale-to", "not-a-percent"); + expect( + readSheetPrintSettings(editor.toPackage(), findTableElement(editor)) + .scalePercent, + ).toBeUndefined(); + }); + + it("round-trips fitToPages", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = { ...BASE, fitToPages: { width: 2, height: 3 } }; + expect(sheet.printSettings.fitToPages).toEqual({ width: 2, height: 3 }); + const properties = currentPageLayoutProperties(editor); + expect(attr(properties, "style:scale-to-X")).toBe("2"); + expect(attr(properties, "style:scale-to-Y")).toBe("3"); + }); + + it("has no fitToPages when the field is omitted", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = BASE; + expect(sheet.printSettings.fitToPages).toBeUndefined(); + }); + + it("fitToPages is undefined when only one of style:scale-to-X/style:scale-to-Y is present", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = BASE; + const properties = currentPageLayoutProperties(editor); + setAttr(properties, "style:scale-to-X", "4"); + expect( + readSheetPrintSettings(editor.toPackage(), findTableElement(editor)) + .fitToPages, + ).toBeUndefined(); + }); + + it("a negative style:scale-to-X/Y value yields no fitToPages", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = BASE; + const properties = currentPageLayoutProperties(editor); + setAttr(properties, "style:scale-to-X", "-1"); + setAttr(properties, "style:scale-to-Y", "3"); + expect( + readSheetPrintSettings(editor.toPackage(), findTableElement(editor)) + .fitToPages, + ).toBeUndefined(); + }); + + it("Number.parseInt truncates a fractional style:scale-to-X/Y value rather than rejecting it, mirroring odf.js's own parseNonNegativeInteger", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = BASE; + const properties = currentPageLayoutProperties(editor); + setAttr(properties, "style:scale-to-X", "2.5"); + setAttr(properties, "style:scale-to-Y", "3"); + expect( + readSheetPrintSettings(editor.toPackage(), findTableElement(editor)) + .fitToPages, + ).toEqual({ width: 2, height: 3 }); + }); +}); + +describe("OdsSheet.printSettings: manualBreaks", () => { + it("round-trips manual breaks on both columns and rows, preserving any width/height already set on the same column/row", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.setColumnWidth(0, 111); + sheet.setRowHeight(2, 33); + sheet.printSettings = { + ...BASE, + manualBreaks: { columns: [0, 4], rows: [2, 6] }, + }; + + const settings = sheet.printSettings; + expect(settings.manualBreaks?.columns).toEqual([0, 4]); + expect(settings.manualBreaks?.rows).toEqual([2, 6]); + + // the pre-existing width/height on column 0 / row 2 survived the manual-break write + const content = readOdsContent(editor.toPackage()); + if (content.kind !== "spreadsheet") { + throw new Error("expected a spreadsheet ContentDocument"); + } + expect( + content.sheets[0]!.columns.find((c) => c.index === 0)?.widthPt, + ).toBeCloseTo(111, 5); + expect( + content.sheets[0]!.rows.find((r) => r.index === 2)?.heightPt, + ).toBeCloseTo(33, 5); + }); + + it("has no manualBreaks when the field is omitted", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = BASE; + expect(sheet.printSettings.manualBreaks).toBeUndefined(); + }); +}); + +describe("OdsSheet.printSettings: repeatColumns/repeatRows", () => { + it("round-trips repeatColumns and repeatRows as table:table-header-columns/-rows wrapping the given range", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + for (let column = 0; column < 5; column++) { + sheet.cell(0, column).value = { kind: "number", value: column }; + } + sheet.printSettings = { + ...BASE, + repeatColumns: { start: 0, end: 1 }, + repeatRows: { start: 0, end: 0 }, + }; + + expect(sheet.printSettings.repeatColumns).toEqual({ start: 0, end: 1 }); + expect(sheet.printSettings.repeatRows).toEqual({ start: 0, end: 0 }); + + const table = findTableElement(editor); + expect(directChild(table, "table:table-header-columns")).toBeDefined(); + expect(directChild(table, "table:table-header-rows")).toBeDefined(); + }); + + it("has no repeatColumns/repeatRows when the fields are omitted", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.printSettings = BASE; + expect(sheet.printSettings.repeatColumns).toBeUndefined(); + expect(sheet.printSettings.repeatRows).toBeUndefined(); + }); + + it("setting a new repeatColumns range dissolves the previous wrapper rather than nesting or duplicating it", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + for (let column = 0; column < 6; column++) { + sheet.cell(0, column).value = { kind: "number", value: column }; + } + sheet.printSettings = { ...BASE, repeatColumns: { start: 0, end: 1 } }; + sheet.printSettings = { ...BASE, repeatColumns: { start: 2, end: 3 } }; + + expect(sheet.printSettings.repeatColumns).toEqual({ start: 2, end: 3 }); + const table = findTableElement(editor); + const wrappers = table.children.filter( + (c) => c.type === "element" && c.tag === "table:table-header-columns", + ); + expect(wrappers).toHaveLength(1); + }); + + it("stamps a real default width/height on the exterior gap-filled columns/rows too, not just the in-range ones", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + // columns/rows 3-5 are individuated (and wrapped) by the repeat range below; positions 0-2 are gap-filled by replaceRun's own case-3 as one compressed run ahead of it, and would otherwise be left at an ambiguous, unstyled 0 -- readOdsContent reports one compressed run as a single entry at its own start index (0), so only that index is checked for the exterior gap-fill. + sheet.printSettings = { + ...BASE, + repeatColumns: { start: 3, end: 5 }, + repeatRows: { start: 3, end: 5 }, + }; + + const content = readOdsContent(editor.toPackage()); + if (content.kind !== "spreadsheet") { + throw new Error("expected a spreadsheet ContentDocument"); + } + for (const index of [0, 3, 4, 5]) { + expect( + content.sheets[0]!.columns.find((c) => c.index === index)?.widthPt, + ).toBeCloseTo(64, 5); + expect( + content.sheets[0]!.rows.find((r) => r.index === index)?.heightPt, + ).toBeCloseTo(15, 5); + } + }); +}); + +describe("hasManualBreak / scanTableStructure (via a hand-crafted table:style-name chain)", () => { + it('a column/row style with no fo:break-before, or one set to something other than "page", is not a manual break', () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.setColumnWidth(0, 80); // mints a style:table-column-properties with no fo:break-before at all + const table = findTableElement(editor); + const column = directChild(table, "table:table-column")!; + const styleName = attr(column, "table:style-name")!; + const styleElement = findStyleElement( + styleName, + "table-column", + editor.toPackage(), + )!; + const properties = directChild( + styleElement, + "style:table-column-properties", + )!; + + expect( + readSheetPrintSettings(editor.toPackage(), table).manualBreaks, + ).toBeUndefined(); + + setAttr(properties, "fo:break-before", "auto"); + expect( + readSheetPrintSettings(editor.toPackage(), table).manualBreaks, + ).toBeUndefined(); + }); +}); + +describe("writeSheetPrintSettings error handling", () => { + it("throws when printRange is set but the table has no table:name", () => { + const editor = createOds(); + const table = findTableElement(editor); + setAttr(table, "table:name", undefined as unknown as string); + // directly deleting the attribute: setAttr(undefined) is not the real removal path, so remove it via the attributes array instead. + table.attributes = table.attributes.filter((a) => a.name !== "table:name"); + + expect(() => { + writeSheetPrintSettings(editor.toPackage(), table, { + ...BASE, + printRange: { startRow: 0, startColumn: 0, endRow: 1, endColumn: 1 }, + }); + }).toThrow(/table:name/); + }); +}); diff --git a/packages/documents.js/src/edit/ods/print-settings.ts b/packages/documents.js/src/edit/ods/print-settings.ts index 73b1058c3..335bcb3e0 100644 --- a/packages/documents.js/src/edit/ods/print-settings.ts +++ b/packages/documents.js/src/edit/ods/print-settings.ts @@ -297,14 +297,12 @@ export function readSheetPrintSettings( : parsePageSize(layoutProperties); const margins = layoutProperties === undefined ? undefined : parseMargins(layoutProperties); - const printTokens = new Set( + // A whitespace-split array (no separate empty-token filtering needed: .includes("grid")/.includes("headers") below finds either token regardless of any empty entries a stray double space or leading/trailing space would otherwise produce) of style:print's own space-separated tokens. + const printWords = (layoutProperties === undefined ? undefined : attr(layoutProperties, "style:print") - ) - ?.split(" ") - .filter((token) => token.length > 0) ?? [], - ); + )?.split(" ") ?? []; const pageOrder = (layoutProperties === undefined ? undefined @@ -347,8 +345,8 @@ export function readSheetPrintSettings( return { pageSize: pageSize ?? PAGE_SIZE_A4, margins: margins ?? DEFAULT_MARGINS, - gridlines: printTokens.has("grid"), - headers: printTokens.has("headers"), + gridlines: printWords.includes("grid"), + headers: printWords.includes("headers"), pageOrder, ...(printRange !== undefined ? { printRange } : {}), ...(scalePercent !== undefined ? { scalePercent } : {}), diff --git a/packages/documents.js/src/edit/ods/scaffold.test.ts b/packages/documents.js/src/edit/ods/scaffold.test.ts new file mode 100644 index 000000000..c2d423133 --- /dev/null +++ b/packages/documents.js/src/edit/ods/scaffold.test.ts @@ -0,0 +1,211 @@ +import type { Package, XmlElement } from "odf.js"; +import { readMimetype, rootElement } from "odf.js"; +import { attr } from "ooxml.js"; +import { describe, expect, it } from "vitest"; +import { createEmptyOdsPackage } from "./scaffold"; + +function xmlRoot(pkg: Package, partName: string): XmlElement { + const part = pkg.parts[partName]; + if (part?.kind !== "xml") { + throw new Error(`expected ${partName} to be an xml part`); + } + const root = rootElement(part.nodes); + if (root === undefined) { + throw new Error(`expected a root element in ${partName}`); + } + return root; +} + +function elementChildren( + node: XmlElement | undefined, + tag: string, +): XmlElement[] { + if (node === undefined) { + return []; + } + return node.children.filter( + (c): c is XmlElement => c.type === "element" && c.tag === tag, + ); +} + +function elementChild(node: XmlElement | undefined, tag: string): XmlElement { + const found = elementChildren(node, tag)[0]; + if (found === undefined) { + throw new Error(`expected a <${tag}> child`); + } + return found; +} + +describe("createEmptyOdsPackage", () => { + it("has every part a minimal ods needs, plus mimetype and manifest", () => { + const pkg = createEmptyOdsPackage(); + expect(Object.keys(pkg.parts).sort()).toEqual( + [ + "content.xml", + "styles.xml", + "meta.xml", + "mimetype", + "META-INF/manifest.xml", + ].sort(), + ); + }); + + it("declares the vnd.oasis.opendocument.spreadsheet media type in both mimetype and the manifest's root entry", () => { + const pkg = createEmptyOdsPackage(); + expect(readMimetype(pkg)).toBe( + "application/vnd.oasis.opendocument.spreadsheet", + ); + + const manifestRoot = xmlRoot(pkg, "META-INF/manifest.xml"); + const rootEntry = elementChildren(manifestRoot, "manifest:file-entry").find( + (entry) => attr(entry, "manifest:full-path") === "/", + ); + if (rootEntry === undefined) { + throw new Error("expected a root manifest:file-entry"); + } + expect(attr(rootEntry, "manifest:media-type")).toBe( + "application/vnd.oasis.opendocument.spreadsheet", + ); + }); + + it("every XML part starts with the standard version/encoding/standalone declaration", () => { + const pkg = createEmptyOdsPackage(); + for (const partName of ["content.xml", "styles.xml", "meta.xml"] as const) { + const part = pkg.parts[partName]; + if (part?.kind !== "xml") { + throw new Error(`expected ${partName} to be an xml part`); + } + expect(part.nodes[0]).toEqual({ + type: "declaration", + attributes: [ + { name: "version", value: "1.0" }, + { name: "encoding", value: "UTF-8" }, + { name: "standalone", value: "yes" }, + ], + }); + } + }); + + it("content.xml declares the of: namespace (required for table:formula's OpenFormula grammar to recalculate on open) alongside version 1.3 and one empty, named default sheet", () => { + const pkg = createEmptyOdsPackage(); + const root = xmlRoot(pkg, "content.xml"); + expect(root.tag).toBe("office:document-content"); + expect(attr(root, "xmlns:of")).toBe( + "urn:oasis:names:tc:opendocument:xmlns:of:1.2", + ); + // xmlns:table specifically (rather than only xmlns:of, hand-declared separately above): pins that CONTENT_NS_PREFIXES's own prefix list is actually spread into the element's attributes, not silently dropped. + expect(attr(root, "xmlns:table")).toBe( + "urn:oasis:names:tc:opendocument:xmlns:table:1.0", + ); + expect(attr(root, "office:version")).toBe("1.3"); + + const automaticStyles = elementChild(root, "office:automatic-styles"); + const sheetStyle = elementChild(automaticStyles, "style:style"); + expect(attr(sheetStyle, "style:name")).toBe("OdsTable"); + expect(attr(sheetStyle, "style:family")).toBe("table"); + expect(attr(sheetStyle, "style:master-page-name")).toBe("Standard"); + + const body = elementChild(root, "office:body"); + const spreadsheet = elementChild(body, "office:spreadsheet"); + const calcSettings = elementChild( + spreadsheet, + "table:calculation-settings", + ); + expect(attr(calcSettings, "table:automatic-find-labels")).toBe("false"); + expect(attr(calcSettings, "table:use-regular-expressions")).toBe("false"); + expect(attr(calcSettings, "table:use-wildcards")).toBe("true"); + expect(attr(calcSettings, "table:null-year")).toBe("1950"); + + const table = elementChild(spreadsheet, "table:table"); + expect(attr(table, "table:name")).toBe("Sheet1"); + expect(attr(table, "table:style-name")).toBe("OdsTable"); + }); + + it("styles.xml declares version 1.3, a PAGE_SIZE_A4/2cm-margin page layout, and the Standard master page referencing it", () => { + const pkg = createEmptyOdsPackage(); + const root = xmlRoot(pkg, "styles.xml"); + expect(root.tag).toBe("office:document-styles"); + expect(attr(root, "office:version")).toBe("1.3"); + // An empty office:styles sibling, distinct from office:automatic-styles below -- odf.js's own consumers expect this element to exist even when this scaffold defines no named paragraph/cell styles in it. + expect(elementChild(root, "office:styles")).toBeDefined(); + + const automaticStyles = elementChild(root, "office:automatic-styles"); + const pageLayout = elementChild(automaticStyles, "style:page-layout"); + expect(attr(pageLayout, "style:name")).toBe("PM1"); + const properties = elementChild(pageLayout, "style:page-layout-properties"); + expect(attr(properties, "fo:page-width")).toBe("595.28pt"); + expect(attr(properties, "fo:page-height")).toBe("841.89pt"); + expect(attr(properties, "fo:margin-top")).toBe("2cm"); + expect(attr(properties, "fo:margin-right")).toBe("2cm"); + expect(attr(properties, "fo:margin-bottom")).toBe("2cm"); + expect(attr(properties, "fo:margin-left")).toBe("2cm"); + + const masterStyles = elementChild(root, "office:master-styles"); + const masterPage = elementChild(masterStyles, "style:master-page"); + expect(attr(masterPage, "style:name")).toBe("Standard"); + expect(attr(masterPage, "style:page-layout-name")).toBe("PM1"); + }); + + it("meta.xml has an empty office:meta when no metadata is given", () => { + const pkg = createEmptyOdsPackage(); + const root = xmlRoot(pkg, "meta.xml"); + expect(root.tag).toBe("office:document-meta"); + expect(attr(root, "office:version")).toBe("1.3"); + const meta = elementChild(root, "office:meta"); + expect(meta.children).toHaveLength(0); + }); + + it("meta.xml carries every given metadata field, XML-encoded, with keywords repeated once per entry", () => { + const pkg = createEmptyOdsPackage({ + metadata: { + title: "A & More", + author: "Ada", + subject: "A Subject", + keywords: ["alpha", "beta"], + creator: "documents.js", + createdIso: "2024-01-01T00:00:00.000Z", + modifiedIso: "2024-06-01T00:00:00.000Z", + }, + }); + const root = xmlRoot(pkg, "meta.xml"); + const meta = elementChild(root, "office:meta"); + + const title = elementChild(meta, "dc:title"); + expect(title.children).toEqual([ + { type: "text", value: "A <Title> & More" }, + ]); + const creator = elementChild(meta, "meta:initial-creator"); + expect(creator.children).toEqual([{ type: "text", value: "Ada" }]); + const subject = elementChild(meta, "dc:subject"); + expect(subject.children).toEqual([{ type: "text", value: "A Subject" }]); + const keywords = elementChildren(meta, "meta:keyword"); + expect(keywords).toHaveLength(2); + expect(keywords[0]?.children).toEqual([{ type: "text", value: "alpha" }]); + expect(keywords[1]?.children).toEqual([{ type: "text", value: "beta" }]); + const generator = elementChild(meta, "meta:generator"); + expect(generator.children).toEqual([ + { type: "text", value: "documents.js" }, + ]); + const creationDate = elementChild(meta, "meta:creation-date"); + expect(creationDate.children).toEqual([ + { type: "text", value: "2024-01-01T00:00:00.000Z" }, + ]); + const date = elementChild(meta, "dc:date"); + expect(date.children).toEqual([ + { type: "text", value: "2024-06-01T00:00:00.000Z" }, + ]); + }); + + it("meta.xml omits each metadata field individually when it is absent, rather than writing an empty element", () => { + const pkg = createEmptyOdsPackage({ metadata: { title: "Only Title" } }); + const root = xmlRoot(pkg, "meta.xml"); + const meta = elementChild(root, "office:meta"); + expect(elementChildren(meta, "dc:title")).toHaveLength(1); + expect(elementChildren(meta, "meta:initial-creator")).toHaveLength(0); + expect(elementChildren(meta, "dc:subject")).toHaveLength(0); + expect(elementChildren(meta, "meta:keyword")).toHaveLength(0); + expect(elementChildren(meta, "meta:generator")).toHaveLength(0); + expect(elementChildren(meta, "meta:creation-date")).toHaveLength(0); + expect(elementChildren(meta, "dc:date")).toHaveLength(0); + }); +}); diff --git a/packages/documents.js/src/edit/ods/sheet.test.ts b/packages/documents.js/src/edit/ods/sheet.test.ts index decba4cef..1089bd49c 100644 --- a/packages/documents.js/src/edit/ods/sheet.test.ts +++ b/packages/documents.js/src/edit/ods/sheet.test.ts @@ -568,6 +568,21 @@ describe("OdsSheet.setColumnWidth / setRowHeight", () => { // sheetB's own column was only ever touched by its own cell() call, never by sheetA's setColumnWidth -- it reads back at the ordinary cell()-materialization default (64pt), proving the two sheets' styles are genuinely independent rather than sharing one automatic style neither of them meant to share. expect(content.sheets[1]!.columns[0]?.widthPt).toBeCloseTo(64, 5); }); + + it("a later cell() on a column/row that already has an explicit width/height never resets it back to the cell()-materialization default", () => { + const editor = createOds(); + const sheet = editor.sheets()[0]!; + sheet.setColumnWidth(0, 130); + sheet.setRowHeight(0, 45); + sheet.cell(0, 0).value = { kind: "string", value: "x" }; // ensureColumnDefaultWidth/ensureRowDefaultHeight run here and must no-op + + const content = readOdsContent(openOds(editor.toBytes()).toPackage()); + if (content.kind !== "spreadsheet") { + throw new Error("expected a spreadsheet ContentDocument"); + } + expect(content.sheets[0]!.columns[0]?.widthPt).toBeCloseTo(130, 5); + expect(content.sheets[0]!.rows[0]?.heightPt).toBeCloseTo(45, 5); + }); }); describe("OdsSheet.setColumnHidden / setRowHidden", () => { diff --git a/packages/documents.js/src/edit/odt/run.test.ts b/packages/documents.js/src/edit/odt/run.test.ts index 6d5851e08..0d214296f 100644 --- a/packages/documents.js/src/edit/odt/run.test.ts +++ b/packages/documents.js/src/edit/odt/run.test.ts @@ -25,17 +25,20 @@ describe("OdtRun text", () => { }); describe("OdtRun toggle properties", () => { - it("bold/italic/underline default to false and can be toggled on and off", () => { + it("bold/italic/underline/strike default to false and can be toggled on and off", () => { const run = freshRun(); expect(run.bold).toBe(false); expect(run.italic).toBe(false); expect(run.underline).toBe(false); + expect(run.strike).toBe(false); run.bold = true; run.italic = true; run.underline = true; + run.strike = true; expect(run.bold).toBe(true); expect(run.italic).toBe(true); expect(run.underline).toBe(true); + expect(run.strike).toBe(true); run.bold = false; expect(run.bold).toBe(false); expect(run.italic).toBe(true); // unaffected by the other toggle @@ -121,14 +124,20 @@ describe("buildRun", () => { text: "Hi", bold: true, italic: true, + underline: true, + strike: true, sizePt: 16, fontFamily: "Arial", + color: { r: 1, g: 0, b: 0 }, }); const run = new OdtRun([runElement], runElement, editor.toPackage()); expect(run.bold).toBe(true); expect(run.italic).toBe(true); + expect(run.underline).toBe(true); + expect(run.strike).toBe(true); expect(run.sizePt).toBe(16); expect(run.fontFamily).toBe("Arial"); + expect(run.color).toEqual({ r: 1, g: 0, b: 0 }); expect(run.text).toBe("Hi"); }); }); diff --git a/packages/documents.js/src/edit/pdf/util.test.ts b/packages/documents.js/src/edit/pdf/util.test.ts new file mode 100644 index 000000000..ef848b6e7 --- /dev/null +++ b/packages/documents.js/src/edit/pdf/util.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it } from "vitest"; +import type { LayoutImageAsset } from "pdf-codec"; +import { encodePng } from "byte-codec"; +import { registerImageBytes, spliceOut } from "./util"; + +// A minimal, spec-shaped baseline JPEG: SOI, an SOF0 frame header (4x3px, 3 components), EOI -- mirrors byte-codec's own readJpegInfo test fixture shape (that package's own image/jpeg-info.test.ts buildJpeg helper), restated inline here since this is the only place in documents.js that needs a real (not merely format-labelled) JPEG byte stream. +const JPEG_WIDTH = 4; +const JPEG_HEIGHT = 3; +const JPEG_BYTES = new Uint8Array([ + 0xff, + 0xd8, // SOI + 0xff, + 0xc0, + 0x00, + 0x08, + 0x08, + 0x00, + JPEG_HEIGHT, + 0x00, + JPEG_WIDTH, + 0x03, // SOF0 + 0xff, + 0xd9, // EOI +]); + +const PNG_BYTES = encodePng({ + width: 2, + height: 2, + channels: 3, + data: new Uint8Array(2 * 2 * 3), +}); + +describe("spliceOut", () => { + it("removes the given node from the container", () => { + const container = ["a", "b", "c"]; + spliceOut(container, "b"); + expect(container).toEqual(["a", "c"]); + }); + + it("leaves the container completely unchanged when the node is not present", () => { + // If the index-not-found guard were skipped, Array.prototype.splice(-1, 1) would silently remove the container's own LAST element instead of doing nothing. + const container = ["a", "b", "c"]; + spliceOut(container, "not present"); + expect(container).toEqual(["a", "b", "c"]); + }); +}); + +describe("registerImageBytes", () => { + it("decodes real JPEG dimensions via readJpegInfo for format 'jpeg'", () => { + const images: Record<string, LayoutImageAsset> = {}; + const imageId = registerImageBytes(JPEG_BYTES, "jpeg", images); + expect(images[imageId]).toMatchObject({ + format: "jpeg", + widthPx: JPEG_WIDTH, + heightPx: JPEG_HEIGHT, + }); + }); + + it("decodes real PNG dimensions via decodePng for format 'png'", () => { + const images: Record<string, LayoutImageAsset> = {}; + const imageId = registerImageBytes(PNG_BYTES, "png", images); + expect(images[imageId]).toMatchObject({ + format: "png", + widthPx: 2, + heightPx: 2, + }); + }); + + it("does not re-decode or overwrite an already-registered image id", () => { + const images: Record<string, LayoutImageAsset> = {}; + const imageId = registerImageBytes(PNG_BYTES, "png", images); + // A sentinel value decodeImageDimensions could never itself produce -- if the "already registered" guard were skipped, the second call would overwrite it with the real decode. + const sentinel: LayoutImageAsset = { + format: "png", + base64: "sentinel", + widthPx: -1, + heightPx: -1, + }; + images[imageId] = sentinel; + const secondId = registerImageBytes(PNG_BYTES, "png", images); + expect(secondId).toBe(imageId); + expect(images[imageId]).toBe(sentinel); + }); +}); diff --git a/packages/documents.js/src/edit/ppt/editor.test.ts b/packages/documents.js/src/edit/ppt/editor.test.ts index efaec7e36..b2de6bef0 100644 --- a/packages/documents.js/src/edit/ppt/editor.test.ts +++ b/packages/documents.js/src/edit/ppt/editor.test.ts @@ -1,7 +1,8 @@ +import type { ContentDocument } from "document-schema.js"; import { SLIDE_SIZE_WIDESCREEN } from "document-schema.js"; import { describe, expect, it } from "vitest"; import { fixedClock } from "../../ports/clock"; -import { createPpt, openPpt } from "./editor"; +import { createPpt, openPpt, PptEditor } from "./editor"; const FIXED_ISO = "2026-01-01T00:00:00.000Z"; @@ -13,6 +14,19 @@ describe("createPpt", () => { }); }); +describe("PptEditor constructor guard", () => { + it("rejects a non-presentation ContentDocument, naming the offending kind", () => { + const spreadsheet: ContentDocument = { + kind: "spreadsheet", + metadata: {}, + sheets: [], + }; + expect(() => new PptEditor(spreadsheet)).toThrow( + 'PptEditor requires a presentation ContentDocument, got "spreadsheet"', + ); + }); +}); + describe("PptEditor slides and shapes", () => { it("round-trips a slide with a text box, its frame, and speaker notes", () => { const editor = createPpt(); diff --git a/packages/documents.js/src/edit/pptx/vector.test.ts b/packages/documents.js/src/edit/pptx/vector.test.ts new file mode 100644 index 000000000..34038883a --- /dev/null +++ b/packages/documents.js/src/edit/pptx/vector.test.ts @@ -0,0 +1,48 @@ +import type { ContentVector } from "document-schema.js"; +import { childrenWithTag, attr } from "ooxml.js"; +import { describe, expect, it } from "vitest"; +import { buildVectorShape } from "./vector"; + +function rect(): ContentVector { + return { + kind: "rect", + frame: { xPt: 0, yPt: 0, widthPt: 10, heightPt: 5 }, + }; +} + +describe("buildVectorShape", () => { + it("wraps the shape properties in a real p:sp/p:nvSpPr with the expected child tags", () => { + const sp = buildVectorShape(rect(), 3); + expect(sp.tag).toBe("p:sp"); + expect( + sp.children.map((c) => (c.type === "element" ? c.tag : c.type)), + ).toEqual(["p:nvSpPr", "p:spPr"]); + + const [nvSpPr] = childrenWithTag(sp, "p:nvSpPr"); + expect(nvSpPr).toBeDefined(); + expect( + nvSpPr === undefined + ? [] + : nvSpPr.children.map((c) => (c.type === "element" ? c.tag : c.type)), + ).toEqual(["p:cNvPr", "p:cNvSpPr", "p:nvPr"]); + + const [cNvPr] = + nvSpPr === undefined ? [] : childrenWithTag(nvSpPr, "p:cNvPr"); + expect(cNvPr).toBeDefined(); + expect(cNvPr === undefined ? undefined : attr(cNvPr, "id")).toBe("3"); + expect(cNvPr === undefined ? undefined : attr(cNvPr, "name")).toBe( + "Rect 3", + ); + }); + + it("derives the shape id and name from the supplied id, not a hardcoded value", () => { + const sp = buildVectorShape(rect(), 7); + const [nvSpPr] = childrenWithTag(sp, "p:nvSpPr"); + const [cNvPr] = + nvSpPr === undefined ? [] : childrenWithTag(nvSpPr, "p:cNvPr"); + expect(cNvPr === undefined ? undefined : attr(cNvPr, "id")).toBe("7"); + expect(cNvPr === undefined ? undefined : attr(cNvPr, "name")).toBe( + "Rect 7", + ); + }); +}); diff --git a/packages/documents.js/src/edit/xls/editor.test.ts b/packages/documents.js/src/edit/xls/editor.test.ts index 415262718..772014fb9 100644 --- a/packages/documents.js/src/edit/xls/editor.test.ts +++ b/packages/documents.js/src/edit/xls/editor.test.ts @@ -1,9 +1,34 @@ +import type { ContentDocument } from "document-schema.js"; import { describe, expect, it } from "vitest"; import { fixedClock } from "../../ports/clock"; -import { createXls, openXls } from "./editor"; +import { createXls, openXls, XlsEditor } from "./editor"; const FIXED_ISO = "2026-01-01T00:00:00.000Z"; +describe("XlsEditor constructor guards", () => { + it("rejects a non-spreadsheet ContentDocument, naming the offending kind", () => { + const wordprocessing: ContentDocument = { + kind: "wordprocessing", + metadata: {}, + sections: [], + }; + expect(() => new XlsEditor(wordprocessing)).toThrow( + 'XlsEditor requires a spreadsheet ContentDocument, got "wordprocessing"', + ); + }); + + it("rejects a spreadsheet ContentDocument with no sheets at all", () => { + const empty: ContentDocument = { + kind: "spreadsheet", + metadata: {}, + sheets: [], + }; + expect(() => new XlsEditor(empty)).toThrow( + "an xls workbook must carry at least one sheet", + ); + }); +}); + describe("createXls", () => { it("builds a one-sheet workbook with real metadata timestamps", () => { const editor = createXls({ diff --git a/packages/documents.js/src/firebird/backup.test.ts b/packages/documents.js/src/firebird/backup.test.ts index 64bd71f1e..198625e26 100644 --- a/packages/documents.js/src/firebird/backup.test.ts +++ b/packages/documents.js/src/firebird/backup.test.ts @@ -179,6 +179,16 @@ describe("readFirebirdBackup: format guards, never a silent wrong result", () => ); }); + it("names its errors FirebirdBackupFormatError, not the bare Error default", () => { + try { + readFirebirdBackup(minimalBurpStream(11)); + throw new Error("expected readFirebirdBackup to throw"); + } catch (error) { + expect(error).toBeInstanceOf(FirebirdBackupFormatError); + expect((error as Error).name).toBe("FirebirdBackupFormatError"); + } + }); + it("throws FirebirdBackupFormatError for a non-transportable (native binary) backup", () => { // No att_backup_transportable attribute present at all -- mvol.cpp only ever writes it when true, so its absence IS "false" (see reader.ts's own Encoding 1 note). expect(() => @@ -198,6 +208,26 @@ describe("readFirebirdBackup: format guards, never a silent wrong result", () => ); }); + it("throws FirebirdBackupFormatError when rec_burp has no att_backup_format attribute at all", () => { + // rec_burp(0) immediately followed by att_end(0) -- a leading record with an empty attribute list. + const bytes = new Uint8Array([0, 0]); + expect(() => readFirebirdBackup(bytes)).toThrow(FirebirdBackupFormatError); + expect(() => readFirebirdBackup(bytes)).toThrow( + /had no att_backup_format attribute/, + ); + }); + + it("reports compressed:false when att_backup_compress is absent, not merely truthy-adjacent", () => { + // A fully valid, transportable, uncompressed rec_burp header -- no att_backup_compress attribute at all -- followed directly by rec_end, so nothing beyond the header is ever parsed. + const bytes = minimalBurpStream( + SUPPORTED_BACKUP_FORMAT_VERSION, + [5, 4, 1, 0, 0, 0], + ); + const { summary } = readFirebirdBackup(bytes); + expect(summary.transportable).toBe(true); + expect(summary.compressed).toBe(false); + }); + it("throws FirebirdCompositeRecordUnsupportedError, not a silent skip, for a genuinely unrecognised top-level record kind", () => { // Valid rec_burp header (transportable=true) followed immediately by an unrecognised record type (250) instead of rec_end. const transportableAttr = [5, 4, 1, 0, 0, 0]; @@ -216,6 +246,103 @@ describe("readFirebirdBackup: format guards, never a silent wrong result", () => expect(() => readFirebirdBackup(bytes)).toThrow( FirebirdCompositeRecordUnsupportedError, ); + expect(() => readFirebirdBackup(bytes)).toThrow( + /while walking the backup stream's own top-level record sequence/, + ); + }); + + it("throws FirebirdCompositeRecordUnsupportedError for a relation carrying an unrecognised nested record (a rec_view child)", () => { + // rec_burp header, then rec_relation("T") whose own nested-record loop opens with an unrecognised record type (99) instead of a rec_field or rec_relation_end. + const transportableAttr = [5, 4, 1, 0, 0, 0]; + const bytes = new Uint8Array([ + 0, + 2, + 4, + SUPPORTED_BACKUP_FORMAT_VERSION, + 0, + 0, + 0, + ...transportableAttr, + 0, + 3, // REC_RELATION + 1, + 1, + 84, // att_relation_name = "T" + 0, // att_end + 99, // unrecognised nested record type + ]); + expect(() => readFirebirdBackup(bytes)).toThrow( + FirebirdCompositeRecordUnsupportedError, + ); + expect(() => readFirebirdBackup(bytes)).toThrow( + /while reading a relation's own schema \(a rec_view child, most likely\)/, + ); + }); + + it("excludes a computed field from the reported columns, since gbak's own row writer never includes one either", () => { + // rec_burp header, then rec_relation("T") with two rec_field children -- "ID" (ordinary) and "CALC" (att_field_computed_flag=1) -- followed by rec_relation_end and rec_end. + const transportableAttr = [5, 4, 1, 0, 0, 0]; + const BLR_LONG = 8; + const idField = [ + 4, // REC_FIELD + 1, + 2, + 73, + 68, // att_field_name = "ID" + 8, + 4, + BLR_LONG, + 0, + 0, + 0, // att_field_type + 0, // att_end + ]; + const calcField = [ + 4, // REC_FIELD + 1, + 4, + 67, + 65, + 76, + 67, // att_field_name = "CALC" + 8, + 4, + BLR_LONG, + 0, + 0, + 0, // att_field_type + 23, + 4, + 1, + 0, + 0, + 0, // att_field_computed_flag = true + 0, // att_end + ]; + const bytes = new Uint8Array([ + 0, + 2, + 4, + SUPPORTED_BACKUP_FORMAT_VERSION, + 0, + 0, + 0, + ...transportableAttr, + 0, + 3, // REC_RELATION + 1, + 1, + 84, // att_relation_name = "T" + 0, // att_end + ...idField, + ...calcField, + 9, // REC_RELATION_END + 10, // REC_END + ]); + const { tables } = readFirebirdBackup(bytes); + expect(tables).toEqual([ + { tableName: "T", columns: [{ name: "ID", type: "INTEGER" }], rows: [] }, + ]); }); }); diff --git a/packages/documents.js/src/firebird/backup.ts b/packages/documents.js/src/firebird/backup.ts index dba1a25c8..73bd34637 100644 --- a/packages/documents.js/src/firebird/backup.ts +++ b/packages/documents.js/src/firebird/backup.ts @@ -163,7 +163,8 @@ export function readFirebirdBackup( let pageSizeBytes: number | undefined; const schema = new Map<string, FirebirdRelation>(); - const tablesInOrder: string[] = []; + // The relation objects themselves, in creation order -- not just their names -- so the final table-building step below can read tableName/columns straight off each one rather than looking a name back up in `schema` (which would always succeed, since every name pushed here was set in `schema` in the same statement, but a lookup that can never fail is exactly the redundant guard this module's own equivalent-mutant policy requires eliminating rather than leaving untestable). + const relationsInOrder: FirebirdRelation[] = []; const rowsByRelation = new Map< string, ReturnType<typeof readRelationData>["rows"] @@ -187,7 +188,7 @@ export function readFirebirdBackup( ); }); schema.set(relation.name, relation); - tablesInOrder.push(relation.name); + relationsInOrder.push(relation); continue; } if (recordType === REC_RELATION_DATA) { @@ -208,14 +209,8 @@ export function readFirebirdBackup( // No check that reader.atEnd() here -- confirmed against a real fixture that rec_end is genuinely NOT the last byte of the stream: mvol.cpp writes backup volumes in fixed-size blocks (att_backup_blksize), zero-padding the final block out to that size, so real trailing bytes after rec_end are legitimate filler, not a sign of a mis-walked stream. restore.epp's own top-level loop (`while (get_record(&record, tdgbl) != rec_end)`) matches this exactly -- it stops at rec_end and never inspects what follows. // Only USER tables (schema.system_flag-free, which this reader never reads at all -- see the README's .odb Tier 3 Fidelity note) are reported: RDB$RELATIONS/RDB$RELATION_FIELDS/RDB$FIELDS and every other system table never appear as their own rec_relation records in a gbak backup at all -- gbak's own schema dump only ever emits user-created relations (plus any user-created VIEWs, which this reader throws on as an unsupported composite record -- see schema.ts's own onUnhandledNested). There is consequently no RDB$RELATIONS-bootstrap step in this reader at all: unlike raw ODS-page reading, gbak's own backup format has ALREADY resolved table/column definitions into rec_relation/rec_field records by the time this reader ever sees them -- see the README's own .odb Tier 3 Gotchas entry for why this is a genuine, load-bearing correction to the design plan's original raw-page-format premise. - const tables: HsqldbTable[] = tablesInOrder.map((name) => { - const relation = schema.get(name); - if (relation === undefined) { - throw new FirebirdBackupFormatError( - `internal error: relation "${name}" missing from its own schema map`, - ); - } - const rows = rowsByRelation.get(name) ?? []; + const tables: HsqldbTable[] = relationsInOrder.map((relation) => { + const rows = rowsByRelation.get(relation.name) ?? []; return { tableName: relation.name, columns: relationToColumns(relation), diff --git a/packages/documents.js/src/firebird/date.test.ts b/packages/documents.js/src/firebird/date.test.ts index 1b528055e..1bf44a505 100644 --- a/packages/documents.js/src/firebird/date.test.ts +++ b/packages/documents.js/src/firebird/date.test.ts @@ -35,6 +35,30 @@ describe("decodeFirebirdDate", () => { ); expect(decodeFirebirdDate(days)).toEqual({ year: 2024, month: 2, day: 29 }); }); + + it("excludes 1700 from the leap years despite being divisible by 4, since it isn't divisible by 400", () => { + // The century-based correction term (the 4-year rule minus a further exception every 100 years, restored every 400) is exactly what distinguishes this from a naive 4-year-only leap rule -- 1700 is the case that rule exists for. + const days = Math.round( + (Date.UTC(1700, 1, 28) - Date.UTC(1858, 10, 17)) / 86400000, + ); + expect(decodeFirebirdDate(days)).toEqual({ year: 1700, month: 2, day: 28 }); + expect(formatFirebirdDate(days)).toBe("1700-02-28"); + }); + + it("formats a year under 1000 with leading zeros", () => { + const days = Math.round( + (Date.UTC(500, 1, 28) - Date.UTC(1858, 10, 17)) / 86400000, + ); + expect(decodeFirebirdDate(days)).toEqual({ year: 500, month: 2, day: 28 }); + expect(formatFirebirdDate(days)).toBe("0500-02-28"); + }); + + it("rolls over correctly into March of the following (non-leap) year, one day after a year ending in 59", () => { + const days = Math.round( + (Date.UTC(1859, 2, 1) - Date.UTC(1858, 10, 17)) / 86400000, + ); + expect(decodeFirebirdDate(days)).toEqual({ year: 1859, month: 3, day: 1 }); + }); }); describe("decodeFirebirdTime", () => { @@ -61,6 +85,12 @@ describe("decodeFirebirdTime", () => { const ticks = (9 * 3600 + 5 * 60 + 1) * 10000; expect(formatFirebirdTime(ticks)).toBe("09:05:01.000"); }); + + it("converts a non-zero fraction of a tick-second to milliseconds by dividing, not multiplying", () => { + // 5000 ticks (of 10000 ticks/second) is half a second -- 500ms, not the 50000 a fractions * 10 mutant would produce. + const ticks = 5000; + expect(formatFirebirdTime(ticks)).toBe("00:00:00.500"); + }); }); describe("formatFirebirdTimestamp", () => { diff --git a/packages/documents.js/src/fonts/registry.test.ts b/packages/documents.js/src/fonts/registry.test.ts index d352f8396..86f206872 100644 --- a/packages/documents.js/src/fonts/registry.test.ts +++ b/packages/documents.js/src/fonts/registry.test.ts @@ -13,7 +13,11 @@ import { embeddedFontOdtPackage, embeddedFontPptxPackage, } from "../test-support/fonts"; -import { createDocumentFontRegistry, extractSourceFonts } from "./registry"; +import { + createDocumentFontRegistry, + extractSourceFonts, + treeEmbeddedFontsOf, +} from "./registry"; // A character no Latin-only face carries -- Caladea's cmap genuinely has no glyph for CJK, so a run containing this is the honest "the embedded face is right for the document but lacks this one synthesised character" case. const UNMAPPED_CHARACTER = "中"; @@ -269,3 +273,23 @@ describe("a cmap miss on a source-embedded face", () => { ).toBeGreaterThan(0); }); }); + +describe("treeEmbeddedFontsOf", () => { + it("returns undefined, not an empty array, for a source package with no embedded fonts", () => { + expect( + treeEmbeddedFontsOf({ kind: "docx", package: minimalDocxPackage() }), + ).toBeUndefined(); + }); + + it("returns the base64-encoded faces for a source package that embeds fonts", () => { + const faces = treeEmbeddedFontsOf({ + kind: "docx", + package: embeddedFontDocxPackage(), + }); + expect(faces).toBeDefined(); + expect(faces?.length).toBeGreaterThan(0); + expect(faces?.[0]?.family).toBe("Caladea"); + expect(typeof faces?.[0]?.base64).toBe("string"); + expect(faces?.[0]?.base64.length).toBeGreaterThan(0); + }); +}); diff --git a/packages/documents.js/src/latex/rational.test.ts b/packages/documents.js/src/latex/rational.test.ts new file mode 100644 index 000000000..5ade58ec6 --- /dev/null +++ b/packages/documents.js/src/latex/rational.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "vitest"; +import { decimalToRational, reduceRational } from "./rational"; + +describe("decimalToRational", () => { + it("reduces a fractional literal to its lowest-terms rational", () => { + expect(decimalToRational("3.14")).toEqual({ + numerator: "157", + denominator: "50", + }); + }); + + it("treats a bare integer literal as an exact whole-number rational", () => { + expect(decimalToRational("42")).toEqual({ + numerator: "42", + denominator: "1", + }); + }); + + it("reduces a zero-valued literal to the schema's 0/1 convention regardless of trailing zeros", () => { + expect(decimalToRational("0.00")).toEqual({ + numerator: "0", + denominator: "1", + }); + }); + + it("returns undefined for a literal with two decimal points", () => { + expect(decimalToRational("3.1.4")).toBeUndefined(); + }); + + it("returns undefined for a literal containing non-digit characters", () => { + expect(decimalToRational("12a")).toBeUndefined(); + }); + + it("returns undefined for an empty literal", () => { + expect(decimalToRational("")).toBeUndefined(); + }); + + it("returns undefined for a literal with a leading sign, even though its trailing characters are digits", () => { + expect(decimalToRational("-5")).toBeUndefined(); + }); +}); + +describe("reduceRational", () => { + it("divides both terms by their greatest common divisor", () => { + expect(reduceRational(6n, 3n)).toEqual({ + numerator: "2", + denominator: "1", + }); + }); + + it("leaves an already-reduced pair unchanged", () => { + expect(reduceRational(7n, 5n)).toEqual({ + numerator: "7", + denominator: "5", + }); + }); + + it("reduces a zero numerator against gcd(0, denominator) == denominator, landing on 0/1", () => { + expect(reduceRational(0n, 9n)).toEqual({ + numerator: "0", + denominator: "1", + }); + }); + + it("treats gcd(0, 0) as 1 rather than dividing by zero for a 0/0-shaped input", () => { + expect(() => reduceRational(0n, 0n)).not.toThrow(); + expect(reduceRational(0n, 0n)).toEqual({ + numerator: "0", + denominator: "0", + }); + }); +}); diff --git a/packages/documents.js/src/layout/lattice.test.ts b/packages/documents.js/src/layout/lattice.test.ts index 4a015a282..5c176c7aa 100644 --- a/packages/documents.js/src/layout/lattice.test.ts +++ b/packages/documents.js/src/layout/lattice.test.ts @@ -206,5 +206,6 @@ describe("findCellRegions: a merged region far larger than the JS engine's argum { rowStart: 0, rowEnd: rowCount, colStart: 0, colEnd: 1 }, { rowStart: 0, rowEnd: rowCount, colStart: 1, colEnd: 2 }, ]); - }); + // Builds and reconciles 200,000 synthetic row dividers, which is genuine work even though it completes in well under a second uncontended -- under Stryker's per-statement instrumentation plus heavy concurrent host load it has measured a 5000ms-plus wall clock, the same "wall-clock dominated by scheduling, not this test's own CPU work" shape documented for read-graph.test.ts's docxToPdf timeout (ExaDev/documents.js#1039) and its sibling ODS mergeCells test (ExaDev/documents.js#1037). + }, 60_000); }); diff --git a/packages/documents.js/src/markdown/read.test.ts b/packages/documents.js/src/markdown/read.test.ts index 03e946ced..0ad02e2a8 100644 --- a/packages/documents.js/src/markdown/read.test.ts +++ b/packages/documents.js/src/markdown/read.test.ts @@ -1,9 +1,12 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; +import type * as MarkdownCodec from "markdown-codec"; import { richMarkdownText, richMarkdownTextWithFrontMatter, } from "../test-support/markdown"; -import { readMarkdownContent } from "./read"; +import { HTML_PREFORMATTED_STYLE_ID } from "markdown-codec"; +import { promoteBlock, readMarkdownContent } from "./read"; +import { PAGE_BREAK_MARKER } from "./write"; describe("readMarkdownContent", () => { it("produces a wordprocessing ContentDocument", () => { @@ -11,6 +14,27 @@ describe("readMarkdownContent", () => { expect(content.kind).toBe("wordprocessing"); }); + it("throws if markdown-codec's own reader ever produced a non-wordprocessing ContentDocument", async () => { + // Not a shape markdown-codec's real readMarkdownContent can ever produce (markdown has no presentation/spreadsheet/drawing/formula equivalent to lower into, per this module's own comment) -- this exercises the defensive guard directly via a mocked reader, since no real markdown text can trigger it. + vi.resetModules(); + vi.doMock("markdown-codec", async () => { + const actual = + await vi.importActual<typeof MarkdownCodec>("markdown-codec"); + return { + ...actual, + readMarkdownContent: () => ({ + document: { kind: "spreadsheet", metadata: {}, sheets: [] }, + }), + }; + }); + const { readMarkdownContent: mockedRead } = await import("./read"); + expect(() => mockedRead("irrelevant")).toThrow( + "readMarkdownContent returned a non-wordprocessing ContentDocument", + ); + vi.doUnmock("markdown-codec"); + vi.resetModules(); + }); + // The read-side inverse of src/markdown/write.ts's page-break marker: an `<!-- page break -->` HTML comment lowers (via markdown-codec's own HTML block arm) to an HTMLPreformatted paragraph carrying that literal text, and this pass promotes exactly that paragraph to a pageBreak block -- so markdownToPdf re-renders a real page boundary and a pdfToMarkdown -> markdownToPdf round trip regenerates markers from real boundaries instead of accumulating them as visible literal text. it("reads a page-break marker back as a pageBreak block", () => { const content = readMarkdownContent( @@ -119,3 +143,30 @@ describe("readMarkdownContent", () => { ).toThrow(); }); }); + +describe("promoteBlock", () => { + it("does not promote a paragraph carrying the exact marker text if it isn't HTML-preformatted styled", () => { + // markdown-codec's own HTML-block lowering never produces this exact combination for real input, but the gate is still styleId AND text, not text alone -- pinned directly. + const block = promoteBlock({ + kind: "paragraph", + runs: [{ text: PAGE_BREAK_MARKER }], + }); + expect(block).toEqual({ + kind: "paragraph", + runs: [{ text: PAGE_BREAK_MARKER }], + }); + }); + + it("promotes runs whose texts concatenate (with no separator) to exactly the marker", () => { + // A real marker paragraph is always a single run; this splits it across two runs so a join that inserted any separator between them would produce a non-matching string and fail to promote, proving the join really does concatenate with "" rather than something else. + const promoted = promoteBlock({ + kind: "paragraph", + styleId: HTML_PREFORMATTED_STYLE_ID, + runs: [ + { text: PAGE_BREAK_MARKER.slice(0, 6) }, + { text: PAGE_BREAK_MARKER.slice(6) }, + ], + }); + expect(promoted).toEqual({ kind: "pageBreak" }); + }); +}); diff --git a/packages/documents.js/src/markdown/read.ts b/packages/documents.js/src/markdown/read.ts index 6a3998d26..3ea42e978 100644 --- a/packages/documents.js/src/markdown/read.ts +++ b/packages/documents.js/src/markdown/read.ts @@ -47,7 +47,8 @@ function promotePageBreakMarkers( }; } -function promoteBlock(block: ContentBlock): ContentBlock { +// Exported (not merely internal) so the two conditions that gate a promotion -- the block's own styleId, and the exact (not merely substring, not merely per-run) text match -- are directly testable: a real markdown-codec-lowered marker paragraph is always a single run, so a hand-built multi-run block is the only way to exercise the join("") boundary, and a same-text-wrong-style paragraph is not a shape markdown-codec's own HTML-block lowering can produce for anything OTHER than this exact marker's own preformatted styling. +export function promoteBlock(block: ContentBlock): ContentBlock { if ( block.kind !== "paragraph" || block.styleId !== HTML_PREFORMATTED_STYLE_ID diff --git a/packages/documents.js/src/markdown/write.test.ts b/packages/documents.js/src/markdown/write.test.ts index c4599418d..54baa2b23 100644 --- a/packages/documents.js/src/markdown/write.test.ts +++ b/packages/documents.js/src/markdown/write.test.ts @@ -3,10 +3,22 @@ import type { ContentBlock, ContentDocument } from "document-schema.js"; import { MarkdownUnsupportedDocumentKindError } from "markdown-codec"; import { describe, expect, it } from "vitest"; import { MarkdownUnbalancedConstructMarkersError } from "markdown-codec"; +import { latexToFormula } from "../latex/lower"; +import { buildFormulaBlock } from "../model/formula"; import { richMarkdownText } from "../test-support/markdown"; import { readMarkdownContent } from "./read"; import { buildMarkdownText } from "./write"; +const FORMULA_FRAME = { xPt: 0, yPt: 0, widthPt: 0, heightPt: 22 }; + +function formulaBlock(latex: string, source: string): ContentBlock { + return buildFormulaBlock( + latexToFormula(latex, { source }).formula, + FORMULA_FRAME, + "test:formula", + ); +} + const CONSTRUCT_START: ContentBlock = { kind: "constructStart", descriptor: { kind: "anchor", anchorType: "bookmark", name: "b1" }, @@ -60,6 +72,18 @@ describe("buildMarkdownText", () => { expect(buildMarkdownText(document)).not.toContain("<!-- page break -->"); }); + it("renders a formula whose provenance source is markdown:math-inline as an inline \\( \\) span", () => { + const document = markerDocument([ + formulaBlock("x+1", "markdown:math-inline"), + ]); + expect(buildMarkdownText(document)).toBe("\\(x+1\\)"); + }); + + it("renders a formula from any other provenance source as a $$ display block, not the inline span", () => { + const document = markerDocument([formulaBlock("x+1", "docx:equation")]); + expect(buildMarkdownText(document)).toBe("$$\nx+1\n$$"); + }); + it("throws MarkdownUnsupportedDocumentKindError for a non-wordprocessing ContentDocument", () => { const presentation: ContentDocument = { kind: "presentation", @@ -122,4 +146,47 @@ describe("buildMarkdownText", () => { ]); expect(buildMarkdownText(document)).toContain("cell"); }); + + it("recurses the pageBreak-to-marker transform into a table cell's own blocks", () => { + const document = markerDocument([ + { + kind: "table", + rows: [ + { + cells: [{ blocks: [{ kind: "pageBreak" }] }], + }, + ], + columnWidthsPt: [80], + }, + ]); + // If the table branch did not recurse markdownBlock into the cell, this cell's own pageBreak block would reach the writer unconverted -- a table cell backslash-escapes the marker's own punctuation (unlike the top-level HTMLPreformatted paragraph the same marker gets outside a table), but "page break" surviving into the cell text either way is still proof the marker text -- not the untransformed pageBreak block -- is what reached the writer. + expect(buildMarkdownText(document)).toContain("page break"); + }); + + it("flattens an embedded formula with no presentation LaTeX to the literal [formula] placeholder", () => { + const document = markerDocument([ + { + kind: "embeddedObject", + objectKind: "formula", + document: { + kind: "formula", + metadata: {}, + // No `presentation` field and no `starMath` field, so formulaPlaceholderText falls all the way through to its own literal "[formula]" fallback. + formula: { + mathml: [ + { + type: "element", + tag: "mi", + attributes: [], + children: [{ type: "text", value: "x" }], + }, + ], + }, + }, + frame: { xPt: 0, yPt: 0, widthPt: 40, heightPt: 24 }, + }, + ]); + // The literal "[" and "]" are backslash-escaped by the plain-paragraph run writer, but the word "formula" itself carries no markdown-special characters and survives unescaped -- proof formulaPlaceholderText's own fallback text (and not an empty run list) reached the writer. + expect(buildMarkdownText(document)).toContain("formula"); + }); }); diff --git a/packages/documents.js/src/mathml/compose.test.ts b/packages/documents.js/src/mathml/compose.test.ts new file mode 100644 index 000000000..241939f04 --- /dev/null +++ b/packages/documents.js/src/mathml/compose.test.ts @@ -0,0 +1,81 @@ +import type { + MathAssembledGlyphs, + MathGlyphRun, + MathLayoutItem, + MathStroke, +} from "document-schema.js"; +import { describe, expect, it } from "vitest"; +import { shiftItems } from "./compose"; + +const BLACK = { r: 0, g: 0, b: 0 }; + +describe("shiftItems", () => { + it("returns an equal result, in a new array, when dxPt and dyPt are both zero", () => { + const glyphRun: MathGlyphRun = { + kind: "glyphs", + xPt: 1, + yPt: 2, + text: "x", + sizePt: 12, + color: BLACK, + }; + const items: MathLayoutItem[] = [glyphRun]; + const shifted = shiftItems(items, 0, 0); + expect(shifted).toEqual(items); + expect(shifted).not.toBe(items); + }); + + it("shifts a flat glyph-run item by adding dxPt/dyPt, not subtracting", () => { + const glyphRun: MathGlyphRun = { + kind: "glyphs", + xPt: 10, + yPt: 20, + text: "x", + sizePt: 12, + color: BLACK, + }; + const [shifted] = shiftItems([glyphRun], 3, 5); + expect(shifted).toMatchObject({ xPt: 13, yPt: 25 }); + }); + + it("shifts every point of a stroke item by adding dxPt/dyPt, not subtracting", () => { + const stroke: MathStroke = { + kind: "stroke", + points: [ + { xPt: 1, yPt: 2 }, + { xPt: 3, yPt: 4 }, + ], + widthPt: 1, + color: BLACK, + }; + const [shifted] = shiftItems([stroke], 10, 100); + if (shifted?.kind !== "stroke") { + throw new Error("expected a stroke item"); + } + expect(shifted.points).toEqual([ + { xPt: 11, yPt: 102 }, + { xPt: 13, yPt: 104 }, + ]); + }); + + it("shifts every placement of an assembled-glyphs item by adding dxPt/dyPt, not subtracting", () => { + const assembled: MathAssembledGlyphs = { + kind: "assembled-glyphs", + placements: [ + { glyphId: 1, xPt: 1, yPt: 2 }, + { glyphId: 2, xPt: 3, yPt: 4 }, + ], + text: "√", + sizePt: 12, + color: BLACK, + }; + const [shifted] = shiftItems([assembled], 10, 100); + if (shifted?.kind !== "assembled-glyphs") { + throw new Error("expected an assembled-glyphs item"); + } + expect(shifted.placements).toEqual([ + { glyphId: 1, xPt: 11, yPt: 102 }, + { glyphId: 2, xPt: 13, yPt: 104 }, + ]); + }); +}); diff --git a/packages/documents.js/src/mathml/compose.ts b/packages/documents.js/src/mathml/compose.ts index 7d068eff7..189fca061 100644 --- a/packages/documents.js/src/mathml/compose.ts +++ b/packages/documents.js/src/mathml/compose.ts @@ -8,15 +8,12 @@ export const EMPTY_BOX: MathBox = { items: [], }; -// Translates every item in `items` by (dxPt, dyPt) -- the one place this module touches an individual MathLayoutItem's own coordinate fields, since MathStroke's points and MathAssembledGlyphs' placements are each a nested array unlike MathGlyphRun/MathRule's flat xPt/yPt. +// Translates every item in `items` by (dxPt, dyPt) -- the one place this module touches an individual MathLayoutItem's own coordinate fields, since MathStroke's points and MathAssembledGlyphs' placements are each a nested array unlike MathGlyphRun/MathRule's flat xPt/yPt. No dxPt===0&&dyPt===0 fast path: adding zero to any coordinate is a no-op, so the general map below already produces an equal (if not reference-identical) result for a zero shift, on every item kind, with nothing for a special case to shortcut. export function shiftItems( items: readonly MathLayoutItem[], dxPt: number, dyPt: number, ): MathLayoutItem[] { - if (dxPt === 0 && dyPt === 0) { - return [...items]; - } return items.map((item) => { if (item.kind === "stroke") { return { diff --git a/packages/documents.js/src/mathml/nodes.test.ts b/packages/documents.js/src/mathml/nodes.test.ts new file mode 100644 index 000000000..98d84c2e5 --- /dev/null +++ b/packages/documents.js/src/mathml/nodes.test.ts @@ -0,0 +1,115 @@ +import { describe, expect, it } from "vitest"; +import type { MathMlElement, MathMlNode } from "./nodes"; +import { + attrValue, + elementChildren, + elementLocalName, + firstChildByLocalName, + isMathMlElement, + localName, + textContent, +} from "./nodes"; + +function element( + tag: string, + attributes: readonly { name: string; value: string }[] = [], + children: readonly MathMlNode[] = [], +): MathMlElement { + return { type: "element", tag, attributes, children }; +} + +function text(value: string): MathMlNode { + return { type: "text", value }; +} + +describe("localName / elementLocalName", () => { + it("strips a single leading namespace prefix", () => { + expect(localName("math:mfrac")).toBe("mfrac"); + expect(elementLocalName(element("math:mfrac"))).toBe("mfrac"); + }); + + it("leaves an unprefixed tag unchanged", () => { + expect(localName("mfrac")).toBe("mfrac"); + }); +}); + +describe("attrValue", () => { + it("finds the value of the attribute matching the requested name, not just the first one present", () => { + const el = element("mo", [ + { name: "stretchy", value: "false" }, + { name: "fence", value: "true" }, + ]); + expect(attrValue(el, "fence")).toBe("true"); + expect(attrValue(el, "stretchy")).toBe("false"); + }); + + it("returns undefined when no attribute matches", () => { + expect( + attrValue(element("mo", [{ name: "fence", value: "true" }]), "missing"), + ).toBeUndefined(); + }); +}); + +describe("elementChildren", () => { + it("keeps only element children, skipping text siblings", () => { + const child = element("mi"); + const node = element("mrow", [], [text("x"), child, text("y")]); + expect(elementChildren(node)).toEqual([child]); + }); +}); + +describe("firstChildByLocalName", () => { + it("finds the first element child whose local name matches, ignoring namespace prefixes", () => { + const numerator = element("math:mn"); + const denominator = element("math:mn"); + const node = element("mfrac", [], [numerator, denominator]); + expect(firstChildByLocalName(node, "mn")).toBe(numerator); + }); + + it("returns undefined when no element child has that local name", () => { + const node = element("mfrac", [], [element("mn")]); + expect(firstChildByLocalName(node, "mrow")).toBeUndefined(); + }); + + it("skips a non-matching child rather than returning it regardless of name", () => { + const wrong = element("mo"); + const right = element("mi"); + const node = element("mrow", [], [wrong, right]); + expect(firstChildByLocalName(node, "mi")).toBe(right); + }); +}); + +describe("isMathMlElement", () => { + it("distinguishes an element node from a text node", () => { + expect(isMathMlElement(element("mi"))).toBe(true); + expect(isMathMlElement(text("x"))).toBe(false); + }); +}); + +describe("textContent", () => { + it("returns a text node's own value", () => { + expect(textContent(text("x"))).toBe("x"); + }); + + it("concatenates every descendant text node depth-first, in document order", () => { + const node = element( + "mrow", + [], + [ + element("mi", [], [text("a")]), + text("b"), + element("mo", [], [text("c")]), + ], + ); + expect(textContent(node)).toBe("abc"); + }); + + it("returns an empty string for a node that is neither text nor element", () => { + const comment: MathMlNode = { type: "comment" }; + expect(textContent(comment)).toBe(""); + }); + + it("returns an empty string for an element with no children, not undefined", () => { + expect(textContent(element("mspace"))).toBe(""); + }); +}); diff --git a/packages/documents.js/src/mathml/nodes.ts b/packages/documents.js/src/mathml/nodes.ts index b760d4983..ac9769c0e 100644 --- a/packages/documents.js/src/mathml/nodes.ts +++ b/packages/documents.js/src/mathml/nodes.ts @@ -31,9 +31,9 @@ function isMathMlText(node: MathMlNode): node is MathMlText { } // Real MathML producers (confirmed against LibreOffice's own content.xml output) write element tags with a "math:" namespace prefix when math is not the document's default namespace (<math:mfrac>, <math:mrow>, ...), and bare, unprefixed tags when it is (<mfrac>, <mrow>, ...) -- odf.js's own readOdfFormulaMathMl already handles exactly this ambiguity for the root element (MATH_ROOT_TAGS = ['math', 'math:math']). This module applies the same tolerance uniformly to every element, not just the root: strip a single leading "prefix:" segment before comparing against a canonical MathML tag name, so this layout engine works unmodified regardless of which form a given producer chose. +// Deliberately branchless: slicing from `indexOf(":") + 1` already returns the whole string when there is no colon (indexOf yields -1, so the slice starts at 0), so a colonIndex === -1 guard would be redundant -- every input this function accepts is already correctly handled by the single slice below. export function localName(tag: string): string { - const colonIndex = tag.indexOf(":"); - return colonIndex === -1 ? tag : tag.slice(colonIndex + 1); + return tag.slice(tag.indexOf(":") + 1); } export function elementLocalName(element: MathMlElement): string { diff --git a/packages/documents.js/src/metadata/core-patch.test.ts b/packages/documents.js/src/metadata/core-patch.test.ts new file mode 100644 index 000000000..b513fe422 --- /dev/null +++ b/packages/documents.js/src/metadata/core-patch.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from "vitest"; +import { hasWritableMetadataOverride, mergeMetadata } from "./core-patch"; + +describe("hasWritableMetadataOverride", () => { + it("is false for an empty overrides object", () => { + expect(hasWritableMetadataOverride({})).toBe(false); + }); + + it("is true when only title is supplied", () => { + expect(hasWritableMetadataOverride({ title: "T" })).toBe(true); + }); + + it("is true when only author is supplied", () => { + expect(hasWritableMetadataOverride({ author: "A" })).toBe(true); + }); + + it("is true when only subject is supplied", () => { + expect(hasWritableMetadataOverride({ subject: "S" })).toBe(true); + }); + + it("is true when keywords is a non-empty array", () => { + expect(hasWritableMetadataOverride({ keywords: ["a"] })).toBe(true); + }); + + it("is false when keywords is present but empty, since nothing would actually be written", () => { + expect(hasWritableMetadataOverride({ keywords: [] })).toBe(false); + }); +}); + +describe("mergeMetadata", () => { + it("keeps fields the overrides object did not mention", () => { + expect( + mergeMetadata({ title: "Original", author: "Ada" }, { title: "New" }), + ).toEqual({ title: "New", author: "Ada" }); + }); + + it("keeps the current subject when overrides does not mention it", () => { + expect( + mergeMetadata({ subject: "Original subject" }, { title: "New" }), + ).toEqual({ subject: "Original subject", title: "New" }); + }); + + it("keeps the current keywords when overrides does not mention them", () => { + expect(mergeMetadata({ keywords: ["a", "b"] }, { title: "New" })).toEqual({ + keywords: ["a", "b"], + title: "New", + }); + }); +}); diff --git a/packages/documents.js/src/metadata/core-patch.ts b/packages/documents.js/src/metadata/core-patch.ts index b839ab26d..cfaff60a2 100644 --- a/packages/documents.js/src/metadata/core-patch.ts +++ b/packages/documents.js/src/metadata/core-patch.ts @@ -32,7 +32,9 @@ export function mergeMetadata( } // Whether `overrides` would actually cause the addCoreProperties/writeOdfMetadata fallback below to write at least one element -- NOT merely whether a field is present in `overrides` at all. An empty keywords array is the gap this distinction closes: overrides.keywords !== undefined is true for `keywords: []`, but addCoreProperties/writeOdfMetadata themselves only ever emit a keywords element when the array's length is nonzero (mirroring how a from-scratch build never writes an empty keywords element), so treating "the key is present" as "something will be written" would create a real metadata part (plus, for OOXML, its Content_Types override and package-root relationship) out of an empty root element, on a document that had none -- contradicting patchOoxmlCorePropertiesOnPackage/patchOdfMetadataOnPackage's own contract that a document with no requested change stays byte-for-byte free of a part it never had. This predicate mirrors addCoreProperties'/buildOdfMetaNodes' own per-field write conditions exactly: title/author/subject count on mere presence, keywords counts only with at least one entry. -function hasWritableMetadataOverride(overrides: MetadataOverrides): boolean { +export function hasWritableMetadataOverride( + overrides: MetadataOverrides, +): boolean { return ( overrides.title !== undefined || overrides.author !== undefined || diff --git a/packages/documents.js/src/model/embedded-drawing.test.ts b/packages/documents.js/src/model/embedded-drawing.test.ts new file mode 100644 index 000000000..f2c293a62 --- /dev/null +++ b/packages/documents.js/src/model/embedded-drawing.test.ts @@ -0,0 +1,100 @@ +import type { + ContentEmbeddedObjectBlock, + ContentVector, +} from "document-schema.js"; +import { describe, expect, it } from "vitest"; +import { + buildDrawingBlock, + drawingOfBlock, + embeddedDrawingVectors, + FLOW_CONTAINER_ORIGIN, +} from "./embedded-drawing"; + +describe("buildDrawingBlock / drawingOfBlock", () => { + it("wraps the given vectors in a one-page drawing document sized to the given page, and drawingOfBlock recovers it", () => { + const rect: ContentVector = { + kind: "rect", + frame: { xPt: 5, yPt: 10, widthPt: 20, heightPt: 30 }, + fill: { r: 1, g: 0, b: 0 }, + }; + const block = buildDrawingBlock({ widthPt: 100, heightPt: 200 }, [rect]); + expect(block.frame).toEqual({ + xPt: 0, + yPt: 0, + widthPt: 100, + heightPt: 200, + }); + const drawing = drawingOfBlock(block); + expect(drawing?.pages).toHaveLength(1); + expect(drawing?.pages[0]?.vectors).toEqual([rect]); + }); + + it("drawingOfBlock returns undefined for a non-drawing embeddedObject block", () => { + const nonDrawing: ContentEmbeddedObjectBlock = { + kind: "embeddedObject", + objectKind: "formula", + document: { kind: "formula", metadata: {}, formula: { mathml: [] } }, + frame: { xPt: 0, yPt: 0, widthPt: 10, heightPt: 10 }, + }; + expect(drawingOfBlock(nonDrawing)).toBeUndefined(); + }); +}); + +describe("embeddedDrawingVectors", () => { + it("translates a line's endpoints by adding dxPt/dyPt to each coordinate, not subtracting", () => { + const line: ContentVector = { + kind: "line", + from: { xPt: 1, yPt: 2 }, + to: { xPt: 3, yPt: 7 }, + stroke: { color: { r: 0, g: 0, b: 0 }, widthPt: 1 }, + }; + const block = buildDrawingBlock({ widthPt: 100, heightPt: 100 }, [line]); + // A non-zero, asymmetric (block frame != container origin) offset, so a sign flip on either axis produces a different result than the correct one. + block.frame.xPt = 10; + block.frame.yPt = 20; + const [translated] = embeddedDrawingVectors(block, { xPt: 1, yPt: 2 }); + expect(translated?.kind).toBe("line"); + if (translated?.kind !== "line") { + throw new Error("expected a line vector"); + } + // dxPt = 10 + 1 = 11, dyPt = 20 + 2 = 22. + expect(translated.from).toEqual({ xPt: 12, yPt: 24 }); + expect(translated.to).toEqual({ xPt: 14, yPt: 29 }); + }); + + it("translates rect, ellipse, and path vectors identically by shifting only their own frame", () => { + const rect: ContentVector = { + kind: "rect", + frame: { xPt: 0, yPt: 0, widthPt: 5, heightPt: 5 }, + fill: { r: 1, g: 0, b: 0 }, + }; + const ellipse: ContentVector = { + kind: "ellipse", + frame: { xPt: 0, yPt: 0, widthPt: 5, heightPt: 5 }, + fill: { r: 0, g: 1, b: 0 }, + }; + const block = buildDrawingBlock({ widthPt: 100, heightPt: 100 }, [ + rect, + ellipse, + ]); + const translated = embeddedDrawingVectors(block, FLOW_CONTAINER_ORIGIN); + expect( + translated[0]?.kind === "rect" ? translated[0].frame : undefined, + ).toEqual({ xPt: 0, yPt: 0, widthPt: 5, heightPt: 5 }); + expect( + translated[1]?.kind === "ellipse" ? translated[1].frame : undefined, + ).toEqual({ xPt: 0, yPt: 0, widthPt: 5, heightPt: 5 }); + }); + + it("returns an empty array for a non-drawing embeddedObject block", () => { + const nonDrawing: ContentEmbeddedObjectBlock = { + kind: "embeddedObject", + objectKind: "formula", + document: { kind: "formula", metadata: {}, formula: { mathml: [] } }, + frame: { xPt: 0, yPt: 0, widthPt: 10, heightPt: 10 }, + }; + expect(embeddedDrawingVectors(nonDrawing, FLOW_CONTAINER_ORIGIN)).toEqual( + [], + ); + }); +}); diff --git a/packages/documents.js/src/model/embedded-drawing.ts b/packages/documents.js/src/model/embedded-drawing.ts index 501cabe17..970ca37f3 100644 --- a/packages/documents.js/src/model/embedded-drawing.ts +++ b/packages/documents.js/src/model/embedded-drawing.ts @@ -63,10 +63,9 @@ function translateVector( from: shiftPoint(vector.from, dxPt, dyPt), to: shiftPoint(vector.to, dxPt, dyPt), }; + // rect/ellipse/path all translate by shifting the frame alone and nothing else -- one shared body under three case labels, not three copies of the identical statement (which would leave rect's and ellipse's own bodies byte-identical and swappable with each other for no observable difference, an equivalent-mutant trap the earlier three-copy form fell into). case "rect": - return { ...vector, frame: shiftBox(vector.frame, dxPt, dyPt) }; case "ellipse": - return { ...vector, frame: shiftBox(vector.frame, dxPt, dyPt) }; case "path": return { ...vector, frame: shiftBox(vector.frame, dxPt, dyPt) }; } diff --git a/packages/documents.js/src/model/formula.test.ts b/packages/documents.js/src/model/formula.test.ts index 9adebdfd2..7406f8a78 100644 --- a/packages/documents.js/src/model/formula.test.ts +++ b/packages/documents.js/src/model/formula.test.ts @@ -215,6 +215,7 @@ describe("collectDocumentFormulas", () => { const entries = collectDocumentFormulas(document); expect(entries).toHaveLength(1); expect(entries[0]?.formula.presentation?.latex).toBe("m \\times a"); + expect(entries[0]?.locate).toBe("slides[0].shapes[0]/blocks[0]"); }); it("walks a drawing page's shapes", () => { diff --git a/packages/documents.js/src/odb/formula/definition.test.ts b/packages/documents.js/src/odb/formula/definition.test.ts index df558735c..34561f96e 100644 --- a/packages/documents.js/src/odb/formula/definition.test.ts +++ b/packages/documents.js/src/odb/formula/definition.test.ts @@ -108,6 +108,19 @@ describe("rptDefinitionFromReport refusals", () => { ).toThrow(/declares no rpt:group-expression/); }); + it("refuses a single-length group level whose one slot holds no group", () => { + // Not a shape odf.js's own reader can ever produce (its groups array is always populated element-for-element) -- this exercises the defensive guard directly, since a length-1 array with a hole is otherwise unreachable through any real .odb fixture. + const holed = emptyReport({ + groups: [undefined] as unknown as OdbReportGroup[], + }); + expect(() => rptDefinitionFromReport(holed)).toThrow( + RptReportStructureError, + ); + expect(() => rptDefinitionFromReport(holed)).toThrow( + /a group nesting level reported a non-zero length but held no group/, + ); + }); + it("refuses sibling groups at one nesting level rather than keeping the first and dropping the rest", () => { const siblings = emptyReport({ groups: [ diff --git a/packages/documents.js/src/odb/spreadsheet.test.ts b/packages/documents.js/src/odb/spreadsheet.test.ts index ea761170b..cb472e3fc 100644 --- a/packages/documents.js/src/odb/spreadsheet.test.ts +++ b/packages/documents.js/src/odb/spreadsheet.test.ts @@ -88,6 +88,15 @@ describe("odbTablesToSpreadsheetDocument", () => { ]); }); + it("sizes one row entry per data row plus the header row", () => { + const content = odbTablesToSpreadsheetDocument([TABLE]); + if (content.kind !== "spreadsheet") { + throw new Error("expected a spreadsheet ContentDocument"); + } + // TABLE has 2 data rows, so the sizing array must cover row 0 (header) through row 2. + expect(content.sheets[0]?.rows.map((row) => row.index)).toEqual([0, 1, 2]); + }); + it("produces a real, non-empty printSettings for every sheet, so the xlsx builder has something to write", () => { const content = odbTablesToSpreadsheetDocument([TABLE]); if (content.kind !== "spreadsheet") { diff --git a/packages/documents.js/src/odb/values.test.ts b/packages/documents.js/src/odb/values.test.ts index df08dc1fc..5671816ce 100644 --- a/packages/documents.js/src/odb/values.test.ts +++ b/packages/documents.js/src/odb/values.test.ts @@ -199,4 +199,16 @@ describe("aggregateCellValues", () => { "SUM requires numeric values, but found a string value", ); }); + + it("keeps the first-seen value on a tie, for both MIN and MAX", () => { + // Two structurally different values (a plain number and a currency) that compare numerically equal -- distinguishable by .kind alone, so which one "won" the tie is directly observable. + const first: ContentCellValue = { kind: "number", value: 5 }; + const second: ContentCellValue = { + kind: "currency", + value: 5, + currency: "GBP", + }; + expect(aggregateCellValues("MIN", [first, second], fail)).toBe(first); + expect(aggregateCellValues("MAX", [first, second], fail)).toBe(first); + }); }); diff --git a/packages/documents.js/src/odb/values.ts b/packages/documents.js/src/odb/values.ts index 9dc0e1bf8..a54137ec3 100644 --- a/packages/documents.js/src/odb/values.ts +++ b/packages/documents.js/src/odb/values.ts @@ -51,18 +51,19 @@ export function compareCellKeys( right: CellComparisonKey, fail: CellValueFailure, ): number { + // Ordered as less-than-first rather than equality-first: with equality checked first, the surrounding guard already rules out left === right by the time a `<` (or `<=`) comparison runs, making the two relational spellings produce identical output for every reachable input -- an unkillable, permanently-equivalent mutant. Checking `<` first means a `<`-to-`<=` mutation is reachable at the equal-values input (it would wrongly report -1 instead of 0), so this ordering carries no equivalent-mutant gap. if (left.valueClass === "numeric" && right.valueClass === "numeric") { - return left.numeric === right.numeric - ? 0 - : left.numeric < right.numeric - ? -1 - : 1; + return left.numeric < right.numeric + ? -1 + : left.numeric > right.numeric + ? 1 + : 0; } if (left.valueClass === "boolean" && right.valueClass === "boolean") { return left.boolean === right.boolean ? 0 : left.boolean ? 1 : -1; } if (left.valueClass === "text" && right.valueClass === "text") { - return left.text === right.text ? 0 : left.text < right.text ? -1 : 1; + return left.text < right.text ? -1 : left.text > right.text ? 1 : 0; } throw fail( `cannot compare a ${left.valueClass} value with a ${right.valueClass} value`, diff --git a/packages/documents.js/src/odf-package/formula.test.ts b/packages/documents.js/src/odf-package/formula.test.ts new file mode 100644 index 000000000..3614209a1 --- /dev/null +++ b/packages/documents.js/src/odf-package/formula.test.ts @@ -0,0 +1,37 @@ +import type { Package } from "odf.js"; +import { describe, expect, it } from "vitest"; +import { nextObjectIndex } from "./formula"; + +function packageWithParts(paths: readonly string[]): Package { + const parts: Package["parts"] = {}; + for (const path of paths) { + parts[path] = { kind: "xml", nodes: [] }; + } + return { parts }; +} + +describe("nextObjectIndex", () => { + it("is 1 for a package with no existing Object directories at all", () => { + expect(nextObjectIndex(packageWithParts(["content.xml"]))).toBe(1); + }); + + it("is one past the highest index present, regardless of encounter order", () => { + expect( + nextObjectIndex( + packageWithParts([ + "Object 3/content.xml", + "Object 1/content.xml", + "Object 2/content.xml", + ]), + ), + ).toBe(4); + }); + + it("resumes from a gap left by a removed object, rather than reusing the lowest free index", () => { + expect( + nextObjectIndex( + packageWithParts(["Object 1/content.xml", "Object 5/content.xml"]), + ), + ).toBe(6); + }); +}); diff --git a/packages/documents.js/src/odf-package/formula.ts b/packages/documents.js/src/odf-package/formula.ts index 0ee7545a2..ba1a53a74 100644 --- a/packages/documents.js/src/odf-package/formula.ts +++ b/packages/documents.js/src/odf-package/formula.ts @@ -28,7 +28,7 @@ export interface AddedOdfFormula { } // One past the highest "Object N" directory already present, so a second formula in the same document never collides with the first -- mirroring src/odf-package/media.ts's own nextPictureIndex exactly, including its tolerance of a gap left by an earlier object that is no longer there. -function nextObjectIndex(pkg: Package): number { +export function nextObjectIndex(pkg: Package): number { const pattern = /^Object (\d+)\//; let max = 0; for (const path of Object.keys(pkg.parts)) { @@ -37,10 +37,8 @@ function nextObjectIndex(pkg: Package): number { if (digits === undefined) { continue; } - const index = Number.parseInt(digits, 10); - if (index > max) { - max = index; - } + // Math.max rather than an if-comparison: every "Object N" directory in a real package is distinct, so no two paths this loop sees ever carry the same index -- an if-guarded assignment and a running max are equally correct here, but only the latter has no tie-boundary comparison left for a mutation to flip unobservably. + max = Math.max(max, Number.parseInt(digits, 10)); } return max + 1; } diff --git a/packages/documents.js/src/odf-package/manifest.test.ts b/packages/documents.js/src/odf-package/manifest.test.ts new file mode 100644 index 000000000..24657ba4c --- /dev/null +++ b/packages/documents.js/src/odf-package/manifest.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it, vi } from "vitest"; +import * as odfJs from "odf.js"; +import { ODF_MEDIA_TYPES } from "odf.js"; +import { createOdt } from "../edit/odt/editor"; +import { syncOdfManifest } from "./manifest"; + +// syncOdfManifest walks every package part path, deriving a mediaTypeOverrides entry for each genuine embedded sub-document directory ("<dir>/content.xml") and handing the whole map to odf.js's own syncManifest. Spying on that call is what makes the guard against the package's own ROOT content.xml (which also happens to have a real office:body -- there is nothing about its shape alone that would exclude it) directly observable: odf.js's real syncManifest silently tolerates a bogus override key, so nothing downstream of it would otherwise notice one leaking in. +describe("syncOdfManifest", () => { + it("never derives a mediaTypeOverrides entry for the package's own root content.xml", () => { + const spy = vi.spyOn(odfJs, "syncManifest"); + const pkg = createOdt().toPackage(); + syncOdfManifest(pkg); + const options = spy.mock.calls.at(-1)?.[1]; + expect(options?.mediaTypeOverrides).toEqual({}); + spy.mockRestore(); + }); + + it("derives the correct media type override for a real embedded sub-document directory", () => { + const pkg = createOdt().toPackage(); + pkg.parts["Object 1/content.xml"] = { + kind: "xml", + nodes: [ + { + type: "element", + tag: "office:document-content", + attributes: [], + children: [ + { + type: "element", + tag: "office:body", + attributes: [], + children: [ + { + type: "element", + tag: "office:spreadsheet", + attributes: [], + children: [], + }, + ], + }, + ], + }, + ], + }; + const spy = vi.spyOn(odfJs, "syncManifest"); + syncOdfManifest(pkg); + const options = spy.mock.calls.at(-1)?.[1]; + expect(options?.mediaTypeOverrides).toEqual({ + "Object 1/": ODF_MEDIA_TYPES.ods, + }); + spy.mockRestore(); + }); +}); diff --git a/packages/documents.js/src/odf-package/manifest.ts b/packages/documents.js/src/odf-package/manifest.ts index 0ba78803c..7e35b3e59 100644 --- a/packages/documents.js/src/odf-package/manifest.ts +++ b/packages/documents.js/src/odf-package/manifest.ts @@ -47,7 +47,9 @@ function subDocumentMediaType( export function syncOdfManifest(pkg: Package): void { const mediaTypeOverrides: Record<string, string> = {}; for (const path of Object.keys(pkg.parts)) { - if (!path.endsWith(CONTENT_PART_SUFFIX) || path === ROOT_CONTENT_PART) { + // No `|| path === ROOT_CONTENT_PART` check alongside this: the bare root content.xml can + // never itself end with "/content.xml" (it has no directory prefix to carry the slash), so that comparison could never be true for any path this `endsWith` check has already let through -- it restated the same exclusion a second, unreachable way. + if (!path.endsWith(CONTENT_PART_SUFFIX)) { continue; } const directory = path.slice(0, path.length - ROOT_CONTENT_PART.length); diff --git a/packages/documents.js/src/odf-package/media.test.ts b/packages/documents.js/src/odf-package/media.test.ts index 29e251e3d..01630e03e 100644 --- a/packages/documents.js/src/odf-package/media.test.ts +++ b/packages/documents.js/src/odf-package/media.test.ts @@ -7,7 +7,7 @@ import { setDocumentMediaType, } from "odf.js"; import { describe, expect, it } from "vitest"; -import { addImageMedia } from "./media"; +import { addImageMedia, nextPictureIndex } from "./media"; const ODT_MEDIA_TYPE = "application/vnd.oasis.opendocument.text"; const PNG_BYTES: Uint8Array<ArrayBuffer> = new Uint8Array([ @@ -95,3 +95,37 @@ describe("addImageMedia", () => { ); }); }); + +describe("nextPictureIndex", () => { + it("ignores a same-named file outside Pictures/", () => { + const pkg: Package = { + parts: { "Other/image9.png": { kind: "binary", base64: "" } }, + }; + expect(nextPictureIndex(pkg, "png")).toBe(1); + }); + + it("continues from a pre-existing higher index rather than starting from 1", () => { + const pkg: Package = { + parts: { "Pictures/image5.png": { kind: "binary", base64: "" } }, + }; + expect(nextPictureIndex(pkg, "png")).toBe(6); + }); + + it("does not let an extension containing a regex-special character match unrelated files", () => { + const pkg: Package = { + parts: { "Pictures/image1.pXg": { kind: "binary", base64: "" } }, + }; + expect(nextPictureIndex(pkg, "p.g")).toBe(1); + }); + + // "Pictures0image5.png" is 9 characters ("Pictures0") ahead of a slice that -- once the leading "Pictures/" (also 9 characters) is stripped off a real Pictures/ path -- looks exactly like "image5.png". A path-prefix check that only LOOKED at whether the loop should skip a part, without actually gating the pattern match against it, would still slice this non-Pictures path at the same fixed offset and misread it as Pictures/image5.png -- this path is deliberately crafted so that coincidence is exercised, unlike a plain "Other/imageN.ext" path (whose own 9-character-in slice does not happen to spell a valid image filename). + it("ignores a same-named file outside Pictures/ even when slicing its path at the Pictures/ prefix length would coincidentally spell a valid image filename", () => { + const pkg: Package = { + parts: { + "Pictures/image1.png": { kind: "binary", base64: "" }, + "Pictures0image5.png": { kind: "binary", base64: "" }, + }, + }; + expect(nextPictureIndex(pkg, "png")).toBe(2); + }); +}); diff --git a/packages/documents.js/src/odf-package/media.ts b/packages/documents.js/src/odf-package/media.ts index f13bb8643..57ec66755 100644 --- a/packages/documents.js/src/odf-package/media.ts +++ b/packages/documents.js/src/odf-package/media.ts @@ -14,7 +14,7 @@ function escapeRegExp(value: string): string { } // Mirrors src/opc/media.ts's own nextMediaIndex -- scans existing Pictures/ part paths for the given extension and returns one past the highest index found, so successive images never collide even if an earlier one was later removed. -function nextPictureIndex(pkg: Package, extension: string): number { +export function nextPictureIndex(pkg: Package, extension: string): number { const pattern = new RegExp(`^image(\\d+)\\.${escapeRegExp(extension)}$`); const prefix = `${PICTURES_DIR}/`; let max = 0; @@ -31,9 +31,11 @@ function nextPictureIndex(pkg: Package, extension: string): number { continue; } const n = Number.parseInt(digits, 10); - if (n > max) { - max = n; - } + // Math.max, not an if/comparison: the two ever differ observably only on a tie, and every + // path here is keyed by its own literal numeric suffix, so no two iterations of this loop can + // ever see the same n twice -- a tie can only be n against its own already-recorded max, which + // assigns the identical value back, an if-based '>' vs '>=' comparison could never distinguish. + max = Math.max(max, n); } return max + 1; } diff --git a/packages/documents.js/src/odf/odp/read.ts b/packages/documents.js/src/odf/odp/read.ts index a0715aad9..992ddf49a 100644 --- a/packages/documents.js/src/odf/odp/read.ts +++ b/packages/documents.js/src/odf/odp/read.ts @@ -63,10 +63,8 @@ export function readOdpContent(pkg: Package): ContentDocument { }; } + // No early return for an empty `groups`: the rebuild loop below already reduces to a no-op copy of the slide's existing shapes when there is nothing to insert (the inner insertion while-loop never runs, so every shapeIndex iteration just re-pushes the shape already at that index) -- an early-return guard here would only skip allocating an equivalent array, never change what gets assigned, making the guard a permanently equivalent mutation target rather than a real correctness branch. const groups = collectSlideVectorGroups(pageElement.children, pkg); - if (groups.length === 0) { - return; - } const shapes: ContentShape[] = []; let shapeIndex = 0; let groupIndex = 0; diff --git a/packages/documents.js/src/odf/vector/detect.test.ts b/packages/documents.js/src/odf/vector/detect.test.ts new file mode 100644 index 000000000..1bd95cf75 --- /dev/null +++ b/packages/documents.js/src/odf/vector/detect.test.ts @@ -0,0 +1,35 @@ +import type { ContentVector } from "document-schema.js"; +import { describe, expect, it } from "vitest"; +import { groupVectorsByShapePosition } from "./detect"; + +function rect(paintOrder: number | undefined): ContentVector { + return { + kind: "rect", + frame: { xPt: 0, yPt: 0, widthPt: 10, heightPt: 10 }, + fill: { r: 1, g: 0, b: 0 }, + paintOrder, + }; +} + +describe("groupVectorsByShapePosition", () => { + it("throws when a vector carries no paintOrder at all, naming odf.js's own stamping contract", () => { + expect(() => { + groupVectorsByShapePosition([], [rect(undefined)]); + }).toThrow( + "expected odf.js's own readDrawPageContent to stamp every shape/vector with a paintOrder", + ); + }); + + it("a vector sharing a shape's own paintOrder exactly is NOT counted as coming before that shape", () => { + // odf.js's real, single shared counter can never actually produce this collision (see the module comment on the function under test), but the boundary itself -- strictly less than, not less-than-or-equal -- is still this function's own contract and worth pinning directly. + const groups = groupVectorsByShapePosition([5], [rect(5)]); + expect(groups).toHaveLength(1); + expect(groups[0]?.insertBeforeShapeIndex).toBe(0); + }); + + it("a vector strictly after a shape's paintOrder is grouped behind it", () => { + const groups = groupVectorsByShapePosition([5], [rect(6)]); + expect(groups).toHaveLength(1); + expect(groups[0]?.insertBeforeShapeIndex).toBe(1); + }); +}); diff --git a/packages/documents.js/src/odf/vector/detect.ts b/packages/documents.js/src/odf/vector/detect.ts index 1977659ef..d71c9538a 100644 --- a/packages/documents.js/src/odf/vector/detect.ts +++ b/packages/documents.js/src/odf/vector/detect.ts @@ -44,14 +44,11 @@ function paintOrderOf(item: { readonly paintOrder?: number }): number { return item.paintOrder; } -// Every vector primitive on one draw:page, grouped by which of odf.js's own readOdpContent-produced ContentShapes each sits immediately before -- so a caller inserting synthetic shapes for them lands each group at its true position among the slide's real shapes, in ONE forward pass, rather than always at the end. -export function collectSlideVectorGroups( - pageChildren: readonly XmlNode[], - pkg: Package, +// The grouping logic itself, split from collectSlideVectorGroups below so it takes plain paintOrder-bearing data rather than an XmlNode tree and a Package -- both to keep the actual algorithm testable against hand-built inputs (a real odf.js-decoded page can never hand this a colliding shape/vector paintOrder, since both arrays are stamped from the one shared counter the module comment above describes) and because it is the whole of what this module adds on top of odf.js's own readDrawPageContent; the XML-facing wrapper below is just that call plus this. +export function groupVectorsByShapePosition( + shapePaintOrders: readonly number[], + vectors: readonly ContentVector[], ): readonly DetectedSlideVectorGroup[] { - const { shapes, vectors } = readDrawPageContent(pageChildren, pkg); - const shapePaintOrders = shapes.map(paintOrderOf); - interface MutableGroup { insertBeforeShapeIndex: number; vectors: ContentVector[]; @@ -80,3 +77,12 @@ export function collectSlideVectorGroups( })), })); } + +// Every vector primitive on one draw:page, grouped by which of odf.js's own readOdpContent-produced ContentShapes each sits immediately before -- so a caller inserting synthetic shapes for them lands each group at its true position among the slide's real shapes, in ONE forward pass, rather than always at the end. +export function collectSlideVectorGroups( + pageChildren: readonly XmlNode[], + pkg: Package, +): readonly DetectedSlideVectorGroup[] { + const { shapes, vectors } = readDrawPageContent(pageChildren, pkg); + return groupVectorsByShapePosition(shapes.map(paintOrderOf), vectors); +} diff --git a/packages/documents.js/src/ooxml/legacy-embedded.test.ts b/packages/documents.js/src/ooxml/legacy-embedded.test.ts index 9bda49947..05ba2abf0 100644 --- a/packages/documents.js/src/ooxml/legacy-embedded.test.ts +++ b/packages/documents.js/src/ooxml/legacy-embedded.test.ts @@ -1,4 +1,5 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; +import * as docCodec from "doc-codec"; import { writeDocContent } from "doc-codec"; import { writeXlsContent } from "xls-codec"; import { writePptContent } from "ppt-codec"; @@ -94,4 +95,12 @@ describe("decodeLegacyEmbeddedObject", () => { decodeLegacyEmbeddedObject(new TextEncoder().encode("not a CFB file")), ).toBeUndefined(); }); + + it("never invokes a legacy reader at all for bytes that carry no compound-file signature", () => { + // Every legacy reader would itself reject non-CFB bytes too (its own first step is archive-codec's readCompoundFile), so the outcome alone can't distinguish the isCompoundFile guard existing from it being skipped -- this spies on readDocContent to prove the guard actually short-circuits before any reader is ever called, rather than merely happening to produce the same undefined result by falling through all three try/catch blocks. + const spy = vi.spyOn(docCodec, "readDocContent"); + decodeLegacyEmbeddedObject(new TextEncoder().encode("not a CFB file")); + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + }); }); diff --git a/packages/documents.js/src/ooxml/pptx/read.test.ts b/packages/documents.js/src/ooxml/pptx/read.test.ts index 92d222f3a..df106eb7a 100644 --- a/packages/documents.js/src/ooxml/pptx/read.test.ts +++ b/packages/documents.js/src/ooxml/pptx/read.test.ts @@ -4,7 +4,7 @@ import { minimalPptxPackage, pptxWithLegacyOleObjectPackage, } from "../../test-support/pptx"; -import { readPptxContent } from "./read"; +import { readPptxContent, slidePathsInOrder } from "./read"; // readPptxContent is now a thin adapter over ooxml.js's own readPptxContent (the flat reader; the bare readPptx name reads the tree-form DocumentTree since ooxml.js 4.0.0): placeholder -> layout -> master -> theme inheritance, the run-property cascade, and group-transform flattening all live upstream in ooxml.js now, with their own test coverage there. These tests exercise only the wrapping this file is actually responsible for -- ContentDocument's discriminant/formatVersion, the metadata/slides passthrough -- not the OOXML semantics readPptx itself resolves. @@ -70,3 +70,38 @@ describe("readPptxContent", () => { ).toBe("Legacy doc text"); }); }); + +describe("slidePathsInOrder", () => { + it("returns an empty array when the package has no ppt/presentation.xml part at all", () => { + const pkg = minimalPptxPackage(); + const rest = Object.fromEntries( + Object.entries(pkg.parts).filter( + ([path]) => path !== "ppt/presentation.xml", + ), + ); + expect(slidePathsInOrder({ ...pkg, parts: rest })).toEqual([]); + }); + + it("returns an empty array when presentation.xml carries no p:sldIdLst element", () => { + const pkg = minimalPptxPackage(); + expect( + slidePathsInOrder({ + ...pkg, + parts: { + ...pkg.parts, + "ppt/presentation.xml": { + kind: "xml", + nodes: [ + { + type: "element", + tag: "p:presentation", + attributes: [], + children: [], + }, + ], + }, + }, + }), + ).toEqual([]); + }); +}); diff --git a/packages/documents.js/src/ooxml/pptx/read.ts b/packages/documents.js/src/ooxml/pptx/read.ts index a6290a815..91962eb7d 100644 --- a/packages/documents.js/src/ooxml/pptx/read.ts +++ b/packages/documents.js/src/ooxml/pptx/read.ts @@ -19,8 +19,8 @@ export interface ReadPptxContentOptions { const PRESENTATION_PART = "ppt/presentation.xml"; -// Every slide's own part path, in p:sldIdLst document order -- the same order the upstream reader itself resolves slides in (see ooxml.js's own readSlidePathsInOrder), needed here only to locate each slide's raw p:sld root for the second, vector-detecting pass below. -function slidePathsInOrder(pkg: Package): readonly string[] { +// Every slide's own part path, in p:sldIdLst document order -- the same order the upstream reader itself resolves slides in (see ooxml.js's own readSlidePathsInOrder), needed here only to locate each slide's raw p:sld root for the second, vector-detecting pass below. Exported (not merely internal) so its own two malformed-package guards -- no ppt/presentation.xml part, or one with no p:sldIdLst -- are directly testable: readPptxContent's own upstream flat reader has no slides to map over at all in either of those same shapes, so nothing calling THIS function through readPptxContent can ever observe which of its two possible return values ("[]" vs "the mutant's own placeholder array") actually came back. +export function slidePathsInOrder(pkg: Package): readonly string[] { const presentationRoot = rootElement(pkg.parts[PRESENTATION_PART]); if (presentationRoot === undefined) { return []; diff --git a/packages/documents.js/src/opc/content-types.test.ts b/packages/documents.js/src/opc/content-types.test.ts index f181e71e8..69e90832e 100644 --- a/packages/documents.js/src/opc/content-types.test.ts +++ b/packages/documents.js/src/opc/content-types.test.ts @@ -42,7 +42,9 @@ describe("defaultContentTypeForExtension", () => { }); it("throws for an unknown extension rather than guessing", () => { - expect(() => defaultContentTypeForExtension("tiff")).toThrow(); + expect(() => defaultContentTypeForExtension("tiff")).toThrow( + "no known default content type for extension: tiff", + ); }); }); @@ -50,6 +52,12 @@ describe("ensureDefaultContentType", () => { it("creates [Content_Types].xml with a Default entry when none exists", () => { const pkg = emptyPackage(); ensureDefaultContentType(pkg, "png", "image/png"); + const part = pkg.parts["[Content_Types].xml"]; + const root = part?.kind === "xml" ? part.nodes[0] : undefined; + expect(root?.type === "element" ? root.tag : undefined).toBe("Types"); + expect(root?.type === "element" ? attr(root, "xmlns") : undefined).toBe( + "http://schemas.openxmlformats.org/package/2006/content-types", + ); const defaults = findChildElements(rootChildren(pkg), "Default"); const node = soleNode(defaults); expect(attr(node, "Extension")).toBe("png"); @@ -69,6 +77,18 @@ describe("ensureDefaultContentType", () => { ensureDefaultContentType(pkg, "jpeg", "image/jpeg"); expect(findChildElements(rootChildren(pkg), "Default")).toHaveLength(2); }); + + it("does not mistake an Override element carrying the same Extension attribute value for an existing Default", () => { + const pkg = emptyPackage(); + ensureContentTypeOverride(pkg, "png", "image/png"); + // Force an Extension attribute onto that Override entry, matching what ensureDefaultContentType would look for on a Default -- proving the presence check keys on the element's own tag, not merely on the attribute value. + const [override] = findChildElements(rootChildren(pkg), "Override"); + if (override !== undefined) { + override.node.attributes.push({ name: "Extension", value: "png" }); + } + ensureDefaultContentType(pkg, "png", "image/png"); + expect(findChildElements(rootChildren(pkg), "Default")).toHaveLength(1); + }); }); describe("ensureContentTypeOverride", () => { diff --git a/packages/documents.js/src/opc/core-properties.test.ts b/packages/documents.js/src/opc/core-properties.test.ts index 4e300f751..ddd2a1956 100644 --- a/packages/documents.js/src/opc/core-properties.test.ts +++ b/packages/documents.js/src/opc/core-properties.test.ts @@ -19,6 +19,23 @@ function emptyPackage(): Package { } describe("addCoreProperties", () => { + it("starts docProps/core.xml with the standard version/encoding/standalone declaration", () => { + const pkg = emptyPackage(); + addCoreProperties(pkg, {}); + const part = pkg.parts[CORE_PROPERTIES_PATH]; + if (part?.kind !== "xml") { + throw new Error("expected an xml part"); + } + expect(part.nodes[0]).toEqual({ + type: "declaration", + attributes: [ + { name: "version", value: "1.0" }, + { name: "encoding", value: "UTF-8" }, + { name: "standalone", value: "yes" }, + ], + }); + }); + it("writes every supplied field to its real OOXML core-properties element", () => { const pkg = emptyPackage(); const metadata: LayoutMetadata = { @@ -34,6 +51,18 @@ describe("addCoreProperties", () => { const root = rootElement(pkg.parts[CORE_PROPERTIES_PATH]); expect(root).toBeDefined(); expect(root?.tag).toBe("cp:coreProperties"); + expect(root === undefined ? undefined : attr(root, "xmlns:cp")).toBe( + "http://schemas.openxmlformats.org/package/2006/metadata/core-properties", + ); + expect(root === undefined ? undefined : attr(root, "xmlns:dc")).toBe( + "http://purl.org/dc/elements/1.1/", + ); + expect(root === undefined ? undefined : attr(root, "xmlns:dcterms")).toBe( + "http://purl.org/dc/terms/", + ); + expect(root === undefined ? undefined : attr(root, "xmlns:xsi")).toBe( + "http://www.w3.org/2001/XMLSchema-instance", + ); const title = root === undefined ? undefined : childrenWithTag(root, "dc:title")[0]; @@ -108,6 +137,15 @@ describe("addCoreProperties", () => { ).toHaveLength(0); }); + it("omits cp:keywords for an empty (but defined) keywords array, not just an undefined one", () => { + const pkg = emptyPackage(); + addCoreProperties(pkg, { title: "Has keywords field", keywords: [] }); + const root = rootElement(pkg.parts[CORE_PROPERTIES_PATH]); + expect( + root === undefined ? [] : childrenWithTag(root, "cp:keywords"), + ).toHaveLength(0); + }); + it("registers the [Content_Types].xml override and the package-root relationship", () => { const pkg = emptyPackage(); addCoreProperties(pkg, { title: "Doc" }); diff --git a/packages/documents.js/src/opc/media.test.ts b/packages/documents.js/src/opc/media.test.ts index 59256cac5..affd51025 100644 --- a/packages/documents.js/src/opc/media.test.ts +++ b/packages/documents.js/src/opc/media.test.ts @@ -7,7 +7,7 @@ import { } from "ooxml.js"; import { describe, expect, it } from "vitest"; import { findChildElements } from "../xml/query"; -import { addImageMedia } from "./media"; +import { addImageMedia, nextMediaIndex } from "./media"; function emptyPackage(): Package { return { parts: {} }; @@ -98,3 +98,54 @@ describe("addImageMedia", () => { ).toHaveLength(1); }); }); + +describe("nextMediaIndex", () => { + it("ignores a same-named file outside the given media directory", () => { + const pkg: Package = { + parts: { + "ppt/media/image9.png": { kind: "binary", base64: "" }, + }, + }; + expect(nextMediaIndex(pkg, "word/media", "image", "png")).toBe(1); + }); + + it("continues from a pre-existing higher index rather than starting from 1", () => { + const pkg: Package = { + parts: { + "word/media/image5.png": { kind: "binary", base64: "" }, + }, + }; + expect(nextMediaIndex(pkg, "word/media", "image", "png")).toBe(6); + }); + + it("does not let an extension containing a regex-special character match unrelated files", () => { + // "p.g" contains a literal dot -- if escapeRegExp's own replacement text were dropped (turning the escape into a no-op deletion instead), the built pattern's dot would match ANY character, wrongly matching "pXg" too. + const pkg: Package = { + parts: { + "word/media/image1.pXg": { kind: "binary", base64: "" }, + }, + }; + expect(nextMediaIndex(pkg, "word/media", "image", "p.g")).toBe(1); + }); + + it("still matches an extension containing a regex-special character against its own literal spelling", () => { + // "p+g" contains a literal plus -- if escapeRegExp deleted the special character instead of escaping it, the built pattern would require the literal text "pg" and this genuinely matching "p+g" part would be missed. + const pkg: Package = { + parts: { + "word/media/image1.p+g": { kind: "binary", base64: "" }, + }, + }; + expect(nextMediaIndex(pkg, "word/media", "image", "p+g")).toBe(2); + }); + + // "word/mediaXimage5.png" is exactly as long as "word/media/" ("word/mediaX" is 11 characters, matching "word/media/"'s own 11), so slicing it at the media-directory-prefix length spells "image5.png" by coincidence -- deliberately exercising the same prefix-check coincidence as src/odf-package/media.test.ts's own nextPictureIndex case, for the sibling OOXML-side implementation. + it("ignores a same-named file outside the media directory even when slicing its path at the prefix length would coincidentally spell a valid image filename", () => { + const pkg: Package = { + parts: { + "word/media/image1.png": { kind: "binary", base64: "" }, + "word/mediaXimage5.png": { kind: "binary", base64: "" }, + }, + }; + expect(nextMediaIndex(pkg, "word/media", "image", "png")).toBe(2); + }); +}); diff --git a/packages/documents.js/src/opc/media.ts b/packages/documents.js/src/opc/media.ts index 22bf1dd33..bf35167c3 100644 --- a/packages/documents.js/src/opc/media.ts +++ b/packages/documents.js/src/opc/media.ts @@ -19,7 +19,7 @@ function escapeRegExp(value: string): string { return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); } -function nextMediaIndex( +export function nextMediaIndex( pkg: Package, mediaDir: string, fileNamePrefix: string, @@ -43,9 +43,11 @@ function nextMediaIndex( continue; } const n = Number.parseInt(digits, 10); - if (n > max) { - max = n; - } + // Math.max, not an if/comparison: the two ever differ observably only on a tie, and every + // path here is keyed by its own literal numeric suffix, so no two iterations of this loop can + // ever see the same n twice -- a tie can only be n against its own already-recorded max, which + // assigns the identical value back, an if-based '>' vs '>=' comparison could never distinguish. + max = Math.max(max, n); } return max + 1; } diff --git a/packages/documents.js/src/opc/paths.test.ts b/packages/documents.js/src/opc/paths.test.ts index 69ccfefa2..a95ea448d 100644 --- a/packages/documents.js/src/opc/paths.test.ts +++ b/packages/documents.js/src/opc/paths.test.ts @@ -14,6 +14,11 @@ describe("relsPathFor", () => { it("handles a root-level part with no directory", () => { expect(relsPathFor("document.xml")).toBe("/_rels/document.xml.rels"); }); + + // A single-character directory puts the slash at index 1 -- deliberately exercising a genuinely different lastSlash value from the -1/no-slash case above, so a mutation swapping which index the filename split point compares against would extract the whole path as the filename rather than just the part after the slash. + it("splits correctly when the directory is a single character", () => { + expect(relsPathFor("a/file.xml")).toBe("a/_rels/file.xml.rels"); + }); }); describe("buildRelativeTarget", () => { @@ -40,4 +45,15 @@ describe("buildRelativeTarget", () => { buildRelativeTarget("ppt/presentation.xml", "ppt/slides/slide1.xml"), ).toBe("slides/slide1.xml"); }); + + it("targets a nested part from a root-level part with no directory of its own", () => { + expect(buildRelativeTarget("document.xml", "word/document.xml")).toBe( + "word/document.xml", + ); + }); + + // Both parts share the identical, fully-matching directory chain ("a/b"), so the common-prefix scan runs all the way to that shared length on both sides at once -- the one case where the two length bounds stop protecting each other (see buildRelativeTarget's own comment on combinedLimit). + it("targets a sibling part in a two-level-deep identical directory chain", () => { + expect(buildRelativeTarget("a/b/x.xml", "a/b/y.xml")).toBe("y.xml"); + }); }); diff --git a/packages/documents.js/src/opc/paths.ts b/packages/documents.js/src/opc/paths.ts index a70965b14..01eb2d840 100644 --- a/packages/documents.js/src/opc/paths.ts +++ b/packages/documents.js/src/opc/paths.ts @@ -2,7 +2,8 @@ export function relsPathFor(partPath: string): string { const lastSlash = partPath.lastIndexOf("/"); const dir = lastSlash === -1 ? "" : partPath.slice(0, lastSlash); - const fileName = lastSlash === -1 ? partPath : partPath.slice(lastSlash + 1); + // No lastSlash === -1 ternary guard here (unlike dir above): slicing from lastSlash + 1 already returns the whole path when there is no slash at all (lastIndexOf yields -1, so the slice starts at 0), making a guard for that case redundant -- see src/mathml/nodes.ts's localName for the identical pattern and reasoning. + const fileName = partPath.slice(lastSlash + 1); return `${dir}/_rels/${fileName}.rels`; } @@ -24,11 +25,9 @@ export function buildRelativeTarget( const toFileName = toPartPath.slice(toPartPath.lastIndexOf("/") + 1); let common = 0; - while ( - common < fromDirs.length && - common < toDirs.length && - fromDirs[common] === toDirs[common] - ) { + // A single combined bound, not two independently-ANDed length checks: with two separate `common < fromDirs.length && common < toDirs.length` clauses, relaxing (or dropping) either one in isolation never changes the loop's outcome on its own -- the OTHER, still-correct clause independently stops the loop at the same `common`, and wherever the two arrays' lengths genuinely differ, the fromDirs[common] === toDirs[common] comparison itself already fails once one side runs out (a real segment can never equal undefined). That made every mutation on either individual clause (and on the && joining them) permanently equivalent. A single combinedLimit bound has no sibling clause left to compensate, so a boundary mutation on it is only masked when the two paths share every directory segment all the way to a shared length -- covered by the identical-directories case below. + const combinedLimit = Math.min(fromDirs.length, toDirs.length); + while (common < combinedLimit && fromDirs[common] === toDirs[common]) { common++; } diff --git a/packages/documents.js/src/opc/rels.test.ts b/packages/documents.js/src/opc/rels.test.ts index de301572f..cb89d6610 100644 --- a/packages/documents.js/src/opc/rels.test.ts +++ b/packages/documents.js/src/opc/rels.test.ts @@ -6,7 +6,12 @@ import { rootElement, } from "ooxml.js"; import { describe, expect, it } from "vitest"; -import { addRelationship, addRootRelationship } from "./rels"; +import { el } from "../xml/fragment"; +import { + addRelationship, + addRootRelationship, + allocateRelationshipId, +} from "./rels"; const IMAGE_TYPE = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/image"; @@ -98,6 +103,10 @@ describe("addRootRelationship", () => { const rels = rootElement(pkg.parts["_rels/.rels"]); expect(rels).toBeDefined(); + expect(rels?.tag).toBe("Relationships"); + expect(rels === undefined ? undefined : attr(rels, "xmlns")).toBe( + "http://schemas.openxmlformats.org/package/2006/relationships", + ); const [relationship] = rels === undefined ? [] : childrenWithTag(rels, "Relationship"); expect(relationship).toBeDefined(); @@ -166,3 +175,22 @@ describe("addRootRelationship", () => { ).toEqual(["rId1", "rId2"]); }); }); + +describe("allocateRelationshipId", () => { + it("allocates rId1 for an empty root", () => { + const root = el("Relationships"); + expect(allocateRelationshipId(root)).toBe("rId1"); + }); + + it("continues from a pre-existing higher id rather than starting from 1", () => { + const root = el("Relationships", {}, [ + el("Relationship", { Id: "rId5", Type: IMAGE_TYPE, Target: "x" }), + ]); + expect(allocateRelationshipId(root)).toBe("rId6"); + }); + + it("ignores a non-Relationship child even if it carries an Id-shaped attribute", () => { + const root = el("Relationships", {}, [el("SomethingElse", { Id: "rId9" })]); + expect(allocateRelationshipId(root)).toBe("rId1"); + }); +}); diff --git a/packages/documents.js/src/opc/rels.ts b/packages/documents.js/src/opc/rels.ts index 6a91a74b0..6786e8fe9 100644 --- a/packages/documents.js/src/opc/rels.ts +++ b/packages/documents.js/src/opc/rels.ts @@ -29,7 +29,7 @@ function ensureRelationshipsRootAtPath( } // The next unused rId in a Relationships root, scanning existing Id attributes for the highest numeric suffix -- never reusing or guessing an id that might already be referenced elsewhere. -function allocateRelationshipId(relationshipsRoot: XmlElement): string { +export function allocateRelationshipId(relationshipsRoot: XmlElement): string { let max = 0; for (const child of relationshipsRoot.children) { if (child.type !== "element" || child.tag !== "Relationship") { @@ -48,9 +48,11 @@ function allocateRelationshipId(relationshipsRoot: XmlElement): string { continue; } const n = Number.parseInt(digits, 10); - if (n > max) { - max = n; - } + // Math.max, not an if/comparison: the two ever differ observably only on a tie, and every + // path here is keyed by its own literal numeric suffix, so no two iterations of this loop can + // ever see the same n twice -- a tie can only be n against its own already-recorded max, which + // assigns the identical value back, an if-based '>' vs '>=' comparison could never distinguish. + max = Math.max(max, n); } return `rId${max + 1}`; } diff --git a/packages/documents.js/src/package-codec.test.ts b/packages/documents.js/src/package-codec.test.ts index 2c42ed2c7..20f665984 100644 --- a/packages/documents.js/src/package-codec.test.ts +++ b/packages/documents.js/src/package-codec.test.ts @@ -118,6 +118,7 @@ describe("decodeDocumentPackage / encodeDocumentPackage: unsupported formats", ( throw error; } expect(error.format).toBe("markdown"); + expect(error.name).toBe("UnsupportedPackageFormatError"); } }); diff --git a/packages/documents.js/src/ports/abort.test.ts b/packages/documents.js/src/ports/abort.test.ts index 9449bf837..064eab418 100644 --- a/packages/documents.js/src/ports/abort.test.ts +++ b/packages/documents.js/src/ports/abort.test.ts @@ -21,5 +21,14 @@ describe("throwIfAborted", () => { expect(() => { throwIfAborted(controller.signal); }).toThrow(DOMException); + let caught: unknown; + try { + throwIfAborted(controller.signal); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(DOMException); + expect((caught as DOMException).name).toBe("AbortError"); + expect((caught as DOMException).message).toBe("Aborted"); }); }); diff --git a/packages/documents.js/src/ppt/write.test.ts b/packages/documents.js/src/ppt/write.test.ts index 151c661db..cfe48abd2 100644 --- a/packages/documents.js/src/ppt/write.test.ts +++ b/packages/documents.js/src/ppt/write.test.ts @@ -148,3 +148,16 @@ describe("ppt/write + ppt/read: OLE-embedded objects", () => { ).toBe("Nested deck"); }); }); + +describe("writePptContent: constructor guard", () => { + it("rejects a non-presentation ContentDocument, naming the offending kind", () => { + const spreadsheet: ContentDocument = { + kind: "spreadsheet", + metadata: {}, + sheets: [], + }; + expect(() => writePptContent(spreadsheet)).toThrow( + "writePptContent requires a presentation ContentDocument, got 'spreadsheet'", + ); + }); +}); diff --git a/packages/documents.js/src/svg/read-write.test.ts b/packages/documents.js/src/svg/read-write.test.ts index 20ae370cc..fd0e6dc04 100644 --- a/packages/documents.js/src/svg/read-write.test.ts +++ b/packages/documents.js/src/svg/read-write.test.ts @@ -652,6 +652,63 @@ describe("buildSvgText", () => { }, ]); }); + + it("falls back to the literal 'shape' when a diagnostic's own shape has neither a name nor a sourcePath", () => { + const diagnostics: SvgDiagnostic[] = []; + const document: ContentDocument = { + kind: "drawing", + metadata: {}, + pages: [ + { + size: { widthPt: 100, heightPt: 60 }, + shapes: [ + { + frame: { xPt: 0, yPt: 0, widthPt: 10, heightPt: 10 }, + insetLeftPt: 0, + insetTopPt: 0, + insetRightPt: 0, + insetBottomPt: 0, + blocks: [], + }, + ], + vectors: [], + }, + ], + }; + buildSvgText(document, { + onSvgDiagnostic: (diagnostic) => diagnostics.push(diagnostic), + }); + expect(diagnostics[0]?.detail).toMatch(/^shape:/); + }); + + it('writes a fill-rule="evenodd" attribute on a path vector whose own fillRule is evenodd', () => { + const document: ContentDocument = { + kind: "drawing", + metadata: {}, + pages: [ + { + size: { widthPt: 100, heightPt: 60 }, + shapes: [], + vectors: [ + { + kind: "path", + frame: { xPt: 0, yPt: 0, widthPt: 10, heightPt: 10 }, + fillRule: "evenodd", + subpaths: [ + { + start: { xPt: 0, yPt: 0 }, + segments: [], + closed: true, + }, + ], + }, + ], + }, + ], + }; + const text = buildSvgText(document); + expect(text).toContain('fill-rule="evenodd"'); + }); }); describe("readSvgContent -> buildSvgText round trip", () => { @@ -688,5 +745,15 @@ describe("decodeSvgText / encodeSvgText", () => { expect(() => decodeSvgText(new Uint8Array([0xff, 0xfe, 0x00]))).toThrow( SvgInvalidUtf8Error, ); + let caught: unknown; + try { + decodeSvgText(new Uint8Array([0xff, 0xfe, 0x00])); + } catch (error) { + caught = error; + } + expect((caught as Error).name).toBe("SvgInvalidUtf8Error"); + expect((caught as Error).message).toBe( + "svg text must be well-formed UTF-8", + ); }); }); diff --git a/packages/documents.js/src/svg/units.test.ts b/packages/documents.js/src/svg/units.test.ts index 0d132e672..2260dd718 100644 --- a/packages/documents.js/src/svg/units.test.ts +++ b/packages/documents.js/src/svg/units.test.ts @@ -28,6 +28,15 @@ describe("parseSvgLengthPt", () => { expect(parseSvgLengthPt("")).toBeUndefined(); expect(parseSvgLengthPt(undefined)).toBeUndefined(); }); + + it("trims surrounding whitespace before matching, rather than rejecting it as malformed", () => { + expect(parseSvgLengthPt(" 100px ")).toBe(75); + }); + + it("returns undefined when the matched number is syntactically valid but not finite", () => { + // The pattern's own exponent grammar accepts a magnitude this large; Number() then overflows to Infinity, which the finiteness guard must still reject rather than propagate. + expect(parseSvgLengthPt("1e400")).toBeUndefined(); + }); }); describe("parseSvgUserUnits", () => { @@ -67,6 +76,7 @@ describe("parseSvgViewBox", () => { expect(parseSvgViewBox("0 0 100")).toBeUndefined(); expect(parseSvgViewBox("0 0 100 60 5")).toBeUndefined(); expect(parseSvgViewBox("0 0 -100 60")).toBeUndefined(); + expect(parseSvgViewBox("0 0 100 -60")).toBeUndefined(); expect(parseSvgViewBox("0 0 100 abc")).toBeUndefined(); expect(parseSvgViewBox(undefined)).toBeUndefined(); }); @@ -78,5 +88,20 @@ describe("parseSvgViewBox", () => { width: 0, height: 60, }); + expect(parseSvgViewBox("0 0 100 0")).toEqual({ + minX: 0, + minY: 0, + width: 100, + height: 0, + }); + }); + + it("trims surrounding whitespace and collapses runs of internal whitespace between numbers", () => { + expect(parseSvgViewBox(" 0 0 100 60 ")).toEqual({ + minX: 0, + minY: 0, + width: 100, + height: 60, + }); }); }); diff --git a/packages/documents.js/src/test-support/markdown.test.ts b/packages/documents.js/src/test-support/markdown.test.ts new file mode 100644 index 000000000..e4c8f6d99 --- /dev/null +++ b/packages/documents.js/src/test-support/markdown.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from "vitest"; +import { readMarkdownContent } from "../markdown/read"; +import { richMarkdownText, richMarkdownTextWithFrontMatter } from "./markdown"; + +// richMarkdownText/richMarkdownTextWithFrontMatter are hand-authored literal markdown source text (see their own top-of-file comment), joined from an array of lines including several deliberately blank ("") separator lines between blocks. A blank line is a genuine CommonMark block boundary, so these assert the fixture actually parses into DISTINCT top-level blocks rather than merging into fewer, larger ones -- the only way a corrupted separator (anything other than a real blank line) would show up. + +describe("richMarkdownText", () => { + it("parses into four distinct top-level blocks: heading, paragraph, list, table", () => { + const content = readMarkdownContent(richMarkdownText()); + if (content.kind !== "wordprocessing") { + throw new Error("expected a wordprocessing ContentDocument"); + } + const blocks = content.sections[0]?.blocks ?? []; + // The heading, the second paragraph, then one paragraph per list item (markdown-codec's own flat block model has no dedicated "list" block kind -- each item is its own paragraph, with list membership carried on the paragraph itself), then the table. + expect(blocks.map((block) => block.kind)).toEqual([ + "paragraph", + "paragraph", + "paragraph", + "paragraph", + "paragraph", + "paragraph", + "table", + ]); + expect(blocks[0]).toMatchObject({ styleId: "Heading1" }); + // Exact per-block text -- proof each blank-line separator genuinely separated two blocks rather than merging stray text into one of them (CommonMark merges consecutive non-blank lines of plain text into a single paragraph, so a corrupted separator would silently widen one paragraph's own text rather than changing the block kind sequence above at all). + function text(block: (typeof blocks)[number]): string { + return block.kind === "paragraph" + ? block.runs.map((run) => run.text).join("") + : ""; + } + expect(text(blocks[0]!)).toBe("Report Title"); + expect(text(blocks[1]!)).toBe( + "Second paragraph with bold and italic text.", + ); + expect(text(blocks[2]!)).toBe("First item"); + }); +}); + +describe("richMarkdownTextWithFrontMatter", () => { + it("separates the closing --- from the body, parsing metadata and richMarkdownText's own four blocks separately", () => { + const content = readMarkdownContent(richMarkdownTextWithFrontMatter(), { + frontMatter: true, + }); + expect(content.metadata.title).toBe("Sample Report"); + expect(content.metadata.author).toBe("Ada Lovelace"); + if (content.kind !== "wordprocessing") { + throw new Error("expected a wordprocessing ContentDocument"); + } + const blocks = content.sections[0]?.blocks ?? []; + expect(blocks.map((block) => block.kind)).toEqual([ + "paragraph", + "paragraph", + "paragraph", + "paragraph", + "paragraph", + "paragraph", + "table", + ]); + }); +}); diff --git a/packages/documents.js/src/test-support/odb.ts b/packages/documents.js/src/test-support/odb.ts index f53edc6c6..cbe0e6dc7 100644 --- a/packages/documents.js/src/test-support/odb.ts +++ b/packages/documents.js/src/test-support/odb.ts @@ -214,10 +214,6 @@ function multiIndexOdbEntries(): (readonly [ ]; } -export function embeddedHsqldbMultiIndexOdbPackage(): Package { - return decodePackage(embeddedHsqldbMultiIndexOdbBytes()); -} - export function embeddedHsqldbMultiIndexOdbBytes(): Uint8Array<ArrayBuffer> { return zipPackage(multiIndexOdbEntries()); } diff --git a/packages/documents.js/src/test-support/odf.ts b/packages/documents.js/src/test-support/odf.ts index 37e935fc6..36e290f56 100644 --- a/packages/documents.js/src/test-support/odf.ts +++ b/packages/documents.js/src/test-support/odf.ts @@ -1,5 +1,4 @@ -import type { Package } from "odf.js"; -import { decodePackage, ODF_MEDIA_TYPES, zipPackage } from "odf.js"; +import { ODF_MEDIA_TYPES, zipPackage } from "odf.js"; // Never imported by src/index.ts and never reaches dist/. Hand-authored ODF formula (.odf) XML zipped via odf.js's own zipPackage/decodePackage, mirroring src/test-support/odt.ts's own established convention exactly (same mimetype-part-first-and-stored requirement, same "not from a real LibreOffice binary" scope) -- see that file's own top-of-file comment for the full reasoning. Every fixture wraps its own MathML content in the real office:body > office:math > math:math structure a genuine LibreOffice-authored .odf uses, with every math element under a "math:" namespace prefix (not the bare, unprefixed form) -- deliberately, since that IS what real LibreOffice output uses (confirmed by src/mathml/nodes.ts's own localName-stripping design, built specifically to handle this), so these fixtures exercise the realistic path, not merely the more lenient one. @@ -30,13 +29,6 @@ export function odfFormulaBytes( ]); } -export function odfFormulaPackage( - mathMlInner: string, - options?: { readonly starMath?: string }, -): Package { - return decodePackage(odfFormulaBytes(mathMlInner, options)); -} - // A small, curated set of real formulas covering every construct the task's own test requirement names: a simple fraction, a square root, a superscript/subscript combination, and a small matrix via mtable. export const FRACTION_FORMULA = diff --git a/packages/documents.js/src/test-support/ods-formula.ts b/packages/documents.js/src/test-support/ods-formula.ts index 334acc4f1..c377fbdd9 100644 --- a/packages/documents.js/src/test-support/ods-formula.ts +++ b/packages/documents.js/src/test-support/ods-formula.ts @@ -1,5 +1,4 @@ -import type { Package } from "odf.js"; -import { base64ToBytes, decodePackage } from "odf.js"; +import { base64ToBytes } from "odf.js"; // Never imported by src/index.ts and never reaches dist/. The ExaDev/odf.js repository's own real fixture (src/typed/ods/fixtures/sheet-formula.ods), base64-embedded here exactly like src/test-support/odb-fixture.ts's own .odb and src/test-support/firebird.ts's own .fbk streams -- a genuine, unmodified LibreOffice 26.2-generated spreadsheet built through that application's own UNO API (a Java client against a headless soffice, saved with the calc8 filter) and never hand-edited afterwards. Embedded rather than read off disk because odf.js ships only dist/ as a dependency: its fixtures directory exists in that repository, not in this package's own node_modules, so a test reading it from a sibling checkout would pass on one machine and fail in CI. // @@ -94,7 +93,3 @@ const SHEET_FORMULA_ODS_BASE64 = export function sheetFormulaOdsBytes(): Uint8Array<ArrayBuffer> { return base64ToBytes(SHEET_FORMULA_ODS_BASE64); } - -export function sheetFormulaOdsPackage(): Package { - return decodePackage(sheetFormulaOdsBytes()); -} diff --git a/packages/documents.js/src/test-support/ods.ts b/packages/documents.js/src/test-support/ods.ts index 94b427cea..486975e19 100644 --- a/packages/documents.js/src/test-support/ods.ts +++ b/packages/documents.js/src/test-support/ods.ts @@ -263,10 +263,6 @@ export function gridOdsBytes(): Uint8Array<ArrayBuffer> { return encodePackage(buildGridFixturePackage()); } -export function gridOdsPackage(): Package { - return decodePackage(gridOdsBytes()); -} - // A third fixture, purpose-built for the ods<->xlsx cross-format bridge's own round-trip tests (src/convert/bridges.test.ts): three explicitly-widthed columns (3cm/4cm/2cm) and every office:value-type ODS distinguishes on one row each -- string, float, boolean, percentage, currency, date, time -- plus a formula cell (table:formula carried verbatim, never evaluated by either side of the bridge) and a genuine 2-column merge. This is deliberately the richest of the three ods.ts fixtures: xlsx write support (ooxml.js's buildXlsxPackageFromContent) is new to the ecosystem, so the bridge's own tests need real, independently-authored ground truth to check against, not a fixture built through the very editor (createOds) the bridge composes with on its own write-back hop. function buildRichFixturePackage(): Package { const columns = [ @@ -445,10 +441,6 @@ export function richOdsBytes(): Uint8Array<ArrayBuffer> { return encodePackage(buildRichFixturePackage()); } -export function richOdsPackage(): Package { - return decodePackage(richOdsBytes()); -} - // A fourth fixture, purpose-built for the per-cell decoration wiring (ContentSheetCell's background/borders/alignment/verticalAlignment, all four added to document-schema.js's ContentSheetCellSchema and all four genuinely populated by odf.js's own readOdsContent -- see typed/shared/table.ts's readCellStyleDecoration). Deliberately hand-authored ODF XML rather than built through createOds/OdsCell, for the same independent-construction reason this module's other fixtures are: OdsCell has no decoration setter at all today, so the editor could not express this fixture even if it were the right tool. // // One sheet, "Decorated", one row of two cells: A1 carries a yellow fo:background-color, a full fo:border shorthand, an explicit fo:text-align="right" and style:vertical-align="top"; B1 carries only a red fo:border-bottom, with no background, no alignment, and no vertical alignment of its own -- so a single fixture exercises both the "declares everything" and the "declares exactly one edge and nothing else" branches of the layout wiring at once. @@ -564,7 +556,3 @@ function buildDecoratedFixturePackage(): Package { export function decoratedOdsBytes(): Uint8Array<ArrayBuffer> { return encodePackage(buildDecoratedFixturePackage()); } - -export function decoratedOdsPackage(): Package { - return decodePackage(decoratedOdsBytes()); -} diff --git a/packages/documents.js/src/test-support/pdf.ts b/packages/documents.js/src/test-support/pdf.ts index 1cb8b77ea..c0b667bb4 100644 --- a/packages/documents.js/src/test-support/pdf.ts +++ b/packages/documents.js/src/test-support/pdf.ts @@ -301,19 +301,6 @@ export function inheritedPageAttributesPdf(): Uint8Array<ArrayBuffer> { return b.bytes(); } -// A page with a hidden /Subtype /Text annotation NOT authored by documents.js's own writer (a different /T, as a real third-party tool's own sticky note would have) -- proves readPageNotes's /T-marker check genuinely discriminates our own notes annotation from someone else's, rather than treating every hidden Text annotation as recovered pptx notes. -export function pdfWithForeignHiddenAnnotationPdf(): Uint8Array<ArrayBuffer> { - const b = new FixtureBuilder().header("1.4"); - catalogPagesPageFontObjects(b, 5, "[0 0 200 100]", "/Annots [6 0 R] "); - b.stream(5, "<< >>", enc(HELLO_CONTENT)); - b.object( - 6, - "<< /Type /Annot /Subtype /Text /Rect [0 0 0 0] /Contents (A real reviewer note, not pptx speaker notes) /T (Some Other Tool) /F 2 >>", - ); - b.classicXrefAndTrailer(6, "/Root 1 0 R"); - return b.bytes(); -} - // An /Info dict mixing the two real-world string encodings a reader must handle: /Title as UTF-16BE-with-BOM (our own writer's own convention, ISO 32000-1 7.9.2.2's "long form"), and /Author/Keywords as plain literal-string PDFDocEncoding (the common case for ASCII-only metadata most third-party producers emit). /CreationDate uses the PDF date format (ISO 32000-1 7.9.4) with an explicit UTC+02:00 offset. export function withInfoDictPdf(): Uint8Array<ArrayBuffer> { const b = new FixtureBuilder().header("1.4"); diff --git a/packages/documents.js/src/xml/edit.test.ts b/packages/documents.js/src/xml/edit.test.ts index c84854de4..1e0036928 100644 --- a/packages/documents.js/src/xml/edit.test.ts +++ b/packages/documents.js/src/xml/edit.test.ts @@ -61,6 +61,35 @@ describe("removeChild / insertBefore / insertAfter", () => { insertAfter(container, b, after); expect(container).toEqual([a, before, b, after]); }); + + it("insertBefore appends at the end when the reference sibling is not in the container, rather than immediately before the last element", () => { + const a = el("a"); + const b = el("b"); + const container: XmlNode[] = [a, b]; + const stray = el("stray"); + const newNode = el("new"); + insertBefore(container, stray, newNode); + expect(container).toEqual([a, b, newNode]); + }); + + it("insertAfter places the node right after a found reference that is not the container's last element", () => { + const a = el("a"); + const b = el("b"); + const container: XmlNode[] = [a, b]; + const newNode = el("new"); + insertAfter(container, a, newNode); + expect(container).toEqual([a, newNode, b]); + }); + + it("insertAfter appends at the end when the reference sibling is not in the container, rather than at the start", () => { + const a = el("a"); + const b = el("b"); + const container: XmlNode[] = [a, b]; + const stray = el("stray"); + const newNode = el("new"); + insertAfter(container, stray, newNode); + expect(container).toEqual([a, b, newNode]); + }); }); describe("insertInSchemaOrder", () => { @@ -108,6 +137,14 @@ describe("insertInSchemaOrder", () => { parent.children.map((c) => (c.type === "element" ? c.tag : c.type)), ).toEqual(RPR_ORDER); }); + + it("appends after a same-rank sibling rather than inserting before it", () => { + const parent = el("w:rPr", {}, [el("w:b")]); + insertInSchemaOrder(parent, el("w:b"), RPR_ORDER); + expect( + parent.children.map((c) => (c.type === "element" ? c.tag : c.type)), + ).toEqual(["w:b", "w:b"]); + }); }); describe("directChildElement / getOrCreateChildElement", () => { diff --git a/packages/documents.js/src/xml/edit.ts b/packages/documents.js/src/xml/edit.ts index 60141fa75..7cb3f4a78 100644 --- a/packages/documents.js/src/xml/edit.ts +++ b/packages/documents.js/src/xml/edit.ts @@ -67,8 +67,9 @@ export function insertInSchemaOrder( if (sibling.type !== "element") { continue; } + // No explicit "not in order" guard: order.indexOf yields -1 for a sibling whose tag is absent from `order`, and childRank is already known non-negative (the -1 case returned above), so -1 > childRank is always false on its own -- an explicit siblingRank !== -1 check ahead of it would never change the outcome, only duplicate what the comparison below already guarantees. const siblingRank = order.indexOf(sibling.tag); - if (siblingRank !== -1 && siblingRank > childRank) { + if (siblingRank > childRank) { insertBefore(parent.children, sibling, child); return; } diff --git a/packages/documents.js/src/xml/odf-text.test.ts b/packages/documents.js/src/xml/odf-text.test.ts index 550aa58f8..3fd0c74d1 100644 --- a/packages/documents.js/src/xml/odf-text.test.ts +++ b/packages/documents.js/src/xml/odf-text.test.ts @@ -1,5 +1,6 @@ import type { XmlNode } from "ooxml.js"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; +import * as odfJs from "odf.js"; import { decodeOdfText, encodeOdfText } from "./odf-text"; // The wrong behaviour decodeOdfText exists specifically to avoid: a naive concatenation of ONLY XmlText nodes, exactly what ooxml.js's own textContent() helper does and exactly why this codebase's own top-of-file warning in odf-text.ts forbids using it on ODF content. Defined only for the one regression test below, never exported. @@ -98,6 +99,15 @@ describe("encodeOdfText", () => { }); describe("decodeOdfText", () => { + it("wraps the given nodes in a real, named synthetic container element", () => { + const spy = vi.spyOn(odfJs, "decodeOdfText"); + decodeOdfText([{ type: "text", value: "x" }]); + expect(spy).toHaveBeenCalledWith( + expect.objectContaining({ tag: "_odf-text-container" }), + ); + spy.mockRestore(); + }); + it("is the exact inverse of encodeOdfText for single spaces, space runs, tabs, newlines, and mixed sequences", () => { for (const value of [ " ", diff --git a/packages/documents.js/src/xml/odf-text.ts b/packages/documents.js/src/xml/odf-text.ts index 44fedee72..8e8688075 100644 --- a/packages/documents.js/src/xml/odf-text.ts +++ b/packages/documents.js/src/xml/odf-text.ts @@ -31,7 +31,8 @@ export function encodeOdfText(text: string): XmlNode[] { const ch = text.charAt(i); if (ch === " ") { let runLength = 1; - while (i + runLength < text.length && text[i + runLength] === " ") { + // No explicit i + runLength < text.length bound check: indexing a string past its end yields undefined in JavaScript, and undefined === " " is already false, so the length comparison could never change the loop's outcome -- it would only ever agree with what the character comparison below already decides on its own, making it a permanently equivalent mutation target. + while (text[i + runLength] === " ") { runLength += 1; } if (runLength >= MIN_SPACE_RUN_FOR_TEXT_S) { diff --git a/packages/documents.js/src/xml/query.test.ts b/packages/documents.js/src/xml/query.test.ts index 3ba85ae26..a53cca40b 100644 --- a/packages/documents.js/src/xml/query.test.ts +++ b/packages/documents.js/src/xml/query.test.ts @@ -21,6 +21,12 @@ describe("xml/query", () => { expect(cursor?.container).toBe(container); }); + it("findChildElement returns undefined when no child element matches the requested tag, even though an element of a different tag is present", () => { + const run = el("w:r"); + const container: XmlNode[] = [run]; + expect(findChildElement(container, "w:p")).toBeUndefined(); + }); + it("findChildElements returns only direct children, in document order", () => { const runA = el("w:r"); const runB = el("w:r");