Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions src/_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -659,15 +659,24 @@ export interface ThreadedComment {
export interface SheetImage {
data: Uint8Array
type: "png" | "jpeg" | "gif" | "svg" | "webp"
/** Anchor to cell */
/**
* Anchor to cell. With `to`, the image is written as a `twoCellAnchor`
* and stretched to fill `from`–`to` ("Move and size with cells"). Without
* it, the image is written as a `oneCellAnchor` at `from`, sized by
* `width` / `height` ("Move but don't size with cells"), and reads back
* without a `to`. See #598.
*/
anchor: {
from: { row: number; col: number }
to?: { row: number; col: number }
}
/**
* Rendered size in pixels at 96 DPI, stored as EMU in the drawing's
* `<a:ext>`. Absent on write means the writer's own default size, which
* is then what the reader reports — a file records a size either way.
* Rendered size in pixels at 96 DPI, stored as EMU in the picture's
* `<a:ext>` and, without `anchor.to`, in the `oneCellAnchor`'s own
* `<xdr:ext>`, which is what sets the size there. Absent on write means
* the writer's own default size, which is then what the reader reports —
* a file records a size either way. With `anchor.to`, Excel and
* LibreOffice size the image from the cells instead (#598).
*/
width?: number
height?: number
Expand Down
44 changes: 28 additions & 16 deletions src/xlsx/drawing-writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,8 @@ export function writeDrawing(
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
const fromCol = img.anchor.from.col
const fromRow = img.anchor.from.row
const toCol = img.anchor.to?.col ?? fromCol + 3
const toRow = img.anchor.to?.row ?? fromRow + 5

const fromElement = xmlElement("xdr:from", undefined, [
xmlElement("xdr:col", undefined, String(fromCol)),
Expand All @@ -142,13 +139,6 @@ export function writeDrawing(
xmlElement("xdr:rowOff", undefined, "0"),
])

const toElement = xmlElement("xdr:to", undefined, [
xmlElement("xdr:col", undefined, String(toCol)),
xmlElement("xdr:colOff", undefined, "0"),
xmlElement("xdr:row", undefined, String(toRow)),
xmlElement("xdr:rowOff", undefined, "0"),
])

const cNvPrAttrs: Record<string, string | number> = {
id: i + 2,
name: `Picture ${i + 1}`,
Expand Down Expand Up @@ -176,12 +166,34 @@ export function writeDrawing(

const pic = xmlElement("xdr:pic", undefined, [nvPicPr, blipFill, spPr])

const anchor = xmlElement("xdr:twoCellAnchor", undefined, [
fromElement,
toElement,
pic,
xmlSelfClose("xdr:clientData"),
])
let anchor: string
if (img.anchor.to) {
const toElement = xmlElement("xdr:to", undefined, [
xmlElement("xdr:col", undefined, String(img.anchor.to.col)),
xmlElement("xdr:colOff", undefined, "0"),
xmlElement("xdr:row", undefined, String(img.anchor.to.row)),
xmlElement("xdr:rowOff", undefined, "0"),
])
anchor = xmlElement("xdr:twoCellAnchor", undefined, [
fromElement,
toElement,
pic,
xmlSelfClose("xdr:clientData"),
])
} else {
// No `to` means the size comes from width/height, and only a
// oneCellAnchor can say that: under a twoCellAnchor Excel and
// LibreOffice size the picture from the from/to cells and ignore
// `a:ext`, so the invented `to` this used to write (from + 3 columns,
// + 5 rows) stretched every such image over that box. The anchor-level
// `xdr:ext` is what the reader reports back. See #598.
anchor = xmlElement("xdr:oneCellAnchor", undefined, [
fromElement,
xmlSelfClose("xdr:ext", { cx: widthEmu, cy: heightEmu }),
pic,
xmlSelfClose("xdr:clientData"),
])
}

anchorElements.push(anchor)
}
Expand Down
6 changes: 4 additions & 2 deletions src/xlsx/reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1097,8 +1097,10 @@ function parseTwoCellAnchor(
* On a `twoCellAnchor` the from/to cells are what Excel actually honours,
* but `a:ext` is still required by the schema and every writer — hucre's
* included — records the intended size there. Reading it is the only way
* to recover {@link SheetImage.width} from a hucre-written file, which
* always uses `twoCellAnchor` (#407).
* to recover {@link SheetImage.width} from a `twoCellAnchor`, which is what
* hucre writes for an image with `anchor.to` (#407). An image without `to`
* is written as a `oneCellAnchor`, whose size is the anchor's own
* `<xdr:ext>` — see `parseOneCellAnchor` (#598).
*/
function findShapeExtent(shapeEl: {
children: Array<unknown>
Expand Down
21 changes: 12 additions & 9 deletions test/images.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,20 +174,23 @@ describe("writeDrawing", () => {
expect(result.drawingXml).toContain('r:embed="rId1"')
})

it("uses default to anchor when not specified", () => {
it("uses a oneCellAnchor sized by ext when to is not specified", () => {
const images: SheetImage[] = [makeImage("png", { row: 2, col: 1 })]
const result = writeDrawing(images, 1)

// Default to: from.col + 3, from.row + 5
// No invented `to` corner: the default size goes in the anchor's ext (#598).
const doc = parseXml(result.drawingXml)
const anchor = findChild(doc, "twoCellAnchor")
expect(findChild(doc, "twoCellAnchor")).toBeUndefined()
const anchor = findChild(doc, "oneCellAnchor")
expect(anchor).toBeTruthy()

const toEl = findChild(anchor, "to")
const toCol = findChild(toEl, "col")
const toRow = findChild(toEl, "row")
expect(toCol.children[0]).toBe("4") // 1 + 3
expect(toRow.children[0]).toBe("7") // 2 + 5
expect(findChild(anchor, "to")).toBeUndefined()

const fromEl = findChild(anchor, "from")
expect(findChild(fromEl, "col").children[0]).toBe("1")
expect(findChild(fromEl, "row").children[0]).toBe("2")
const ext = findChild(anchor, "ext")
expect(ext.attrs.cx).toBe("3000000")
expect(ext.attrs.cy).toBe("2000000")
})
})

Expand Down
158 changes: 158 additions & 0 deletions test/one-cell-anchor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,26 @@ import { ZipReader } from "../src/zip/reader"
import { ZipWriter } from "../src/zip/writer"
import { writeXlsx } from "../src/xlsx/writer"
import { readXlsx } from "../src/xlsx/reader"
import { openXlsx, saveXlsx } from "../src/xlsx/roundtrip"
import { writeDrawing } from "../src/xlsx/drawing-writer"
import { parseXml } from "../src/xml/parser"
import type { WriteSheet } from "../src/_types"

// ── Helpers ──────────────────────────────────────────────────────────

const encoder = new TextEncoder()
const decoder = new TextDecoder()

async function drawingXmlOf(xlsx: Uint8Array): Promise<string> {
return decoder.decode(await new ZipReader(xlsx).extract("xl/drawings/drawing1.xml"))
}

/** Local names of an element's child elements, in document order. */
function childNames(el: { children: Array<unknown> }): string[] {
return el.children
.filter((c): c is { local?: string; tag: string } => typeof c !== "string")
.map((c) => c.local || c.tag)
}

/** Create a simple fake PNG-like image */
function fakePng(size = 64): Uint8Array {
Expand Down Expand Up @@ -282,3 +297,146 @@ describe("mixed anchor types", () => {
expect(img2.height).toBe(100)
})
})

// ── oneCellAnchor writing (#598) ─────────────────────────────────────

describe("image without anchor.to is written as oneCellAnchor (#598)", () => {
// The issue's reproduction: a 200×40 px logo anchored at B2 with no `to`.
const logoSheet = (): WriteSheet => ({
name: "Sheet1",
rows: [["x"]],
images: [
{
data: fakePng(),
type: "png",
anchor: { from: { row: 1, col: 1 } },
width: 200,
height: 40,
},
],
})

it("emits oneCellAnchor with from, ext, pic, clientData and no to", async () => {
const xml = await drawingXmlOf(await writeXlsx({ sheets: [logoSheet()] }))

expect(xml).not.toContain("twoCellAnchor")
expect(xml).not.toContain("<xdr:to>")
const doc = parseXml(xml)
expect(childNames(doc)).toEqual(["oneCellAnchor"])
const anchor = doc.children.find((c) => typeof c !== "string") as { children: Array<unknown> }
// CT_OneCellAnchor is a sequence: from, ext, object, clientData.
expect(childNames(anchor)).toEqual(["from", "ext", "pic", "clientData"])
// 200 px × 9525 EMU/px, 40 px × 9525 EMU/px
expect(xml).toContain('<xdr:ext cx="1905000" cy="381000"/>')
})

it("reads back with only `from` and the written size", async () => {
const wb = await readXlsx(await writeXlsx({ sheets: [logoSheet()] }))
const img = wb.sheets[0].images![0]
expect(img.anchor).toEqual({ from: { row: 1, col: 1 } })
expect(img.width).toBe(200)
expect(img.height).toBe(40)
})

it("uses the default size for ext when width/height are absent", async () => {
const xlsx = await writeXlsx({
sheets: [
{
name: "Sheet1",
rows: [["x"]],
images: [{ data: fakePng(), type: "png", anchor: { from: { row: 0, col: 0 } } }],
},
],
})
const xml = await drawingXmlOf(xlsx)
expect(xml).toContain("<xdr:oneCellAnchor>")
expect(xml).toContain('<xdr:ext cx="3000000" cy="2000000"/>')

// The default is what the reader reports, as SheetImage documents.
const img = (await readXlsx(xlsx)).sheets[0].images![0]
expect(img.anchor).toEqual({ from: { row: 0, col: 0 } })
expect(img.width).toBe(315)
expect(img.height).toBe(210)
})

it("keeps altText and title on the picture", async () => {
const sheet = logoSheet()
sheet.images![0].altText = "Company logo"
sheet.images![0].title = "Logo"
const xlsx = await writeXlsx({ sheets: [sheet] })

const xml = await drawingXmlOf(xlsx)
expect(xml).toContain("<xdr:oneCellAnchor>")
expect(xml).toContain('<xdr:cNvPr id="2" name="Picture 1" title="Logo" descr="Company logo"/>')
const img = (await readXlsx(xlsx)).sheets[0].images![0]
expect(img.altText).toBe("Company logo")
expect(img.title).toBe("Logo")
})

it("still writes twoCellAnchor, unchanged, when `to` is given", () => {
const result = writeDrawing(
[
{
data: fakePng(),
type: "png",
anchor: { from: { row: 1, col: 1 }, to: { row: 5, col: 3 } },
width: 200,
height: 40,
altText: "a<b",
title: "T",
},
],
1,
)
// Byte-for-byte what the writer produced before #598.
expect(result.drawingXml).toContain(
"<xdr:twoCellAnchor>" +
"<xdr:from><xdr:col>1</xdr:col><xdr:colOff>0</xdr:colOff><xdr:row>1</xdr:row><xdr:rowOff>0</xdr:rowOff></xdr:from>" +
"<xdr:to><xdr:col>3</xdr:col><xdr:colOff>0</xdr:colOff><xdr:row>5</xdr:row><xdr:rowOff>0</xdr:rowOff></xdr:to>" +
'<xdr:pic><xdr:nvPicPr><xdr:cNvPr id="2" name="Picture 1" title="T" descr="a&lt;b"/>' +
'<xdr:cNvPicPr><a:picLocks noChangeAspect="1"/></xdr:cNvPicPr></xdr:nvPicPr>' +
'<xdr:blipFill><a:blip r:embed="rId1"/><a:stretch><a:fillRect/></a:stretch></xdr:blipFill>' +
'<xdr:spPr><a:xfrm><a:off x="0" y="0"/><a:ext cx="1905000" cy="381000"/></a:xfrm>' +
'<a:prstGeom prst="rect"><a:avLst/></a:prstGeom></xdr:spPr></xdr:pic>' +
"<xdr:clientData/></xdr:twoCellAnchor>",
)
expect(result.drawingXml).not.toContain("oneCellAnchor")
})

it("mixes both anchor kinds on one sheet in declaration order", async () => {
const a = fakePng(70)
const b = fakePng(90)
const xlsx = await writeXlsx({
sheets: [
{
name: "Sheet1",
rows: [["x"]],
images: [
{ data: a, type: "png", anchor: { from: { row: 0, col: 0 }, to: { row: 4, col: 2 } } },
{ data: b, type: "png", anchor: { from: { row: 6, col: 3 } }, width: 64, height: 32 },
],
},
],
})
expect(childNames(parseXml(await drawingXmlOf(xlsx)))).toEqual([
"twoCellAnchor",
"oneCellAnchor",
])

const images = (await readXlsx(xlsx)).sheets[0].images!
expect(images.map((i) => i.data)).toEqual([a, b])
expect(images[0].anchor).toEqual({ from: { row: 0, col: 0 }, to: { row: 4, col: 2 } })
expect(images[1].anchor).toEqual({ from: { row: 6, col: 3 } })
expect(images[1].width).toBe(64)
expect(images[1].height).toBe(32)
})

it("stays a oneCellAnchor through openXlsx / saveXlsx", async () => {
const saved = await saveXlsx(await openXlsx(await writeXlsx({ sheets: [logoSheet()] })))
expect(await drawingXmlOf(saved)).toContain('<xdr:ext cx="1905000" cy="381000"/>')
const img = (await readXlsx(saved)).sheets[0].images![0]
expect(img.anchor).toEqual({ from: { row: 1, col: 1 } })
expect(img.width).toBe(200)
expect(img.height).toBe(40)
})
})
12 changes: 12 additions & 0 deletions test/xlsx-write-read-parity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,9 @@ const SHEET_FIELDS: { [K in keyof Required<WriteSheet>]: Entry<WriteSheet[K]> }
altText: "a chart of nothing",
title: "Figure 1",
},
// No `to`: written as a oneCellAnchor, so it reads back without one
// rather than with a corner the writer made up (#598).
{ data: PNG_1X1, type: "png", anchor: { from: { row: 12, col: 1 } }, width: 200, height: 40 },
],
read: (sheet) =>
sheet.images?.map((i) => ({
Expand All @@ -192,6 +195,15 @@ const SHEET_FIELDS: { [K in keyof Required<WriteSheet>]: Entry<WriteSheet[K]> }
title: "Figure 1",
bytes: PNG_1X1.length,
},
{
type: "png",
anchor: { from: { row: 12, col: 1 } },
width: 200,
height: 40,
altText: undefined,
title: undefined,
bytes: PNG_1X1.length,
},
],
},

Expand Down