SD-2974 - Bug: image resizing in suggesting mode duplicates the image

packages/super-editor/src/editors/v1/components/ImageResizeOverlay.test.js

@@ -9,12 +9,29 @@ vi.mock('@superdoc/layout-bridge', () => ({
 }));
 
 function createMockEditor(overrides = {}) {
+  // Block ancestor carrying sdBlockRev, as resolved when bumping the
+  // containing block's revision after the AttrStep resize.
+  const paragraphNode = {
+    isBlock: true,
+    type: { name: 'paragraph', spec: { attrs: { sdBlockId: {}, sdBlockRev: {} } } },
+    attrs: { sdBlockRev: 3 },
+  };
   return {
     options: { documentMode: 'editing' },
     isEditable: true,
     view: {
       dom: document.createElement('div'),
-      state: { doc: { nodeAt: vi.fn() }, tr: { setNodeMarkup: vi.fn().mockReturnThis() } },
+      state: {
+        doc: {
+          nodeAt: vi.fn(),
+          resolve: vi.fn(() => ({
+            depth: 1,
+            node: () => paragraphNode,
+            before: () => 0,
+          })),
+        },
+        tr: { setNodeAttribute: vi.fn().mockReturnThis() },
+      },
       dispatch: vi.fn(),
     },
     ...overrides,
@@ -153,7 +170,7 @@ describe('ImageResizeOverlay', () => {
       expect(wrapper.vm.dragState).toBe(null);
       expect(wrapper.find('.resize-guideline').exists()).toBe(false);
       expect(editor.view.dispatch).not.toHaveBeenCalled();
-      expect(editor.view.state.tr.setNodeMarkup).not.toHaveBeenCalled();
+      expect(editor.view.state.tr.setNodeAttribute).not.toHaveBeenCalled();
 
       wrapper.unmount();
       remove();
@@ -207,11 +224,10 @@ describe('ImageResizeOverlay', () => {
     document.dispatchEvent(new MouseEvent('mousemove', { clientX: 150, clientY: 90 }));
     document.dispatchEvent(new MouseEvent('mouseup', { clientX: 150, clientY: 90 }));
 
-    expect(editor.view.state.tr.setNodeMarkup).toHaveBeenCalledWith(
-      0,
-      null,
-      expect.objectContaining({ size: { width: 140, height: 70 } }),
-    );
+    expect(editor.view.state.tr.setNodeAttribute).toHaveBeenCalledWith(0, 'size', { width: 140, height: 70 });
+    // AttrSteps have no changed range, so the containing block's sdBlockRev
+    // must be bumped explicitly for the layout engine to repaint.
+    expect(editor.view.state.tr.setNodeAttribute).toHaveBeenCalledWith(0, 'sdBlockRev', 4);
     expect(editor.view.dispatch).toHaveBeenCalledWith(editor.view.state.tr);
 
     wrapper.unmount();
@@ -274,11 +290,10 @@ describe('ImageResizeOverlay', () => {
 
     expect(headerFooterEditor.view.state.doc.nodeAt).toHaveBeenCalledWith(0);
     expect(bodyEditor.view.state.doc.nodeAt).not.toHaveBeenCalled();
-    expect(headerFooterEditor.view.state.tr.setNodeMarkup).toHaveBeenCalledWith(
-      0,
-      null,
-      expect.objectContaining({ size: { width: 140, height: 70 } }),
-    );
+    expect(headerFooterEditor.view.state.tr.setNodeAttribute).toHaveBeenCalledWith(0, 'size', {
+      width: 140,
+      height: 70,
+    });
     expect(headerFooterEditor.view.dispatch).toHaveBeenCalledWith(headerFooterEditor.view.state.tr);
     expect(bodyEditor.view.dispatch).not.toHaveBeenCalled();
 

packages/super-editor/src/editors/v1/components/ImageResizeOverlay.vue

@@ -23,6 +23,7 @@
 import { ref, computed, watch, onBeforeUnmount } from 'vue';
 import { measureCache } from '@superdoc/layout-bridge';
 import { isContentLockedMode } from '../extensions/structured-content/lockModes.js';
+import { nodeAllowsSdBlockRevAttr } from '../extensions/block-node/block-node.js';
 
 // Configuration constants
 const OVERLAY_EXPANSION_PX = 2000;
@@ -622,18 +623,31 @@ function dispatchResizeTransaction(blockId, newWidth, newHeight) {
       return;
     }
 
-    // Store pixel dimensions directly (converted to EMU only during DOCX export)
-    const newAttrs = {
-      ...imageNode.attrs,
-      size: {
-        width: Math.round(newWidth),
-        height: Math.round(newHeight),
-      },
-    };
+    // Store pixel dimensions directly (converted to EMU only during DOCX export).
+    // Use setNodeAttribute (AttrStep) instead of setNodeMarkup (ReplaceStep):
+    // Word does not track image resizes as revisions, and AttrSteps pass through
+    // the track-changes machinery untracked, so the resize applies in place in
+    // suggesting mode — even when the image is itself a pending tracked
+    // insertion. A ReplaceStep would be split into a tracked insert + delete,
+    // rendering as a duplicate image (SD-2974).
+    tr.setNodeAttribute(imagePos, 'size', {
+      width: Math.round(newWidth),
+      height: Math.round(newHeight),
+    });
 
-    tr.setNodeMarkup(imagePos, null, newAttrs);
+    // AttrSteps have no changed range, so blockNodePlugin won't bump the
+    // containing blocks' sdBlockRev and the layout engine would reuse the
+    // cached FlowBlock (stale paint). Bump the revs here, same pattern as
+    // numberingPlugin's bumpBlockRev.
+    const $imagePos = state.doc.resolve(imagePos);
+    for (let depth = $imagePos.depth; depth > 0; depth--) {
+      const ancestor = $imagePos.node(depth);
+      if (!nodeAllowsSdBlockRevAttr(ancestor)) continue;
+      const currentRev = Number.parseInt(ancestor.attrs?.sdBlockRev, 10);
+      if (!Number.isFinite(currentRev)) continue;
+      tr.setNodeAttribute($imagePos.before(depth), 'sdBlockRev', currentRev + 1);
+    }
 
-    // Dispatch transaction
     dispatch(tr);
 
     // Invalidate the measure cache for this image to force re-measurement with new size

tests/behavior/tests/images/image-resize-in-suggesting-mode.spec.ts

@@ -0,0 +1,208 @@
+import { test, expect } from '../../fixtures/superdoc.js';
+import path from 'node:path';
+
+/**
+ * SD-2974: resizing an image in suggesting mode must not produce a duplicate
+ * image. Word does not track image resizes as revisions; the resize should
+ * apply in place rather than being split into a tracked insert + delete.
+ */
+
+test.use({ config: { toolbar: 'full', showSelection: true, trackChanges: true } });
+
+const FIXTURE = path.resolve(import.meta.dirname, 'fixtures/sd-2323-image-resize-test.docx');
+
+async function getImageCount(superdoc: any): Promise<number> {
+  return superdoc.page.evaluate(() => {
+    const doc = (window as any).editor?.state?.doc;
+    if (!doc) throw new Error('Editor document is unavailable.');
+    let count = 0;
+    doc.descendants((node: any) => {
+      if (node.type?.name === 'image') count += 1;
+    });
+    return count;
+  });
+}
+
+async function getFirstImageSize(superdoc: any): Promise<{ width: number; height: number }> {
+  return superdoc.page.evaluate(() => {
+    const doc = (window as any).editor?.state?.doc;
+    if (!doc) throw new Error('Editor document is unavailable.');
+    let size: { width: number; height: number } | null = null;
+    doc.descendants((node: any) => {
+      if (!size && node.type?.name === 'image') {
+        size = node.attrs?.size ?? null;
+      }
+    });
+    if (!size) throw new Error('No image node with a size attr found.');
+    return { width: size.width, height: size.height };
+  });
+}
+
+async function firstImageIsPendingTrackedInsertion(superdoc: any): Promise<boolean> {
+  return superdoc.page.evaluate(() => {
+    const doc = (window as any).editor?.state?.doc;
+    if (!doc) throw new Error('Editor document is unavailable.');
+    let tracked = false;
+    let found = false;
+    doc.descendants((node: any) => {
+      if (!found && node.type?.name === 'image') {
+        found = true;
+        tracked = node.marks?.some((mark: any) => mark.type?.name === 'trackInsert') ?? false;
+      }
+    });
+    if (!found) throw new Error('No image node found.');
+    return tracked;
+  });
+}
+
+/**
+ * Hover the image to surface the resize overlay, then drag the SE handle by
+ * (dx, dy). Negative deltas shrink the image. The exact end-size doesn't
+ * matter — the delta only needs to exceed the 1px DIMENSION_CHANGE_THRESHOLD
+ * so the transaction actually dispatches; callers assert the size changed.
+ */
+async function dragSeResizeHandle(superdoc: any, dx: number, dy: number): Promise<void> {
+  const img = superdoc.page.locator('.superdoc-inline-image').first();
+  await expect(img).toBeAttached({ timeout: 5000 });
+  await img.hover();
+  await superdoc.waitForStable();
+
+  const overlay = superdoc.page.locator('.superdoc-image-resize-overlay');
+  await expect(overlay).toBeAttached({ timeout: 5000 });
+
+  const handle = overlay.locator('.resize-handle--se');
+  await expect(handle).toBeAttached({ timeout: 5000 });
+  const handleBox = await handle.boundingBox();
+  if (!handleBox) throw new Error('Could not locate SE resize handle.');
+
+  const startX = handleBox.x + handleBox.width / 2;
+  const startY = handleBox.y + handleBox.height / 2;
+
+  await superdoc.page.mouse.move(startX, startY);
+  await superdoc.page.mouse.down();
+  // Use multiple steps so the throttled mousemove handler in
+  // ImageResizeOverlay.vue produces an updated constrainedWidth/Height
+  // before mouseup commits.
+  await superdoc.page.mouse.move(startX + dx, startY + dy, { steps: 10 });
+  await superdoc.page.mouse.up();
+
+  await superdoc.waitForStable();
+}
+
+test.describe('Image resize in suggesting mode (SD-2974)', () => {
+  test.beforeEach(async ({ superdoc }) => {
+    await superdoc.loadDocument(FIXTURE);
+  });
+
+  test('@behavior SD-2974: resizing an image in suggesting mode does not duplicate the image', async ({ superdoc }) => {
+    // Sanity: exactly one image to start with.
+    expect(await getImageCount(superdoc)).toBe(1);
+
+    await superdoc.setDocumentMode('suggesting');
+    await superdoc.waitForStable();
+    await superdoc.assertDocumentMode('suggesting');
+
+    const before = await getFirstImageSize(superdoc);
+    await dragSeResizeHandle(superdoc, -60, -60);
+
+    // The size must actually have changed — otherwise the drag no-opped (the
+    // dispatch is skipped under the 1px threshold) and the duplicate-count
+    // assertion below would pass without exercising the fix.
+    const after = await getFirstImageSize(superdoc);
+    expect(after.width).toBeLessThan(before.width);
+
+    // After the resize commit there must still be exactly one image node.
+    // Before the SD-2974 fix the tracked-change machinery split the
+    // ReplaceStep (generated by setNodeMarkup on the leaf image node) into
+    // a tracked insert + delete, leaving two image nodes in the document.
+    expect(await getImageCount(superdoc)).toBe(1);
+  });
+
+  test('@behavior SD-2974: repeated resizes do not accumulate duplicate images', async ({ superdoc }) => {
+    expect(await getImageCount(superdoc)).toBe(1);
+
+    await superdoc.setDocumentMode('suggesting');
+    await superdoc.waitForStable();
+    await superdoc.assertDocumentMode('suggesting');
+
+    const initial = await getFirstImageSize(superdoc);
+    await dragSeResizeHandle(superdoc, -60, -60);
+
+    const afterFirst = await getFirstImageSize(superdoc);
+    expect(afterFirst.width).toBeLessThan(initial.width);
+    expect(await getImageCount(superdoc)).toBe(1);
+
+    await dragSeResizeHandle(superdoc, -30, -30);
+
+    const afterSecond = await getFirstImageSize(superdoc);
+    expect(afterSecond.width).toBeLessThan(afterFirst.width);
+    expect(await getImageCount(superdoc)).toBe(1);
+  });
+});
+
+test.describe('Resize of a pending tracked image insertion (SD-2974)', () => {
+  test('@behavior SD-2974: resizing an image that is itself a pending tracked insertion does not duplicate it', async ({
+    superdoc,
+  }) => {
+    // Set up an image that is itself a pending tracked insertion (insert in
+    // suggesting mode → resize before acceptance — the customer flow in
+    // SD-2974's Loom). This is the case most likely to regress: a ReplaceStep
+    // resize of a tracked-inserted node is re-routed through the tracking
+    // machinery by the protect-tracked-review-state guard regardless of
+    // skipTrackChanges. The AttrStep resize never enters that machinery.
+    //
+    // The state is constructed directly (image node carrying a trackInsert
+    // mark) rather than via editor.commands.setImage in suggesting mode:
+    // image registration (imageRegistrationPlugin) asynchronously rewrites
+    // freshly inserted images via an untracked setNodeMarkup, which has a
+    // suggesting-mode duplication of its own and would contaminate this
+    // test's premise.
+    await superdoc.setDocumentMode('suggesting');
+    await superdoc.waitForStable();
+    await superdoc.assertDocumentMode('suggesting');
+
+    await superdoc.page.evaluate(() => {
+      const editor = (window as any).editor;
+      const { state } = editor;
+      const mark = state.schema.marks.trackInsert.create({
+        id: 'sd-2974-pending-insert',
+        author: 'Behavior Test',
+        authorEmail: 'test@example.com',
+        date: new Date().toISOString(),
+      });
+      const image = state.schema.nodes.image.create(
+        {
+          src: 'assets/image-landscape.png',
+          alt: 'Pending insertion',
+          size: { width: 240, height: 160 },
+          sdImageId: 'sd-2974-pending-image',
+          // Mark the image as already registered so imageRegistrationPlugin
+          // does not asynchronously re-dispatch a setNodeMarkup for it (that
+          // un-tracked rewrite path has its own suggesting-mode duplication,
+          // which would contaminate this test's premise).
+          rId: 'rId-sd-2974-test',
+        },
+        null,
+        [mark],
+      );
+      const tr = state.tr.insert(state.selection.from, image);
+      tr.setMeta('skipTrackChanges', true);
+      editor.view.dispatch(tr);
+    });
+    await superdoc.waitForStable();
+
+    // Sanity: one image, and it is a pending tracked insertion.
+    expect(await getImageCount(superdoc)).toBe(1);
+    expect(await firstImageIsPendingTrackedInsertion(superdoc)).toBe(true);
+
+    const before = await getFirstImageSize(superdoc);
+    await dragSeResizeHandle(superdoc, -40, -40);
+
+    const after = await getFirstImageSize(superdoc);
+    expect(after.width).toBeLessThan(before.width);
+    expect(await getImageCount(superdoc)).toBe(1);
+
+    // The resize must not have disturbed the pending insertion itself.
+    expect(await firstImageIsPendingTrackedInsertion(superdoc)).toBe(true);
+  });
+});