fix: guard drawing export against invalid structures and zero IDs (SD-824)

packages/super-editor/src/core/super-converter/v3/handlers/w/drawing/drawing-translator.js

@@ -70,6 +70,11 @@ function decode(params) {
   const childTranslator = node.attrs.isAnchor ? wpAnchorTranslator : wpInlineTranslator;
   const resultNode = childTranslator.decode(params);
 
+  // Guard: only wrap when we have valid drawing content.
+  if (!resultNode || (resultNode.name !== 'wp:inline' && resultNode.name !== 'wp:anchor')) {
+    return resultNode;
+  }
+
   return wrapTextInRun(
     {
       name: 'w:drawing',

packages/super-editor/src/core/super-converter/v3/handlers/w/drawing/drawing-translator.test.js

@@ -95,38 +95,82 @@ describe('w:drawing translator', () => {
   });
 
   describe('decode', () => {
-    it('delegates to wp:anchor when node.attrs.isAnchor is true', () => {
+    it('wraps valid wp:anchor child in w:drawing', () => {
       const params = { node: { type: 'element', attrs: { isAnchor: true } } };
-      anchorTranslatorMock.decode.mockReturnValue({ decoded: 'anchor' });
+      anchorTranslatorMock.decode.mockReturnValue({ name: 'wp:anchor', decoded: 'anchor' });
 
       const result = translator.decode(params);
 
       expect(anchorTranslatorMock.decode).toHaveBeenCalledWith(params);
       expect(wrapTextInRun).toHaveBeenCalledWith(
         expect.objectContaining({
           name: 'w:drawing',
-          elements: [{ decoded: 'anchor' }],
+          elements: [expect.objectContaining({ name: 'wp:anchor' })],
         }),
         [],
       );
-      expect(result).toEqual({ wrapped: { name: 'w:drawing', elements: [{ decoded: 'anchor' }] } });
+      expect(result).toEqual({
+        wrapped: {
+          name: 'w:drawing',
+          elements: [expect.objectContaining({ name: 'wp:anchor' })],
+        },
+      });
     });
 
-    it('delegates to wp:inline when node.attrs.isAnchor is false', () => {
+    it('wraps valid wp:inline child in w:drawing', () => {
       const params = { node: { type: 'element', attrs: { isAnchor: false } } };
-      inlineTranslatorMock.decode.mockReturnValue({ decoded: 'inline' });
+      inlineTranslatorMock.decode.mockReturnValue({ name: 'wp:inline', decoded: 'inline' });
 
       const result = translator.decode(params);
 
       expect(inlineTranslatorMock.decode).toHaveBeenCalledWith(params);
       expect(wrapTextInRun).toHaveBeenCalledWith(
         expect.objectContaining({
           name: 'w:drawing',
-          elements: [{ decoded: 'inline' }],
+          elements: [expect.objectContaining({ name: 'wp:inline' })],
         }),
         [],
       );
-      expect(result).toEqual({ wrapped: { name: 'w:drawing', elements: [{ decoded: 'inline' }] } });
+      expect(result).toEqual({
+        wrapped: {
+          name: 'w:drawing',
+          elements: [expect.objectContaining({ name: 'wp:inline' })],
+        },
+      });
+    });
+
+    it('returns child as-is when anchor child is not a drawing node', () => {
+      const params = { node: { type: 'element', attrs: { isAnchor: true } } };
+      const textRun = { name: 'w:r', elements: [{ name: 'w:t', elements: [{ text: 'x' }] }] };
+      anchorTranslatorMock.decode.mockReturnValue(textRun);
+
+      const result = translator.decode(params);
+
+      expect(anchorTranslatorMock.decode).toHaveBeenCalledWith(params);
+      expect(wrapTextInRun).not.toHaveBeenCalled();
+      expect(result).toEqual(textRun);
+    });
+
+    it('returns child as-is when inline child is not a drawing node', () => {
+      const params = { node: { type: 'element', attrs: { isAnchor: false } } };
+      const textRun = { name: 'w:r', elements: [{ name: 'w:t', elements: [{ text: 'y' }] }] };
+      inlineTranslatorMock.decode.mockReturnValue(textRun);
+
+      const result = translator.decode(params);
+
+      expect(inlineTranslatorMock.decode).toHaveBeenCalledWith(params);
+      expect(wrapTextInRun).not.toHaveBeenCalled();
+      expect(result).toEqual(textRun);
+    });
+
+    it('returns null when child translator returns null', () => {
+      const params = { node: { type: 'element', attrs: { isAnchor: false } } };
+      inlineTranslatorMock.decode.mockReturnValue(null);
+
+      const result = translator.decode(params);
+
+      expect(wrapTextInRun).not.toHaveBeenCalled();
+      expect(result).toBeNull();
     });
 
     it('returns null if node is missing', () => {

packages/super-editor/src/core/super-converter/v3/handlers/wp/anchor/helpers/translate-anchor-node.js

@@ -68,6 +68,11 @@ export function translateAnchorNode(params) {
 
   const nodeElements = translateImageNode(params);
 
+  // Guard: bail out if translateImageNode produced a non-drawing result (e.g. text fallback).
+  if (!nodeElements?.elements?.some((el) => el?.name === 'wp:extent')) {
+    return nodeElements;
+  }
+
   const inlineAttrs = {
     ...(attrs.originalAttributes || {}),
     ...(nodeElements.attributes || {}),

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

@@ -15,10 +15,10 @@ describe('translateAnchorNode', () => {
   beforeEach(() => {
     vi.clearAllMocks();
 
-    // default mock for translateImageNode
+    // default mock for translateImageNode — must include wp:extent so the guard passes
     translateImageNode.mockReturnValue({
       attributes: { fakeAttr: 'val' },
-      elements: [{ name: 'pic:fake' }],
+      elements: [{ name: 'wp:extent' }, { name: 'wp:effectExtent' }, { name: 'pic:fake' }],
     });
 
     // default mock for pixelsToEmu

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

@@ -37,9 +37,9 @@ function buildHlinkClickElement(attrs, hlinkRId) {
  * - `a:hlinkClick` child when hyperlink is set (Word's canonical placement per §20.4.2.5)
  * - Decorative extension child when attrs.decorative is true
  */
-function buildDocPrElement(attrs, imageName, hlinkRId) {
+function buildDocPrElement(attrs, imageName, hlinkRId, drawingId) {
   const docPrAttrs = {
-    id: attrs.id || 0,
+    id: drawingId,
     name: attrs.alt || `Picture ${imageName}`,
   };
   // Emit descr (accessibility description) unless decorative
@@ -84,7 +84,7 @@ function buildDocPrElement(attrs, imageName, hlinkRId) {
  * - `a:hlinkClick` child when hyperlink is set (mirrors wp:docPr for compatibility)
  * - `a:picLocks/@noChangeAspect` ← dynamic from attrs.lockAspectRatio
  */
-function buildNvPicPrElement(attrs, imageName, hlinkRId) {
+function buildNvPicPrElement(attrs, imageName, hlinkRId, drawingId) {
   // --- pic:cNvPr children (hyperlink) ---
   const cNvPrChildren = [];
   const hlinkEl = buildHlinkClickElement(attrs, hlinkRId);
@@ -96,7 +96,7 @@ function buildNvPicPrElement(attrs, imageName, hlinkRId) {
       {
         name: 'pic:cNvPr',
         attributes: {
-          id: attrs.id || 0,
+          id: drawingId,
           name: attrs.alt || `Picture ${imageName}`,
         },
         ...(cNvPrChildren.length ? { elements: cNvPrChildren } : {}),
@@ -295,6 +295,9 @@ export const translateImageNode = (params) => {
   // Resolve hyperlink relationship once; shared by wp:docPr and pic:cNvPr.
   const hlinkRId = resolveHyperlinkRId(attrs, params);
 
+  // Ensure valid positive docPr/cNvPr IDs (OOXML requires id > 0).
+  const drawingId = attrs.id && Number(attrs.id) > 0 ? attrs.id : Math.max(1, parseInt(generateDocxRandomId(), 16));
+
   return {
     attributes: inlineAttrs,
     elements: [
@@ -309,7 +312,7 @@ export const translateImageNode = (params) => {
         name: 'wp:effectExtent',
         attributes: effectExtentAttrs,
       },
-      buildDocPrElement(attrs, imageName, hlinkRId),
+      buildDocPrElement(attrs, imageName, hlinkRId, drawingId),
       {
         name: 'wp:cNvGraphicFramePr',
         elements: [
@@ -334,7 +337,7 @@ export const translateImageNode = (params) => {
                 name: 'pic:pic',
                 attributes: { 'xmlns:pic': pictureXmlns },
                 elements: [
-                  buildNvPicPrElement(attrs, imageName, hlinkRId),
+                  buildNvPicPrElement(attrs, imageName, hlinkRId, drawingId),
                   {
                     name: 'pic:blipFill',
                     elements: [

packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/merge-drawing-children.js

@@ -15,8 +15,13 @@ import { carbonCopy } from '@core/utilities/carbonCopy.js';
 export function mergeDrawingChildren({ order, generated, original }) {
   const genQueues = groupByName(generated);
   const originalsByIndex = groupByIndex(original);
+  const merged = mergeWithOrder(order, genQueues, originalsByIndex);
 
-  return mergeWithOrder(order, genQueues, originalsByIndex);
+  // Originals may carry invalid IDs (e.g. id="0") that Word rejects.
+  // Patch them using the valid ID from the generated wp:docPr.
+  fixZeroDrawingIds(merged, generated);
+
+  return merged;
 }
 
 function groupByIndex(entries = []) {
@@ -70,6 +75,7 @@ function mergeWithOrder(order = [], genQueues, originalsByIndex) {
 
   return out;
 }
+
 function groupByName(nodes = []) {
   const map = new Map();
   nodes.forEach((el) => {
@@ -80,3 +86,28 @@ function groupByName(nodes = []) {
   });
   return map;
 }
+
+/**
+ * Patch zero/missing IDs on wp:docPr and pic:cNvPr in merged output.
+ * When the merge prefers an original element, it may carry an invalid id="0"
+ * that Word rejects. We fix it using the valid ID from the generated wp:docPr.
+ */
+function fixZeroDrawingIds(merged, generated) {
+  const genDocPr = generated?.find((el) => el?.name === 'wp:docPr');
+  const validId = genDocPr?.attributes?.id;
+  if (!validId || !(Number(validId) > 0)) return;
+
+  const docPr = merged.find((el) => el?.name === 'wp:docPr');
+  if (docPr?.attributes && !(Number(docPr.attributes.id) > 0)) {
+    docPr.attributes.id = validId;
+  }
+
+  const graphic = merged.find((el) => el?.name === 'a:graphic');
+  const graphicData = graphic?.elements?.find((el) => el?.name === 'a:graphicData');
+  const pic = graphicData?.elements?.find((el) => el?.name === 'pic:pic');
+  const nvPicPr = pic?.elements?.find((el) => el?.name === 'pic:nvPicPr');
+  const cNvPr = nvPicPr?.elements?.find((el) => el?.name === 'pic:cNvPr');
+  if (cNvPr?.attributes && !(Number(cNvPr.attributes.id) > 0)) {
+    cNvPr.attributes.id = validId;
+  }
+}

packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/merge-drawing-children.test.js

@@ -204,6 +204,111 @@ describe('mergeDrawingChildren', () => {
     });
   });
 
+  describe('zero drawing ID fix', () => {
+    it('patches wp:docPr id=0 using the generated ID', () => {
+      const result = mergeDrawingChildren({
+        order: ['wp:extent', 'wp:docPr', 'a:graphic'],
+        generated: [
+          { name: 'wp:extent', attributes: { cx: 100 } },
+          { name: 'wp:docPr', attributes: { id: 42, name: 'Picture 1' } },
+          { name: 'a:graphic', attributes: {} },
+        ],
+        original: [
+          { index: 1, xml: { name: 'wp:docPr', attributes: { id: 0, name: 'Picture 1', descr: 'alt text' } } },
+          { index: 2, xml: { name: 'a:graphic', attributes: {} } },
+        ],
+      });
+
+      const docPr = result.find((el) => el.name === 'wp:docPr');
+      expect(docPr.attributes.id).toBe(42);
+      expect(docPr.attributes.descr).toBe('alt text');
+    });
+
+    it('patches pic:cNvPr id=0 inside original a:graphic subtree', () => {
+      const result = mergeDrawingChildren({
+        order: ['wp:extent', 'wp:docPr', 'a:graphic'],
+        generated: [
+          { name: 'wp:extent', attributes: { cx: 100 } },
+          { name: 'wp:docPr', attributes: { id: 7 } },
+          {
+            name: 'a:graphic',
+            elements: [
+              {
+                name: 'a:graphicData',
+                elements: [
+                  {
+                    name: 'pic:pic',
+                    elements: [
+                      {
+                        name: 'pic:nvPicPr',
+                        elements: [{ name: 'pic:cNvPr', attributes: { id: 7 } }],
+                      },
+                    ],
+                  },
+                ],
+              },
+            ],
+          },
+        ],
+        original: [
+          { index: 1, xml: { name: 'wp:docPr', attributes: { id: 0 } } },
+          {
+            index: 2,
+            xml: {
+              name: 'a:graphic',
+              elements: [
+                {
+                  name: 'a:graphicData',
+                  elements: [
+                    {
+                      name: 'pic:pic',
+                      elements: [
+                        {
+                          name: 'pic:nvPicPr',
+                          elements: [{ name: 'pic:cNvPr', attributes: { id: 0, name: 'Original' } }],
+                        },
+                      ],
+                    },
+                  ],
+                },
+              ],
+            },
+          },
+        ],
+      });
+
+      const graphic = result.find((el) => el.name === 'a:graphic');
+      const cNvPr = graphic.elements[0].elements[0].elements[0].elements[0];
+      expect(cNvPr.attributes.id).toBe(7);
+      expect(cNvPr.attributes.name).toBe('Original');
+    });
+
+    it('does not overwrite valid positive IDs on originals', () => {
+      const result = mergeDrawingChildren({
+        order: ['wp:extent', 'wp:docPr'],
+        generated: [
+          { name: 'wp:extent', attributes: {} },
+          { name: 'wp:docPr', attributes: { id: 99 } },
+        ],
+        original: [{ index: 1, xml: { name: 'wp:docPr', attributes: { id: 5 } } }],
+      });
+
+      const docPr = result.find((el) => el.name === 'wp:docPr');
+      expect(docPr.attributes.id).toBe(5);
+    });
+
+    it('handles missing generated wp:docPr gracefully', () => {
+      const result = mergeDrawingChildren({
+        order: ['wp:extent', 'wp:docPr'],
+        generated: [{ name: 'wp:extent', attributes: {} }],
+        original: [{ index: 1, xml: { name: 'wp:docPr', attributes: { id: 0 } } }],
+      });
+
+      const docPr = result.find((el) => el.name === 'wp:docPr');
+      expect(docPr.attributes.id).toBe(0);
+    });
+  });
+
   describe('deep copy behavior', () => {
     it('returns deep copies, not references to original objects', () => {
       const originalXml = { name: 'wp:docPr', attributes: { id: 1 } };

packages/super-editor/src/core/super-converter/v3/handlers/wp/inline/helpers/translate-inline-node.js

@@ -10,6 +10,11 @@ export function translateInlineNode(params) {
   const { attrs } = params.node;
   const nodeElements = translateImageNode(params);
 
+  // Guard: bail out if translateImageNode produced a non-drawing result (e.g. text fallback).
+  if (!nodeElements?.elements?.some((el) => el?.name === 'wp:extent')) {
+    return nodeElements;
+  }
+
   const inlineAttrs = {
     ...(attrs.originalAttributes || {}),
     ...(nodeElements.attributes || {}),