From 8f51b4f62fedce5ce649fc20fc0b732dd2f93cd7 Mon Sep 17 00:00:00 2001 From: n374 <4954114+n374@users.noreply.github.com> Date: Fri, 3 Jul 2026 15:16:03 +0800 Subject: [PATCH] fix(editor): repair empty list items parsed from a markdown draft (#4869) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The real ordered-list caret bug (MUL-3973). Typing 1. in the comment box persists the draft "1. \n\n"; on remount @tiptap/markdown parses that empty item into a schema-invalid, childless listItem, leaving the document with an AllSelection instead of a text cursor — so the browser paints the caret on the following block and it can't be moved back into the list. Verified against the REAL editor (real createEditorExtensions + @tiptap/markdown, no @tiptap/react mock). Non-empty items round-trip fine; only the empty-item round-trip corrupts, which the reverted #4813 never exercised. - repairEmptyListItems(): rebuild from JSON so every list/task item leads with a paragraph (covers empty items and nested items whose first child is a sub-list); reset the AllSelection first (else setContent collapses the list); keep the whole repair off the undo stack; restore the prior caret (sync path) or land in the list item (mount). - Called in onCreate (mount) and after the sync-effect setContent. - Real-editor tests incl. undo-does-not-revive and nested-list schema validity. Co-authored-by: multica-agent --- packages/views/editor/content-editor.test.tsx | 7 + packages/views/editor/content-editor.tsx | 23 ++- .../editor/utils/repair-list-items.test.ts | 175 ++++++++++++++++++ .../views/editor/utils/repair-list-items.ts | 105 +++++++++++ 4 files changed, 303 insertions(+), 7 deletions(-) create mode 100644 packages/views/editor/utils/repair-list-items.test.ts create mode 100644 packages/views/editor/utils/repair-list-items.ts diff --git a/packages/views/editor/content-editor.test.tsx b/packages/views/editor/content-editor.test.tsx index ae38388ee..c4dab764b 100644 --- a/packages/views/editor/content-editor.test.tsx +++ b/packages/views/editor/content-editor.test.tsx @@ -42,6 +42,13 @@ vi.mock("./utils/preprocess", () => ({ preprocessMarkdown: (value: string) => value, })); +// Empty-list repair needs a live ProseMirror doc (covered by +// repair-list-items.test.ts against the real editor). Here it is a no-op so the +// mocked editor's sync path exercises the normal (non-repair) branch. +vi.mock("./utils/repair-list-items", () => ({ + repairEmptyListItems: vi.fn(() => false), +})); + vi.mock("./bubble-menu", () => ({ EditorBubbleMenu: () => null, })); diff --git a/packages/views/editor/content-editor.tsx b/packages/views/editor/content-editor.tsx index 51e724972..4db183e77 100644 --- a/packages/views/editor/content-editor.tsx +++ b/packages/views/editor/content-editor.tsx @@ -54,6 +54,7 @@ import type { MentionItem } from "./extensions/mention-suggestion"; import { createEditorExtensions } from "./extensions"; import { uploadAndInsertFile } from "./extensions/file-upload"; import { preprocessMarkdown } from "./utils/preprocess"; +import { repairEmptyListItems } from "./utils/repair-list-items"; import { openLink, isMentionHref } from "./utils/link-handler"; import { EditorBubbleMenu } from "./bubble-menu"; import { useLinkHover, LinkHoverCard } from "./link-hover-card"; @@ -325,6 +326,10 @@ const ContentEditor = forwardRef( }); } } + // A markdown draft ending in an empty list item (e.g. `"1. \n\n"` left + // after typing `1.`) parses into a caretless, schema-invalid item; + // repair it so the mounted editor has a real cursor in the list. + repairEmptyListItems(ed); lastEmittedRef.current = normalizeEditorMarkdown(ed); }, content: mountChunked ? "" : initialContent, @@ -467,13 +472,17 @@ const ContentEditor = forwardRef( }); } - // Clamp prior selection to the new doc size so the caret doesn't snap - // to position 0 after ProseMirror replaces the document. - const docSize = editor.state.doc.content.size; - editor.commands.setTextSelection({ - from: Math.min(from, docSize), - to: Math.min(to, docSize), - }); + // An empty list item in the incoming markdown parses into a caretless, + // schema-invalid node; repair it and let it own the caret. Otherwise clamp + // the prior selection to the new doc size so the caret doesn't snap to + // position 0 after ProseMirror replaces the document. + if (!repairEmptyListItems(editor, { from, to })) { + const docSize = editor.state.doc.content.size; + editor.commands.setTextSelection({ + from: Math.min(from, docSize), + to: Math.min(to, docSize), + }); + } lastEmittedRef.current = normalizeEditorMarkdown(editor); }, [defaultValue, editor]); diff --git a/packages/views/editor/utils/repair-list-items.test.ts b/packages/views/editor/utils/repair-list-items.test.ts new file mode 100644 index 000000000..cb72af1d8 --- /dev/null +++ b/packages/views/editor/utils/repair-list-items.test.ts @@ -0,0 +1,175 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { Editor } from "@tiptap/core"; +import { TextSelection } from "@tiptap/pm/state"; +import type { Node as PMNode } from "@tiptap/pm/model"; +import { createEditorExtensions } from "../extensions"; +import { repairEmptyListItems } from "./repair-list-items"; + +// A REAL editor — real createEditorExtensions + real @tiptap/markdown, no +// @tiptap/react mock. This is the layer the reverted #4813 failed to exercise: +// its green suite validated a hand-fabricated `editorState.markdown`, not the +// actual markdown round-trip. +let editor: Editor | undefined; + +function makeEditor(content?: string): Editor { + const el = document.createElement("div"); + document.body.appendChild(el); + editor = new Editor({ + element: el, + extensions: createEditorExtensions({}), + content, + contentType: content ? "markdown" : undefined, + }); + return editor; +} + +function firstItem(ed: Editor, name = "listItem"): PMNode { + let found: PMNode | undefined; + ed.state.doc.descendants((node) => { + if (!found && node.type.name === name) found = node; + return true; + }); + if (!found) throw new Error(`no ${name} found`); + return found; +} + +afterEach(() => { + editor?.destroy(); + editor = undefined; + document.body.innerHTML = ""; +}); + +describe("repairEmptyListItems (real editor)", () => { + it("reproduces the corrupt empty-ordered-list round-trip, then repairs it", () => { + // The draft persisted after typing `1.` in a comment, restored on remount. + const ed = makeEditor("1. \n\n"); + + // Failing-first: @tiptap/markdown parses the empty item into a childless + // listItem, and the document is left with no real text cursor. + expect(firstItem(ed).childCount).toBe(0); + const before = ed.state.selection; + expect(before instanceof TextSelection && before.$cursor != null).toBe(false); + + repairEmptyListItems(ed); + + // The list item now holds a paragraph and the caret sits inside it (line 1), + // ready to keep typing — not on the following block. + const li = firstItem(ed); + expect(li.childCount).toBe(1); + expect(li.firstChild?.type.name).toBe("paragraph"); + const after = ed.state.selection; + expect(after instanceof TextSelection).toBe(true); + expect((after as TextSelection).$cursor).not.toBeNull(); + expect(after.$from.parent.type.name).toBe("paragraph"); + expect(after.$from.node(-1)?.type.name).toBe("listItem"); + // The repaired document round-trips cleanly. + expect(ed.getMarkdown()).toBe("1. \n\n"); + }); + + it("does not leave the repair on the undo stack (Ctrl-Z must not revive the bug)", () => { + const ed = makeEditor("1. \n\n"); + repairEmptyListItems(ed); + expect(firstItem(ed).childCount).toBe(1); + + // Undo must not restore the childless item / AllSelection. + expect(ed.can().undo()).toBe(false); + ed.commands.undo(); + expect(firstItem(ed).childCount).toBe(1); + expect(ed.state.selection instanceof TextSelection).toBe(true); + }); + + it("keeps the document schema-valid across undo/redo when a prior edit history exists", () => { + const ed = makeEditor("hello\n\n"); + // Build real undo history, then simulate an external empty-list update. + ed.commands.insertContentAt(ed.state.doc.content.size - 1, " world"); + ed.commands.setContent("1. \n\n", { contentType: "markdown" }); + repairEmptyListItems(ed); + + for (let i = 0; i < 4; i++) { + ed.commands.undo(); + expect(() => ed.state.doc.check()).not.toThrow(); + } + for (let i = 0; i < 4; i++) { + ed.commands.redo(); + expect(() => ed.state.doc.check()).not.toThrow(); + } + }); + + it("repairs a nested empty ordered list to a schema-valid document", () => { + // The outer item's only child is the sub-list (not a paragraph), so it is + // itself schema-invalid until we prepend a paragraph. + const ed = makeEditor("1. \n 1. \n\n"); + expect(() => ed.state.doc.check()).toThrow(); + + repairEmptyListItems(ed); + + // No schema violation remains, and there is a real text cursor. + expect(() => ed.state.doc.check()).not.toThrow(); + expect(ed.state.selection instanceof TextSelection).toBe(true); + expect((ed.state.selection as TextSelection).$cursor).not.toBeNull(); + }); + + it("repairs a directly-constructed malformed list (bullet item with no paragraph)", () => { + // Guards the general contract beyond the ordered-list markdown case. + const ed = makeEditor(); + ed.view.dispatch( + ed.state.tr.setSelection(TextSelection.near(ed.state.doc.resolve(0))), + ); + ed.commands.setContent( + { + type: "doc", + content: [{ type: "bulletList", content: [{ type: "listItem" }] }], + }, + { emitUpdate: false }, + ); + expect(firstItem(ed).childCount).toBe(0); + + repairEmptyListItems(ed); + + expect(firstItem(ed).childCount).toBe(1); + expect(firstItem(ed).firstChild?.type.name).toBe("paragraph"); + expect(ed.state.selection instanceof TextSelection).toBe(true); + expect(ed.state.selection.$from.node(-1)?.type.name).toBe("listItem"); + }); + + it("is a no-op for a non-empty list (round-trips fine) — caret untouched", () => { + const ed = makeEditor("1. buy milk\n\n"); + const pos = 4; // mid-word + ed.view.dispatch( + ed.state.tr.setSelection(TextSelection.create(ed.state.doc, pos)), + ); + const json = JSON.stringify(ed.getJSON()); + + repairEmptyListItems(ed); + + expect(ed.state.selection.from).toBe(pos); + expect(JSON.stringify(ed.getJSON())).toBe(json); + }); + + it("is a no-op for plain paragraph content", () => { + const ed = makeEditor("hello world\n\n"); + const json = JSON.stringify(ed.getJSON()); + repairEmptyListItems(ed); + expect(JSON.stringify(ed.getJSON())).toBe(json); + }); + + it("works from the onCreate mount hook (the path ContentEditor uses)", async () => { + const el = document.createElement("div"); + document.body.appendChild(el); + editor = new Editor({ + element: el, + extensions: createEditorExtensions({}), + content: "1. \n\n", + contentType: "markdown", + onCreate: ({ editor: ed }) => repairEmptyListItems(ed), + }); + // Tiptap defers onCreate a tick past construction. + await new Promise((r) => setTimeout(r, 0)); + + const li = firstItem(editor); + expect(li.childCount).toBe(1); + expect(li.firstChild?.type.name).toBe("paragraph"); + expect(editor.state.selection instanceof TextSelection).toBe(true); + expect(editor.state.selection.$from.node(-1)?.type.name).toBe("listItem"); + }); +}); diff --git a/packages/views/editor/utils/repair-list-items.ts b/packages/views/editor/utils/repair-list-items.ts new file mode 100644 index 000000000..64277bc67 --- /dev/null +++ b/packages/views/editor/utils/repair-list-items.ts @@ -0,0 +1,105 @@ +import { TextSelection } from "@tiptap/pm/state"; +import type { Editor } from "@tiptap/core"; + +/** Minimal shape of a ProseMirror JSON node — enough to walk and patch. */ +interface JsonNode { + type?: string; + content?: JsonNode[]; + [key: string]: unknown; +} + +/** + * Ensure every list item starts with a paragraph. + * + * `listItem`/`taskItem` content is `paragraph block*`, so a valid item's first + * child must be a paragraph. `@tiptap/markdown` violates that two ways for an + * empty item: a top-level empty item (`"1. "`) parses to a childless item, and + * a nested one (`"1. \n 1. "`) parses to an outer item whose only child is the + * sub-list. Both are schema-invalid and leave the document without a text + * cursor. Recurse depth-first, then prepend an empty paragraph to any item that + * doesn't already lead with one. Attrs (e.g. a task item's `checked`) are kept. + */ +function repairListItems(node: JsonNode): { node: JsonNode; changed: boolean } { + let changed = false; + let content = node.content; + + if (content) { + const mapped = content.map((child) => { + const result = repairListItems(child); + if (result.changed) changed = true; + return result.node; + }); + if (changed) content = mapped; + } + + if (node.type === "listItem" || node.type === "taskItem") { + if (!content || content.length === 0 || content[0]?.type !== "paragraph") { + content = [{ type: "paragraph" }, ...(content ?? [])]; + changed = true; + } + } + + return changed ? { node: { ...node, content }, changed } : { node, changed }; +} + +/** + * Repair markdown-parsed empty list items so they hold a valid text cursor. + * + * The document left after typing `1.` (or `- `) in a comment is persisted as the + * draft `"1. \n\n"`. On remount `@tiptap/markdown` parses that empty item into a + * schema-invalid, childless `listItem` (see {@link repairListItems}); the + * document is then left with an `AllSelection` instead of a cursor, so the + * browser paints the caret on the following block — the reported bug: "type + * `1.`, switch issues, come back, the caret jumps off the list item and can't be + * moved back." Note this only affects the round-trip of an *empty* item; + * non-empty items (`"1. buy milk"`) round-trip byte-identically. + * + * `preferredSelection` (the caret captured before an external re-parse) is + * restored when given, snapped to a valid text position; otherwise the caret + * lands at the first valid text position (the list item's paragraph). No-op when + * every list item is already valid, so ordinary content is untouched. + * + * Returns true when it repaired the document (and therefore set the caret). + */ +export function repairEmptyListItems( + editor: Editor, + preferredSelection?: { from: number; to: number }, +): boolean { + if (editor.isDestroyed) return false; + + const { node: repaired, changed } = repairListItems( + editor.getJSON() as JsonNode, + ); + if (!changed) return false; + + // The corrupt parse leaves an `AllSelection`. Re-setting content while that + // selection is live makes ProseMirror's replace collapse the whole list into + // bare paragraphs, so first pin the selection to a real position. + editor.view.dispatch( + editor.state.tr + .setSelection(TextSelection.near(editor.state.doc.resolve(0))) + .setMeta("addToHistory", false), + ); + + // Rebuild from the repaired tree. This is a normalization of externally + // parsed content, not a user edit: `setMeta("addToHistory", false)` keeps it + // off the undo stack (otherwise Ctrl-Z would restore the broken item), and + // `emitUpdate: false` avoids a self-write back to the draft/server. + editor + .chain() + .setMeta("addToHistory", false) + .setContent(repaired, { emitUpdate: false }) + .run(); + + const { doc } = editor.state; + const size = doc.content.size; + const clamp = (pos: number) => Math.min(Math.max(pos, 0), size); + const anchor = preferredSelection ? clamp(preferredSelection.from) : 0; + const head = preferredSelection ? clamp(preferredSelection.to) : 0; + editor.view.dispatch( + editor.state.tr + .setSelection(TextSelection.between(doc.resolve(anchor), doc.resolve(head))) + .setMeta("addToHistory", false), + ); + return true; +}