fix(super-editor): handle partial comment file-sets and clean up stale parts on export (#2123)

Author: harbournick

packages/super-editor/src/core/DocxZipper.js

@@ -3,6 +3,7 @@ import JSZip from 'jszip';
 import { getContentTypesFromXml, base64ToUint8Array } from './super-converter/helpers.js';
 import { ensureXmlString, isXmlLike } from './encoding-helpers.js';
 import { DOCX } from '@superdoc/common';
+import { COMMENT_FILE_BASENAMES } from './super-converter/constants.js';
 
 /**
  * Class to handle unzipping and zipping of docx files
@@ -143,9 +144,13 @@ class DocxZipper {
       (el) => el.name === 'Override' && el.attributes.PartName === '/word/commentsExtensible.xml',
     );
 
+    /**
+     * Check if a file will exist in the final zip output.
+     * A null value in updatedDocs means the file is explicitly deleted.
+     */
     const hasFile = (filename) => {
       if (updatedDocs && Object.prototype.hasOwnProperty.call(updatedDocs, filename)) {
-        return true;
+        return updatedDocs[filename] !== null;
       }
       if (!docx?.files) return false;
       if (!fromJson) return Boolean(docx.files[filename]);
@@ -205,9 +210,23 @@ class DocxZipper {
       }
     });
 
+    // Prune stale comment Override entries for parts that will not exist in the final zip.
+    const commentPartNames = COMMENT_FILE_BASENAMES.map((name) => `/word/${name}`);
+    const staleOverridePartNames = commentPartNames.filter((partName) => {
+      const filename = partName.slice(1); // strip leading /
+      return !hasFile(filename);
+    });
+
     const beginningString = '<Types xmlns="http://schemas.openxmlformats.org/package/2006/content-types">';
     let updatedContentTypesXml = contentTypesXml.replace(beginningString, `${beginningString}${typesString}`);
 
