diff --git a/packages/super-editor/src/editors/v1/components/ImageResizeOverlay.test.js b/packages/super-editor/src/editors/v1/components/ImageResizeOverlay.test.js index fb6faf50a6..093d5f128d 100644 --- a/packages/super-editor/src/editors/v1/components/ImageResizeOverlay.test.js +++ b/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(); diff --git a/packages/super-editor/src/editors/v1/components/ImageResizeOverlay.vue b/packages/super-editor/src/editors/v1/components/ImageResizeOverlay.vue index c77d97d44f..e8e03982ad 100644 --- a/packages/super-editor/src/editors/v1/components/ImageResizeOverlay.vue +++ b/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 diff --git a/tests/behavior/tests/images/image-resize-in-suggesting-mode.spec.ts b/tests/behavior/tests/images/image-resize-in-suggesting-mode.spec.ts new file mode 100644 index 0000000000..82b0028432 --- /dev/null +++ b/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 { + 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 { + 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 { + 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); + }); +});