Skip to content

Commit 2b8dbce

Browse files
authored
fix(super-converter): handle empty pic:spPr in image import (#2254)
Guard against undefined spPr.elements when pic:spPr is a self-closing empty element (e.g. <pic:spPr/>). This is valid OOXML per ECMA-376 §20.2.2.6 — all CT_ShapeProperties children are optional. Tools like docx-templates v4.15.0 generate this pattern, causing a TypeError that silently breaks all image imports in the document. SD-2085
1 parent a4c3ab8 commit 2b8dbce

2 files changed

Lines changed: 193 additions & 1 deletion

File tree

packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/encode-image-node-helpers.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ export function handleImageNode(node, params, isAnchor) {
345345

346346
const spPr = picture.elements.find((el) => el.name === 'pic:spPr');
347347
if (spPr) {
348-
const xfrm = spPr.elements.find((el) => el.name === 'a:xfrm');
348+
const xfrm = spPr.elements?.find((el) => el.name === 'a:xfrm');
349349
if (xfrm?.attributes) {
350350
transformData = {
351351
...transformData,
@@ -370,6 +370,7 @@ export function handleImageNode(node, params, isAnchor) {
370370
const { elements } = relationships || [];
371371

372372
const rel = elements?.find((el) => el.attributes['Id'] === rEmbed);
373+
373374
if (!rel) {
374375
return null;
375376
}

packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/encode-image-node-helpers.test.js

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1460,4 +1460,195 @@ describe('getVectorShape', () => {
14601460
expect(result.attrs.textContent.parts[0].text).toBe('[[notspace]] [other]');
14611461
});
14621462
});
1463+
1464+
describe('IT-632: docx-templates duplicate pic:cNvPr id and non-standard rIds', () => {
1465+
/**
1466+
* docx-templates generates images with:
1467+
* 1. All pic:cNvPr id="0" (duplicate, non-conformant per OOXML spec §20.1.2.2.8)
1468+
* 2. All wp:docPr id="0" (also duplicated from template cloning)
1469+
* 3. Non-standard relationship IDs like "img{hash}" instead of "rId{n}"
1470+
* 4. Different relationship targets for each image
1471+
*
1472+
* This test verifies each image resolves to a unique src path.
1473+
*/
1474+
1475+
const makeDocxTemplatesImageNode = ({ rEmbed, docPrName, picCNvPrName }) => ({
1476+
attributes: {
1477+
distT: '0',
1478+
distB: '0',
1479+
distL: '0',
1480+
distR: '0',
1481+
},
1482+
elements: [
1483+
{ name: 'wp:extent', attributes: { cx: '5000000', cy: '3000000' } },
1484+
{
1485+
name: 'a:graphic',
1486+
elements: [
1487+
{
1488+
name: 'a:graphicData',
1489+
attributes: { uri: 'pic' },
1490+
elements: [
1491+
{
1492+
name: 'pic:pic',
1493+
elements: [
1494+
{
1495+
name: 'pic:nvPicPr',
1496+
elements: [
1497+
{
1498+
name: 'pic:cNvPr',
1499+
attributes: { id: '0', name: picCNvPrName },
1500+
},
1501+
],
1502+
},
1503+
{
1504+
name: 'pic:blipFill',
1505+
elements: [
1506+
{
1507+
name: 'a:blip',
1508+
attributes: { 'r:embed': rEmbed },
1509+
},
1510+
],
1511+
},
1512+
],
1513+
},
1514+
],
1515+
},
1516+
],
1517+
},
1518+
// wp:docPr also duplicated with id="0"
1519+
{ name: 'wp:docPr', attributes: { id: '0', name: docPrName } },
1520+
],
1521+
});
1522+
1523+
const makeDocxTemplatesParams = () => ({
1524+
filename: 'document.xml',
1525+
docx: {
1526+
'word/_rels/document.xml.rels': {
1527+
elements: [
1528+
{
1529+
name: 'Relationships',
1530+
elements: [
1531+
{
1532+
name: 'Relationship',
1533+
attributes: {
1534+
Id: 'img2073076884',
1535+
Target: 'media/template_document.xml_img2073076884.jpg',
1536+
},
1537+
},
1538+
{
1539+
name: 'Relationship',
1540+
attributes: {
1541+
Id: 'img3891234567',
1542+
Target: 'media/template_document.xml_img3891234567.jpg',
1543+
},
1544+
},
1545+
{
1546+
name: 'Relationship',
1547+
attributes: {
1548+
Id: 'img5678901234',
1549+
Target: 'media/template_document.xml_img5678901234.jpg',
1550+
},
1551+
},
1552+
],
1553+
},
1554+
],
1555+
},
1556+
},
1557+
});
1558+
1559+
it('should produce distinct src paths for images with duplicate pic:cNvPr id=0', () => {
1560+
const params = makeDocxTemplatesParams();
1561+
1562+
const image1 = makeDocxTemplatesImageNode({
1563+
rEmbed: 'img2073076884',
1564+
docPrName: 'image1.jpg',
1565+
picCNvPrName: 'image1.jpg',
1566+
});
1567+
const image2 = makeDocxTemplatesImageNode({
1568+
rEmbed: 'img3891234567',
1569+
docPrName: 'image2.jpg',
1570+
picCNvPrName: 'image2.jpg',
1571+
});
1572+
const image3 = makeDocxTemplatesImageNode({
1573+
rEmbed: 'img5678901234',
1574+
docPrName: 'image3.jpg',
1575+
picCNvPrName: 'image3.jpg',
1576+
});
1577+
1578+
const result1 = handleImageNode(image1, params, false);
1579+
const result2 = handleImageNode(image2, params, false);
1580+
const result3 = handleImageNode(image3, params, false);
1581+
1582+
// All should produce valid image nodes
1583+
expect(result1).not.toBeNull();
1584+
expect(result2).not.toBeNull();
1585+
expect(result3).not.toBeNull();
1586+
1587+
// Each should have a DISTINCT src path
1588+
expect(result1.attrs.src).toBe('word/media/template_document.xml_img2073076884.jpg');
1589+
expect(result2.attrs.src).toBe('word/media/template_document.xml_img3891234567.jpg');
1590+
expect(result3.attrs.src).toBe('word/media/template_document.xml_img5678901234.jpg');
1591+
1592+
// Verify all three are different
1593+
const srcs = [result1.attrs.src, result2.attrs.src, result3.attrs.src];
1594+
expect(new Set(srcs).size).toBe(3);
1595+
1596+
// rIds should also be distinct
1597+
expect(result1.attrs.rId).toBe('img2073076884');
1598+
expect(result2.attrs.rId).toBe('img3891234567');
1599+
expect(result3.attrs.rId).toBe('img5678901234');
1600+
});
1601+
1602+
it('should handle empty pic:spPr element (SD-2085)', () => {
1603+
const params = makeDocxTemplatesParams();
1604+
1605+
// pic:spPr as a self-closing empty element — valid per ECMA-376 §20.2.2.6
1606+
// (all CT_ShapeProperties children are optional)
1607+
const imageWithEmptySpPr = {
1608+
...makeDocxTemplatesImageNode({
1609+
rEmbed: 'img2073076884',
1610+
docPrName: 'image1.jpg',
1611+
picCNvPrName: 'image1.jpg',
1612+
}),
1613+
};
1614+
1615+
// Add empty pic:spPr to the pic:pic element (no elements array)
1616+
const graphicData = imageWithEmptySpPr.elements
1617+
.find((el) => el.name === 'a:graphic')
1618+
.elements.find((el) => el.name === 'a:graphicData');
1619+
const picPic = graphicData.elements.find((el) => el.name === 'pic:pic');
1620+
picPic.elements.push({ name: 'pic:spPr', attributes: {} });
1621+
1622+
const result = handleImageNode(imageWithEmptySpPr, params, false);
1623+
1624+
expect(result).not.toBeNull();
1625+
expect(result.attrs.src).toBe('word/media/template_document.xml_img2073076884.jpg');
1626+
expect(result.attrs.rId).toBe('img2073076884');
1627+
});
1628+
1629+
it('should handle images where all wp:docPr ids are "0"', () => {
1630+
const params = makeDocxTemplatesParams();
1631+
1632+
const image1 = makeDocxTemplatesImageNode({
1633+
rEmbed: 'img2073076884',
1634+
docPrName: 'image1.jpg',
1635+
picCNvPrName: 'image1.jpg',
1636+
});
1637+
const image2 = makeDocxTemplatesImageNode({
1638+
rEmbed: 'img3891234567',
1639+
docPrName: 'image2.jpg',
1640+
picCNvPrName: 'image2.jpg',
1641+
});
1642+
1643+
const result1 = handleImageNode(image1, params, false);
1644+
const result2 = handleImageNode(image2, params, false);
1645+
1646+
// Both have id="0" from wp:docPr — this should NOT cause deduplication
1647+
expect(result1.attrs.id).toBe('0');
1648+
expect(result2.attrs.id).toBe('0');
1649+
1650+
// But src should still be different
1651+
expect(result1.attrs.src).not.toBe(result2.attrs.src);
1652+
});
1653+
});
14631654
});

0 commit comments

Comments
 (0)