[lexical-y] Bug Fix: End a restored comment or format run with the writer's own null also where the writer opened the run itself

.changeset/lexical-y-own-run-restore-end.md

@@ -0,0 +1,7 @@
+---
+'@lexical/y': patch
+---
+
+[fix] A comment (or format) no longer spreads over the rest of the document when its author types unformatted text inside it while a collaborator's delete drops the comment's end
+
+Verify: comment a word, then, offline, type uncommented text inside it while a collaborator deletes the word and text next to it; after the client reconnects, the words after it stay uncommented on every client.

packages/lexical-y/src/__tests__/unit/OwnRunRestoreEnd.test.ts

@@ -0,0 +1,159 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+
+// @vitest-environment node
+
+import type {Item} from '@y/y';
+
+import {$addMarkID, $getNodeMarkIDs} from '@lexical/mark';
+import {ContentFormat} from '@y/y';
+import {
+  $createTextNode,
+  $getRoot,
+  type LexicalNode,
+  type TextNode,
+} from 'lexical';
+import {describe, expect, test} from 'vitest';
+
+import {
+  $block,
+  $p,
+  $selectIn,
+  expectConverged,
+  type Peer,
+  setup,
+} from '../utils/harness';
+import {prepareMarkState as prepare} from '../utils/sizeOps';
+
+// A client types uncommented text inside a word it commented itself, so its
+// write restores the comment after that text and relies on the comment's own
+// end (the null its comment wrote after the word) to end it. A collaborator
+// that has not seen that text deletes the word and then deletes the
+// comment's opener and end there, which for it bracket deleted content only,
+// so deleting them changes nothing it shows. That second step is simulated
+// with raw Yjs deletes (dropCommentItems): it stands for any concurrent
+// deletion of that end, such as the bounded collapse a delete next to a run
+// of deleted content runs (Cursor#collapseGap, which keeps a bounded number
+// of the format items there), and isolates the condition this guards: the
+// restore's run losing the end it relied on. Merged, the restore must still
+// end where the comment ended, not run on over the rest of the document.
+
+/** Each word of `peer`'s first line, with its comment ids. */
+function words(peer: Peer): string {
+  return peer.read(() =>
+    ($block([0]).getAllTextNodes() as TextNode[])
+      .flatMap(n => {
+        const ids = $getNodeMarkIDs(n as LexicalNode);
+        return n
+          .getTextContent()
+          .split(' ')
+          .filter(w => w !== '')
+          .map(w => (ids.length > 0 ? `${w}[${ids.join(',')}]` : w));
+      })
+      .join(' '),
+  );
+}
+
+/** Delete `peer`'s live format items of comment `id` with raw Yjs deletes. */
+function dropCommentItems(peer: Peer, id: string) {
+  const doc = peer.doc;
+  doc.transact(tr => {
+    const walk = (type: {_start: Item | null}) => {
+      for (let it = type._start; it !== null; it = it.right) {
+        if (
+          !it.deleted &&
+          it.content instanceof ContentFormat &&
+          it.content.key.endsWith(id)
+        ) {
+          it.delete(tr);
+        }
+        const content = it.content as {type?: {_start?: Item | null}};
+        if (content.type !== undefined && content.type._start !== undefined) {
+          walk(content.type as {_start: Item | null});
+        }
+      }
+    };
+    walk(doc.get('lexical-y') as unknown as {_start: Item | null});
+  });
+}
+
+describe('@lexical/y a restore of a comment its own client opened', () => {
+  for (const ids of [
+    [1, 2],
+    [2, 1],
+  ]) {
+    test(`ends where the comment ended when a collaborator's delete drops the comment's end (ids ${ids})`, async () => {
+      const t = await setup(
+        () => $getRoot().append($p('aa Kilo bb cc'), $p('tail line')),
+        {ids, prepare},
+      );
+      t.a.update(() => void $addMarkID('c1', $selectIn([0], 3, 7)));
+      await t.sync();
+      expect(words(t.a)).toBe('aa Kilo[c1] bb cc');
+      t.offline();
+      // A types uncommented "X" inside its own commented word
+      t.a.update(() => {
+        const node = ($block([0]).getAllTextNodes() as TextNode[]).find(n =>
+          n.getTextContent().startsWith('Kilo'),
+        )!;
+        const [, tail] = node.splitText(2);
+        tail.insertBefore($createTextNode('X'));
+      });
+      expect(words(t.a)).toBe('aa Ki[c1] X lo[c1] bb cc');
+      // B deletes the word, then drops the comment's items around it
+      t.b.update(() => void $selectIn([0], 3, 8).removeText());
+      dropCommentItems(t.b, 'c1');
+      expect(words(t.b)).toBe('aa bb cc');
+      await t.sync();
+      await expectConverged(t.peers);
+      // A's "X" is uncommented, and so is everything after it
+      expect(words(t.a)).toBe('aa Xbb cc');
+      expect(
+        t.a.read(() =>
+          ($block([1]).getAllTextNodes() as TextNode[]).some(
+            n => $getNodeMarkIDs(n).length > 0,
+          ),
+        ),
+      ).toBe(false);
+    });
+    test(`ends where the comment ended when the comment was written with a hole in it (ids ${ids})`, async () => {
+      const t = await setup(
+        () => $getRoot().append($p('aa Kilo bb cc'), $p('tail line')),
+        {ids, prepare},
+      );
+      // One write: the comment, with uncommented "X" inside it, so the
+      // comment's end ends the run its restore after "X" opens
+      t.a.update(() => {
+        $addMarkID('c1', $selectIn([0], 3, 7));
+        const node = ($block([0]).getAllTextNodes() as TextNode[]).find(n =>
+          n.getTextContent().startsWith('Kilo'),
+        )!;
+        const [, tail] = node.splitText(2);
+        tail.insertBefore($createTextNode('X'));
+      });
+      await t.sync();
+      expect(words(t.a)).toBe('aa Ki[c1] X lo[c1] bb cc');
+      t.offline();
+      // A types uncommented "Y" inside "lo"
+      t.a.update(() => {
+        const node = ($block([0]).getAllTextNodes() as TextNode[]).find(
+          n => n.getTextContent() === 'lo',
+        )!;
+        const [, tail] = node.splitText(1);
+        tail.insertBefore($createTextNode('Y'));
+      });
+      expect(words(t.a)).toBe('aa Ki[c1] X l[c1] Y o[c1] bb cc');
+      t.b.update(() => void $selectIn([0], 3, 9).removeText());
+      dropCommentItems(t.b, 'c1');
+      expect(words(t.b)).toBe('aa bb cc');
+      await t.sync();
+      await expectConverged(t.peers);
+      expect(words(t.a)).toBe('aa Ybb cc');
+    });
+  }
+});

