fix(prosemirror-markdown): guard three-way list coalesce against transitive style loss (#535)
Review follow-up on the list-seam coalescing:
1. The three-way merge only checked each PRE-EXISTING list against the
inserted one. A default-typed inserted orderedList between two lists with
explicit DIFFERENT numbering styles passed both pairwise checks through the
null-typed middle, collapsing all three and silently losing the right
list's style. Add a `listsMergeable(left, right)` guard to the three-way
condition. On failure fall through to the single-seam path: a single
inserted block can be absorbed by at most one neighbour, so prefer the LEFT
seam (consistent with the three-way survivor choice) and leave the
incompatible right list separate with its own style.
2. Lock the footnotesList exclusion with a test — inserting a footnotesList
next to a footnotesList must leave TWO separate blocks (a refactor of the
allow-list to endsWith("List") would otherwise silently merge them and
corrupt footnotes).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -705,26 +705,44 @@ function coalesceSeams(parent: any[], i: number, j: number): void {
|
||||
|
||||
// A seam fires only when the pre-existing neighbour and the inserted boundary
|
||||
// list are mergeable AND the inserted boundary list is non-empty.
|
||||
const singleBlock = i === j - 1;
|
||||
const leftMergeable =
|
||||
i - 1 >= 0 &&
|
||||
listsMergeable(left, boundaryLeft) &&
|
||||
boundaryLeft.content.length > 0;
|
||||
const rightMergeable =
|
||||
let rightMergeable =
|
||||
j < n &&
|
||||
listsMergeable(boundaryRight, right) &&
|
||||
boundaryRight.content.length > 0;
|
||||
|
||||
// Three-way collision: a SINGLE inserted list (i === j-1) landed exactly
|
||||
// between two pre-existing same-type lists. The LEFT pre-existing list wins:
|
||||
// the inserted items then the right list's items fold into it, and both the
|
||||
// Three-way collision: a SINGLE inserted list (singleBlock) landed exactly
|
||||
// between two pre-existing lists. The LEFT pre-existing list wins: the
|
||||
// inserted items then the right list's items fold into it, and both the
|
||||
// inserted wrapper and the right pre-existing list are deleted (the right
|
||||
// block id is NOT preserved — rare, documented).
|
||||
if (i === j - 1 && leftMergeable && rightMergeable) {
|
||||
//
|
||||
// The `listsMergeable(left, right)` guard is REQUIRED: leftMergeable and
|
||||
// rightMergeable only check each PRE-EXISTING list against the inserted one.
|
||||
// A default-typed inserted orderedList is compatible with BOTH neighbours
|
||||
// even when the neighbours carry explicit DIFFERENT numbering styles, so
|
||||
// without this guard the two would collapse transitively through the middle
|
||||
// and the right list's style would be silently lost. When it fails we fall
|
||||
// through to the single-seam path below (never a transitive merge).
|
||||
if (singleBlock && leftMergeable && rightMergeable && listsMergeable(left, right)) {
|
||||
left.content.push(...boundaryLeft.content, ...right.content);
|
||||
parent.splice(i, 2);
|
||||
return;
|
||||
}
|
||||
|
||||
// A single inserted block can be absorbed by at most ONE neighbour. When both
|
||||
// seams are individually valid but the neighbours are mutually incompatible
|
||||
// (the three-way guard above failed), prefer the LEFT seam — consistent with
|
||||
// the three-way survivor choice — and drop the right so the incompatible
|
||||
// right list stays separate with its own style.
|
||||
if (singleBlock && leftMergeable && rightMergeable) {
|
||||
rightMergeable = false;
|
||||
}
|
||||
|
||||
// Otherwise the two seams are independent. Process the RIGHT seam FIRST so its
|
||||
// deletion at the higher indices cannot shift the left seam's [i-1, i].
|
||||
if (rightMergeable) {
|
||||
|
||||
@@ -158,6 +158,31 @@ describe("#535 list seam coalescing — markdown path (insertNodesRelative)", ()
|
||||
expect(out.content[2].attrs.id).toBe("E");
|
||||
});
|
||||
|
||||
it("crit4b: three-way with DIFFERING explicit ordered types does NOT transitively merge through a default-typed insertion", () => {
|
||||
// [ol(type:"a")[1], ol(type:"i")[9]] + insert ol(type:null)[5] after A.
|
||||
// The null-typed middle is pairwise-compatible with BOTH neighbours, but the
|
||||
// neighbours are mutually incompatible ("a" vs "i"), so they must NOT
|
||||
// collapse. The inserted list is absorbed by the LEFT survivor; the RIGHT
|
||||
// list stays separate and keeps its roman numbering style.
|
||||
const doc = {
|
||||
type: "doc",
|
||||
content: [orderedList("A", 1, "a", 1), orderedList("C", 9, "i", 9)],
|
||||
};
|
||||
const { doc: out } = insertNodeRelative(doc, orderedList("B", 5, null, 5), {
|
||||
position: "after",
|
||||
anchorNodeId: "A",
|
||||
});
|
||||
expect(out.content.length).toBe(2);
|
||||
// Left survivor absorbs the inserted item.
|
||||
expect(out.content[0].attrs.id).toBe("A");
|
||||
expect(out.content[0].attrs.type).toBe("a");
|
||||
expect(labels(out.content[0])).toEqual(["1", "5"]);
|
||||
// Right list is untouched: its explicit style survives.
|
||||
expect(out.content[1].attrs.id).toBe("C");
|
||||
expect(out.content[1].attrs.type).toBe("i");
|
||||
expect(labels(out.content[1])).toEqual(["9"]);
|
||||
});
|
||||
|
||||
it("crit6: before for orderedList{start:5} keeps start:5 (positional survivor = pre-existing)", () => {
|
||||
const doc = { type: "doc", content: [orderedList("A", 5, null, "a", "b")] };
|
||||
const { doc: out } = insertNodesRelative(
|
||||
@@ -205,6 +230,27 @@ describe("#535 list seam coalescing — markdown path (insertNodesRelative)", ()
|
||||
expect(out.content.length).toBe(2);
|
||||
});
|
||||
|
||||
it("footnotesList is NEVER structurally coalesced (allow-list excludes it, guarding against endsWith(\"List\"))", () => {
|
||||
const fn = (id: string): any => ({
|
||||
type: "footnotesList",
|
||||
attrs: { id },
|
||||
content: [
|
||||
{ type: "footnoteDefinition", attrs: { id: id + "d" }, content: [] },
|
||||
],
|
||||
});
|
||||
const doc = { type: "doc", content: [fn("A")] };
|
||||
const { doc: out } = insertNodeRelative(doc, fn("B"), {
|
||||
position: "append",
|
||||
});
|
||||
// Two footnotesLists must stay two separate blocks — merging them would
|
||||
// corrupt footnotes.
|
||||
expect(out.content.length).toBe(2);
|
||||
expect(out.content.map((n: any) => n.type)).toEqual([
|
||||
"footnotesList",
|
||||
"footnotesList",
|
||||
]);
|
||||
});
|
||||
|
||||
it("taskList next to taskList coalesces, item checked attrs move with items", () => {
|
||||
const doc = {
|
||||
type: "doc",
|
||||
|
||||
Reference in New Issue
Block a user