fix(docx-core): clean only vacated selected paragraph-mark properties

packages/docx-compare/src/tagged/terminalMarkCleanupParity.test.ts

@@ -0,0 +1,78 @@
+import { describe, expect } from 'vitest';
+import { XMLSerializer } from '@xmldom/xmldom';
+import { testAllure } from '../testing/allure-test.js';
+import { parseXml, serializeXml, acceptChanges, rejectChanges } from '@usejunior/docx-core';
+import { acceptAllChanges, rejectAllChanges } from './trackChangesAcceptorAst.js';
+const test = testAllure.epic('Document Comparison').withLabels({ feature: 'Terminal mark cleanup parity' });
+const W = 'http://schemas.openxmlformats.org/wordprocessingml/2006/main';
+const subtree = (element: Element) => new XMLSerializer().serializeToString(element);
+const wrap = (body: string) => `<w:document xmlns:w="${W}"><w:body>${body}<w:sectPr/></w:body></w:document>`;
+const paragraph = (kind: string, color: boolean, text: boolean) => '<w:p><w:pPr><w:jc w:val="right"/><w:rPr>' +
+  `<w:${kind} w:id="1" w:author="AI"/>${color ? '<w:color w:val="FF0000"/>' : ''}` +
+  `</w:rPr></w:pPr>${text ? '<w:r><w:t>Surviving text</w:t></w:r>' : ''}</w:p>`;
+const cell = (p: string) => '<w:tbl><w:tblPr/><w:tblGrid><w:gridCol w:w="6000"/></w:tblGrid><w:tr><w:tc>' + p + '</w:tc></w:tr></w:tbl>';
+
+describe('resolved terminal paragraph-mark properties', () => {
+  for (const [name, kind, nativeProject, astProject] of [
+    ['Accept', 'del', acceptChanges, acceptAllChanges], ['Reject', 'ins', rejectChanges, rejectAllChanges],
+    ['Accept inserted', 'ins', acceptChanges, acceptAllChanges], ['Reject deleted', 'del', rejectChanges, rejectAllChanges],
+  ] as const) {
+    const revisionTest = test.conformance({ spec: 'ECMA-376', edition: 5, part: 1, section: kind === 'del' ? '17.13.5.15' : '17.13.5.20' });
+    for (const inCell of [false, true]) {
+      for (const color of [false, true]) {
+        revisionTest(`${name} terminal ${inCell ? 'cell' : 'body'} mark ${color ? 'retains color' : 'drops empty rPr'}`, () => {
+          const p = paragraph(kind, color, true);
+          const input = wrap(inCell ? cell(p) : p);
+          const doc = parseXml(input);
+          nativeProject(doc);
+          expect(serializeXml(doc)).toBe(serializeXml(parseXml(astProject(input))));
+          expect(doc.getElementsByTagNameNS(W, 'p').length).toBe(1);
+          expect(doc.getElementsByTagNameNS(W, 't')[0]!.textContent).toBe('Surviving text');
+          expect(doc.getElementsByTagNameNS(W, 'rPr').length).toBe(color ? 1 : 0);
+        });
+      }
+    }
+
+    revisionTest(`${name} preserves the required empty last table-cell paragraph`, () => {
+      const input = wrap(cell(paragraph(kind, false, false)));
+      const doc = parseXml(input);
+      nativeProject(doc);
+      expect(doc.getElementsByTagNameNS(W, 'p').length).toBe(1);
+      expect(doc.getElementsByTagNameNS(W, 'rPr').length).toBe(0);
+      expect(serializeXml(doc)).toBe(serializeXml(parseXml(astProject(input))));
+    });
+
+    revisionTest(`${name} resolves one author while preserving a foreign terminal mark and formatting`, () => {
+      const foreign = paragraph(kind, true, true).replace('w:author="AI"', 'w:author="Human"');
+      const input = wrap(cell(paragraph(kind, false, true)) + cell(foreign));
+      const doc = parseXml(input);
+      const foreignBefore = subtree(doc.getElementsByTagNameNS(W, 'p')[1]!);
+      nativeProject(doc, { filter: e => e.getAttributeNS(W, 'author') === 'AI' });
+      expect(subtree(doc.getElementsByTagNameNS(W, 'p')[1]!)).toBe(foreignBefore);
+      expect(doc.getElementsByTagNameNS(W, 'p')[0]!.getElementsByTagNameNS(W, 'rPr').length).toBe(0);
+      expect(doc.getElementsByTagNameNS(W, 'p').length).toBe(2);
+    });
+  }
+
+  for (const [name, kind, project, ast] of [['Accept', 'del', acceptChanges, acceptAllChanges], ['Reject', 'ins', rejectChanges, rejectAllChanges]] as const) {
+    for (const metadata of ['<!--PRESERVE-ME-->', '<?keep data?>']) {
+      for (const scope of ['pPr', 'rPr']) {
+        test.conformance({ spec: 'ECMA-376', edition: 5, part: 1, section: kind === 'del' ? '17.13.5.15' : '17.13.5.20' })(`${name} retains ${metadata.startsWith('<!--') ? 'comments' : 'PIs'} in ${scope}`, () => {
+          const input = wrap(paragraph(kind, false, true).replace(`<w:${scope}>`, `<w:${scope}>${metadata}`));
+          const doc = parseXml(input); project(doc);
+          expect(serializeXml(doc)).toContain(metadata);
+          expect(ast(input)).toContain(metadata);
+        });
+      }
+    }
+  }
+
+  test.conformance({ spec: 'ECMA-376', edition: 5, part: 1, section: '17.13.5.30' })('Accept cleans vacated paragraph-mark history without touching unselected metadata', () => {
+    const input = wrap('<w:p><w:pPr><w:rPr><w:rPrChange w:id="1" w:author="AI"><w:rPr><w:b/></w:rPr></w:rPrChange></w:rPr></w:pPr><w:r><w:t>Text</w:t></w:r></w:p>');
+    const doc = parseXml(input); acceptChanges(doc);
+    expect(doc.getElementsByTagNameNS(W, 'rPr').length).toBe(0);
+    const foreign = parseXml(input), before = serializeXml(foreign);
+    acceptChanges(foreign, { filter: e => e.getAttributeNS(W, 'author') === 'Human' });
+    expect(serializeXml(foreign)).toBe(before);
+  });
+});

