diff --git a/.github/workflows/drawing-layout-integration.yml b/.github/workflows/drawing-layout-integration.yml new file mode 100644 index 00000000..2d9efa24 --- /dev/null +++ b/.github/workflows/drawing-layout-integration.yml @@ -0,0 +1,77 @@ +name: Drawing layout integration +on: + pull_request: + push: + branches: [main, feature/drawing-layout-contract] +permissions: + contents: read +concurrency: + group: drawing-layout-${{ github.ref }} + cancel-in-progress: true +jobs: + check: + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + with: + persist-credentials: false + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version: 24 + - name: Install build dependencies + run: npm install --ignore-scripts --package-lock=false + - name: Check drawing source formatting with actionable diff + run: | + if ! node_modules/.bin/oxfmt --check src/xlsx/drawing-layout.ts test/drawing-layout.test.ts; then + node_modules/.bin/oxfmt src/xlsx/drawing-layout.ts test/drawing-layout.test.ts + git diff -- src/xlsx/drawing-layout.ts test/drawing-layout.test.ts + exit 1 + fi + - name: Check public parser and writer contract + run: npm run test:drawing-layout + - name: Type check + run: npm run typecheck + - name: Full library regression suite + run: node_modules/.bin/vitest run + - name: Preserve successful source and build for integration verification + run: | + git rev-parse HEAD > integration-revision.txt + tar -czf drawing-layout-runtime.tar.gz src dist scripts docs test package.json build.config.ts tsconfig.json tsconfig.cli.json integration-revision.txt + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + with: + name: drawing-layout-runtime + path: drawing-layout-runtime.tar.gz + retention-days: 3 + coverage-baseline: + name: Measure unchanged base coverage (informational) + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - name: Check out the PR base, not the proposed changes + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + with: + ref: ${{ github.event.pull_request.base.sha }} + persist-credentials: false + - run: corepack enable + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version: 24 + - run: pnpm install --frozen-lockfile + - name: Record baseline without changing coverage gates + shell: bash + run: | + git rev-parse HEAD + set +e + pnpm coverage > "$RUNNER_TEMP/base-coverage.log" 2>&1 + status=$? + set -e + tail -180 "$RUNNER_TEMP/base-coverage.log" + echo "Unchanged base coverage exit status: $status" + { + echo '## Unchanged base coverage (informational only)' + echo "Revision: $(git rev-parse HEAD)" + echo "Coverage exit status: $status" + echo 'The main CI coverage job still independently enforces all original thresholds.' + } >> "$GITHUB_STEP_SUMMARY" diff --git a/docs/drawing-layout.md b/docs/drawing-layout.md new file mode 100644 index 00000000..56b2d417 --- /dev/null +++ b/docs/drawing-layout.md @@ -0,0 +1,29 @@ +# Drawing and Normal-font metadata + +The maintained parser is the single owner of XLSX package relationships and +DrawingML coordinates. Renderers must not reopen the ZIP to repair missing fields. + +`SheetImage.anchor` now preserves the container `kind` (`twoCell`, `oneCell`, +`absolute`), an explicitly saved `editAs`, exact EMU `extent` and (for absolute +placement) EMU `position`. Existing `from`/`to` markers and offsets remain intact. +Absent metadata remains optional for older caller-created models. `width` and +`height` are compatibility dimensions at 96 DPI, without premature integer rounding. +One inch is 914400 EMU; DPI, display zoom, hidden-axis layout, and browser coordinate +conversion belong to the renderer, not the parser. + +A fixed-size placement must not be stretched just because it has a second saved +cell marker. A normal two-cell placement still derives its current displayed size +from the cell markers. The writer preserves container kind, edit behavior, marker +offsets and exact extents; cloning and worker structured cloning retain them too. +This change does not claim full support for grouped-shape transforms or image effects. + +`Workbook.defaultFont` follows the built-in Normal style's reference chain: +`cellStyles[builtinId=0].xfId -> cellStyleXfs[xfId].fontId -> fonts[fontId]`. +It is not inferred from a cell or from a screenshot. Invalid references fall back +to the first font, as before. View, page setup, print area and page-break metadata +continue to come from the existing parser. + +Run `npm run test:drawing-layout` for the post-build parser/writer/clone regression +checks. Fixtures are generated in memory and contain no customer data. A `prepare` +lifecycle script builds the package when installed through a pinned Git dependency; +consumers do not need a separately installed pnpm executable to run that build. diff --git a/package.json b/package.json index 49eb2856..b66b9df6 100644 --- a/package.json +++ b/package.json @@ -95,7 +95,9 @@ "test": "pnpm lint && pnpm typecheck && vitest run", "typecheck": "tsc --noEmit && tsc --noEmit -p tsconfig.cli.json", "release": "pnpm test && pnpm build && bumpp --commit --tag --push --all", - "prepack": "pnpm build" + "prepack": "npm run build", + "prepare": "npm run build", + "test:drawing-layout": "npm run build && node --test scripts/verify-drawing-layout.mjs" }, "devDependencies": { "@types/node": "^26.1.2", diff --git a/scripts/verify-drawing-layout.mjs b/scripts/verify-drawing-layout.mjs new file mode 100644 index 00000000..25615fb5 --- /dev/null +++ b/scripts/verify-drawing-layout.mjs @@ -0,0 +1,173 @@ +/** Post-build public parser/writer contract checks; no customer documents. */ +import assert from "node:assert/strict" +import test from "node:test" +import { readXlsx, writeXlsx, cloneSheet } from "../dist/index.mjs" +import { ZipReader } from "../dist/zip/reader.mjs" +import { ZipWriter } from "../dist/zip/writer.mjs" + +const encode = new TextEncoder() +const decode = new TextDecoder() +const png = Uint8Array.from([137, 80, 78, 71, 13, 10, 26, 10]) +const NS = "http://schemas.openxmlformats.org" +const marker = (name, row, col, rowOff = 0, colOff = 0) => + `${col}${colOff}${row}${rowOff}` +const extent = { cx: 1252822, cy: 962025 } +const pic = `` +const from = marker("from", 0, 1, 257175, 133350) +const to = marker("to", 3, 8, 100965, 123158) +const two = (edit = "") => + `${from}${to}${pic}` +const one = `${from}${pic}` +const absolute = `${pic}` + +async function fixture(anchor, styleXml) { + const original = await writeXlsx({ + sheets: [ + { + name: "Geometry", + rows: [["ordinary cell"]], + images: [{ data: png, type: "png", anchor: { from: { row: 0, col: 0 } } }], + }, + ], + }) + const zip = new ZipReader(original) + const out = new ZipWriter() + for (const path of zip.entries()) { + let bytes = await zip.extract(path) + if (path === "xl/drawings/drawing1.xml") + bytes = encode.encode( + `${anchor}`, + ) + if (path === "xl/styles.xml" && styleXml) bytes = encode.encode(styleXml) + if (path === "xl/worksheets/sheet1.xml" && styleXml) + bytes = encode.encode(decode.decode(bytes).replace(" (await readXlsx(bytes, { readStyles: true })).sheets[0].images[0] + +for (const edit of ["", "twoCell", "oneCell", "absolute"]) { + test(`read/write two-cell container retains markers and editAs=${edit || "(omitted)"}`, async () => { + const image = await imageOf(await fixture(two(edit))) + assert.equal(image.anchor.kind, "twoCell") + assert.equal(image.anchor.editAs, edit || undefined) + assert.deepEqual(image.anchor.extent, extent) + assert.deepEqual(image.anchor.from, { row: 0, col: 1, rowOff: 257175, colOff: 133350 }) + assert.deepEqual(image.anchor.to, { row: 3, col: 8, rowOff: 100965, colOff: 123158 }) + assert.equal(image.width, extent.cx / 9525) + if (edit === "absolute") assert.deepEqual(image.anchor.position, { x: 10001, y: 20003 }) + const again = await imageOf( + await writeXlsx({ sheets: [{ name: "Roundtrip", rows: [], images: [image] }] }), + ) + assert.deepEqual(again.anchor, image.anchor) + assert.equal(again.width, image.width) + }) +} +for (const [kind, xml] of [ + ["oneCell", one], + ["absolute", absolute], +]) { + test(`${kind} container survives a read/write/read cycle without fabricated end marker`, async () => { + const image = await imageOf(await fixture(xml)) + assert.equal(image.anchor.kind, kind) + assert.equal(image.anchor.to, undefined) + assert.deepEqual(image.anchor.extent, extent) + if (kind === "absolute") assert.deepEqual(image.anchor.position, { x: -3175, y: 19051 }) + const again = await imageOf( + await writeXlsx({ sheets: [{ name: "Roundtrip", rows: [], images: [image] }] }), + ) + assert.deepEqual(again.anchor, image.anchor) + }) +} +test("one-cell extent is the container extent, not stale picture transform ext", async () => { + const image = await imageOf( + await fixture( + one.replace(``, ''), + ), + ) + assert.deepEqual(image.anchor.extent, { cx: 1, cy: 2 }) + assert.equal(image.width, 1 / 9525) +}) +test("zero-size extents remain zero and do not turn into a default-size image", async () => { + const input = await imageOf( + await fixture( + one.replace(``, ''), + ), + ) + assert.deepEqual(input.anchor.extent, { cx: 0, cy: 0 }) + const output = await imageOf( + await writeXlsx({ sheets: [{ name: "Zero", rows: [], images: [input] }] }), + ) + assert.deepEqual(output.anchor.extent, { cx: 0, cy: 0 }) +}) +for (const value of ["NaN", "Infinity", "-1", "9007199254740992", "1.5"]) { + test(`rejects invalid saved EMU extent ${value}`, async () => { + const image = await imageOf(await fixture(two().replace(`cx="${extent.cx}"`, `cx="${value}"`))) + assert.equal(image.anchor.extent, undefined) + }) +} +test("explicit zero offsets and subpixel offsets survive serialization", async () => { + const image = await imageOf(await fixture(two("oneCell"))) + image.anchor.from = { row: 0, col: 0, rowOff: 0, colOff: 1 } + image.anchor.to = { row: 1, col: 1, rowOff: 2, colOff: 0 } + const output = await imageOf( + await writeXlsx({ sheets: [{ name: "Tiny", rows: [], images: [image] }] }), + ) + assert.deepEqual( + { ...output.anchor.from, rowOff: output.anchor.from.rowOff ?? 0 }, + image.anchor.from, + ) + assert.deepEqual({ ...output.anchor.to, colOff: output.anchor.to.colOff ?? 0 }, image.anchor.to) +}) +test("cloneSheet isolates all nested drawing coordinates", async () => { + const original = (await readXlsx(await fixture(two("absolute")))).sheets[0] + const cloned = cloneSheet(original, "Copy") + cloned.images[0].anchor.extent.cx++ + cloned.images[0].anchor.position.x++ + cloned.images[0].anchor.from.rowOff++ + assert.equal(original.images[0].anchor.extent.cx, extent.cx) + assert.equal(original.images[0].anchor.position.x, 10001) + assert.equal(original.images[0].anchor.from.rowOff, 257175) +}) +test("structuredClone preserves the full worker-safe geometry contract", async () => { + const image = await imageOf(await fixture(two("absolute"))) + assert.deepEqual(structuredClone(image).anchor, image.anchor) +}) +function styles(xfId = "1", fontId = "2") { + return `` +} +test("Normal font follows builtinId -> xfId -> fontId, not fonts[0] or the first cell", async () => { + const book = await readXlsx(await fixture(two(), styles()), { readStyles: true }) + assert.equal(book.defaultFont.name, "Normal Font") + assert.equal(book.defaultFont.size, 10) + assert.equal(book.defaultFont.bold, true) + assert.equal(book.sheets[0].cells.get("0,0").style.font.name, "Cell") +}) +for (const [xf, font] of [ + ["999", "2"], + ["-1", "2"], + ["bad", "2"], + ["1", "999"], + ["1", "bad"], +]) { + test(`invalid Normal reference ${xf}/${font} falls back safely`, async () => { + const book = await readXlsx(await fixture(two(), styles(xf, font)), { readStyles: true }) + assert.equal(book.defaultFont.name, "Fallback") + }) +} +test("legacy caller-created image anchors remain supported", async () => { + const image = { + data: png, + type: "png", + anchor: { from: { row: 1, col: 2 }, to: { row: 4, col: 5 } }, + width: 17, + height: 23, + } + const bytes = await writeXlsx({ sheets: [{ name: "Legacy", rows: [], images: [image] }] }) + const saved = await imageOf(bytes) + assert.equal(saved.anchor.kind, "twoCell") + assert.deepEqual(saved.anchor.extent, { cx: 17 * 9525, cy: 23 * 9525 }) + const xml = decode.decode(await new ZipReader(bytes).extract("xl/drawings/drawing1.xml")) + assert.ok(!xml.includes("NaN") && !xml.includes("undefined")) +}) diff --git a/src/_types.ts b/src/_types.ts index cc069c99..5df3792e 100644 --- a/src/_types.ts +++ b/src/_types.ts @@ -668,6 +668,14 @@ export interface SheetImage { type: "png" | "jpeg" | "gif" | "svg" | "webp" /** Anchor to cell */ anchor: { + /** Original DrawingML container. Omitted by legacy caller-created models. */ + kind?: "twoCell" | "oneCell" | "absolute" + /** Resizing behavior of a two-cell container; does not discard its markers. */ + editAs?: "twoCell" | "oneCell" | "absolute" + /** Exact saved drawing extent in EMUs (914400 per inch), before display rounding. */ + extent?: { cx: number; cy: number } + /** Sheet-relative position in EMUs for absolute placement. */ + position?: { x: number; y: number } from: { row: number; col: number; rowOff?: number; colOff?: number } to?: { row: number; col: number; rowOff?: number; colOff?: number } } diff --git a/src/sheet-ops.ts b/src/sheet-ops.ts index 27ec68e7..6d4c3344 100644 --- a/src/sheet-ops.ts +++ b/src/sheet-ops.ts @@ -1021,6 +1021,8 @@ export function cloneSheet(sheet: Sheet, newName: string): Sheet { const copy = { ...img, data: new Uint8Array(img.data) } copy.anchor = { ...img.anchor, from: { ...img.anchor.from } } if (img.anchor.to) copy.anchor.to = { ...img.anchor.to } + if (img.anchor.extent) copy.anchor.extent = { ...img.anchor.extent } + if (img.anchor.position) copy.anchor.position = { ...img.anchor.position } return copy }) } diff --git a/src/xlsx/drawing-layout.ts b/src/xlsx/drawing-layout.ts new file mode 100644 index 00000000..159e9687 --- /dev/null +++ b/src/xlsx/drawing-layout.ts @@ -0,0 +1,68 @@ +import type { SheetImage } from "../_types" +import type { XmlElement } from "../xml/parser" + +type Layout = Pick + +function child(parent: XmlElement | undefined, name: string): XmlElement | undefined { + return parent?.children.find( + (node): node is XmlElement => typeof node !== "string" && (node.local || node.tag) === name, + ) +} + +function picture(anchor: XmlElement): XmlElement | undefined { + const direct = child(anchor, "pic") + if (direct) return direct + // Follow compatibility wrappers only. Grouped-child transforms use a + // different coordinate system and must not leak into the sheet anchor. + const pending = [...anchor.children].reverse() + while (pending.length) { + const node = pending.pop() + if (!node || typeof node === "string") continue + const local = node.local || node.tag + if (local === "pic") return node + if (local === "AlternateContent" || local === "Choice" || local === "Fallback") { + for (let index = node.children.length - 1; index >= 0; index--) { + pending.push(node.children[index]!) + } + } + } + return undefined +} + +function pair(node: XmlElement | undefined, a: string, b: string): [number, number] | undefined { + if (!node || node.attrs[a] === undefined || node.attrs[b] === undefined) return undefined + if (!/^[+-]?\d+$/.test(node.attrs[a].trim()) || !/^[+-]?\d+$/.test(node.attrs[b].trim())) { + return undefined + } + const x = Number(node.attrs[a]) + const y = Number(node.attrs[b]) + return Number.isSafeInteger(x) && Number.isSafeInteger(y) ? [x, y] : undefined +} + +/** Preserve file geometry in EMUs; DPI, zoom and grid rounding belong to the renderer. */ +export function readDrawingLayout(anchor: XmlElement): Layout { + const local = anchor.local || anchor.tag + let kind: Layout["kind"] + if (local === "twoCellAnchor") kind = "twoCell" + else if (local === "oneCellAnchor") kind = "oneCell" + else if (local === "absoluteAnchor") kind = "absolute" + else return {} + const result: Layout = { kind } + const editAs = anchor.attrs["editAs"] + if ( + kind === "twoCell" && + (editAs === "oneCell" || editAs === "absolute" || editAs === "twoCell") + ) { + result.editAs = editAs + } + const xfrm = child(child(picture(anchor), "spPr"), "xfrm") + const ext = kind === "twoCell" ? child(xfrm, "ext") : child(anchor, "ext") + const size = pair(ext, "cx", "cy") + if (size && size[0] >= 0 && size[1] >= 0) result.extent = { cx: size[0], cy: size[1] } + let positionNode: XmlElement | undefined + if (kind === "absolute") positionNode = child(anchor, "pos") + else if (editAs === "absolute") positionNode = child(xfrm, "off") + const pos = pair(positionNode, "x", "y") + if (pos) result.position = { x: pos[0], y: pos[1] } + return result +} diff --git a/src/xlsx/drawing-writer.ts b/src/xlsx/drawing-writer.ts index c660dffd..11c95bc1 100644 --- a/src/xlsx/drawing-writer.ts +++ b/src/xlsx/drawing-writer.ts @@ -125,11 +125,15 @@ export function writeDrawing( }), ) - // Calculate dimensions in EMU - const widthEmu = img.width ? img.width * EMU_PER_PIXEL : DEFAULT_WIDTH_EMU - const heightEmu = img.height ? img.height * EMU_PER_PIXEL : DEFAULT_HEIGHT_EMU - - // Build twoCellAnchor element + // Prefer exact parser-owned EMUs. Pixel fields remain a compatibility + // input for caller-created images; only quantize at XML serialization. + const widthEmu = imageExtent(img.anchor.extent?.cx, img.width, DEFAULT_WIDTH_EMU) + const heightEmu = imageExtent(img.anchor.extent?.cy, img.height, DEFAULT_HEIGHT_EMU) + const kind = img.anchor.kind ?? "twoCell" + const editAs = img.anchor.editAs + const position = img.anchor.position + + // Retain both markers even when a two-cell container has editAs=oneCell. const fromCol = img.anchor.from.col const fromRow = img.anchor.from.row const toCol = img.anchor.to?.col ?? fromCol + 3 @@ -137,16 +141,16 @@ export function writeDrawing( const fromElement = xmlElement("xdr:from", undefined, [ xmlElement("xdr:col", undefined, String(fromCol)), - xmlElement("xdr:colOff", undefined, "0"), + xmlElement("xdr:colOff", undefined, String(imageCoordinate(img.anchor.from.colOff))), xmlElement("xdr:row", undefined, String(fromRow)), - xmlElement("xdr:rowOff", undefined, "0"), + xmlElement("xdr:rowOff", undefined, String(imageCoordinate(img.anchor.from.rowOff))), ]) const toElement = xmlElement("xdr:to", undefined, [ xmlElement("xdr:col", undefined, String(toCol)), - xmlElement("xdr:colOff", undefined, "0"), + xmlElement("xdr:colOff", undefined, String(imageCoordinate(img.anchor.to?.colOff))), xmlElement("xdr:row", undefined, String(toRow)), - xmlElement("xdr:rowOff", undefined, "0"), + xmlElement("xdr:rowOff", undefined, String(imageCoordinate(img.anchor.to?.rowOff))), ]) const cNvPrAttrs: Record = { @@ -168,7 +172,7 @@ export function writeDrawing( const spPr = xmlElement("xdr:spPr", undefined, [ xmlElement("a:xfrm", undefined, [ - xmlSelfClose("a:off", { x: 0, y: 0 }), + xmlSelfClose("a:off", { x: imageCoordinate(position?.x), y: imageCoordinate(position?.y) }), xmlSelfClose("a:ext", { cx: widthEmu, cy: heightEmu }), ]), xmlElement("a:prstGeom", { prst: "rect" }, [xmlSelfClose("a:avLst")]), @@ -176,9 +180,27 @@ export function writeDrawing( const pic = xmlElement("xdr:pic", undefined, [nvPicPr, blipFill, spPr]) - const anchor = xmlElement("xdr:twoCellAnchor", undefined, [ - fromElement, - toElement, + const anchorTag = + kind === "absolute" + ? "xdr:absoluteAnchor" + : kind === "oneCell" + ? "xdr:oneCellAnchor" + : "xdr:twoCellAnchor" + const anchorAttrs = kind === "twoCell" && editAs ? { editAs } : undefined + const geometry = + kind === "absolute" + ? [ + xmlSelfClose("xdr:pos", { + x: imageCoordinate(position?.x), + y: imageCoordinate(position?.y), + }), + xmlSelfClose("xdr:ext", { cx: widthEmu, cy: heightEmu }), + ] + : kind === "oneCell" + ? [fromElement, xmlSelfClose("xdr:ext", { cx: widthEmu, cy: heightEmu })] + : [fromElement, toElement] + const anchor = xmlElement(anchorTag, anchorAttrs, [ + ...geometry, pic, xmlSelfClose("xdr:clientData"), ]) @@ -403,3 +425,25 @@ export function writeDrawing( charts: chartList, } } + +/** Do not serialize NaN/Infinity or fractional EMUs from public write inputs. */ +function imageCoordinate(value: number | undefined): number { + return value !== undefined && Number.isFinite(value) && Number.isSafeInteger(Math.round(value)) + ? Math.round(value) + : 0 +} + +function imageExtent( + emu: number | undefined, + pixels: number | undefined, + fallback: number, +): number { + if (emu !== undefined && Number.isSafeInteger(emu) && emu >= 0) return emu + const value = pixels === undefined ? undefined : pixels * EMU_PER_PIXEL + return value !== undefined && + Number.isFinite(value) && + value >= 0 && + Number.isSafeInteger(Math.round(value)) + ? Math.round(value) + : fallback +} diff --git a/src/xlsx/reader.ts b/src/xlsx/reader.ts index 7b72a2f4..af7e115b 100644 --- a/src/xlsx/reader.ts +++ b/src/xlsx/reader.ts @@ -43,6 +43,7 @@ import { parseContentTypes } from "./content-types" import { dirname, findRIdAttr, parseRelationships, resolvePath } from "./relationships" import { parseSharedStrings } from "./shared-strings" import { parseStyles } from "./styles" +import { readDrawingLayout } from "./drawing-layout" import { parseWorksheet, parseWorksheetStream } from "./worksheet" import type { WorksheetContext } from "./worksheet" import { parseDynamicArrayCellMetadata } from "./metadata" @@ -785,10 +786,9 @@ export async function readXlsx(input: ReadInput, options?: ReadOptions): Promise workbook.activeSheet = activeSheet } - // The workbook's default font is fonts[0] in styles.xml — the entry - // every xf inherits from unless it names another. Surfacing it closes - // the WriteOptions.defaultFont round trip. - const baseFont = parsedStyles?.fonts[0] + // Column-width units use the builtin Normal style, which may reference + // any cellStyleXf/font. Keep the font table itself untouched for cell XFs. + const baseFont = parsedStyles?.normalFont ?? parsedStyles?.fonts[0] if (baseFont && Object.keys(baseFont).length > 0) { workbook.defaultFont = baseFont } @@ -989,7 +989,7 @@ async function extractSheetDrawing( const img: SheetImage = { data, type: imageInfo.type, - anchor: imageInfo.anchor, + anchor: { ...imageInfo.anchor, ...readDrawingLayout(child) }, } if (imageInfo.width !== undefined) img.width = imageInfo.width if (imageInfo.height !== undefined) img.height = imageInfo.height @@ -998,7 +998,7 @@ async function extractSheetDrawing( images.push(img) } } - } else if (local === "oneCellAnchor") { + } else if (local === "oneCellAnchor" || local === "absoluteAnchor") { const imageInfo = parseOneCellAnchor(child, imageRelMap) if (imageInfo) { const imagePath = imageInfo.mediaPath @@ -1007,7 +1007,7 @@ async function extractSheetDrawing( const img: SheetImage = { data, type: imageInfo.type, - anchor: imageInfo.anchor, + anchor: { ...imageInfo.anchor, ...readDrawingLayout(child) }, } if (imageInfo.width !== undefined) img.width = imageInfo.width if (imageInfo.height !== undefined) img.height = imageInfo.height @@ -1191,8 +1191,9 @@ function findShapeExtent(shapeEl: { const cy = Number(ext.attrs["cy"]) // A zero / absent extent is a placeholder, not a 0×0 shape — the chart // writer emits exactly that. Report nothing rather than a size of 0. - if (!(cx > 0) || !(cy > 0)) return undefined - return { width: Math.round(cx / EMU_PER_PIXEL), height: Math.round(cy / EMU_PER_PIXEL) } + if (!Number.isSafeInteger(cx) || !Number.isSafeInteger(cy) || !(cx > 0) || !(cy > 0)) + return undefined + return { width: cx / EMU_PER_PIXEL, height: cy / EMU_PER_PIXEL } } /** Parse a twoCellAnchor element that contains a textbox shape (sp with txBox="1") */ @@ -1497,11 +1498,11 @@ function parseOneCellAnchor( }, } - if (widthEmu > 0) { - result.width = Math.round(widthEmu / EMU_PER_PIXEL) + if (Number.isSafeInteger(widthEmu) && widthEmu > 0) { + result.width = widthEmu / EMU_PER_PIXEL } - if (heightEmu > 0) { - result.height = Math.round(heightEmu / EMU_PER_PIXEL) + if (Number.isSafeInteger(heightEmu) && heightEmu > 0) { + result.height = heightEmu / EMU_PER_PIXEL } if (altText) result.altText = altText if (title) result.title = title diff --git a/src/xlsx/styles.ts b/src/xlsx/styles.ts index f175a317..f1a0677d 100644 --- a/src/xlsx/styles.ts +++ b/src/xlsx/styles.ts @@ -24,6 +24,8 @@ import { FPB_XF_EXT_URI } from "./feature-property-bag" export interface ParsedStyles { numFmts: Map fonts: FontStyle[] + /** Resolved through builtin Normal → cellStyleXfs → fonts, never by array order. */ + normalFont?: FontStyle fills: FillStyle[] borders: BorderStyle[] cellXfs: CellXf[] @@ -143,7 +145,23 @@ export function parseStyles(xml: string): ParsedStyles { const palette = readIndexedPalette(doc) for (const group of [fonts, fills, borders, dxfs]) resolveIndexed(group, palette) - return { numFmts, fonts, fills, borders, cellXfs, dxfs } + const normal = elementChildren(doc, "cellStyles") + .flatMap((el) => elementChildren(el, "cellStyle")) + .find((el) => el.attrs["builtinId"] === "0") + const styleXfs = elementChildren(doc, "cellStyleXfs").flatMap((el) => elementChildren(el, "xf")) + const xfId = normal ? Number(normal.attrs["xfId"]) : 0 + const fontId = + Number.isInteger(xfId) && xfId >= 0 ? Number(styleXfs[xfId]?.attrs["fontId"] ?? 0) : 0 + const normalFont = + (Number.isInteger(fontId) && fontId >= 0 ? fonts[fontId] : undefined) ?? fonts[0] + return { numFmts, fonts, fills, borders, cellXfs, dxfs, normalFont } +} + +function elementChildren(parent: XmlElement, local: string): XmlElement[] { + return parent.children.filter( + (child): child is XmlElement => + typeof child !== "string" && (child.local || child.tag) === local, + ) } // ── Indexed colours ────────────────────────────────────────────────── diff --git a/test/coverage-xlsx-reader.test.ts b/test/coverage-xlsx-reader.test.ts index 2f07578b..8d267156 100644 --- a/test/coverage-xlsx-reader.test.ts +++ b/test/coverage-xlsx-reader.test.ts @@ -604,7 +604,7 @@ describe("drawing images", () => { expect(wb.sheets[0].images![0]).toEqual({ data: PNG, type: "png", - anchor: { from: { row: 3, col: 2 } }, + anchor: { from: { row: 3, col: 2 }, kind: "oneCell", extent: { cx: 952500, cy: 476250 } }, width: 100, height: 50, altText: "A logo", diff --git a/test/drawing-layout.test.ts b/test/drawing-layout.test.ts new file mode 100644 index 00000000..13a2d2a0 --- /dev/null +++ b/test/drawing-layout.test.ts @@ -0,0 +1,123 @@ +import { describe, expect, it } from "vitest" +import { parseXml } from "../src/xml/parser" +import { readDrawingLayout } from "../src/xlsx/drawing-layout" +import { parseStyles } from "../src/xlsx/styles" + +function layout(xml: string) { + return readDrawingLayout(parseXml(xml)) +} + +function picture(extent: string, position = "") { + return `${position}${extent}` +} + +const extent = '' + +describe("DrawingML metadata ownership", () => { + it("ignores unrelated containers", () => { + expect(layout(`${picture(extent)}`)).toEqual({}) + }) + + it("does not invent an extent or behavior for incomplete anchors", () => { + expect(layout('text')).toEqual({ + kind: "twoCell", + }) + expect(layout(`${picture("")}`)).toEqual({ + kind: "twoCell", + }) + }) + + for (const editAs of ["twoCell", "oneCell", "absolute"]) { + it(`retains the explicit ${editAs} behavior`, () => { + const xml = `${picture(extent)}` + expect(layout(xml)).toEqual({ + kind: "twoCell", + editAs, + extent: { cx: 1252822, cy: 962025 }, + }) + }) + } + + it("takes one-cell extents from the container rather than a stale picture transform", () => { + const xml = `${picture(extent)}` + expect(layout(xml)).toEqual({ kind: "oneCell", extent: { cx: 0, cy: 1 } }) + }) + + it("keeps signed absolute coordinates without floating-point pixel conversion", () => { + expect(layout('')).toEqual({ + kind: "absolute", + position: { x: -3175, y: 19051 }, + }) + const xml = picture(extent, '') + expect(layout(`${xml}`).position).toEqual({ + x: 10, + y: 20, + }) + expect(layout(`${xml}`).position).toBeUndefined() + }) + + for (const attribute of ["cx", "cy"]) { + for (const value of ["", "NaN", "Infinity", "1.5", "-1", "9007199254740992"]) { + it(`rejects invalid ${attribute}=${value}`, () => { + const ext = extent.replace(new RegExp(`${attribute}="[^"]*"`), `${attribute}="${value}"`) + expect(layout(`${picture(ext)}`).extent).toBeUndefined() + }) + } + } + + for (const attrs of ['cx="1"', 'cy="1"', ""]) { + it(`rejects incomplete extent ${attrs}`, () => { + const xml = `` + expect(layout(xml).extent).toBeUndefined() + }) + } + + it("accepts valid signed and whitespace-padded integer coordinates", () => { + const xml = '' + expect(layout(xml).extent).toEqual({ cx: 1, cy: 2 }) + }) + + it("finds fallback pictures through compatibility wrappers, not grouped transforms", () => { + const wrapped = `\n\n${picture(extent)}` + expect(layout(`${wrapped}`).extent).toEqual({ + cx: 1252822, + cy: 962025, + }) + const group = `${picture(extent)}` + expect(layout(`${group}`).extent).toBeUndefined() + expect(layout(`text`).extent).toBeUndefined() + }) + + it("supports local-name fallback on caller-created XML nodes", () => { + const node = parseXml(`${picture(extent)}`) + node.local = "" + expect(readDrawingLayout(node).extent).toEqual({ cx: 1252822, cy: 962025 }) + }) +}) + +function styles(xf = "1", font = "1") { + return parseStyles(` + + + + `) +} + +describe("Normal style indirection", () => { + it("uses builtin Normal rather than font table order", () => { + expect(styles().normalFont?.name).toBe("Normal") + }) + + for (const value of ["-1", "1.5", "999", "NaN"]) { + it(`falls back for an invalid style or font reference ${value}`, () => { + expect(styles(value).normalFont?.name).toBe("Fallback") + expect(styles("1", value).normalFont?.name).toBe("Fallback") + }) + } + + it("tolerates missing styles and fonts", () => { + expect(parseStyles("").normalFont).toBeUndefined() + const xml = '' + expect(parseStyles(xml).normalFont?.name).toBe("Only") + }) +}) diff --git a/test/excel365-in-cell-images.test.ts b/test/excel365-in-cell-images.test.ts index 53aebc01..547fe9b2 100644 --- a/test/excel365-in-cell-images.test.ts +++ b/test/excel365-in-cell-images.test.ts @@ -129,9 +129,15 @@ describe("Excel 365 in-cell and AlternateContent pictures", () => { expect(sheet.images).toHaveLength(1) expect(sheet.images?.[0]?.data.at(-1)).toBe(2) expect(sheet.images?.[0]?.anchor).toEqual({ + kind: "twoCell", + extent: { cx: 2257798, cy: 2562028 }, from: { row: 8, col: 4, rowOff: 77884, colOff: 636399 }, to: { row: 19, col: 7, rowOff: 125312, colOff: 36697 }, }) - expect(sheet.images?.[0]).toMatchObject({ width: 237, height: 269, altText: "Preview" }) + expect(sheet.images?.[0]).toMatchObject({ + width: 2257798 / 9525, + height: 2562028 / 9525, + altText: "Preview", + }) }) }) diff --git a/test/xlsx-write-read-parity.test.ts b/test/xlsx-write-read-parity.test.ts index 6dd0b044..8d16212f 100644 --- a/test/xlsx-write-read-parity.test.ts +++ b/test/xlsx-write-read-parity.test.ts @@ -185,7 +185,12 @@ const SHEET_FIELDS: { [K in keyof Required]: Entry } expected: [ { type: "png", - anchor: { from: { row: 4, col: 0 }, to: { row: 9, col: 3 } }, + anchor: { + from: { row: 4, col: 0 }, + to: { row: 9, col: 3 }, + kind: "twoCell", + extent: { cx: 1143000, cy: 762000 }, + }, width: 120, height: 80, altText: "a chart of nothing",