mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-05 21:39:54 +02:00
fix(editor): repair empty list items parsed from a markdown draft (#4869)
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 <github@multica.ai>
This commit is contained in:
@@ -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,
|
||||
}));
|
||||
|
||||
@@ -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<ContentEditorRef, ContentEditorProps>(
|
||||
});
|
||||
}
|
||||
}
|
||||
// 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<ContentEditorRef, ContentEditorProps>(
|
||||
});
|
||||
}
|
||||
|
||||
// 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]);
|
||||
|
||||
175
packages/views/editor/utils/repair-list-items.test.ts
Normal file
175
packages/views/editor/utils/repair-list-items.test.ts
Normal file
@@ -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");
|
||||
});
|
||||
});
|
||||
105
packages/views/editor/utils/repair-list-items.ts
Normal file
105
packages/views/editor/utils/repair-list-items.ts
Normal file
@@ -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;
|
||||
}
|
||||
Reference in New Issue
Block a user