fix(diffing): ignore volatile OOXML attrs in image and paragraph diff comparison (#2421)

Author: harbournick

packages/super-editor/src/extensions/diffing/algorithm/inline-diffing.test.js

@@ -64,6 +64,29 @@ const buildInlineNodeToken = (attrs = {}, type = { name: 'link' }, pos = 0) => {
   };
 };
 
+/**
+ * Builds a mock image inline-node token for diff tests.
+ *
+ * @param {Record<string, unknown>} attrs Image node attributes.
+ * @param {number} pos Position offset for the image node.
+ * @returns {import('./inline-diffing.ts').InlineNodeToken}
+ */
+const buildImageNodeToken = (attrs = {}, pos = 0) => {
+  const nodeAttrs = { ...attrs };
+  const type = { name: 'image' };
+  return {
+    kind: 'inlineNode',
+    nodeType: 'image',
+    node: {
+      type,
+      attrs: nodeAttrs,
+      toJSON: () => ({ type: 'image', attrs: nodeAttrs }),
+    },
+    nodeJSON: { type: 'image', attrs: nodeAttrs },
+    pos,
+  };
+};
+
 /**
  * Builds text tokens without offsets for tokenizer assertions.
  *
@@ -426,3 +449,131 @@ describe('tokenizeInlineContent', () => {
     expect(tokens[5]?.offset).toBe(16);
   });
 });
+
+describe('image semantic normalization in inline diff', () => {
+  it('produces no diff when images differ only in volatile originalAttributes', () => {
+    const baseAttrs = {
+      src: 'image1.png',
+      size: { width: 100, height: 50 },
+      originalAttributes: {
+        'wp14:anchorId': 'AAAA1111',
+        'wp14:editId': 'BBBB2222',
+        cx: '914400',
+      },
+    };
+    const changedAttrs = {
+      src: 'image1.png',
+      size: { width: 100, height: 50 },
+      originalAttributes: {
+        'wp14:anchorId': 'CCCC3333',
+        'wp14:editId': 'DDDD4444',
+        cx: '914400',
+      },
+    };
+
+    const oldToken = buildImageNodeToken(baseAttrs, 5);
+    const newToken = buildImageNodeToken(changedAttrs, 5);
+
+    const diffs = getInlineDiff([oldToken], [newToken], 6);
+    expect(diffs).toEqual([]);
+  });
+
+  it('detects a real image change even when volatile attrs also differ', () => {
+    const oldAttrs = {
+      src: 'old-image.png',
+      originalAttributes: { 'wp14:anchorId': 'A1', cx: '100' },
+    };
+    const newAttrs = {
+      src: 'new-image.png',
+      originalAttributes: { 'wp14:anchorId': 'A2', cx: '100' },
+    };
+
+    const oldToken = buildImageNodeToken(oldAttrs, 3);
+    const newToken = buildImageNodeToken(newAttrs, 3);
+
+    const diffs = getInlineDiff([oldToken], [newToken], 4);
+
+    expect(diffs).toHaveLength(1);
+    expect(diffs[0].action).toBe('modified');
+    expect(diffs[0].kind).toBe('inlineNode');
+    expect(diffs[0].attrsDiff?.modified).toHaveProperty('src');
+  });
+
+  it('handles multiple images in one paragraph using type-based pairing', () => {
+    const mkImage = (src, anchorId, pos) =>
+      buildImageNodeToken({ src, originalAttributes: { 'wp14:anchorId': anchorId, cx: '100' } }, pos);
+
+    const oldTokens = [mkImage('a.png', 'ID1', 1), mkImage('b.png', 'ID2', 3)];
+    const newTokens = [mkImage('a.png', 'ID3', 1), mkImage('b.png', 'ID4', 3)];
+
+    const diffs = getInlineDiff(oldTokens, newTokens, 5);
+    expect(diffs).toEqual([]);
+  });
+
+  it('emits a diff when one of multiple images genuinely changes', () => {
+    const mkImage = (src, anchorId, pos) =>
+      buildImageNodeToken({ src, originalAttributes: { 'wp14:anchorId': anchorId } }, pos);
+
+    const oldTokens = [mkImage('a.png', 'ID1', 1), mkImage('b.png', 'ID2', 3)];
+    const newTokens = [mkImage('a.png', 'ID3', 1), mkImage('c.png', 'ID4', 3)];
+
+    const diffs = getInlineDiff(oldTokens, newTokens, 5);
+
+    expect(diffs).toHaveLength(1);
+    expect(diffs[0].action).toBe('modified');
+    expect(diffs[0].attrsDiff?.modified).toHaveProperty('src');
+  });
+
+  it('correctly detects an image insertion when a new image is prepended', () => {
+    const mkImage = (src, pos) => buildImageNodeToken({ src }, pos);
+
+    const oldTokens = [mkImage('a.png', 1), mkImage('b.png', 3)];
+    const newTokens = [mkImage('x.png', 1), mkImage('a.png', 3), mkImage('b.png', 5)];
+
+    const diffs = getInlineDiff(oldTokens, newTokens, 5);
+
+    // Should be a single insertion of x.png, not two modifications + addition
+    expect(diffs).toHaveLength(1);
+    expect(diffs[0].action).toBe('added');
+    expect(diffs[0].kind).toBe('inlineNode');
+    expect(diffs[0].nodeJSON.attrs.src).toBe('x.png');
+  });
+
+  it('correctly detects image reordering as delete + add', () => {
+    const mkImage = (src, pos) => buildImageNodeToken({ src }, pos);
+
+    const oldTokens = [mkImage('a.png', 1), mkImage('b.png', 3)];
+    const newTokens = [mkImage('b.png', 1), mkImage('a.png', 3)];
+
+    const diffs = getInlineDiff(oldTokens, newTokens, 5);
+
+    // Reorder produces diffs — at minimum some combination of added/deleted
+    expect(diffs.length).toBeGreaterThan(0);
+  });
+
+  it('excludes volatile attrs from attrsDiff when a real image change occurs', () => {
+    const oldAttrs = {
+      src: 'v1.png',
+      size: { width: 100 },
+      originalAttributes: { 'wp14:anchorId': 'OLD', 'wp14:editId': 'OLD', cx: '100' },
+    };
+    const newAttrs = {
+      src: 'v2.png',
+      size: { width: 200 },
+      originalAttributes: { 'wp14:anchorId': 'NEW', 'wp14:editId': 'NEW', cx: '100' },
+    };
+
+    const diffs = getInlineDiff([buildImageNodeToken(oldAttrs, 1)], [buildImageNodeToken(newAttrs, 1)], 2);
+
+    expect(diffs).toHaveLength(1);
+    const attrsDiff = diffs[0].attrsDiff;
+
+    // Semantic changes are reported
+    expect(attrsDiff?.modified).toHaveProperty('src');
+    expect(attrsDiff?.modified).toHaveProperty('size.width');
+
+    // Volatile changes are NOT reported
+    expect(attrsDiff?.modified).not.toHaveProperty('originalAttributes.wp14:anchorId');
+    expect(attrsDiff?.modified).not.toHaveProperty('originalAttributes.wp14:editId');
+  });
+});

packages/super-editor/src/extensions/diffing/algorithm/inline-diffing.ts

@@ -1,5 +1,6 @@
 import type { Node as PMNode } from 'prosemirror-model';
 import { getAttributesDiff, getMarksDiff, type AttributesDiff, type MarksDiff } from './attributes-diffing';
+import { normalizeInlineNodeJSON, normalizeInlineNodeAttrs, semanticInlineNodeKey } from './semantic-normalization';
 import { diffSequences } from './sequence-diffing';
 
 type NodeJSON = ReturnType<PMNode['toJSON']>;
@@ -237,7 +238,9 @@ export function getInlineDiff(
     buildDeleted: (token, oldIdx) => buildInlineDiff('deleted', token, oldIdx),
     buildModified: (oldToken, newToken, oldIdx) => {
       if (oldToken.kind !== 'text' && newToken.kind !== 'text') {
-        const attrsDiff = getAttributesDiff(oldToken.node.attrs, newToken.node.attrs);
+        const oldNormalized = normalizeInlineNodeAttrs(oldToken.node.type.name, oldToken.node.attrs);
+        const newNormalized = normalizeInlineNodeAttrs(newToken.node.type.name, newToken.node.attrs);
+        const attrsDiff = getAttributesDiff(oldNormalized, newNormalized);
         return {
           action: 'modified',
           idx: oldIdx,
@@ -270,7 +273,8 @@ export function getInlineDiff(
 
 /**
  * Compares two inline tokens to decide if they can be considered equal for the Myers diff.
- * Text tokens compare character equality while inline nodes compare their type.
+ * Text tokens compare character equality. Inline nodes compare by semantic identity
+ * (normalized JSON), not just type name, so that distinct images are not falsely paired.
  */
 function inlineComparator(a: InlineDiffToken, b: InlineDiffToken): boolean {
   if (a.kind !== b.kind) {
@@ -281,7 +285,7 @@ function inlineComparator(a: InlineDiffToken, b: InlineDiffToken): boolean {
     return a.char === b.char;
   }
   if (a.kind === 'inlineNode' && b.kind === 'inlineNode') {
-    return a.node.type.name === b.node.type.name;
+    return semanticInlineNodeKey(a.node) === semanticInlineNodeKey(b.node);
   }
   return false;
 }
@@ -299,8 +303,8 @@ function shouldProcessEqualAsModification(oldToken: InlineDiffToken, newToken: I
   }
 
   if (oldToken.kind === 'inlineNode' && newToken.kind === 'inlineNode') {
-    const oldJSON = oldToken.node.toJSON();
-    const newJSON = newToken.node.toJSON();
+    const oldJSON = normalizeInlineNodeJSON(oldToken.node.toJSON());
+    const newJSON = normalizeInlineNodeJSON(newToken.node.toJSON());
     return JSON.stringify(oldJSON) !== JSON.stringify(newJSON);
   }