+    // Remove Override elements for comment parts that no longer exist
+    for (const partName of staleOverridePartNames) {
+      const escapedPartName = partName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+      const overrideRegex = new RegExp(`\\s*<Override[^>]*PartName="${escapedPartName}"[^>]*/>`, 'g');
+      updatedContentTypesXml = updatedContentTypesXml.replace(overrideRegex, '');
+    }
+
     // Include any header/footer targets referenced from document relationships
     let relationshipsXml = updatedDocs['word/_rels/document.xml.rels'];
     if (!relationshipsXml) {
@@ -298,10 +317,13 @@ class DocxZipper {
       zip.file(file.name, content);
     }
 
-    // Replace updated docs
+    // Replace updated docs (null = delete from zip)
     Object.keys(updatedDocs).forEach((key) => {
-      const content = updatedDocs[key];
-      zip.file(key, content);
+      if (updatedDocs[key] === null) {
+        zip.remove(key);
+      } else {
+        zip.file(key, updatedDocs[key]);
+      }
     });
 
     Object.keys(media).forEach((path) => {
@@ -337,9 +359,13 @@ class DocxZipper {
     });
     await Promise.all(filePromises);
 
-    // Make replacements of updated docs
+    // Make replacements of updated docs (null = delete from zip)
     Object.keys(updatedDocs).forEach((key) => {
-      unzippedOriginalDocx.file(key, updatedDocs[key]);
+      if (updatedDocs[key] === null) {
+        unzippedOriginalDocx.remove(key);
+      } else {
+        unzippedOriginalDocx.file(key, updatedDocs[key]);
+      }
     });
 
     Object.keys(media).forEach((path) => {

packages/super-editor/src/core/DocxZipper.test.js

@@ -256,6 +256,44 @@ describe('DocxZipper - updateContentTypes', () => {
     expect(updatedContentTypes).toContain('/word/header1.xml');
     expect(updatedContentTypes).toContain('/word/footer1.xml');
   });
+
+  it('removes stale comment overrides when updated docs mark comment files as deleted', async () => {
+    const zipper = new DocxZipper();
+    const zip = new JSZip();
+
+    const contentTypes = `<?xml version="1.0" encoding="UTF-8"?>
+      <Types xmlns="http://schemas.openxmlformats.org/package/2006/content-types">
+        <Default Extension="rels" ContentType="application/vnd.openxmlformats-package.relationships+xml"/>
+        <Default Extension="xml" ContentType="application/xml"/>
+        <Override PartName="/word/document.xml" ContentType="application/vnd.openxmlformats-officedocument.wordprocessingml.document.main+xml"/>
+        <Override PartName="/word/comments.xml" ContentType="application/vnd.openxmlformats-officedocument.wordprocessingml.comments+xml"/>
+        <Override PartName="/word/comments.xml" ContentType="application/vnd.openxmlformats-officedocument.wordprocessingml.comments+xml"/>
+        <Override PartName="/word/commentsExtended.xml" ContentType="application/vnd.openxmlformats-officedocument.wordprocessingml.commentsExtended+xml"/>
+        <Override PartName="/word/commentsIds.xml" ContentType="application/vnd.openxmlformats-officedocument.wordprocessingml.commentsIds+xml"/>
+        <Override PartName="/word/commentsExtensible.xml" ContentType="application/vnd.openxmlformats-officedocument.wordprocessingml.commentsExtensible+xml"/>
+      </Types>`;
+    zip.file('[Content_Types].xml', contentTypes);
+    zip.file(
+      'word/document.xml',
+      '<w:document xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"/>',
+    );
+
+    const updatedDocs = {
+      'word/comments.xml': null,
+      'word/commentsExtended.xml': null,
+      'word/commentsIds.xml': null,
+      'word/commentsExtensible.xml': null,
+    };
+
+    await zipper.updateContentTypes(zip, {}, false, updatedDocs);
+
+    const updatedContentTypes = await zip.file('[Content_Types].xml').async('string');
+    expect(updatedContentTypes).toContain('PartName="/word/document.xml"');
+    expect(updatedContentTypes).not.toContain('PartName="/word/comments.xml"');
+    expect(updatedContentTypes).not.toContain('PartName="/word/commentsExtended.xml"');
+    expect(updatedContentTypes).not.toContain('PartName="/word/commentsIds.xml"');
+    expect(updatedContentTypes).not.toContain('PartName="/word/commentsExtensible.xml"');
+  });
 });
 
 describe('DocxZipper - exportFromCollaborativeDocx media handling', () => {
@@ -299,3 +337,119 @@ describe('DocxZipper - exportFromCollaborativeDocx media handling', () => {
     expect(Array.from(img2)).toEqual([87, 111, 114, 108, 100]);
   });
 });
+
+describe('DocxZipper - comment file deletion', () => {
+  const contentTypesWithComments = `<?xml version="1.0" encoding="UTF-8"?>
+    <Types xmlns="http://schemas.openxmlformats.org/package/2006/content-types">
+      <Default Extension="rels" ContentType="application/vnd.openxmlformats-package.relationships+xml"/>
+      <Default Extension="xml" ContentType="application/xml"/>
+      <Override PartName="/word/document.xml" ContentType="application/vnd.openxmlformats-officedocument.wordprocessingml.document.main+xml"/>
+      <Override PartName="/word/comments.xml" ContentType="application/vnd.openxmlformats-officedocument.wordprocessingml.comments+xml"/>
+      <Override PartName="/word/commentsExtended.xml" ContentType="application/vnd.openxmlformats-officedocument.wordprocessingml.commentsExtended+xml"/>
+      <Override PartName="/word/commentsIds.xml" ContentType="application/vnd.openxmlformats-officedocument.wordprocessingml.commentsIds+xml"/>
+      <Override PartName="/word/commentsExtensible.xml" ContentType="application/vnd.openxmlformats-officedocument.wordprocessingml.commentsExtensible+xml"/>
+    </Types>`;
+
+  const updatedDocsWithCommentDeletes = {
+    'word/document.xml': '<w:document xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"/>',
+    'word/comments.xml': null,
+    'word/commentsExtended.xml': null,
+    'word/commentsIds.xml': null,
+    'word/commentsExtensible.xml': null,
+  };
+
+  it('removes stale comment files in collaborative export path when null sentinels are provided', async () => {
+    const zipper = new DocxZipper();
+    const docx = [
+      { name: '[Content_Types].xml', content: contentTypesWithComments },
+      {
+        name: 'word/document.xml',
+        content: '<w:document xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"/>',
+      },
+      {
+        name: 'word/comments.xml',
+        content: '<w:comments xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"/>',
+      },
+      {
+        name: 'word/commentsExtended.xml',
+        content: '<w15:commentsEx xmlns:w15="http://schemas.microsoft.com/office/word/2012/wordml"/>',
+      },
+      {
+        name: 'word/commentsIds.xml',
+        content: '<w16cid:commentsIds xmlns:w16cid="http://schemas.microsoft.com/office/word/2016/wordml/cid"/>',
+      },
+      {
+        name: 'word/commentsExtensible.xml',
+        content: '<w16cex:commentsExtensible xmlns:w16cex="http://schemas.microsoft.com/office/word/2018/wordml/cex"/>',
+      },
+    ];
+
+    const result = await zipper.updateZip({
+      docx,
+      updatedDocs: updatedDocsWithCommentDeletes,
+      media: {},
+      fonts: {},
+      isHeadless: true,
+    });
+
+    const readBack = await new JSZip().loadAsync(result);
+    expect(readBack.file('word/comments.xml')).toBeNull();
+    expect(readBack.file('word/commentsExtended.xml')).toBeNull();
+    expect(readBack.file('word/commentsIds.xml')).toBeNull();
+    expect(readBack.file('word/commentsExtensible.xml')).toBeNull();
+
+    const updatedContentTypes = await readBack.file('[Content_Types].xml').async('string');
+    expect(updatedContentTypes).not.toContain('PartName="/word/comments.xml"');
+    expect(updatedContentTypes).not.toContain('PartName="/word/commentsExtended.xml"');
+    expect(updatedContentTypes).not.toContain('PartName="/word/commentsIds.xml"');
+    expect(updatedContentTypes).not.toContain('PartName="/word/commentsExtensible.xml"');
+  });
+
+  it('removes stale comment files in original-file export path when null sentinels are provided', async () => {
+    const zipper = new DocxZipper();
+    const originalZip = new JSZip();
+    originalZip.file('[Content_Types].xml', contentTypesWithComments);
+    originalZip.file(
+      'word/document.xml',
+      '<w:document xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"/>',
+    );
+    originalZip.file(
+      'word/comments.xml',
+      '<w:comments xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"/>',
+    );
+    originalZip.file(
+      'word/commentsExtended.xml',
+      '<w15:commentsEx xmlns:w15="http://schemas.microsoft.com/office/word/2012/wordml"/>',
+    );
+    originalZip.file(
+      'word/commentsIds.xml',
+      '<w16cid:commentsIds xmlns:w16cid="http://schemas.microsoft.com/office/word/2016/wordml/cid"/>',
+    );
+    originalZip.file(
+      'word/commentsExtensible.xml',
+      '<w16cex:commentsExtensible xmlns:w16cex="http://schemas.microsoft.com/office/word/2018/wordml/cex"/>',
+    );
+    const originalDocxFile = await originalZip.generateAsync({ type: 'nodebuffer' });
+
+    const result = await zipper.updateZip({
+      docx: [],
+      updatedDocs: updatedDocsWithCommentDeletes,
+      originalDocxFile,
+      media: {},
+      fonts: {},
+      isHeadless: true,
+    });
+
+    const readBack = await new JSZip().loadAsync(result);
+    expect(readBack.file('word/comments.xml')).toBeNull();
+    expect(readBack.file('word/commentsExtended.xml')).toBeNull();
+    expect(readBack.file('word/commentsIds.xml')).toBeNull();
+    expect(readBack.file('word/commentsExtensible.xml')).toBeNull();
+
+    const updatedContentTypes = await readBack.file('[Content_Types].xml').async('string');
+    expect(updatedContentTypes).not.toContain('PartName="/word/comments.xml"');
+    expect(updatedContentTypes).not.toContain('PartName="/word/commentsExtended.xml"');
+    expect(updatedContentTypes).not.toContain('PartName="/word/commentsIds.xml"');
+    expect(updatedContentTypes).not.toContain('PartName="/word/commentsExtensible.xml"');
+  });
+});

packages/super-editor/src/core/Editor.ts

@@ -47,6 +47,7 @@ import { createLinkedChildEditor } from '@core/child-editor/index.js';
 import { unflattenListsInHtml } from './inputRules/html/html-helpers.js';
 import { SuperValidator } from '@core/super-validator/index.js';
 import { createDocFromMarkdown, createDocFromHTML } from '@core/helpers/index.js';
+import { COMMENT_FILE_BASENAMES } from '@core/super-converter/constants.js';
 import { isHeadless } from '../utils/headless-helpers.js';
 import { canUseDOM } from '../utils/canUseDOM.js';
 import { buildSchemaSummary } from './schema-summary.js';
@@ -2586,7 +2587,7 @@ export class Editor extends EventEmitter<EditorEventMap> {
     getUpdatedDocs?: boolean;
     fieldsHighlightColor?: string | null;
     compression?: 'DEFLATE' | 'STORE';
-  } = {}): Promise<Blob | ArrayBuffer | Buffer | Record<string, string> | ProseMirrorJSON | string | undefined> {
+  } = {}): Promise<Blob | ArrayBuffer | Buffer | Record<string, string | null> | ProseMirrorJSON | string | undefined> {
     try {
       // Use provided comments, or fall back to imported comments from converter
       const effectiveComments = comments ?? this.converter.comments ?? [];
@@ -2654,7 +2655,7 @@ export class Editor extends EventEmitter<EditorEventMap> {
       const coreXmlData = this.converter.convertedXml['docProps/core.xml'];
       const coreXml = coreXmlData?.elements?.[0] ? this.converter.schemaToXml(coreXmlData.elements[0]) : null;
 
-      const updatedDocs: Record<string, string> = {
+      const updatedDocs: Record<string, string | null> = {
         ...this.options.customUpdatedFiles,
         'word/document.xml': String(documentXml),
         'docProps/custom.xml': String(customXml),
@@ -2677,26 +2678,15 @@ export class Editor extends EventEmitter<EditorEventMap> {
         updatedDocs['word/_rels/footnotes.xml.rels'] = String(footnotesRelsXml);
       }
 
-      if (preparedComments.length) {
-        const commentsXml = this.converter.schemaToXml(this.converter.convertedXml['word/comments.xml'].elements[0]);
-        updatedDocs['word/comments.xml'] = String(commentsXml);
-
-        const commentsExtended = this.converter.convertedXml['word/commentsExtended.xml'];
-        if (commentsExtended?.elements?.[0]) {
-          const commentsExtendedXml = this.converter.schemaToXml(commentsExtended.elements[0]);
-          updatedDocs['word/commentsExtended.xml'] = String(commentsExtendedXml);
-        }
-
-        const commentsExtensible = this.converter.convertedXml['word/commentsExtensible.xml'];
-        if (commentsExtensible?.elements?.[0]) {
-          const commentsExtensibleXml = this.converter.schemaToXml(commentsExtensible.elements[0]);
-          updatedDocs['word/commentsExtensible.xml'] = String(commentsExtensibleXml);
-        }
-
-        const commentsIds = this.converter.convertedXml['word/commentsIds.xml'];
-        if (commentsIds?.elements?.[0]) {
-          const commentsIdsXml = this.converter.schemaToXml(commentsIds.elements[0]);
-          updatedDocs['word/commentsIds.xml'] = String(commentsIdsXml);
+      // Serialize each comment file if it exists in convertedXml, otherwise mark as null
+      // for deletion from the zip (removes stale originals).
+      const commentFiles = COMMENT_FILE_BASENAMES.map((name) => `word/${name}`);
+      for (const path of commentFiles) {
+        const data = this.converter.convertedXml[path];
+        if (data?.elements?.[0]) {
+          updatedDocs[path] = String(this.converter.schemaToXml(data.elements[0]));
+        } else {
+          updatedDocs[path] = null;
         }
       }