packages/lexical-y/src/internal/Cursor.ts

@@ -217,18 +217,20 @@ function liveItemBefore(item: Item, key: string): Item | 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.
+ * The items that opened the run live format item `item` gives its value in,
+ * from `item` back: `item` itself, and, 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 item that gave the run its value before that text,
+ * and so on; the last one opened the run.
  */
-export function runOpener(doc: Doc, item: Item): Item {
+function runChain(doc: Doc, item: Item): Item[] {
   const {key, value} = item.content as ContentFormat;
+  const chain = [item];
   let current = item;
   for (let depth = 0; depth < 16; depth++) {
     const negation = lastOwnItemBefore(doc, current, key);
     if (negation === null || valueOf(negation) !== null) {
-      return current;
+      return chain;
     }
     let found = false;
     let steps = 0;
@@ -247,11 +249,12 @@ export function runOpener(doc: Doc, item: Item): Item {
     }
     const source = found ? liveItemBefore(negation, key) : null;
     if (source === null || !equalFormatValue(valueOf(source), value)) {
-      return current;
+      return chain;
     }
     current = source;
+    chain.push(current);
   }
-  return current;
+  return chain;
 }
 
 /** A live countable item: text, a child node, an embed. */
@@ -611,14 +614,13 @@ export class Cursor {
   }
 
   /**
-   * 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.
+   * For a restore of `key` to `value` written at the cursor: where the next
+   * live item of the key after the cursor with another value is the end that
+   * a write that gave the restated run its value wrote for it (see runChain,
+   * from the item that gave the content the value before this write: the
+   * nearest item of the key before the cursor that this write did not make;
+   * 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;
@@ -642,10 +644,7 @@ export class Cursor {
     if (source === null || !equalFormatValue(valueOf(source), value)) {
       return null;
     }
-    const opener = runOpener(doc, source);
-    if (opener.id.client === doc.clientID) {
-      return null;
-    }
+    const chain = runChain(doc, source);
     for (let it = this.right; it !== null; it = it.right) {
       if (
         it.deleted ||
@@ -655,7 +654,9 @@ export class Cursor {
       ) {
         continue;
       }
-      return valueOf(it) === null && changeEnd(doc, opener) === it ? it : null;
+      return valueOf(it) === null && chain.some(c => changeEnd(doc, c) === it)
+        ? it
+        : null;
     }
     return null;
   }
@@ -1105,18 +1106,26 @@ export class Cursor {
    *    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,
+   *    inserted text, see insertNegatedFormats), and the item that ends the
+   *    restored run is the end that a write that gave that run its value
+   *    wrote for it (see runChain: the change that opened the run, or one
+   *    that restored it there before; see changeEnd), it writes a null of
+   *    its own right after that end, whichever client wrote it, this one
+   *    too. 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; a comment whose author typed
+   *    uncommented text into it while a collaborator's delete next to the
+   *    commented word, deleted before, dropped the comment's end). The run's
+   *    end is that of the write that restored it last where there is one: a
+   *    comment written with uncommented text inside it ends with the end its
+   *    restore after that text wrote, not the null before that text. 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