From 04154cc6ee4f3e7516fb1373a1dd35a171a3e995 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Wed, 28 Oct 2026 20:14:30 -0400 Subject: [PATCH 1/3] [lexical-website][lexical-y] Bug Fix: Stop a format, comment or link from spreading to the end of the document after an undo or delete races an edit inside it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description In `@lexical/y`, a format, comment or link could spread to the end of the document when a collaborator edited inside it while it was undone, or while another peer deleted text at its end. All of these converge on main, on every client and in a fresh read: - A makes "t56z t57z" italic and undoes it while B types plain text after "t56z": everything after B's text, the next paragraph included, turns italic. - B comments "t32z t33z", where a comment of another member of the same set ends, and undoes it while A types after "t33z": the removed comment covers the rest of the document. - B bolds "t6z t7z t8z" and undoes it while A removes bold from "t7z": " t8z t9z" and the next paragraph turn bold. A removal from the whole range, then its undo, does the same. - A undoes bold or a comment while B pastes a list or paragraphs inside it; B undoes a link while A makes text across the link's start italic. - B pastes inside a comment while C, offline for a long time, deletes text at the comment's end: once C is back, the comment covers words far after it. The cause is shared. The client that edits inside the range restates the value there: the write cursor restores the old value after the text it inserts or at the end of the range it formats, and an undo brings back the format items its change deleted. That restated run ended at the closer of the change that opened the run. When that change is undone, its opener and its closer are deleted; a concurrent delete can also drop the closer (it keeps a bounded number of the format items it leaves). Either way the restatement runs on to the end of the document. This change adds one rule, for every stored inline kind (text formats, styles, NodeState and its set members, links): a run that a client restates ends with an item of that client's own. - `Cursor`, format rule 5 (also used after inserted text): where a restore restates a value whose run another client's change opened (following restores back to the run's opener), and the item that ends the restored run is that change's own end of the run, the writer puts a null right after that end. If the change is undone, or a delete drops that end, the null ends the restored run there. It goes after the end, not before it, so text a collaborator types at the end of the run at the same time stays inside the run. A null by which a change made text inside the run plain is not such an end, so undoing that change still brings the value back. - `YBinding#closeRestoredRuns`, the undo path: a format item an undo brings back gets a null of the undoer's own right after the end its original change gave it, where that end is gone. This generalizes the guard added for #9493. The editor reads those nulls as part of the undo, with the selection the undo puts back, so the undoer's editor matches a fresh read. The stored layout does not change, and nothing more is written where no other client's change is involved. The restored value can still stay on the part of the range after the collaborator's edit, and an undone link stays on its own text when a collaborator's format change rewrote the link's textFormat (both documented in y.md); neither reaches past the range any more. ## Changeset `.changeset/lexical-y-undo-run-end.md`: `[fix]` for `@lexical/y`, with a `Verify:` paragraph. ## Test plan New regressions in `packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts`: each case in both client orders, with comment variants (another member of the set, three comments, comment and bold), the selection an undo puts back (direct and inside an editor update), and a matrix of bold or a comment removed (by undo or by the opposite change) while a collaborator types, presses Enter, pastes paragraphs or a list, or deletes inside the range. ### Before On main (0cd7448): ``` $ npx vitest --project unit --no-watch packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts Test Files 1 failed (1) Tests 38 failed | 146 passed | 4 expected fail (188) AssertionError: expected '"t56z t85z" " t57z t59z t60z"[I] / "t…' to be '"t56z t85z" " t57z"[I] " t59z t60z" /…' // Object.is equality Expected: ""t56z t85z" " t57z"[I] " t59z t60z" / "t78z t79z"" Received: ""t56z t85z" " t57z t59z t60z"[I] / "t78z t79z"[I]" AssertionError: expected '"t5z t6z t7z" " t8z t9z"[B] / "tail l…' to be '"t5z t6z t7z" " t8z"[B] " t9z" / "tai…' // Object.is equality Expected: ""t5z t6z t7z" " t8z"[B] " t9z" / "tail line"" Received: ""t5z t6z t7z" " t8z t9z"[B] / "tail line"[B]" AssertionError: expected 't7z t8z t9z / tail line' to be 't7z t8z t9z / ' // Object.is equality AssertionError: expected [ ' t57z t59z t60z', 't78z t79z' ] to deeply equal [] ``` ### After ``` $ npx vitest --project unit --no-watch packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts Test Files 1 passed (1) Tests 184 passed | 4 expected fail (188) $ npx vitest --project unit --no-watch --maxWorkers=6 packages/lexical-y Test Files 148 passed | 2 skipped (150) Tests 4983 passed | 16 expected fail | 2 skipped (5001) $ pnpm run ci-check Changeset check passed: 5 pending changeset(s) valid. Found 0 errors ``` Randomized collaborative sessions with formats, comments and undo (a local soak harness, not committed), compared with main: concurrent undo sessions 112 -> 107 failing of 4000 (none new), sequential undo sessions 200 -> 199 of 4000 (none new), format sessions and the format growth probes unchanged. An undo-vs-comment fuzz spread in 9 of 300 seeds on main and in 0 of 1000 with this change. A replay of the 4-hour session where a peer returned after 81 minutes offline no longer spreads the comment (main: 4 untouched words get it). Long sessions store about 2.5% more format items. Browser e2e was not run. --- .changeset/lexical-y-undo-run-end.md | 7 + .../lexical-website/docs/collaboration/y.md | 14 +- .../unit/regressions/Formats.test.ts | 408 +++++++++++++++++- packages/lexical-y/src/internal/Binding.ts | 138 ++++-- packages/lexical-y/src/internal/Cursor.ts | 199 ++++++++- 5 files changed, 716 insertions(+), 50 deletions(-) create mode 100644 .changeset/lexical-y-undo-run-end.md diff --git a/.changeset/lexical-y-undo-run-end.md b/.changeset/lexical-y-undo-run-end.md new file mode 100644 index 00000000000..7bcc2586208 --- /dev/null +++ b/.changeset/lexical-y-undo-run-end.md @@ -0,0 +1,7 @@ +--- +'@lexical/y': patch +--- + +[fix] Stop a format, comment or link from spreading to the end of the document in `@lexical/y` when a collaborator edits inside it while it is undone or its end is deleted + +Verify: undo a format, a comment or a link, or delete the text at its end, while a collaborator types, pastes or changes a format inside it; on every client, no text past it gets the value. diff --git a/packages/lexical-website/docs/collaboration/y.md b/packages/lexical-website/docs/collaboration/y.md index f4058747f2c..fa809b6198b 100644 --- a/packages/lexical-website/docs/collaboration/y.md +++ b/packages/lexical-website/docs/collaboration/y.md @@ -841,11 +841,15 @@ not merge as a user might expect: Undoing a format change does not undo a collaborator's later change of part of its range, nor the value that change restored after itself: color `abc`, let a collaborator recolor `b`, then undo the color, and `c` keeps - the old color. Where a collaborator overwrote a step of yours entirely, - the undo manager skips that step as changing nothing, and the same undo - also reverts your step before it. When you undo a format change while a - collaborator removes that format and undoes the removal, the text keeps - the format: the collaborator's undo brings it back. + the old color, up to the end of the range (the same holds for text a + collaborator types or pastes inside the range as you undo). Undoing a link + while a collaborator formats text inside it keeps the link on its text: + formatting text in a link also sets the link's text format, which writes + the link there again. Where a collaborator overwrote a step of yours + entirely, the undo manager skips that step as changing nothing, and the + same undo also reverts your step before it. When you undo a format change + while a collaborator removes that format and undoes the removal, the text + keeps the format: the collaborator's undo brings it back. - **Large lists.** A structural edit in a list costs time proportional to the list on every client. - **The caret when a collaborator rewrites its line.** A caret stays after diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts index a5800453dd1..8081479ae9b 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts @@ -11,7 +11,7 @@ // the editors are attached to DOM roots, so the reconciler derives elements' // text fields from the text it renders. -import {$createLinkNode, $isLinkNode} from '@lexical/link'; +import {$createLinkNode, $isLinkNode, $toggleLink} from '@lexical/link'; import {$createListItemNode, $createListNode} from '@lexical/list'; import {$addMarkID, $removeMarkID} from '@lexical/mark'; import {$patchStyleText} from '@lexical/selection'; @@ -21,7 +21,9 @@ import { $createRangeSelection, $createTextNode, $getRoot, + $getSelection, $isElementNode, + $isRangeSelection, $isTextNode, $setSelection, type ElementNode, @@ -30,7 +32,13 @@ import { } from 'lexical'; import {describe, expect, test} from 'vitest'; -import {freshReader, type Peer, setup, shape} from '../../utils/harness'; +import { + $selectIn, + freshReader, + type Peer, + setup, + shape, +} from '../../utils/harness'; function attach(peer: Peer) { const element = document.createElement('div'); @@ -1424,3 +1432,399 @@ describe('an undo of a format change ‖ its removal by a collaborator, undone', } } }); + +// S7-24, S7-7, S7-10, S7-20, S7-26: a value one client restates (the restore +// after its typed text or its format range, or a value its undo brings back) +// ended at the closer of the change that gave it. The undo of that change by +// its author deleted the closer, and the restated value ran on to the end of +// the document. A run that one client restates now ends with an item of that +// client's own, right after the closer it relied on (see Cursor#restatedEnd +// and YBinding#closeRestoredRuns), for every kind of value: text formats, +// styles, NodeState and its set members, and links. +describe('a value one client restates ‖ the undo of the change that gave it', () => { + const twoLines = (first: string, second: string) => () => + $getRoot().append( + $createParagraphNode().append($createTextNode(first)), + $createParagraphNode().append($createTextNode(second)), + ); + // As the toolbar does: on where the first character lacks it, off otherwise. + const $format = + (format: 'bold' | 'italic' | 'underline', from: number, to: number) => + () => { + const sel = $selectLine(from, to); + sel.format = (sel.anchor.getNode() as TextNode).getFormat(); + $setSelection(sel); + sel.formatText(format); + $setSelection(null); + }; + const $comment = (id: string, from: number, to: number) => () => { + $addMarkID(id, $selectLine(from, to)); + $setSelection(null); + }; + // Plain text typed at first-line offset `at`: a text node of its own, with + // no format or NodeState (as a comment's end does not extend to it). + const $typePlain = (at: number, text: string) => () => { + let acc = 0; + for (const node of ( + $getRoot().getFirstChildOrThrow() as ElementNode + ).getAllTextNodes()) { + const size = node.getTextContentSize(); + if (at <= acc + size) { + const [before] = at - acc < size ? node.splitText(at - acc) : [node]; + before.insertAfter($createTextNode(text)); + return; + } + acc += size; + } + throw new Error('offset'); + }; + // Blocks pasted at first-line offset `at`. + const $paste = + (at: number, ...blocks: (() => LexicalNode)[]) => + () => { + const sel = $selectLine(at, at); + $setSelection(sel); + sel.insertNodes(blocks.map(block => block())); + $setSelection(null); + }; + const $list = () => + $createListNode('bullet').append( + $createListItemNode().append($createTextNode('t20z t67z')), + $createListItemNode().append($createTextNode('t23z')), + ); + for (const ids of [ + [1, 2], + [2, 1], + ]) { + // S7-24: A formats, then undoes, while B types inside the range. + for (const [name, $add, expected] of [ + [ + 'italic', + $format('italic', 0, 9), + '"t56z t85z" " t57z"[I] " t59z t60z" / "t78z t79z"', + ], + [ + 'a comment', + $comment('c1', 0, 9), + '"t56z t85z" " t57z"[c1] " t59z t60z" / "t78z t79z"', + ], + ] as const) { + test(`S7-24: A adds ${name} on "t56z t57z" and undoes it ‖ B types plain text after "t56z" (ids ${ids})`, async () => { + const t = await setup(twoLines('t56z t57z t59z t60z', 't78z t79z'), { + ids, + }); + t.a.update($add); + await t.sync(); + t.offline(); + t.a.undo(); + t.b.update($typePlain(4, ' t85z')); + expect(await merged(t)).toBe(expected); + }); + } + // S7-7: a comment's undo ‖ plain text typed right after it, where + // other values (a comment of a set's other member, bold) end too. + for (const [name, $others, expected] of [ + [ + 'another comment', + [$comment('c5', 10, 14)], + '"t31z t32z " "t33z"[c5] " t1012z t1008z" / "t34z t35z t1005z"', + ], + [ + 'two other comments', + [$comment('c5', 10, 14), $comment('c6', 0, 14)], + '"t31z t32z "[c6] "t33z"[c5+c6] " t1012z t1008z" / "t34z t35z t1005z"', + ], + [ + 'bold', + [$format('bold', 10, 14)], + '"t31z t32z " "t33z"[B] " t1012z t1008z" / "t34z t35z t1005z"', + ], + ] as const) { + test(`S7-7: B comments "t32z t33z", where ${name} ends, and undoes it ‖ A types after "t33z" (ids ${ids})`, async () => { + const t = await setup( + twoLines('t31z t32z t33z t1008z', 't34z t35z t1005z'), + {ids}, + ); + for (const $other of $others) { + t.a.update($other); + } + await t.sync(); + t.b.update($comment('c4', 5, 14)); + await t.sync(); + t.offline(); + t.b.undo(); + t.a.update($typePlain(14, ' t1012z')); + expect(await merged(t)).toBe(expected); + }); + } + // S7-10: a format's undo ‖ its removal from part of the range (the part + // after it keeps the format, up to the end of the range), or from all of + // it, and then the removal's undo (which brings the format back on the + // range). + for (const [format, tag] of [ + ['bold', 'B'], + ['underline', 'U'], + ] as const) { + test(`S7-10: B ${format}s "t6z t7z t8z" and undoes it ‖ A removes it from "t7z" (ids ${ids})`, async () => { + const t = await setup(twoLines('t5z t6z t7z t8z t9z', 'tail line'), { + ids, + }); + t.b.update($format(format, 4, 15)); + await t.sync(); + t.offline(); + t.a.update($format(format, 8, 11)); + t.b.undo(); + expect(await merged(t)).toBe( + `"t5z t6z t7z" " t8z"[${tag}] " t9z" / "tail line"`, + ); + }); + test(`S7-10: B ${format}s "t6z t7z t8z" and undoes it ‖ A removes it, then A undoes that (ids ${ids})`, async () => { + const t = await setup(twoLines('t5z t6z t7z t8z t9z', 'tail line'), { + ids, + }); + t.b.update($format(format, 4, 15)); + await t.sync(); + t.offline(); + t.a.update($format(format, 4, 15)); + t.b.undo(); + await t.sync(); + t.a.undo(); + expect(await merged(t)).toBe( + `"t5z " "t6z t7z t8z"[${tag}] " t9z" / "tail line"`, + ); + }); + } + test(`S7-10: B comments "t9z" and undoes it ‖ A resolves it, then A undoes that (ids ${ids})`, async () => { + const t = await setup(twoLines('t5z t6z t7z t8z t9z', 'tail line'), { + ids, + }); + t.b.update($comment('c2', 16, 19)); + await t.sync(); + t.offline(); + t.a.update(() => $removeMarkID('c2')); + t.b.undo(); + await t.sync(); + t.a.undo(); + expect(await merged(t)).toBe( + '"t5z t6z t7z t8z " "t9z"[c2] / "tail line"', + ); + }); + // The nulls an undo writes after itself (see closeRestoredRuns) are read + // as part of that undo: the selection it puts back stays, also where the + // undo runs inside an editor update (as UNDO_COMMAND's handler does). + for (const nested of [false, true]) { + test(`S7-10: A's undo of a removal of B's undone bold puts back A's selection (${nested ? 'inside an update' : 'direct'}, ids ${ids})`, async () => { + const t = await setup(twoLines('t5z t6z t7z t8z t9z', 'tail line'), { + ids, + }); + t.b.update(() => { + $selectIn([0], 4, 15).formatText('bold'); + $setSelection(null); + }); + await t.sync(); + t.offline(); + t.a.update(() => void $selectIn([0], 4, 15)); + t.a.update(() => { + const sel = $getSelection(); + if (!$isRangeSelection(sel)) { + throw new Error('no range selection'); + } + sel.format = 1; + sel.formatText('bold'); + $selectIn([0], 0, 0); + }); + t.b.undo(); + await t.sync(); + if (nested) { + t.a.editor.update(() => void t.a.binding.undo(), {discrete: true}); + } else { + t.a.binding.undo(); + } + const offsets = t.a.read(() => { + const sel = $getSelection(); + const texts = $getRoot().getAllTextNodes(); + const at = (key: string, offset: number) => { + let acc = 0; + for (const text of texts) { + if (text.getKey() === key) { + return acc + offset; + } + acc += text.getTextContentSize(); + } + return -1; + }; + return $isRangeSelection(sel) + ? [ + at(sel.anchor.key, sel.anchor.offset), + at(sel.focus.key, sel.focus.offset), + ] + : null; + }); + expect(offsets).toEqual([4, 15]); + expect(await merged(t)).toBe( + '"t5z " "t6z t7z t8z"[B] " t9z" / "tail line"', + ); + }); + } + // S7-20: a comment's undo ‖ a list pasted inside the commented words. + test(`S7-20: A comments "t59z t60z t61z" and undoes it ‖ B pastes a list after "t60z" (ids ${ids})`, async () => { + const t = await setup(twoLines('t58z t59z t60z t61z t62z', 't78z t79z'), { + ids, + }); + t.a.update($comment('c9', 5, 19)); + await t.sync(); + t.offline(); + t.a.undo(); + t.b.update($paste(14, $list)); + expect(await merged(t)).toBe( + '"t58z t59z t60z" / "t20z t67z" "t23z" / " t61z"[c9] " t62z" / "t78z t79z"', + ); + }); + // S7-26: a link's undo ‖ a format change over the link's start, which + // also sets the link's textFormat (RangeSelection#formatText): that + // write restates the link over its text, which keeps it there. + for (const format of ['italic', 'bold'] as const) { + test(`S7-26: B links "t7z t8z t9z" and undoes it ‖ A makes "t5z t6z t7z" ${format} (ids ${ids})`, async () => { + const t = await setup( + twoLines('t1z t2z t3z t4z t5z t6z t7z t8z t9z', 'tail line'), + {ids}, + ); + t.b.update(() => { + $setSelection($selectLine(24, 35)); + $toggleLink('https://u0.test'); + $setSelection(null); + }); + await t.sync(); + t.offline(); + t.a.update($format(format, 16, 27)); + t.b.undo(); + await merged(t); + const $links = () => + $getRoot() + .getChildren() + .map(p => + ($isElementNode(p) ? p.getChildren() : []) + .filter($isLinkNode) + .map(link => link.getTextContent()) + .join(' '), + ) + .join(' / '); + const fresh = await freshReader(t.a); + for (const p of [t.a, t.b, fresh]) { + expect(p.read($links)).toBe('t7z t8z t9z / '); + } + }); + } + } +}); + +// The undo race matrix of soak7: A adds bold or a comment on "t56z t57z" +// and removes it (an undo, or the opposite change) while B edits inside the +// range. Nothing outside the range gets the value. +describe('a format or comment removed ‖ an edit inside its range', () => { + const $outside = () => + $getRoot() + .getAllTextNodes() + .filter( + n => + /t(59|60|78|79)z/.test(n.getTextContent()) && + (n.hasFormat('bold') || + ((n.exportJSON() as {$?: {markIds?: string[]}}).$?.markIds ?? []) + .length > 0), + ) + .map(n => n.getTextContent()); + const $at = (at: number) => { + const sel = $selectLine(at, at); + $setSelection(sel); + return sel; + }; + const edits: [string, () => void][] = [ + [ + 'types plain text', + () => { + const sel = $at(4); + sel.format = 0; + sel.insertText(' t85z'); + }, + ], + ['presses Enter', () => void $at(4).insertParagraph()], + [ + 'pastes two paragraphs', + () => + void $at(4).insertNodes([ + $createParagraphNode().append($createTextNode('t20z')), + $createParagraphNode().append($createTextNode('t23z')), + ]), + ], + [ + 'pastes a list', + () => + void $at(4).insertNodes([ + $createListNode('bullet').append( + $createListItemNode().append($createTextNode('t20z')), + $createListItemNode().append($createTextNode('t23z')), + ), + ]), + ], + ['deletes "t56z"', () => void $selectLine(0, 5).removeText()], + ]; + const kinds: [string, () => void, () => void][] = [ + [ + 'bold', + () => $selectLine(0, 9).formatText('bold'), + () => { + const sel = $selectLine(0, 9); + sel.format = 1; + sel.formatText('bold'); + }, + ], + [ + 'a comment', + () => $addMarkID('c1', $selectLine(0, 9)), + () => $removeMarkID('c1'), + ], + ]; + for (const [kind, $add, $remove] of kinds) { + for (const undo of [true, false]) { + for (const [edit, $edit] of edits) { + for (const ids of [ + [1, 2], + [2, 1], + ]) { + test(`A adds ${kind} on "t56z t57z" and ${undo ? 'undoes it' : 'removes it'} ‖ B ${edit} after "t56z" (ids ${ids})`, async () => { + const t = await setup( + () => + $getRoot().append( + $createParagraphNode().append( + $createTextNode('t56z t57z t59z t60z'), + ), + $createParagraphNode().append($createTextNode('t78z t79z')), + ), + {ids}, + ); + t.a.update(() => { + $add(); + $setSelection(null); + }); + await t.sync(); + t.offline(); + if (undo) { + t.a.undo(); + } else { + t.a.update(() => { + $remove(); + $setSelection(null); + }); + } + t.b.update(() => { + $edit(); + $setSelection(null); + }); + await merged(t); + expect(t.a.read($outside)).toEqual([]); + }); + } + } + } + } +}); diff --git a/packages/lexical-y/src/internal/Binding.ts b/packages/lexical-y/src/internal/Binding.ts index 2ca6eb97389..ab4924b7f86 100644 --- a/packages/lexical-y/src/internal/Binding.ts +++ b/packages/lexical-y/src/internal/Binding.ts @@ -78,7 +78,12 @@ import { LEGACY_ROOTS, SET_MEMBERS_LAYOUT_VERSION, } from './constants'; -import {isFormatCloser, markFormatCloser} from './Cursor'; +import { + changeEnd, + equalFormatValue, + isFormatCloser, + markFormatCloser, +} from './Cursor'; import {$canonicalizeGroup, $switchesWrapper} from './Groups'; import {Hidden, shownLength} from './Hidden'; import {$applyStoredAttrs, Reader} from './Read'; @@ -316,6 +321,24 @@ export interface YCursor { focus: unknown; } +/** + * Whether live format item `item` changes its key's value: the nearest live + * item of its key before it gives another value (none gives null). + */ +function opensRun(item: Item): boolean { + const {key, value} = item.content as ContentFormat; + for (let it = item.left; it !== null; it = it.left) { + if ( + !it.deleted && + it.content instanceof ContentFormat && + it.content.key === key + ) { + return !equalFormatValue(it.content.value, value); + } + } + return value !== null; +} + /** * The layout version `root` records: the highest of its `lexical-y:` * attributes (see LAYOUT_PREFIX), null when it has none, and NaN when one is @@ -538,6 +561,12 @@ export class YBinding { private remoteSeq = 0; /** Origin of read-only scans (no changes). */ private readonly scanOrigin = {binding: this, scan: true}; + /** + * The origin of the nulls an undo or redo writes to end the runs it + * brings back (see closeRestoredRuns): read into the editor as a + * collaborator's change is, and not part of the undo stack. + */ + private readonly runEndOrigin = {binding: this, runEnd: true}; /** Stored nodes the running undo/redo restores (see isKeptByUndo). */ private restoring: Set | null = null; /** Stored nodes the running undo/redo deletes (it inserted them). */ @@ -1772,11 +1801,17 @@ export class YBinding { let cursor = this.pendingCursor; this.pendingCursor = null; // An undo puts back the selection of its step (a range, after typing - // over one); any other read keeps the user's caret a caret. + // over one); any other read keeps the user's caret a caret. The nulls an + // undo writes after itself (see closeRestoredRuns) are read as part of + // it: their read puts back the same selection, which the editor may not + // have committed yet (an undo run inside an update). const undoing = - tr.origin === this._undoManager && this._undoManager !== null; + (tr.origin === this._undoManager && this._undoManager !== null) || + tr.origin === this.runEndOrigin; if (undoing) { - this.undoneCursor = cursor; + if (tr.origin !== this.runEndOrigin) { + this.undoneCursor = cursor; + } cursor = this.stepCursor ?? cursor; } let rebuilt: ReadonlySet = new Set(); @@ -3976,25 +4011,32 @@ export class YBinding { /** * End each run that a format item the undo or redo of `step` brought back - * opens, where a null of another client's ends it, with a null of this - * client's own right before that one. An undo brings back the items its - * change deleted, and the closer that ended such a run was not among - * them: a collaborator's concurrent undo of the change that wrote it - * removed it, and the value ran on to the end of the document (a comment - * added, undone while a collaborator resolved it and undid the resolve, - * put the comment on everything after it). Invariant: a run an undo or - * redo brings back ends where a closer ended it for this client, whatever - * another client removes at the same time. A run that another value ends - * is left as it is: where that value's change is undone, the run going on - * over its text is what it had before (a copy of that value kept it there - * after its own undo). + * opens with an item of this client's own, where the item that ended it is + * another client's: right before a live null of another client's that ends + * it, or right after the end the original's own change gave its run (see + * changeEnd), where that is gone and nothing ends the run before it. An + * undo brings back the items its change deleted, and the closer that ended + * such a run was not among them: a collaborator's concurrent undo of the + * change that wrote it removed it, and the value ran on to the end of the + * document (a comment added, undone while a collaborator resolved it and + * undid the resolve, put the comment on everything after it; bold removed + * from a collaborator's bold text while the collaborator undid it, and that + * removal undone, made everything after it bold). Invariant: a run an undo + * or redo brings back ends where a closer ended it for this client, + * whatever another client removes at the same time. A run that another + * value ends is left as it is: where that value's change is undone, the run + * going on over its text is what it had before (a copy of that value kept + * it there after its own undo). A copy inside a run of its own value (see + * opensRun) opens no run: the run it is in ends where it ends. */ private closeRestoredRuns(step: UndoManager['undoStack'][number]): void { const doc = this._doc; const store = doc.store; const client = doc.clientID; + // Where to write each null: after `left`, before `right`. + const ends: [left: Item | null, right: Item | null, key: string][] = []; doc.transact(tr => { - const copies: Item[] = []; + const copies: [original: Item, copy: Item][] = []; iterateStructsByIdSet(tr, step.deletes, s => { if ( s instanceof Item && @@ -4008,50 +4050,62 @@ export class YBinding { copy.content instanceof ContentFormat && copy.content.value !== null ) { - copies.push(copy); + copies.push([s, copy]); } } }); - for (const copy of copies) { + for (const [original, copy] of copies) { const {key} = copy.content as ContentFormat; + // The end of the run the original opened in its own write (see + // changeEnd): where that is gone (the undo of that write), the + // copy's run ends there. + const gone = opensRun(copy) ? changeEnd(doc, original) : null; let shown = false; - let end: Item | null = null; for (let it = copy.right; it !== null; it = it.right) { + if (it === gone && it.deleted) { + if (shown) { + ends.push([it, it.right, key]); + } + break; + } if (it.deleted) { continue; } if (it.content instanceof ContentFormat) { if (it.content.key === key) { - end = it; + if ( + shown && + it.id.client !== client && + it.content.value === null + ) { + ends.push([it.left, it, key]); + } break; } } else if (it.countable) { shown = true; } } - if ( - end === null || - !shown || - end.id.client === client || - (end.content as ContentFormat).value !== null - ) { - continue; - } - const left = end.left; - const item = new Item( - createID(client, store.getClock(client)), - left, - left && left.lastId, - end, - end.id, - end.parent, - null, - new ContentFormat(key, null as unknown as object), - ); - item.integrate(tr, 0); - markFormatCloser(item); } }, this.scanOrigin); + if (ends.length > 0) { + doc.transact(tr => { + for (const [left, right, key] of ends) { + const item = new Item( + createID(client, store.getClock(client)), + left, + left && left.lastId, + right, + right && right.id, + (left ?? right)!.parent, + null, + new ContentFormat(key, null as unknown as object), + ); + item.integrate(tr, 0); + markFormatCloser(item); + } + }, this.runEndOrigin); + } } /** diff --git a/packages/lexical-y/src/internal/Cursor.ts b/packages/lexical-y/src/internal/Cursor.ts index 8bdcf800c4f..2e2b1ee86a6 100644 --- a/packages/lexical-y/src/internal/Cursor.ts +++ b/packages/lexical-y/src/internal/Cursor.ts @@ -14,6 +14,7 @@ import { ContentString, ContentType, createID, + type Doc, getItemCleanStart, Item, type Node as YNode, @@ -142,6 +143,103 @@ export const markFormatCloser = (item: Item): void => { formatClosers.add(item); }; +/** + * The item that ends the run `item`, a format item, opened in the write that + * made it: the first item of its key, with another value, that its client + * made after it (one write's items are one after the other in its client's + * clock order; looked for among the next 64). Null when none is found. + */ +export function changeEnd(doc: Doc, item: Item): Item | null { + const {key, value} = item.content as ContentFormat; + const {client} = item.id; + const end = doc.store.getClock(client); + let clock = item.id.clock + item.length; + for (let steps = 0; clock < end && steps < 64; steps++) { + const next = doc.store.getItem(createID(client, clock)); + if (!(next instanceof Item)) { + return null; + } + if ( + next.content instanceof ContentFormat && + next.content.key === key && + !equalFormatValue(next.content.value, value) + ) { + return next; + } + clock = next.id.clock + next.length; + } + return null; +} + +/** + * The item its client made last before `item` (looked for among the 64 + * structs before it, in clock order) of the key `key`, or null. + */ +function lastOwnItemBefore(doc: Doc, item: Item, key: string): Item | null { + const {client} = item.id; + let clock = item.id.clock - 1; + for (let steps = 0; clock >= 0 && steps < 64; steps++) { + const prev = doc.store.getItem(createID(client, clock)); + if (!(prev instanceof Item)) { + return null; + } + if (prev.content instanceof ContentFormat && prev.content.key === key) { + return prev; + } + clock = prev.id.clock - 1; + } + return null; +} + +/** The nearest live item of `key` before `item`, among the 4096 before it. */ +function liveItemBefore(item: Item, key: string): Item | null { + let steps = 0; + for (let it = item.left; it !== null && steps < 4096; it = it.left, steps++) { + if (!it.deleted && isFormat(it) && keyOf(it) === key) { + return it; + } + } + return null; +} + +/** + * The item that opened the run live format item `item` gives its value in: + * `item` itself, or, where it is a restore (its client wrote it after text + * it inserted inside the run, with a null of the key before that text), the + * opener of the run that restore restates, and so on. + */ +export function runOpener(doc: Doc, item: Item): Item { + const {key, value} = item.content as ContentFormat; + let current = item; + for (let depth = 0; depth < 16; depth++) { + const negation = lastOwnItemBefore(doc, current, key); + if (negation === null || valueOf(negation) !== null) { + return current; + } + let found = false; + let steps = 0; + for ( + let it = current.left; + it !== null && steps < 512; + it = it.left, steps++ + ) { + if (it === negation) { + found = true; + break; + } + if (!it.deleted && isFormat(it) && keyOf(it) === key) { + break; + } + } + const source = found ? liveItemBefore(negation, key) : null; + if (source === null || !equalFormatValue(valueOf(source), value)) { + return current; + } + current = source; + } + return current; +} + /** A live countable item: text, a child node, an embed. */ export const isLiveContent = (item: Item): boolean => !isGone(item) && item.countable; @@ -409,7 +507,9 @@ export class Cursor { * formatFamily) is never written after the items of a unit of it there * (see unitFrom), where a concurrent format change of the content after * puts its own item: it is written before them (see - * minimizeFormatChanges). + * minimizeFormatChanges). A restore of a value whose run another client's + * change opened gets an end of this client's own after that change's end + * (see format, rule 5). */ private insertNegatedFormats(negated: Formats, closers = false): void { while (this.right !== null) { @@ -441,13 +541,92 @@ export class Cursor { } this.forward(); } + const ends: Item[] = []; for (const [key, val] of negated) { + const end = val === null ? null : this.restatedEnd(key, val); + if (end !== null) { + ends.push(end); + } this.right = this.newItem(new ContentFormat(key, val as object)); if (closers && val === null) { formatClosers.add(this.right); } this.forward(); } + for (const end of ends) { + this.insertAfter( + end, + new ContentFormat(keyOf(end), null as unknown as object), + ); + } + } + + /** + * For a restore of `key` to `value` written at the cursor: where the run it + * restates was opened by another client's change (see runOpener; the item + * that gave the content the value before this write is the nearest item of + * the key before the cursor that this write did not make), and the next + * live item of the key after the cursor with another value is that change's + * own end of the run (see changeEnd), that end, after which the restore's + * run gets an end of this client's own (see format, rule 5). Null + * otherwise. + */ + private restatedEnd(key: string, value: unknown): Item | null { + const doc = this.tr.doc; + let source: Item | null = null; + let steps = 0; + for ( + let it = this.left; + it !== null && steps < 4096; + it = it.left, steps++ + ) { + if ( + isFormat(it) && + keyOf(it) === key && + !this.tr.insertSet.hasId(it.id) && + (!it.deleted || this.tr.deleteSet.hasId(it.id)) + ) { + source = it; + break; + } + } + if (source === null || !equalFormatValue(valueOf(source), value)) { + return null; + } + const opener = runOpener(doc, source); + if (opener.id.client === doc.clientID) { + return null; + } + for (let it = this.right; it !== null; it = it.right) { + if ( + it.deleted || + !isFormat(it) || + keyOf(it) !== key || + equalFormatValue(valueOf(it), value) + ) { + continue; + } + return valueOf(it) === null && changeEnd(doc, opener) === it ? it : null; + } + return null; + } + + /** Write `content` right after `left`, away from the cursor. */ + private insertAfter(left: Item, content: ContentFormat): Item { + const doc = this.tr.doc; + const right = left.right; + const item = new Item( + createID(doc.clientID, doc.store.getClock(doc.clientID)), + left, + left.lastId, + right, + right && right.id, + this.parent, + null, + content, + ); + item.integrate(this.tr, 0); + return item; } /** @@ -847,6 +1026,24 @@ export class Cursor { * there (see minimizeFormatChanges), so a comment resolved over two * characters while a peer typed plain text between them came back on the * second. A key whose old value there is null is restored by no one. + * 5. Where it restores a value (at the end of the range, as after + * inserted text, see insertNegatedFormats) whose run another client's + * change opened (see runOpener), and the item that ends the restored run + * is that change's own end of the run (see changeEnd), it writes a null + * of its own right after that end. Why: that end can go while the + * restore stays, as the undo of that change deletes it and a concurrent + * delete may drop it (it keeps a bounded number of the items it leaves, + * see collapseGap), and the restored value ran on to the end of the + * document (a format undone while a collaborator typed plain text inside + * it, pasted a list there or removed it from part of the range; a + * comment undone while a collaborator typed right after it where another + * comment ended; a comment whose end a long offline peer's delete + * dropped while a collaborator pasted inside it). Right after the end, + * not before it: text a collaborator types at the end of the run at the + * same time lands before the end, inside the run, and keeps the run's + * value. A null by which a change made text inside the run plain (typed + * plain text, a removal) is no such end: the undo of that change brings + * the value back after it. */ format(n: number, formats: Record): void { const passed = new Set(); From 94795667415d35a1474a02a781ba5b4982abe318 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Wed, 28 Oct 2026 21:32:45 -0400 Subject: [PATCH 2/3] [lexical-y] Bug Fix: End a run a write opens with its own null where it reuses another client's change end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description A follow-up to the rule above, for runs a write opens itself. When a write closes the run it opens (its range's value, or inserted text's) by reusing an item already there, and that item is another client's change end, the undo of that change takes the run's end away. With links only, on the previous commit: - B links "t2z t3z t4z" while C links "t3z" with the same URL. Then B undoes its link while C links "t2z t3z t4z" again. C's link over " t4z" closed on B's closer, and once B's undo removed it, the link covered t3z to t9z and the whole next paragraph, on every client and in a fresh read. The write now puts a null of its own right after such a reused end (`Cursor#insertNegatedFormats`, rule 5): the run ends where the write ended it, whatever happens to the other change. ## Test plan New regression in `packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts`, three clients in both orders. ### Before On the previous commit (091a734): ``` $ npx vitest --project unit --no-watch packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts -t "C links" × S7-26: B links "t2z t3z t4z" ‖ C links "t3z"; then B undoes it ‖ C links "t2z t3z t4z" (ids 1,2,3) × S7-26: B links "t2z t3z t4z" ‖ C links "t3z"; then B undoes it ‖ C links "t2z t3z t4z" (ids 3,2,1) Expected: "t3z t4z / " Received: "t3z t4z t5z t6z t7z t8z t9z / tail line" Test Files 1 failed (1) Tests 2 failed | 188 skipped (190) ``` ### After ``` $ npx vitest --project unit --no-watch --maxWorkers=6 packages/lexical-y Test Files 148 passed | 2 skipped (150) Tests 4985 passed | 16 expected fail | 2 skipped (5003) $ pnpm run ci-check Changeset check passed: 5 pending changeset(s) valid. Found 0 errors ``` The local soak lanes are unchanged from the previous commit (no new failures). The undo-vs-comment fuzz stays at 0 of 1000. Long sessions store about 0.5% more format items than the previous commit. Browser e2e was not run. --- .../unit/regressions/Formats.test.ts | 52 +++++++++++++++---- packages/lexical-y/src/internal/Cursor.ts | 39 +++++++++++++- 2 files changed, 80 insertions(+), 11 deletions(-) diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts index 8081479ae9b..3177e4f7d39 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts @@ -1487,6 +1487,22 @@ describe('a value one client restates ‖ the undo of the change that gave it', sel.insertNodes(blocks.map(block => block())); $setSelection(null); }; + // Each line's links' text. + const $links = () => + $getRoot() + .getChildren() + .map(p => + ($isElementNode(p) ? p.getChildren() : []) + .filter($isLinkNode) + .map(link => link.getTextContent()) + .join(' '), + ) + .join(' / '); + const $link = (from: number, to: number) => () => { + $setSelection($selectLine(from, to)); + $toggleLink('https://u1.test'); + $setSelection(null); + }; const $list = () => $createListNode('bullet').append( $createListItemNode().append($createTextNode('t20z t67z')), @@ -1699,16 +1715,6 @@ describe('a value one client restates ‖ the undo of the change that gave it', t.a.update($format(format, 16, 27)); t.b.undo(); await merged(t); - const $links = () => - $getRoot() - .getChildren() - .map(p => - ($isElementNode(p) ? p.getChildren() : []) - .filter($isLinkNode) - .map(link => link.getTextContent()) - .join(' '), - ) - .join(' / '); const fresh = await freshReader(t.a); for (const p of [t.a, t.b, fresh]) { expect(p.read($links)).toBe('t7z t8z t9z / '); @@ -1716,6 +1722,32 @@ describe('a value one client restates ‖ the undo of the change that gave it', }); } } + // S7-26, links only: the run a write opens ends at another client's + // change end, which it reuses as its own closer: it gets a null of its own + // after it too, so that change's undo does not take the run's end away. + for (const ids of [ + [1, 2, 3], + [3, 2, 1], + ]) { + test(`S7-26: B links "t2z t3z t4z" ‖ C links "t3z"; then B undoes it ‖ C links "t2z t3z t4z" (ids ${ids})`, async () => { + const t = await setup( + twoLines('t1z t2z t3z t4z t5z t6z t7z t8z t9z', 'tail line'), + {ids}, + ); + t.offline(); + t.b.update($link(4, 15)); + t.c.update($link(8, 11)); + await t.sync(); + t.offline(); + t.b.undo(); + t.c.update($link(4, 15)); + await merged(t); + const fresh = await freshReader(t.a); + for (const p of [t.a, t.b, t.c, fresh]) { + expect(p.read($links)).toBe('t3z t4z / '); + } + }); + } }); // The undo race matrix of soak7: A adds bold or a comment on "t56z t57z" diff --git a/packages/lexical-y/src/internal/Cursor.ts b/packages/lexical-y/src/internal/Cursor.ts index 2e2b1ee86a6..262099fb121 100644 --- a/packages/lexical-y/src/internal/Cursor.ts +++ b/packages/lexical-y/src/internal/Cursor.ts @@ -191,6 +191,20 @@ function lastOwnItemBefore(doc: Doc, item: Item, key: string): Item | null { return null; } +/** + * Whether null `item` is the end its client gave a run it opened in the + * same write (see changeEnd): the item of its key its client made last + * before it opens a run that ends at `item`. + */ +function isChangeEnd(doc: Doc, item: Item): boolean { + const opener = lastOwnItemBefore(doc, item, keyOf(item)); + return ( + opener !== null && + valueOf(opener) !== null && + changeEnd(doc, opener) === item + ); +} + /** The nearest live item of `key` before `item`, among the 4096 before it. */ function liveItemBefore(item: Item, key: string): Item | null { let steps = 0; @@ -512,6 +526,9 @@ export class Cursor { * (see format, rule 5). */ private insertNegatedFormats(negated: Formats, closers = false): void { + // The keys whose run, which this write opens, ends at another client's + // change end it reuses (see format, rule 5). + const reusedEnds: string[] = []; while (this.right !== null) { const r = this.right; if (!isGone(r)) { @@ -537,10 +554,26 @@ export class Cursor { break; } } + if ( + valueOf(r) === null && + r.id.client !== this.tr.doc.clientID && + isChangeEnd(this.tr.doc, r) + ) { + reusedEnds.push(keyOf(r)); + } negated.delete(keyOf(r)); } this.forward(); } + for (const key of reusedEnds) { + this.right = this.newItem( + new ContentFormat(key, null as unknown as object), + ); + if (closers) { + formatClosers.add(this.right); + } + this.forward(); + } const ends: Item[] = []; for (const [key, val] of negated) { const end = val === null ? null : this.restatedEnd(key, val); @@ -1043,7 +1076,11 @@ export class Cursor { * same time lands before the end, inside the run, and keeps the run's * value. A null by which a change made text inside the run plain (typed * plain text, a removal) is no such end: the undo of that change brings - * the value back after it. + * the value back after it. Likewise, where the run this write opens (its + * range's value, or inserted text's) ends at another client's change end + * that it reuses as its own closer, it writes a null of its own right + * after it (a link a collaborator undid while another linked the text + * up to its end ran on to the end of the document). */ format(n: number, formats: Record): void { const passed = new Set(); From 603812b76a44313ff8c5f7ec8df7b838d4caaa26 Mon Sep 17 00:00:00 2001 From: Gerard Rovira Date: Thu, 29 Oct 2026 02:42:24 -0700 Subject: [PATCH 3/3] [lexical-y] Bug Fix: Keep a comment's end when a paste and an undo race plain text typed after a word two comments cover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description In `@lexical/y`, a comment could spread to the end of the document. A comments "t1z", and a second comment covers it too (from A or another client); J types plain text right after "t1z"; then J undoes the typing while C pastes over J's text. On every client and in a fresh read, the first comment then covered the rest of the line and every paragraph after it. With one comment, or with a manual delete instead of the undo, it did not happen. J's typing ends each comment with a null of J's own written right after the end the comment's author wrote, so that the comment ends even where the author's end goes. C's paste deletes J's text, and the delete collapses the format items it leaves after that text. Of the two nulls that end the first comment there, it kept only the last one, J's, as the one that gives the content after the run its value; J's concurrent undo deleted that null, and nothing ended the comment any more. With this change, where the delete keeps a key's last item, it also keeps the last item before it of that key with the same value (no other value between them) that another client wrote. Each client's undo deletes its own items only, so a run's end and the null another client wrote right after it each keep ending the run where the other's writer removes theirs. It keeps at most one more item per key, and only where two clients ended the same value at the same place. ## Test plan New test in `packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts`, for four client id orders, with the two comments from one client and from two: A comments "t1z", the second comment covers it, J types " t1011z" right after it, then J undoes the typing while C pastes "t1013z" over "t1011z". The comments end after the word on every client and in a fresh read (C's text, now right after "t1z" inside both comments' runs, takes both comments). `./node_modules/.bin/vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts -t "L1B-1" --reporter=verbose` ### Before ``` × regressions/Formats.test.ts > L1B-1: undo of plain text typed after a word two comments cover ‖ a paste over that text > L1B-1: the comments end after the word, from one client (ids 1,2,3,4) 223ms × regressions/Formats.test.ts > L1B-1: undo of plain text typed after a word two comments cover ‖ a paste over that text > L1B-1: the comments end after the word, from two clients (ids 1,2,3,4) 78ms (the same for ids 4,3,2,1, 2,4,1,3 and 3,1,4,2) AssertionError: expected '"t1zt1013z"[c12+c4] " t2"[c4] / "t3z"…' to be '"t1zt1013z"[c12+c4] " t2" / "t3z"' // Object.is equality Expected: ""t1zt1013z"[c12+c4] " t2" / "t3z"" Received: ""t1zt1013z"[c12+c4] " t2"[c4] / "t3z"[c4]" Tests 8 failed | 190 skipped (198) ``` ### After ``` ✓ regressions/Formats.test.ts > L1B-1: undo of plain text typed after a word two comments cover ‖ a paste over that text > L1B-1: the comments end after the word, from one client (ids 1,2,3,4) 256ms ✓ regressions/Formats.test.ts > L1B-1: undo of plain text typed after a word two comments cover ‖ a paste over that text > L1B-1: the comments end after the word, from two clients (ids 1,2,3,4) 88ms ✓ regressions/Formats.test.ts > L1B-1: undo of plain text typed after a word two comments cover ‖ a paste over that text > L1B-1: the comments end after the word, from one client (ids 4,3,2,1) 93ms ✓ regressions/Formats.test.ts > L1B-1: undo of plain text typed after a word two comments cover ‖ a paste over that text > L1B-1: the comments end after the word, from two clients (ids 4,3,2,1) 87ms ✓ regressions/Formats.test.ts > L1B-1: undo of plain text typed after a word two comments cover ‖ a paste over that text > L1B-1: the comments end after the word, from one client (ids 2,4,1,3) 85ms ✓ regressions/Formats.test.ts > L1B-1: undo of plain text typed after a word two comments cover ‖ a paste over that text > L1B-1: the comments end after the word, from two clients (ids 2,4,1,3) 80ms ✓ regressions/Formats.test.ts > L1B-1: undo of plain text typed after a word two comments cover ‖ a paste over that text > L1B-1: the comments end after the word, from one client (ids 3,1,4,2) 80ms ✓ regressions/Formats.test.ts > L1B-1: undo of plain text typed after a word two comments cover ‖ a paste over that text > L1B-1: the comments end after the word, from two clients (ids 3,1,4,2) 89ms Tests 8 passed | 190 skipped (198) ``` The other tests in `regressions/Formats.test.ts`, the S7-10, S7-20, S7-24, S7-26 and S7-33 cases included, pass unchanged. ``` $ pnpm exec vitest run --project unit --maxWorkers=8 Test Files 240 passed | 2 skipped (242) Tests 7180 passed | 14 expected fail | 2 skipped (7196) ``` `tsc`, `tsc-test`, `tsc-scripts`, `tsc-extension`, `tsc-website`, `flow`, `eslint`, `prettier` and the changeset check pass on the repository's files. --- .../unit/regressions/Formats.test.ts | 57 +++++++++++++++++++ packages/lexical-y/src/internal/Cursor.ts | 36 ++++++++++-- 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts b/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts index 3177e4f7d39..bbc6371f7ec 100644 --- a/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts +++ b/packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts @@ -1860,3 +1860,60 @@ describe('a format or comment removed ‖ an edit inside its range', () => { } } }); + +// L1B-1: J typed plain text right after a word two comments cover, which +// ends each comment with a null of J's own after the end its author wrote +// (see Cursor#format, rule 5). C pasted over J's word, and the delete +// collapsed the format items after it keeping only J's null of the first +// comment; J's concurrent undo of its typing deleted that null, and the +// comment covered the rest of the document. +describe('L1B-1: undo of plain text typed after a word two comments cover ‖ a paste over that text', () => { + for (const ids of [ + [1, 2, 3, 4], + [4, 3, 2, 1], + [2, 4, 1, 3], + [3, 1, 4, 2], + ]) { + for (const sameClient of [true, false]) { + test(`L1B-1: the comments end after the word, from ${sameClient ? 'one client' : 'two clients'} (ids ${ids})`, async () => { + const t = await setup( + () => + $getRoot().append( + $createParagraphNode().append($createTextNode('t1z t2')), + $createParagraphNode().append($createTextNode('t3z')), + ), + {ids}, + ); + const [a, b, c, j] = t.peers; + const $comment = (id: string) => () => { + $addMarkID(id, $selectLine(0, 3)); + $setSelection(null); + }; + a.update($comment('c4')); + await t.sync(); + (sameClient ? a : b).update($comment('c12')); + await t.sync(); + for (const peer of t.peers) { + peer.binding.undoManager?.clear(); + } + // Plain text typed right after "t1z", a text node of its own. + j.update(() => { + const [word] = ( + $getRoot().getFirstChildOrThrow() as ElementNode + ).getAllTextNodes(); + word.splitText(3)[0].insertAfter($createTextNode(' t1011z')); + }); + await t.sync(); + t.offline(); + j.undo(); + c.update(() => { + const sel = $selectLine(4, 10); + $setSelection(sel); + sel.insertText('t1013z'); + $setSelection(null); + }); + expect(await merged(t)).toBe('"t1zt1013z"[c12+c4] " t2" / "t3z"'); + }); + } + } +}); diff --git a/packages/lexical-y/src/internal/Cursor.ts b/packages/lexical-y/src/internal/Cursor.ts index 262099fb121..7b9b4fee172 100644 --- a/packages/lexical-y/src/internal/Cursor.ts +++ b/packages/lexical-y/src/internal/Cursor.ts @@ -754,7 +754,10 @@ export class Cursor { * there: a format change keeps the item before its opener at a place (see * format), and with several keys a whole value's item there gives every key * its value. Delete the items of the rest, except, of each key, the last item - * where without it the key would have another value after the run. Invariant: + * where without it the key would have another value after the run, and, + * where the key's last item is kept, the last item before it of that key with + * the same value (no other value between them) that another client wrote. + * Invariant: * such a run holds, besides the units of the last delete in it, at most five * units per family, the items that give a family its value at the end of the * places of those, and items that change a key's value between the content @@ -777,7 +780,14 @@ export class Cursor { * at once each left the space's own item, which the other relied on); and the * one before it gives that content the value it falls back to where a * concurrent format change of it removes its item (a comment resolved on a - * character took the comment of the text deleted before it). The items a kept + * character took the comment of the text deleted before it). The other + * client's item with the key's last value stays as each client's undo deletes + * its own items only: a run's end and the null another client wrote right + * after it (see format, rule 5) each end the run where the other's writer + * removes theirs (a paste over plain text typed after a word two comments + * cover kept only the typist's null of one comment, and the typist's + * concurrent undo left that comment on to the end of the document). The + * items a kept * unit's place holds for the other keys of its family give content restored * there, as an undo of its delete restores it, the value it had: a word two * clients gave a comment each, deleted, kept one comment's opener once a @@ -924,14 +934,26 @@ export class Cursor { i = j; } } - // The value of each key after the run stays. + // The value of each key after the run stays; where the key's last item + // stays, so does the last item before it with the same value that + // another client wrote. const lastOf = new Map(); const lastKept = new Map(); + const otherClient = new Map(); for (const unit of units) { for (const it of unit) { - lastOf.set(keyOf(it), it); + const key = keyOf(it); + const before = lastOf.get(key); + if (before !== undefined) { + if (!equalFormatValue(valueOf(before), valueOf(it))) { + otherClient.delete(key); + } else if (before.id.client !== it.id.client) { + otherClient.set(key, before); + } + } + lastOf.set(key, it); if (keep.has(it)) { - lastKept.set(keyOf(it), it); + lastKept.set(key, it); } } } @@ -945,6 +967,10 @@ export class Cursor { ) { keep.add(it); } + const other = otherClient.get(key); + if (keep.has(it) && other !== undefined) { + keep.add(other); + } } const values = new Map(); const changed = new Set();