From d37bc01e1a4040cdf342b9254460f6a5fc1dfb6d Mon Sep 17 00:00:00 2001 From: Ran Luo Date: Wed, 24 Jun 2026 08:32:01 +0800 Subject: [PATCH] =?UTF-8?q?fix(muya):=20undo/redo=20corruption=20on=20coal?= =?UTF-8?q?esced=20paragraph=E2=86=92list=20+=20text=20edits=20(#4669)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(muya): invert undo/redo ops against the live document History._change rebuilt the reverse op with the bare `json1.type.invert`, which trusts the removed values carried in the op. A coalesced entry that removes a subtree containing a text edit records the POST-edit removed value (e.g. an empty list item after the typed characters were composed away), so redo re-inserted a node missing the typed text and a follow-up edit threw "The op is too long for this document". Invert against the current json state with `invertWithDoc` so the true removed value is reread, keeping undo/redo lossless. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(muya): append at end when undo/redo replaces the last block The incremental updateContents walker mishandled a block replace at the last child index: the pick phase removes the old block, then drop calls `insertBefore(newBlock, ref)` where `ref` is now undefined — a no-op, so the new block was silently dropped and the live DOM desynced from the json state. A later op then walked a missing path and crashed with "Cannot read properties of undefined (reading 'blockName')". Append the block when there is no insert reference (end of the list). Co-Authored-By: Claude Opus 4.8 (1M context) * fix(muya): restore selection by path after a structural undo/redo Restoring an undo/redo selection trusted the cached `anchor.block` / `focus.block` references, but a structural op (e.g. a paragraph→list replace) detaches those blocks. Setting a DOM range on a detached node logged "addRange(): The given range isn't in document", and reading the detached block's path/blockName in a later getSelection() threw. Re-resolve both endpoints from their path against the fresh tree and focus a valid block when the path no longer points at content; bail out of getSelection() when an endpoint is detached. Co-Authored-By: Claude Opus 4.8 (1M context) * test(muya): cover undo/redo of a coalesced paragraph→list + text edit Reproduces the reported corruption (type "- " to make a list, type text, undo twice, redo): redo must restore the typed text losslessly and the caret-inside-converted-block path must not crash. Asserts the live block tree (not just getMarkdown, which reads the json state) so a DOM/state desync can't pass silently. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- packages/muya/src/editor/index.ts | 23 ++-- .../undoRedoCoalescedListRoundTrip.spec.ts | 122 ++++++++++++++++++ packages/muya/src/history/index.ts | 5 +- packages/muya/src/selection/TextSelection.ts | 3 + 4 files changed, 144 insertions(+), 9 deletions(-) create mode 100644 packages/muya/src/history/__tests__/undoRedoCoalescedListRoundTrip.spec.ts diff --git a/packages/muya/src/editor/index.ts b/packages/muya/src/editor/index.ts index 4ef6afd7ca..8917825b8f 100644 --- a/packages/muya/src/editor/index.ts +++ b/packages/muya/src/editor/index.ts @@ -36,6 +36,7 @@ type BlockNode = { remove?: (source: string) => void; replaceWith?: (newBlock: BlockNode, source: string) => void; insertBefore?: (newBlock: BlockNode, ref: BlockNode, source: string) => void; + append?: (newBlock: BlockNode, source: string) => void; update?: (value?: unknown, source?: string) => void; blockName?: string; align?: string; @@ -152,8 +153,12 @@ function drop(root: BlockNode, descent: JSONOpList, muya: Muya): BlockNode { if (typeof key === 'number') { const insertedState = comp.i as { name: string }; const newBlock = ScrollPage.loadBlock(insertedState.name).create(muya, insertedState) as BlockNode; - if (cur && ref && newBlock) - cur.insertBefore?.(newBlock, ref, 'api'); + if (cur && newBlock) { + if (ref) + cur.insertBefore?.(newBlock, ref, 'api'); + else + cur.append?.(newBlock, 'api'); + } subDoc = newBlock; } @@ -442,13 +447,15 @@ export class Editor { return; } - // Incremental (updateContents) path: blocks are still attached. Clone the - // paths so `queryBlock(path)` can't drain the caller's arrays — notably - // the selection object stored in the undo stack. - const anchorBlock = anchor.block ?? this.scrollPage?.queryBlock([...anchor.path]); - const focusBlock = focus.block ?? this.scrollPage?.queryBlock([...focus.path]); - if (!anchorBlock || !anchorBlock.isContent() || !focusBlock || !focusBlock.isContent()) + // Incremental (updateContents) path. Clone the paths so + // `queryBlock(path)` can't drain the caller's arrays — notably the + // selection object stored in the undo stack. + const anchorBlock = this.scrollPage?.queryBlock([...anchor.path]); + const focusBlock = this.scrollPage?.queryBlock([...focus.path]); + if (!anchorBlock || !anchorBlock.isContent() || !focusBlock || !focusBlock.isContent()) { + this.focus(); return; + } this.selection.setSelection( { offset: anchor.offset, block: anchorBlock, path: [...anchor.path] }, diff --git a/packages/muya/src/history/__tests__/undoRedoCoalescedListRoundTrip.spec.ts b/packages/muya/src/history/__tests__/undoRedoCoalescedListRoundTrip.spec.ts new file mode 100644 index 0000000000..78a23557ba --- /dev/null +++ b/packages/muya/src/history/__tests__/undoRedoCoalescedListRoundTrip.spec.ts @@ -0,0 +1,122 @@ +// @vitest-environment happy-dom + +import type Format from '../../block/base/format'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { Muya } from '../../muya'; + +const bootedHosts: HTMLElement[] = []; + +beforeEach(() => { + window.MUYA_VERSION = 'test'; +}); + +afterEach(() => { + while (bootedHosts.length) + bootedHosts.pop()!.remove(); + delete (window as Partial).MUYA_VERSION; +}); + +function bootMuya(markdown: string): Muya { + const host = document.createElement('div'); + document.body.appendChild(host); + const muya = new Muya(host, { markdown } as ConstructorParameters[1]); + muya.init(); + bootedHosts.push(muya.domNode); + return muya; +} + +function placeCursorOnFirstContent(muya: Muya): void { + const first = muya.editor.scrollPage!.firstContentInDescendant()!; + muya.editor.activeContentBlock = first; + first.setCursor(0, 0, true); +} + +function secondBlockContent(muya: Muya): Format { + const blocks: { firstContentInDescendant: () => Format }[] = []; + (muya.editor.scrollPage as unknown as { + children: { forEach: (cb: (b: { firstContentInDescendant: () => Format }) => void) => void }; + }).children.forEach(b => blocks.push(b)); + return blocks[1].firstContentInDescendant(); +} + +function undoDepth(muya: Muya): number { + // @ts-expect-error — reach into the private stack for test assertions. + return muya.editor.history._stack.undo.length; +} + +describe('undo/redo of a coalesced paragraph→list + text edit', () => { + it('restores the typed list-item text on redo', async () => { + const muya = bootMuya('# anchor\n\nseed\n'); + + const para = secondBlockContent(muya); + para.text = '- '; + para.checkInlineUpdate(); + await vi.waitFor(() => { + expect(muya.getMarkdown()).toContain('- '); + expect(undoDepth(muya)).toBe(1); + }); + + const listContent = secondBlockContent(muya); + listContent.text = 'foo'; + await vi.waitFor(() => { + expect(muya.getMarkdown()).toContain('foo'); + }); + expect(undoDepth(muya)).toBe(1); + + placeCursorOnFirstContent(muya); + muya.undo(); + await vi.waitFor(() => { + const md = muya.getMarkdown(); + expect(md).toContain('seed'); + expect(md).not.toContain('foo'); + }); + expect(muya.getMarkdown()).not.toContain('- '); + + placeCursorOnFirstContent(muya); + muya.redo(); + await vi.waitFor(() => { + expect(muya.getMarkdown()).toContain('foo'); + }); + }); + + it('undo/redo with the caret left inside the converted block does not crash', async () => { + const muya = bootMuya('hello world\n\nx\n'); + + const para = secondBlockContent(muya); + para.setCursor(0, 0, true); + para.text = '- '; + para.checkInlineUpdate(); + await vi.waitFor(() => { + expect(muya.getMarkdown()).toContain('- '); + }); + + const listContent = secondBlockContent(muya); + listContent.setCursor(0, 0, true); + listContent.text = 'foo'; + await vi.waitFor(() => { + expect(muya.getMarkdown()).toContain('foo'); + }); + + muya.editor.history.cutoff(); + const listContent2 = secondBlockContent(muya); + listContent2.setCursor(3, 3, true); + listContent2.text = 'foo bar'; + await vi.waitFor(() => { + expect(muya.getMarkdown()).toContain('foo bar'); + }); + + muya.undo(); + await vi.waitFor(() => expect(muya.getMarkdown()).not.toContain(' bar')); + muya.undo(); + await vi.waitFor(() => expect(muya.getMarkdown()).not.toContain('foo')); + + muya.redo(); + await vi.waitFor(() => expect(muya.getMarkdown()).toContain('foo')); + muya.redo(); + await vi.waitFor(() => expect(muya.getMarkdown()).toContain('foo bar')); + + const live = secondBlockContent(muya); + expect((live as unknown as { text: string }).text).toBe('foo bar'); + expect((live as unknown as { blockName: string }).blockName).toContain('paragraph'); + }); +}); diff --git a/packages/muya/src/history/index.ts b/packages/muya/src/history/index.ts index b7d73e3284..a29fc43a01 100644 --- a/packages/muya/src/history/index.ts +++ b/packages/muya/src/history/index.ts @@ -151,7 +151,10 @@ class History { return; const { operation, selection, rebuild } = this._stack[source].pop()!; - const inverseOperation = json1.type.invert(operation); + const inverseOperation = json1.type.invertWithDoc( + operation, + asDoc(this._muya.editor.jsonState.getState()), + ); this._stack[dest].push({ operation: inverseOperation as JSONOpList, diff --git a/packages/muya/src/selection/TextSelection.ts b/packages/muya/src/selection/TextSelection.ts index 22d211c96f..401297d39f 100644 --- a/packages/muya/src/selection/TextSelection.ts +++ b/packages/muya/src/selection/TextSelection.ts @@ -177,6 +177,9 @@ class TextSelection { if (!anchorBlock || !focusBlock) return null; + if (!anchorBlock.outMostBlock || !focusBlock.outMostBlock) + return null; + const anchorPath = anchorBlock.path; const focusPath = focusBlock.path;