From 9cda36165ae1a1a51bb9f997aab754b55da1031c Mon Sep 17 00:00:00 2001 From: "Yu, Wang" <727842003@qq.com> Date: Sat, 19 Sep 2026 17:58:22 +0800 Subject: [PATCH 01/12] chore: stage verified drawing metadata source delta for branch integration --- scripts/.drawing-layout-source.patch | 254 +++++++++++++++++++++++++++ 1 file changed, 254 insertions(+) create mode 100644 scripts/.drawing-layout-source.patch diff --git a/scripts/.drawing-layout-source.patch b/scripts/.drawing-layout-source.patch new file mode 100644 index 00000000..1f1d4b81 --- /dev/null +++ b/scripts/.drawing-layout-source.patch @@ -0,0 +1,254 @@ +diff --git a/src/_types.ts b/src/_types.ts +--- a/src/_types.ts ++++ b/src/_types.ts +@@ -668,6 +668,14 @@ + 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/xlsx/styles.ts b/src/xlsx/styles.ts +--- a/src/xlsx/styles.ts ++++ b/src/xlsx/styles.ts +@@ -24,6 +24,8 @@ + 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,22 @@ + 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/src/xlsx/reader.ts b/src/xlsx/reader.ts +--- a/src/xlsx/reader.ts ++++ b/src/xlsx/reader.ts +@@ -43,6 +43,7 @@ + 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 @@ + 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 @@ + 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 @@ + 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 @@ + 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,8 @@ + 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 +1497,11 @@ + }, + } + +- if (widthEmu > 0) { +- result.width = Math.round(widthEmu / EMU_PER_PIXEL) +- } +- if (heightEmu > 0) { +- result.height = Math.round(heightEmu / EMU_PER_PIXEL) ++ if (Number.isSafeInteger(widthEmu) && widthEmu > 0) { ++ result.width = widthEmu / 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/drawing-writer.ts b/src/xlsx/drawing-writer.ts +--- a/src/xlsx/drawing-writer.ts ++++ b/src/xlsx/drawing-writer.ts +@@ -125,11 +125,15 @@ + }), + ) + +- // 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 @@ + + 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 @@ + + 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,11 +180,18 @@ + + const pic = xmlElement("xdr:pic", undefined, [nvPicPr, blipFill, spPr]) + +- const anchor = xmlElement("xdr:twoCellAnchor", undefined, [ ++ 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, +- toElement, +- pic, +- xmlSelfClose("xdr:clientData"), ++ xmlSelfClose("xdr:ext", { cx: widthEmu, cy: heightEmu }), ++ ] : [fromElement, toElement] ++ const anchor = xmlElement(anchorTag, anchorAttrs, [ ++ ...geometry, pic, xmlSelfClose("xdr:clientData"), + ]) + + anchorElements.push(anchor) +@@ -403,3 +414,16 @@ + 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/sheet-ops.ts b/src/sheet-ops.ts +--- a/src/sheet-ops.ts ++++ b/src/sheet-ops.ts +@@ -1021,6 +1021,8 @@ + 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/package.json b/package.json +--- 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", From 051ae878604b23864e411d3e1332687ecf1e96e9 Mon Sep 17 00:00:00 2001 From: "Yu, Wang" <727842003@qq.com> Date: Sat, 19 Sep 2026 17:59:28 +0800 Subject: [PATCH 02/12] test: add DrawingML contract regressions and branch integration check --- .../workflows/drawing-layout-integration.yml | 49 +++++++ docs/drawing-layout.md | 29 ++++ scripts/verify-drawing-layout.mjs | 126 ++++++++++++++++++ src/xlsx/drawing-layout.ts | 41 ++++++ 4 files changed, 245 insertions(+) create mode 100644 .github/workflows/drawing-layout-integration.yml create mode 100644 docs/drawing-layout.md create mode 100644 scripts/verify-drawing-layout.mjs create mode 100644 src/xlsx/drawing-layout.ts diff --git a/.github/workflows/drawing-layout-integration.yml b/.github/workflows/drawing-layout-integration.yml new file mode 100644 index 00000000..34a39b03 --- /dev/null +++ b/.github/workflows/drawing-layout-integration.yml @@ -0,0 +1,49 @@ +name: Drawing layout integration +on: + push: + branches: [feature/drawing-layout-contract] +permissions: + contents: write +jobs: + integrate: + if: github.repository == 'flyfish-dev/hucre' + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v4 + with: + ref: feature/drawing-layout-contract + fetch-depth: 2 + - name: Apply reviewed source delta to this feature branch only + run: | + set -euo pipefail + test "$(git rev-parse HEAD)" = "$GITHUB_SHA" + if test -f scripts/.drawing-layout-source.patch; then + git apply --check scripts/.drawing-layout-source.patch + git apply --index scripts/.drawing-layout-source.patch + git rm scripts/.drawing-layout-source.patch + git config user.name 'github-actions[bot]' + git config user.email '41898282+github-actions[bot]@users.noreply.github.com' + git commit -m 'fix: preserve exact DrawingML placement and Normal font references' + git push origin HEAD:refs/heads/feature/drawing-layout-contract + fi + - uses: actions/setup-node@v4 + with: + node-version: 24 + - name: Install build dependencies without changing the lockfile + run: npm install --ignore-scripts --package-lock=false + - name: Check public parser and writer contract + run: npm run test:drawing-layout + - name: Type check + run: npm run typecheck + - name: Collect exact source and built runtime for integration checks + if: always() + run: | + git rev-parse HEAD > integration-revision.txt + tar -czf drawing-layout-runtime.tar.gz src dist scripts docs package.json build.config.ts tsconfig.json tsconfig.cli.json integration-revision.txt + - uses: actions/upload-artifact@v4 + if: always() + with: + name: drawing-layout-runtime + path: drawing-layout-runtime.tar.gz + retention-days: 3 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/scripts/verify-drawing-layout.mjs b/scripts/verify-drawing-layout.mjs new file mode 100644 index 00000000..ce77334d --- /dev/null +++ b/scripts/verify-drawing-layout.mjs @@ -0,0 +1,126 @@ +/** 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/xlsx/drawing-layout.ts b/src/xlsx/drawing-layout.ts new file mode 100644 index 00000000..9bc535cf --- /dev/null +++ b/src/xlsx/drawing-layout.ts @@ -0,0 +1,41 @@ +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 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]), y = Number(node.attrs[b]) + return Number.isSafeInteger(x) && Number.isSafeInteger(y) ? [x, y] : undefined +} + +/** + * Container semantics belong to the parser, not a renderer's second ZIP read. + * Keep EMUs intact: rounding to whole 96-DPI pixels here loses precision at + * other DPIs/zoom levels. A twoCellAnchor with editAs=oneCell still has two + * saved markers; consumers need both the markers AND its resizing behavior. + */ +export function readDrawingLayout(anchor: XmlElement): Layout { + const local = anchor.local || anchor.tag + const kind = local === "twoCellAnchor" ? "twoCell" : local === "oneCellAnchor" ? "oneCell" + : local === "absoluteAnchor" ? "absolute" : undefined + if (!kind) 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(child(anchor, "pic"), "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] } + const pos = pair(kind === "absolute" ? child(anchor, "pos") + : editAs === "absolute" ? child(xfrm, "off") : undefined, "x", "y") + if (pos) result.position = { x: pos[0], y: pos[1] } + return result +} From 65d956c22774c070ad15e87a97ae42e02765ac3a Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 19 Sep 2026 09:59:43 +0000 Subject: [PATCH 03/12] fix: preserve exact DrawingML placement and Normal font references --- package.json | 4 +- scripts/.drawing-layout-source.patch | 254 --------------------------- src/_types.ts | 8 + src/sheet-ops.ts | 2 + src/xlsx/drawing-writer.ts | 52 ++++-- src/xlsx/reader.ts | 26 +-- src/xlsx/styles.ts | 19 +- 7 files changed, 82 insertions(+), 283 deletions(-) delete mode 100644 scripts/.drawing-layout-source.patch 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/.drawing-layout-source.patch b/scripts/.drawing-layout-source.patch deleted file mode 100644 index 1f1d4b81..00000000 --- a/scripts/.drawing-layout-source.patch +++ /dev/null @@ -1,254 +0,0 @@ -diff --git a/src/_types.ts b/src/_types.ts ---- a/src/_types.ts -+++ b/src/_types.ts -@@ -668,6 +668,14 @@ - 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/xlsx/styles.ts b/src/xlsx/styles.ts ---- a/src/xlsx/styles.ts -+++ b/src/xlsx/styles.ts -@@ -24,6 +24,8 @@ - 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,22 @@ - 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/src/xlsx/reader.ts b/src/xlsx/reader.ts ---- a/src/xlsx/reader.ts -+++ b/src/xlsx/reader.ts -@@ -43,6 +43,7 @@ - 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 @@ - 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 @@ - 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 @@ - 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 @@ - 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,8 @@ - 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 +1497,11 @@ - }, - } - -- if (widthEmu > 0) { -- result.width = Math.round(widthEmu / EMU_PER_PIXEL) -- } -- if (heightEmu > 0) { -- result.height = Math.round(heightEmu / EMU_PER_PIXEL) -+ if (Number.isSafeInteger(widthEmu) && widthEmu > 0) { -+ result.width = widthEmu / 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/drawing-writer.ts b/src/xlsx/drawing-writer.ts ---- a/src/xlsx/drawing-writer.ts -+++ b/src/xlsx/drawing-writer.ts -@@ -125,11 +125,15 @@ - }), - ) - -- // 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 @@ - - 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 @@ - - 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,11 +180,18 @@ - - const pic = xmlElement("xdr:pic", undefined, [nvPicPr, blipFill, spPr]) - -- const anchor = xmlElement("xdr:twoCellAnchor", undefined, [ -+ 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, -- toElement, -- pic, -- xmlSelfClose("xdr:clientData"), -+ xmlSelfClose("xdr:ext", { cx: widthEmu, cy: heightEmu }), -+ ] : [fromElement, toElement] -+ const anchor = xmlElement(anchorTag, anchorAttrs, [ -+ ...geometry, pic, xmlSelfClose("xdr:clientData"), - ]) - - anchorElements.push(anchor) -@@ -403,3 +414,16 @@ - 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/sheet-ops.ts b/src/sheet-ops.ts ---- a/src/sheet-ops.ts -+++ b/src/sheet-ops.ts -@@ -1021,6 +1021,8 @@ - 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/package.json b/package.json ---- 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/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-writer.ts b/src/xlsx/drawing-writer.ts index c660dffd..985ad218 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,11 +180,18 @@ export function writeDrawing( const pic = xmlElement("xdr:pic", undefined, [nvPicPr, blipFill, spPr]) - const anchor = xmlElement("xdr:twoCellAnchor", undefined, [ + 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, - toElement, - pic, - xmlSelfClose("xdr:clientData"), + xmlSelfClose("xdr:ext", { cx: widthEmu, cy: heightEmu }), + ] : [fromElement, toElement] + const anchor = xmlElement(anchorTag, anchorAttrs, [ + ...geometry, pic, xmlSelfClose("xdr:clientData"), ]) anchorElements.push(anchor) @@ -403,3 +414,16 @@ 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..38711b98 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,8 @@ 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 +1497,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..44a65481 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,22 @@ 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 ────────────────────────────────────────────────── From 37c570697dd4003522d3a68002759ad942791391 Mon Sep 17 00:00:00 2001 From: "Yu, Wang" <727842003@qq.com> Date: Sat, 19 Sep 2026 18:01:08 +0800 Subject: [PATCH 04/12] chore: verify drawing changes with repository formatter and full test suite --- .../workflows/drawing-layout-integration.yml | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/drawing-layout-integration.yml b/.github/workflows/drawing-layout-integration.yml index 34a39b03..4d13c92d 100644 --- a/.github/workflows/drawing-layout-integration.yml +++ b/.github/workflows/drawing-layout-integration.yml @@ -13,29 +13,29 @@ jobs: - uses: actions/checkout@v4 with: ref: feature/drawing-layout-contract - fetch-depth: 2 - - name: Apply reviewed source delta to this feature branch only + - uses: actions/setup-node@v4 + with: + node-version: 24 + - name: Install build dependencies without changing the lockfile + run: npm install --ignore-scripts --package-lock=false + - name: Format only the reviewed changed files run: | set -euo pipefail test "$(git rev-parse HEAD)" = "$GITHUB_SHA" - if test -f scripts/.drawing-layout-source.patch; then - git apply --check scripts/.drawing-layout-source.patch - git apply --index scripts/.drawing-layout-source.patch - git rm scripts/.drawing-layout-source.patch + node_modules/.bin/oxfmt src/_types.ts src/sheet-ops.ts src/xlsx/reader.ts src/xlsx/styles.ts src/xlsx/drawing-writer.ts src/xlsx/drawing-layout.ts scripts/verify-drawing-layout.mjs docs/drawing-layout.md package.json + git add src/_types.ts src/sheet-ops.ts src/xlsx/reader.ts src/xlsx/styles.ts src/xlsx/drawing-writer.ts src/xlsx/drawing-layout.ts scripts/verify-drawing-layout.mjs docs/drawing-layout.md package.json + if ! git diff --cached --quiet; then git config user.name 'github-actions[bot]' git config user.email '41898282+github-actions[bot]@users.noreply.github.com' - git commit -m 'fix: preserve exact DrawingML placement and Normal font references' + git commit -m 'style: format drawing contract changes with repository conventions' git push origin HEAD:refs/heads/feature/drawing-layout-contract fi - - uses: actions/setup-node@v4 - with: - node-version: 24 - - name: Install build dependencies without changing the lockfile - run: npm install --ignore-scripts --package-lock=false - 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: Collect exact source and built runtime for integration checks if: always() run: | From a7e89141b34463d099f450aa1164c9c00f3fb8ed Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 19 Sep 2026 10:01:27 +0000 Subject: [PATCH 05/12] style: format drawing contract changes with repository conventions --- scripts/verify-drawing-layout.mjs | 267 ++++++++++++++++++------------ src/xlsx/drawing-layout.ts | 37 ++++- src/xlsx/drawing-writer.ts | 48 ++++-- src/xlsx/reader.ts | 3 +- src/xlsx/styles.ts | 17 +- 5 files changed, 230 insertions(+), 142 deletions(-) diff --git a/scripts/verify-drawing-layout.mjs b/scripts/verify-drawing-layout.mjs index ce77334d..25615fb5 100644 --- a/scripts/verify-drawing-layout.mjs +++ b/scripts/verify-drawing-layout.mjs @@ -1,126 +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'; +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}`; +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(); + 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('${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]; +const imageOf = async (bytes) => (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 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]]) { +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); - }); + 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("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); - }); + 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("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("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'); - }); + 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')); -}); +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/xlsx/drawing-layout.ts b/src/xlsx/drawing-layout.ts index 9bc535cf..b9b8313d 100644 --- a/src/xlsx/drawing-layout.ts +++ b/src/xlsx/drawing-layout.ts @@ -4,13 +4,16 @@ 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) + return parent?.children.find( + (node): node is XmlElement => typeof node !== "string" && (node.local || node.tag) === name, + ) } 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]), y = Number(node.attrs[b]) + if (!/^[+-]?\d+$/.test(node.attrs[a].trim()) || !/^[+-]?\d+$/.test(node.attrs[b].trim())) + return undefined + const x = Number(node.attrs[a]), + y = Number(node.attrs[b]) return Number.isSafeInteger(x) && Number.isSafeInteger(y) ? [x, y] : undefined } @@ -22,20 +25,36 @@ function pair(node: XmlElement | undefined, a: string, b: string): [number, numb */ export function readDrawingLayout(anchor: XmlElement): Layout { const local = anchor.local || anchor.tag - const kind = local === "twoCellAnchor" ? "twoCell" : local === "oneCellAnchor" ? "oneCell" - : local === "absoluteAnchor" ? "absolute" : undefined + const kind = + local === "twoCellAnchor" + ? "twoCell" + : local === "oneCellAnchor" + ? "oneCell" + : local === "absoluteAnchor" + ? "absolute" + : undefined if (!kind) return {} const result: Layout = { kind } const editAs = anchor.attrs["editAs"] - if (kind === "twoCell" && (editAs === "oneCell" || editAs === "absolute" || editAs === "twoCell")) { + if ( + kind === "twoCell" && + (editAs === "oneCell" || editAs === "absolute" || editAs === "twoCell") + ) { result.editAs = editAs } const xfrm = child(child(child(anchor, "pic"), "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] } - const pos = pair(kind === "absolute" ? child(anchor, "pos") - : editAs === "absolute" ? child(xfrm, "off") : undefined, "x", "y") + const pos = pair( + kind === "absolute" + ? child(anchor, "pos") + : editAs === "absolute" + ? child(xfrm, "off") + : undefined, + "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 985ad218..11c95bc1 100644 --- a/src/xlsx/drawing-writer.ts +++ b/src/xlsx/drawing-writer.ts @@ -180,18 +180,29 @@ export function writeDrawing( const pic = xmlElement("xdr:pic", undefined, [nvPicPr, blipFill, spPr]) - const anchorTag = kind === "absolute" ? "xdr:absoluteAnchor" - : kind === "oneCell" ? "xdr:oneCellAnchor" : "xdr:twoCellAnchor" + 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 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"), + ...geometry, + pic, + xmlSelfClose("xdr:clientData"), ]) anchorElements.push(anchor) @@ -418,12 +429,21 @@ export function writeDrawing( /** 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 + ? Math.round(value) + : 0 } -function imageExtent(emu: number | undefined, pixels: number | undefined, fallback: number): number { +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 + 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 38711b98..af7e115b 100644 --- a/src/xlsx/reader.ts +++ b/src/xlsx/reader.ts @@ -1191,7 +1191,8 @@ 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 (!Number.isSafeInteger(cx) || !Number.isSafeInteger(cy) || !(cx > 0) || !(cy > 0)) return undefined + if (!Number.isSafeInteger(cx) || !Number.isSafeInteger(cy) || !(cx > 0) || !(cy > 0)) + return undefined return { width: cx / EMU_PER_PIXEL, height: cy / EMU_PER_PIXEL } } diff --git a/src/xlsx/styles.ts b/src/xlsx/styles.ts index 44a65481..f1a0677d 100644 --- a/src/xlsx/styles.ts +++ b/src/xlsx/styles.ts @@ -148,19 +148,20 @@ export function parseStyles(xml: string): ParsedStyles { 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 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] + 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) + return parent.children.filter( + (child): child is XmlElement => + typeof child !== "string" && (child.local || child.tag) === local, + ) } // ── Indexed colours ────────────────────────────────────────────────── From 0b0109d4b2dab8ddfbdbc87e9f41e1611c3ca313 Mon Sep 17 00:00:00 2001 From: "Yu, Wang" <727842003@qq.com> Date: Sat, 19 Sep 2026 18:06:17 +0800 Subject: [PATCH 06/12] test: align existing image assertions with additive DrawingML contract --- .../workflows/drawing-layout-integration.yml | 15 ++++---- scripts/.drawing-test-contract.patch | 35 +++++++++++++++++++ 2 files changed, 44 insertions(+), 6 deletions(-) create mode 100644 scripts/.drawing-test-contract.patch diff --git a/.github/workflows/drawing-layout-integration.yml b/.github/workflows/drawing-layout-integration.yml index 4d13c92d..66b80d86 100644 --- a/.github/workflows/drawing-layout-integration.yml +++ b/.github/workflows/drawing-layout-integration.yml @@ -18,16 +18,19 @@ jobs: node-version: 24 - name: Install build dependencies without changing the lockfile run: npm install --ignore-scripts --package-lock=false - - name: Format only the reviewed changed files + - name: Integrate additive contract expectations run: | set -euo pipefail test "$(git rev-parse HEAD)" = "$GITHUB_SHA" - node_modules/.bin/oxfmt src/_types.ts src/sheet-ops.ts src/xlsx/reader.ts src/xlsx/styles.ts src/xlsx/drawing-writer.ts src/xlsx/drawing-layout.ts scripts/verify-drawing-layout.mjs docs/drawing-layout.md package.json - git add src/_types.ts src/sheet-ops.ts src/xlsx/reader.ts src/xlsx/styles.ts src/xlsx/drawing-writer.ts src/xlsx/drawing-layout.ts scripts/verify-drawing-layout.mjs docs/drawing-layout.md package.json - if ! git diff --cached --quiet; then + if test -f scripts/.drawing-test-contract.patch; then + git apply --check scripts/.drawing-test-contract.patch + git apply --index scripts/.drawing-test-contract.patch + git rm scripts/.drawing-test-contract.patch + node_modules/.bin/oxfmt test/coverage-xlsx-reader.test.ts test/excel365-in-cell-images.test.ts test/xlsx-write-read-parity.test.ts + git add test/coverage-xlsx-reader.test.ts test/excel365-in-cell-images.test.ts test/xlsx-write-read-parity.test.ts git config user.name 'github-actions[bot]' git config user.email '41898282+github-actions[bot]@users.noreply.github.com' - git commit -m 'style: format drawing contract changes with repository conventions' + git commit -m 'test: expect preserved drawing metadata without weakening parity checks' git push origin HEAD:refs/heads/feature/drawing-layout-contract fi - name: Check public parser and writer contract @@ -40,7 +43,7 @@ jobs: if: always() run: | git rev-parse HEAD > integration-revision.txt - tar -czf drawing-layout-runtime.tar.gz src dist scripts docs package.json build.config.ts tsconfig.json tsconfig.cli.json 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@v4 if: always() with: diff --git a/scripts/.drawing-test-contract.patch b/scripts/.drawing-test-contract.patch new file mode 100644 index 00000000..6b64358e --- /dev/null +++ b/scripts/.drawing-test-contract.patch @@ -0,0 +1,35 @@ +diff --git a/test/coverage-xlsx-reader.test.ts b/test/coverage-xlsx-reader.test.ts +--- a/test/coverage-xlsx-reader.test.ts ++++ b/test/coverage-xlsx-reader.test.ts +@@ -604,7 +604,7 @@ + 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/excel365-in-cell-images.test.ts b/test/excel365-in-cell-images.test.ts +--- a/test/excel365-in-cell-images.test.ts ++++ b/test/excel365-in-cell-images.test.ts +@@ -129,6 +129,7 @@ + expect(sheet.images).toHaveLength(1) + expect(sheet.images?.[0]?.data.at(-1)).toBe(2) + expect(sheet.images?.[0]?.anchor).toEqual({ ++ kind: "twoCell", + from: { row: 8, col: 4, rowOff: 77884, colOff: 636399 }, + to: { row: 19, col: 7, rowOff: 125312, colOff: 36697 }, + }) +diff --git a/test/xlsx-write-read-parity.test.ts b/test/xlsx-write-read-parity.test.ts +--- a/test/xlsx-write-read-parity.test.ts ++++ b/test/xlsx-write-read-parity.test.ts +@@ -185,7 +185,7 @@ + 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", From 8c950e5150f4461d722eda65bb0a4836475819ff Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 19 Sep 2026 10:06:46 +0000 Subject: [PATCH 07/12] test: expect preserved drawing metadata without weakening parity checks --- scripts/.drawing-test-contract.patch | 35 ---------------------------- test/coverage-xlsx-reader.test.ts | 2 +- test/excel365-in-cell-images.test.ts | 1 + test/xlsx-write-read-parity.test.ts | 7 +++++- 4 files changed, 8 insertions(+), 37 deletions(-) delete mode 100644 scripts/.drawing-test-contract.patch diff --git a/scripts/.drawing-test-contract.patch b/scripts/.drawing-test-contract.patch deleted file mode 100644 index 6b64358e..00000000 --- a/scripts/.drawing-test-contract.patch +++ /dev/null @@ -1,35 +0,0 @@ -diff --git a/test/coverage-xlsx-reader.test.ts b/test/coverage-xlsx-reader.test.ts ---- a/test/coverage-xlsx-reader.test.ts -+++ b/test/coverage-xlsx-reader.test.ts -@@ -604,7 +604,7 @@ - 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/excel365-in-cell-images.test.ts b/test/excel365-in-cell-images.test.ts ---- a/test/excel365-in-cell-images.test.ts -+++ b/test/excel365-in-cell-images.test.ts -@@ -129,6 +129,7 @@ - expect(sheet.images).toHaveLength(1) - expect(sheet.images?.[0]?.data.at(-1)).toBe(2) - expect(sheet.images?.[0]?.anchor).toEqual({ -+ kind: "twoCell", - from: { row: 8, col: 4, rowOff: 77884, colOff: 636399 }, - to: { row: 19, col: 7, rowOff: 125312, colOff: 36697 }, - }) -diff --git a/test/xlsx-write-read-parity.test.ts b/test/xlsx-write-read-parity.test.ts ---- a/test/xlsx-write-read-parity.test.ts -+++ b/test/xlsx-write-read-parity.test.ts -@@ -185,7 +185,7 @@ - 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", 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/excel365-in-cell-images.test.ts b/test/excel365-in-cell-images.test.ts index 53aebc01..946c072a 100644 --- a/test/excel365-in-cell-images.test.ts +++ b/test/excel365-in-cell-images.test.ts @@ -129,6 +129,7 @@ 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", from: { row: 8, col: 4, rowOff: 77884, colOff: 636399 }, to: { row: 19, col: 7, rowOff: 125312, colOff: 36697 }, }) 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", From 638a8f29f26883fa03ee254259e395c7d9e962e6 Mon Sep 17 00:00:00 2001 From: "Yu, Wang" <727842003@qq.com> Date: Sat, 19 Sep 2026 18:11:00 +0800 Subject: [PATCH 08/12] fix: retain AlternateContent picture extents and finalize read-only regression CI --- .../workflows/drawing-layout-integration.yml | 38 ++++++----------- src/xlsx/drawing-layout.ts | 42 ++++++++++++------- test/excel365-in-cell-images.test.ts | 7 +++- 3 files changed, 47 insertions(+), 40 deletions(-) diff --git a/.github/workflows/drawing-layout-integration.yml b/.github/workflows/drawing-layout-integration.yml index 66b80d86..f6b74791 100644 --- a/.github/workflows/drawing-layout-integration.yml +++ b/.github/workflows/drawing-layout-integration.yml @@ -1,50 +1,38 @@ name: Drawing layout integration on: + pull_request: push: - branches: [feature/drawing-layout-contract] + branches: [main, feature/drawing-layout-contract] permissions: - contents: write + contents: read +concurrency: + group: drawing-layout-${{ github.ref }} + cancel-in-progress: true jobs: - integrate: - if: github.repository == 'flyfish-dev/hucre' + check: runs-on: ubuntu-latest timeout-minutes: 20 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: - ref: feature/drawing-layout-contract - - uses: actions/setup-node@v4 + persist-credentials: false + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 24 - - name: Install build dependencies without changing the lockfile + - name: Install build dependencies run: npm install --ignore-scripts --package-lock=false - - name: Integrate additive contract expectations - run: | - set -euo pipefail - test "$(git rev-parse HEAD)" = "$GITHUB_SHA" - if test -f scripts/.drawing-test-contract.patch; then - git apply --check scripts/.drawing-test-contract.patch - git apply --index scripts/.drawing-test-contract.patch - git rm scripts/.drawing-test-contract.patch - node_modules/.bin/oxfmt test/coverage-xlsx-reader.test.ts test/excel365-in-cell-images.test.ts test/xlsx-write-read-parity.test.ts - git add test/coverage-xlsx-reader.test.ts test/excel365-in-cell-images.test.ts test/xlsx-write-read-parity.test.ts - git config user.name 'github-actions[bot]' - git config user.email '41898282+github-actions[bot]@users.noreply.github.com' - git commit -m 'test: expect preserved drawing metadata without weakening parity checks' - git push origin HEAD:refs/heads/feature/drawing-layout-contract - 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: Collect exact source and built runtime for integration checks + - name: Preserve exact build and tests for integration verification if: always() 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@v4 + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 if: always() with: name: drawing-layout-runtime diff --git a/src/xlsx/drawing-layout.ts b/src/xlsx/drawing-layout.ts index b9b8313d..25856c84 100644 --- a/src/xlsx/drawing-layout.ts +++ b/src/xlsx/drawing-layout.ts @@ -8,21 +8,38 @@ function child(parent: XmlElement | undefined, name: string): XmlElement | undef (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. A grouped child's transform is not + // expressed in the anchor's coordinate system and must not leak out here. + 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())) + if (!/^[+-]?\d+$/.test(node.attrs[a].trim()) || !/^[+-]?\d+$/.test(node.attrs[b].trim())) { return undefined - const x = Number(node.attrs[a]), - y = Number(node.attrs[b]) + } + const x = Number(node.attrs[a]) + const y = Number(node.attrs[b]) return Number.isSafeInteger(x) && Number.isSafeInteger(y) ? [x, y] : undefined } -/** - * Container semantics belong to the parser, not a renderer's second ZIP read. - * Keep EMUs intact: rounding to whole 96-DPI pixels here loses precision at - * other DPIs/zoom levels. A twoCellAnchor with editAs=oneCell still has two - * saved markers; consumers need both the markers AND its resizing behavior. - */ +/** 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 const kind = @@ -36,13 +53,10 @@ export function readDrawingLayout(anchor: XmlElement): Layout { if (!kind) return {} const result: Layout = { kind } const editAs = anchor.attrs["editAs"] - if ( - kind === "twoCell" && - (editAs === "oneCell" || editAs === "absolute" || editAs === "twoCell") - ) { + if (kind === "twoCell" && (editAs === "oneCell" || editAs === "absolute" || editAs === "twoCell")) { result.editAs = editAs } - const xfrm = child(child(child(anchor, "pic"), "spPr"), "xfrm") + 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] } diff --git a/test/excel365-in-cell-images.test.ts b/test/excel365-in-cell-images.test.ts index 946c072a..547fe9b2 100644 --- a/test/excel365-in-cell-images.test.ts +++ b/test/excel365-in-cell-images.test.ts @@ -130,9 +130,14 @@ describe("Excel 365 in-cell and AlternateContent pictures", () => { 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", + }) }) }) From 96025767fe91aa02db865f2ea0dfb7572d3d27e3 Mon Sep 17 00:00:00 2001 From: "Yu, Wang" <727842003@qq.com> Date: Sat, 19 Sep 2026 18:29:40 +0800 Subject: [PATCH 09/12] refactor: clarify DrawingML container and position selection --- src/xlsx/drawing-layout.ts | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/src/xlsx/drawing-layout.ts b/src/xlsx/drawing-layout.ts index 25856c84..04c2ad42 100644 --- a/src/xlsx/drawing-layout.ts +++ b/src/xlsx/drawing-layout.ts @@ -12,8 +12,8 @@ function child(parent: XmlElement | undefined, name: string): XmlElement | undef function picture(anchor: XmlElement): XmlElement | undefined { const direct = child(anchor, "pic") if (direct) return direct - // Follow compatibility wrappers only. A grouped child's transform is not - // expressed in the anchor's coordinate system and must not leak out here. + // 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() @@ -42,15 +42,11 @@ function pair(node: XmlElement | undefined, a: string, b: string): [number, numb /** 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 - const kind = - local === "twoCellAnchor" - ? "twoCell" - : local === "oneCellAnchor" - ? "oneCell" - : local === "absoluteAnchor" - ? "absolute" - : undefined - if (!kind) return {} + 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")) { @@ -60,15 +56,10 @@ export function readDrawingLayout(anchor: XmlElement): Layout { 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] } - const pos = pair( - kind === "absolute" - ? child(anchor, "pos") - : editAs === "absolute" - ? child(xfrm, "off") - : undefined, - "x", - "y", - ) + 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 } From 8820f83f7062f87d6c9795309d566bc7bf591edb Mon Sep 17 00:00:00 2001 From: "Yu, Wang" <727842003@qq.com> Date: Sat, 19 Sep 2026 18:32:35 +0800 Subject: [PATCH 10/12] test: cover drawing metadata validation and measure unchanged coverage baseline --- .../workflows/drawing-layout-integration.yml | 32 +++++ test/drawing-layout.test.ts | 123 ++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 test/drawing-layout.test.ts diff --git a/.github/workflows/drawing-layout-integration.yml b/.github/workflows/drawing-layout-integration.yml index f6b74791..bb090d30 100644 --- a/.github/workflows/drawing-layout-integration.yml +++ b/.github/workflows/drawing-layout-integration.yml @@ -38,3 +38,35 @@ jobs: 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/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") + }) +}) From 34b14c476281bbee7c9f87e5078dfc109cd08b74 Mon Sep 17 00:00:00 2001 From: "Yu, Wang" <727842003@qq.com> Date: Sat, 19 Sep 2026 18:35:38 +0800 Subject: [PATCH 11/12] ci: show precise drawing formatter differences instead of filenames only --- .github/workflows/drawing-layout-integration.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/drawing-layout-integration.yml b/.github/workflows/drawing-layout-integration.yml index bb090d30..8316b283 100644 --- a/.github/workflows/drawing-layout-integration.yml +++ b/.github/workflows/drawing-layout-integration.yml @@ -21,6 +21,13 @@ jobs: 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 From d2ec74c41e551fc4f7412c3ee5678f07241ef86a Mon Sep 17 00:00:00 2001 From: "Yu, Wang" <727842003@qq.com> Date: Sat, 19 Sep 2026 18:37:13 +0800 Subject: [PATCH 12/12] style: apply repository formatter output and archive successful builds only --- .github/workflows/drawing-layout-integration.yml | 4 +--- src/xlsx/drawing-layout.ts | 5 ++++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/drawing-layout-integration.yml b/.github/workflows/drawing-layout-integration.yml index 8316b283..2d9efa24 100644 --- a/.github/workflows/drawing-layout-integration.yml +++ b/.github/workflows/drawing-layout-integration.yml @@ -34,13 +34,11 @@ jobs: run: npm run typecheck - name: Full library regression suite run: node_modules/.bin/vitest run - - name: Preserve exact build and tests for integration verification - if: always() + - 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 - if: always() with: name: drawing-layout-runtime path: drawing-layout-runtime.tar.gz diff --git a/src/xlsx/drawing-layout.ts b/src/xlsx/drawing-layout.ts index 04c2ad42..159e9687 100644 --- a/src/xlsx/drawing-layout.ts +++ b/src/xlsx/drawing-layout.ts @@ -49,7 +49,10 @@ export function readDrawingLayout(anchor: XmlElement): Layout { else return {} const result: Layout = { kind } const editAs = anchor.attrs["editAs"] - if (kind === "twoCell" && (editAs === "oneCell" || editAs === "absolute" || editAs === "twoCell")) { + if ( + kind === "twoCell" && + (editAs === "oneCell" || editAs === "absolute" || editAs === "twoCell") + ) { result.editAs = editAs } const xfrm = child(child(picture(anchor), "spPr"), "xfrm")