packages/docx-compare/src/tagged/trackChangesAcceptorAst.ts

@@ -97,14 +97,16 @@ function removeParaMarkers(root: Element): void {
 }
 
 function removeEmptyParagraphMarkerContainers(root: Element): void {
+  const vacated = (element: Element): boolean => element.attributes.length === 0 &&
+    Array.from(element.childNodes).every(node => node.nodeType === 3 && !node.nodeValue?.trim());
   for (const p of findAllByTagName(root, 'w:p')) {
     const pPr = getParagraphPPr(p);
     if (!pPr) continue;
     const rPr = childElements(pPr).find((child) => child.tagName === 'w:rPr');
-    if (rPr && childElements(rPr).length === 0 && !(rPr.textContent ?? '').trim()) {
+    if (rPr && vacated(rPr)) {
       pPr.removeChild(rPr);
     }
-    if (childElements(pPr).length === 0 && !(pPr.textContent ?? '').trim()) p.removeChild(pPr);
+    if (vacated(pPr)) p.removeChild(pPr);
   }
 }
 

packages/docx-core/src/primitives/accept_changes.ts

@@ -20,7 +20,7 @@
  */
 
 import { OOXML } from './namespaces.js';
-import { retainLeadingParagraphFormatting, isEmptyParagraphFormattingRun } from './paragraph_merge_formatting.js';
+import { retainLeadingParagraphFormatting, isEmptyParagraphFormattingRun, removeEmptyParagraphMarkProperties } from './paragraph_merge_formatting.js';
 
 const W_NS = OOXML.W_NS;
 
@@ -289,6 +289,7 @@ function resolveParagraphMarkRevision(p: Element): void {
 
   const target = findFollowingSiblingParagraph(p);
   if (!target) {
+    removeEmptyParagraphMarkProperties(p);
     if (!paragraphHasContent(p) && canSafelyRemoveEmptyParagraph(p)) {
       parent.removeChild(p);
     }
@@ -365,6 +366,10 @@ export function acceptChanges(
   // Phase A — Identify paragraphs whose MARK is a tracked deletion
   const markDeletedParagraphs: Element[] = [];
   const allParagraphs = collectByLocalName(root, 'p');
+  // Capture selected direct mark/property histories before other phases remove
+  // them, including accepted insertions which do not remove a paragraph break.
+  const resolvedMarkProperties = allParagraphs.filter(p =>
+    ['ins', 'del', 'moveFrom', 'moveTo', 'rPrChange'].some(kind => paragraphHasParaMarker(p, kind, filter)));
 
   for (const p of allParagraphs) {
     // A paragraph-mark deletion (w:p > w:pPr > w:rPr > w:del) means the
@@ -490,6 +495,7 @@ export function acceptChanges(
   for (const p of markDeletedParagraphs) {
     resolveParagraphMarkRevision(p);
   }
+  for (const p of resolvedMarkProperties) removeEmptyParagraphMarkProperties(p);
 
   // Strip w:rsidDel attributes on remaining elements. Skipped in selective
   // mode: rsidDel is a document-wide save-id, and a selective accept must not

packages/docx-core/src/primitives/paragraph_merge_formatting.ts

@@ -6,6 +6,21 @@ const children = (node: Element): Element[] => Array.from(node.childNodes)
 const properties = (paragraph: Element): Element | undefined => children(paragraph)
   .find(child => child.namespaceURI === W && child.localName === 'pPr');
 
+/** Remove vacated mark-property containers after selected revision resolution.
+ * This is an implementation cleanup policy, not a normative requirement.
+ * @internal
+ * @see https://github.com/UseJunior/safe-docx/issues/982
+ */
+export function removeEmptyParagraphMarkProperties(paragraph: Element): void {
+  const pPr = properties(paragraph);
+  if (!pPr) return;
+  const rPr = children(pPr).find(child => child.namespaceURI === W && child.localName === 'rPr');
+  const vacated = (element: Element): boolean => element.attributes.length === 0 &&
+    Array.from(element.childNodes).every(node => node.nodeType === 3 && !node.nodeValue?.trim());
+  if (rPr && vacated(rPr)) pPr.removeChild(rPr);
+  if (vacated(pPr)) paragraph.removeChild(pPr);
+}
+
 /** Formatting-only test; never use this to decide whether to remove a paragraph. */
 export function isEmptyParagraphFormattingRun(element: Element): boolean {
   return element.namespaceURI === W && element.localName === 'r' && children(element).every(child =>

packages/docx-core/src/primitives/reject_changes.ts

@@ -19,7 +19,7 @@
  */
 
 import { OOXML } from './namespaces.js';
-import { retainLeadingParagraphFormatting, isEmptyParagraphFormattingRun } from './paragraph_merge_formatting.js';
+import { retainLeadingParagraphFormatting, isEmptyParagraphFormattingRun, removeEmptyParagraphMarkProperties } from './paragraph_merge_formatting.js';
 import type { RevisionFilter } from './accept_changes.js';
 
 const W_NS = OOXML.W_NS;
@@ -290,6 +290,7 @@ function resolveParagraphMarkRevision(p: Element): void {
 
   const target = findFollowingSiblingParagraph(p);
   if (!target) {
+    removeEmptyParagraphMarkProperties(p);
     if (!paragraphHasContent(p) && canSafelyRemoveEmptyParagraph(p)) {
       parent.removeChild(p);
     }
@@ -460,6 +461,8 @@ export function rejectChanges(
   // Phase A — Identify paragraphs whose MARK is a tracked insertion
   const markInsertedParagraphs = new Set<Element>();
   const allParagraphs = collectByLocalName(root, 'p');
+  const resolvedMarkProperties = allParagraphs.filter(p =>
+    ['ins', 'del', 'moveFrom', 'moveTo', 'rPrChange'].some(kind => paragraphHasParaMarker(p, kind, filter)));
 
   for (const p of allParagraphs) {
     // A paragraph-mark insertion (w:p > w:pPr > w:rPr > w:ins) means the
@@ -687,6 +690,7 @@ export function rejectChanges(
   for (const p of markInsertedParagraphs) {
     resolveParagraphMarkRevision(p);
   }
+  for (const p of resolvedMarkProperties) removeEmptyParagraphMarkProperties(p);
 
   // Strip w:rsidDel attributes on remaining elements. Skipped in selective mode
   // so a targeted reject leaves foreign elements byte-untouched (#125).