Repository navigation
Conversation
…it is written next to a collaborator's node that is undone
## Description
Text nodes that Lexical keeps apart from an equal neighbour (a tab, text in
token or segmented mode, and every `TextNode` subclass such as a hashtag or
a code highlight token) are stored as runs with a segment key, opened and
closed by format items their writer adds. Two cases lost such a node:
- A node written right after a collaborator's node of the same kind landed
inside that node's run and wrote only the items whose values differed (its
segment key), reading its type and formats from the collaborator's opener.
When the collaborator undid their node, the undo deleted that opener and
its closers, and the node was read as something else: a tab as a `"\t"` in
the text before it, a hashtag or a code highlight token as plain text.
The writer now gives content that starts a segment other than the one in
effect at the cursor items of its own for every key it has, so it never
depends on another node's run. A node written before a collaborator's was
already safe (the writer never steps past another node's type opener).
- Two clients' `"\t"` under one tab opener (left by the case above through
undo and redo, in documents written before this fix) were read as one tab
node: one tab was not shown, and the binding's self-check reported the
line (`text "code\t\txx" != "code\txx"`). Each `"\t"` is now read as a tab
node of its own.
Reported downstream (a collaborative notes app's randomized test with undo).
## Changeset
`.changeset/lexical-y-own-segment-run.md` (`@lexical/y` patch, `[fix]` with
`Verify:`).
## Test plan
New tests in `packages/lexical-y/src/__tests__/unit/TabLeak.test.ts`:
- "a node kept apart written next to another client's": B adds a node next
to an existing one, A adds one right after (or before) B's, B undoes (or
removes) its node, for a tab, token text, segmented text, bold token text
and a hashtag in a paragraph and a tab and a code highlight token in a code
block, with both client id orders. Every peer keeps A's node as written,
no tab character is left outside a tab node, and the peers converge with a
fresh reader.
- "tab characters under one tab opener": a second `"\t"` written inside a
tab's run (paragraph and code block) reads as two tab nodes, and both
peers still edit the line and converge.
```
pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/TabLeak.test.ts
```
### Before
```
× paragraph: B adds a tab, A adds one after it, B undos (ids 1,2) 17ms
× paragraph: B adds a tab, A adds one after it, B undos (ids 2,1) 14ms
× paragraph: B adds a hashtag, A adds one after it, B undos (ids 1,2) 27ms
× paragraph: B adds a hashtag, A adds one after it, B undos (ids 2,1) 20ms
× code: B adds a tab, A adds one after it, B undos (ids 1,2) 18ms
× code: B adds a tab, A adds one after it, B undos (ids 2,1) 16ms
× code: B adds a code-highlight, A adds one after it, B undos (ids 1,2) 15ms
× code: B adds a code-highlight, A adds one after it, B undos (ids 2,1) 15ms
× paragraph: two "\t" in one run read as two tabs 8ms
× code: two "\t" in one run read as two tabs 8ms
Test Files 1 failed (1)
Tests 10 failed | 79 passed (89)
```
A randomized three-peer test over a notes app's node set (lists, tables,
code blocks, links, tabs; typing, splits, merges, range edits, undo and
redo, reloads, self-check on), 2000 seeds: `Tests 5 failed | 1995 passed
(2000)` (seeds 1070, 1288, 1529, 1760, 1870, each a self-check warning
`text "code\t\txx" != "code\txx"`).
### After
```
Test Files 1 passed (1)
Tests 89 passed (89)
```
The randomized test: `Tests 2000 passed (2000)`.
```
pnpm exec vitest run --project unit packages/lexical-y packages/lexical-code packages/lexical-hashtag
Test Files 188 passed | 2 skipped (190)
Tests 7935 passed | 46 expected fail | 5 skipped (7986)
pnpm exec vitest run --project unit packages/lexical/ packages/lexical-mark packages/lexical-extension packages/lexical-html
Test Files 157 passed | 1 skipped (158)
Tests 4147 passed | 1 skipped (4148)
pnpm exec vitest run --project scripts-unit scripts/__tests__/unit/treeShakingSource.test.ts
Tests 374 passed (374)
```
`Fuzz.test.ts` with `FUZZ_SEED_START=1 FUZZ_SEEDS=3000` and
`FUZZ_SEED_START=50000 FUZZ_SEEDS=2000`: the same 8 seeds fail with and
without this change (2277, 50180, 50231, 50372, 51439, 51507, 51972, 51976;
the same messages on main), and every other seed passes.
`pnpm run ci-check` passes. Browsers were not exercised (headless and jsdom
unit tests only).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
lexical
@lexical/a11y
@lexical/clipboard
@lexical/code
@lexical/code-core
@lexical/code-prism
@lexical/code-shiki
@lexical/compiler
@lexical/devtools-core
@lexical/dragon
@lexical/eslint-plugin
@lexical/extension
@lexical/fast-check
@lexical/file
@lexical/hashtag
@lexical/headless
@lexical/history
@lexical/html
@lexical/internal
@lexical/link
@lexical/list
@lexical/mark
@lexical/markdown
@lexical/mdast
@lexical/offset
@lexical/overflow
@lexical/plain-text
@lexical/react
@lexical/rich-text
@lexical/selection
@lexical/table
@lexical/tailwind
@lexical/text
@lexical/utils
@lexical/y
@lexical/yjs
commit: |
![meta-code-review[bot]](https://avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for tracking this down. The undo case is real: against the base source the new TabLeak.test.ts fails exactly the 10 tests listed in the Test plan, and with this change they pass (a redo by B afterwards also comes back clean). A few things need attention before this can go in.
1. A concurrent link or format removal is reverted (please fix)
ownSegmentKeys (packages/lexical-y/src/internal/Cursor.ts:532-547) returns every key with a non-null value whenever the inserted content's segment differs from the cursor's, including when the cursor sits in plain text with no segment at all. insertFormats (lines 557-558) then writes an opener for each of those keys and records it in negated, so a restore of the surrounding run's value is written after the content. A tab typed inside a link is now stored as {w:link}{t:"tab"}{k}\t{w:link}{t:null}{k:null} (on main: {t:"tab"}{k}\t{t:null}{k:null}). A peer that removes the link (or bold) at the same time never sees those items, so every peer converges with the link back on the inserted node and on the text after it. With the existing harness:
test('a tab typed inside a link while a peer removes the link', async () => {
const t = await setup(() =>
$getRoot().append(
$createParagraphNode().append(
$createLinkNode('https://example.com').append($createTextNode('abcd')),
),
$createParagraphNode().append($createTextNode('next line')),
),
);
t.offline();
t.a.update(() => {
const link = $block([0]).getFirstChild() as ElementNode;
const [, right] = (link.getFirstChild() as TextNode).splitText(2);
right.insertBefore($createTabNode());
});
t.b.update(() => {
const link = $block([0]).getFirstChild() as ElementNode;
for (const child of link.getChildren()) {
link.insertBefore(child);
}
link.remove();
});
await t.sync();
// main: paragraph("ab", tab, "cd"); this branch: paragraph("ab", link(tab, "cd"))
for (const peer of t.peers) {
expect(peer.read(() => $block([0]).getChildren().some($isLinkNode))).toBe(false);
}
await expectConverged(t.peers);
});The same happens with a hashtag or token text instead of the tab, with a bold hashtag or bold token typed into bold text that a peer unbolds ("cd" comes back bold), and with A's tab right after B's tab inside a link. This is the situation the cursor's own invariants are written to prevent (the minimizeFormatChanges comment and rule 4 of format: a comment resolved while a peer typed came back). A narrower rule should keep your fix: own only the keys whose value at the cursor comes from the segment's own run (the items the segment author's write opened, for example this.sources entries from the same client and write as the k source, as oneFormatWrite checks), return null when the cursor has no segment, and never own an inherited wrapper key. Please add the link and bold cases as tests.
2. The token and segmented cases do not exercise the bug
In TabLeak.test.ts the token, segmented and bold token variants also pass without the fix: B's node is inserted right before the setup's token PP (lines 405 and 414), B's write steps over PP's p:mode opener (only t and k openers are protected, Cursor.ts:466-470), and A's node then depends on the setup writer's opener, which B's undo never removes. If B's token opens a run of its own instead (for example B splits "ab cd" and inserts its token after "ab", A adds one right after it, B undoes), main loses A's token and this branch keeps it. Using that setup would make the token and segmented claim in the description and changeset tested.
3. Changeset Verify: wording
.changeset/lexical-y-own-segment-run.md ends with "Documents written before this fix keep how they read", but with the reader change a stored line whose two tabs had collapsed into one now shows both. CONTRIBUTING.md asks a fix that changes observable behaviour to say what to verify, so please mention that case.
Minor
- With two
"\t"under one opener the two tab nodes share a segment key, andtakeTextmaps a segment to a single node (Read.ts:2372-2390), so each remote edit of that line recreates one of the tab nodes (the caret stays in place). Mapping a segment to a list of nodes would keep both keys. - Owned items add stored format items (eight Tab presses in one line: 18 before, 26 after); the narrower rule above should shrink that as well.
| const own = new Set<string>(); | ||
| for (const key in formats) { | ||
| if (formats[key] !== null) { | ||
| own.add(key); | ||
| } | ||
| } | ||
| return own; |
There was a problem hiding this comment.
This owns every key with a non-null value, including inherited ones such as a link (w:link) or f:bold, and also when the cursor has no segment at all (plain text). insertFormats then records each in negated, so a restore of the surrounding run's value is written after the content. If a peer removes that link or bold concurrently, the restore brings it back over the inserted node and the text after it (a tab typed into linked abcd while a peer unlinks it ends as "ab" link(tab "cd") on every peer; on main the link is gone). Could ownership be limited to the keys the segment author's own write opened, with null when the cursor has no segment?
There was a problem hiding this comment.
Fixed in 0f080d9 and narrowed again in 904c01c: content that starts a segment inside another segment's run now owns only a tab's type, and only when that segment's own write opened it; never a wrapper or a format, and nothing when the cursor has no segment. Your link case and the bold case are tests in TabLeak.test.ts ("a node kept apart written where a peer removes the link or format around it"), plus A's tab right after B's tab inside a link while C removes the link.
| let bKey = ''; | ||
| t.b.update(() => { | ||
| const bs = $make(kind, 'BB'); | ||
| $kindNodes(kind)[0].insertBefore(bs); |
There was a problem hiding this comment.
For the token, segmented and bold token kinds these cases also pass without the fix: B's node lands right before the setup's token PP, B's write steps over PP's p:mode opener (only t/k openers are protected in minimizeFormatChanges), so A's node depends on the setup writer's opener, which B's undo never removes. Inserting B's token where it opens its own run (for example after "ab" in a plain "ab cd") makes these fail on main and pass here.
There was a problem hiding this comment.
Thanks: B's node now opens a run of its own (B inserts it inside plain ab cd), so the token, segmented and bold token cases fail on main. The "before" placement passes for every kind since 0f080d9 (the cursor no longer steps past any item of another node's opening write). The "after + undo" cases for hashtag, code highlight, token and segmented text are test.fails naming #9586: only a tab's type is owned (904c01c explains why: line-dependent types must follow their run).
|
|
||
| [fix] Keep a tab, a hashtag, a code highlight token or text in token or segmented mode what it is when it was written right next to a collaborator's node of the same kind and that collaborator undoes theirs | ||
|
|
||
| Verify: in a shared document, one person presses Tab right next to a tab another person added; when the other person undoes, the first person's tab is still a tab for everyone (not a tab character in the text before it). Documents written before this fix keep how they read. |
There was a problem hiding this comment.
With the reader change, a stored line whose two tabs had collapsed into one now shows both, so "Documents written before this fix keep how they read" is not quite accurate. CONTRIBUTING.md asks a fix that changes observable behaviour to say what to verify; worth adding that case here.
There was a problem hiding this comment.
Updated: the Verify: paragraph now says a stored line where two tabs were shown as one shows both tabs, and that a link or format removed around a tab at the same time stays removed.
…t is written in, and keep each tab node of a run
## Description
Follow-up to the previous commit, from review:
- Content that starts a segment of its own inside another segment's run
wrote items for every key it had, so it also restated a link or a format
it had from the text around it, and a restore of that value after it. A
collaborator who removed that link or format at the same time saw it come
back on the node and on the text after it (a tab typed into a link while
the link was removed ended inside the link on every peer). It now owns
only the keys whose value at the cursor the other segment's own write
opened (its type, and the formats and fields written with it), never a
wrapper, and nothing where the cursor is not inside another segment's run.
- A node written right before another client's node kept apart could still
land inside that node's run: the cursor stepped past the opener items of
that node's write other than its type and segment keys (the mode of token
or segmented text), and the node then read its mode from that write, which
the other client's undo deletes. The cursor no longer steps past any item
of the write that opens another node's run.
- With two tab characters under one tab opener read as two tab nodes, the
reader's lookup of a node by segment key kept one node per segment, so a
remote edit of the line recreated the other tab node. It now keeps every
node of a segment.
The tests now open B's node as a run of its own (B adds it inside plain
text), so the token, segmented and bold token cases fail without the change,
and cover a tab, hashtag or token typed inside a link while a peer removes
the link, a bold one typed into bold text while a peer unbolds it, and A's
tab right after B's tab inside a link while C removes the link.
## Test plan
```
pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/TabLeak.test.ts
```
### Before
On main (`e28f4dd`):
```
× paragraph: B adds a tab, A adds one after it, B undos (ids 1,2) 15ms
× paragraph: B adds a tab, A adds one after it, B undos (ids 2,1) 13ms
× paragraph: B adds a token, A adds one after it, B undos (ids 1,2) 15ms
× paragraph: B adds a token, A adds one after it, B undos (ids 2,1) 18ms
× paragraph: B adds a token, A adds one before it, B undos (ids 1,2) 19ms
× paragraph: B adds a token, A adds one before it, B undos (ids 2,1) 19ms
× paragraph: B adds a segmented, A adds one after it, B undos (ids 1,2) 20ms
× paragraph: B adds a segmented, A adds one after it, B undos (ids 2,1) 24ms
× paragraph: B adds a segmented, A adds one before it, B undos (ids 1,2) 11ms
× paragraph: B adds a segmented, A adds one before it, B undos (ids 2,1) 12ms
× paragraph: B adds a bold token, A adds one after it, B undos (ids 1,2) 12ms
× paragraph: B adds a bold token, A adds one after it, B undos (ids 2,1) 12ms
× paragraph: B adds a bold token, A adds one before it, B undos (ids 1,2) 12ms
× paragraph: B adds a bold token, A adds one before it, B undos (ids 2,1) 12ms
× paragraph: B adds a hashtag, A adds one after it, B undos (ids 1,2) 13ms
× paragraph: B adds a hashtag, A adds one after it, B undos (ids 2,1) 12ms
× code: B adds a tab, A adds one after it, B undos (ids 1,2) 22ms
× code: B adds a tab, A adds one after it, B undos (ids 2,1) 18ms
× code: B adds a code-highlight, A adds one after it, B undos (ids 1,2) 14ms
× code: B adds a code-highlight, A adds one after it, B undos (ids 2,1) 12ms
× paragraph: two "\t" in one run read as two tabs 8ms
× code: two "\t" in one run read as two tabs 8ms
Failed Tests 22
Test Files 1 failed (1)
Tests 22 failed | 81 passed (103)
```
On the previous commit (`ee30f21`): the link and unbold cases, and the
tab nodes' keys after a remote edit:
```
× paragraph: two "\t" in one run read as two tabs 17ms
× code: two "\t" in one run read as two tabs 13ms
× a tab typed inside a link while a peer removes the link (ids 1,2) 9ms
× a bold tab typed into bold text while a peer unbolds it (ids 1,2) 6ms
× a tab typed inside a link while a peer removes the link (ids 2,1) 7ms
× a bold tab typed into bold text while a peer unbolds it (ids 2,1) 7ms
× a hashtag typed inside a link while a peer removes the link (ids 1,2) 7ms
× a bold hashtag typed into bold text while a peer unbolds it (ids 1,2) 7ms
× a hashtag typed inside a link while a peer removes the link (ids 2,1) 7ms
× a bold hashtag typed into bold text while a peer unbolds it (ids 2,1) 7ms
× a token typed inside a link while a peer removes the link (ids 1,2) 7ms
× a bold token typed into bold text while a peer unbolds it (ids 1,2) 6ms
× a token typed inside a link while a peer removes the link (ids 2,1) 12ms
× a bold token typed into bold text while a peer unbolds it (ids 2,1) 14ms
× A's tab right after B's tab inside a link while C removes the link (ids 1,2,3) 19ms
× A's tab right after B's tab inside a link while C removes the link (ids 3,2,1) 17ms
Failed Tests 16
Test Files 1 failed (1)
Tests 16 failed | 87 passed (103)
```
### After
```
Test Files 1 passed (1)
Tests 103 passed (103)
```
```
pnpm exec vitest run --project unit packages/lexical-y packages/lexical-code packages/lexical-hashtag packages/lexical-link
Test Files 199 passed | 2 skipped (201)
Tests 8176 passed | 46 expected fail | 5 skipped (8227)
```
The randomized three-peer test over a notes app's node set: 2000 of 2000
seeds pass. `Fuzz.test.ts` with `FUZZ_SEED_START=1 FUZZ_SEEDS=3000` and
`FUZZ_SEED_START=50000 FUZZ_SEEDS=2000`: the same 8 seeds fail as on main
(2277, 50180, 50231, 50372, 51439, 51507, 51972, 51976), every other seed
passes. `pnpm run ci-check` passes. Eight Tab presses at the end of a line
store 26 format items (18 on main): each tab next to the one before writes
its own type key and the ends of its run.
… so a concurrent change of the line reaches other types ## Description The previous commit gave content that starts a segment inside another segment's run items of its own for the keys that segment's write opened, its type included. A code highlight token written that way kept its type when a collaborator made the code line a paragraph at the same time (that change turns the line's tokens into plain text, but did not know the new token's own type item): the token stayed a code highlight token inside a paragraph, the editor that made the change showed it as plain text merged with its neighbours, a fresh read of its document showed it as a node of its own, and the self-check reported the line. A randomized session run found it. Now only a tab's type is owned there. A tab is a tab in any line, and no change of a range or a line removes that type. Other types and modes follow the run they are written in, as on main: written right after a collaborator's node of the same kind that the collaborator undoes, a hashtag, a code highlight token or token or segmented text still becomes plain text (#9586); those cases are kept in the tests as expected failures. A node written right before a collaborator's node keeps its type and mode for every kind (the cursor does not step into the other node's opening write). ## Test plan ``` pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/TabLeak.test.ts ``` ### Before The recorded session (four clients, tab-heavy catalog documents, seed 2500572, 77 of 91 operations), on the previous commit: client D reports `self-check (remote): container root[8].children: editor [...] != shared [...]` in round 10, and D's editor differs from a fresh read of its document (one text node `" t1003z = t35z t1014z t1021z t1010z t1027z; ..."` where the document holds three). On main: no report. ### After ``` Test Files 1 passed (1) Tests 93 passed | 10 expected fail (103) ``` The recorded session: no self-check report. 150 sessions of the same lane (seeds 2500500 to 2500649): the same counts of every flag kind as on main. ``` pnpm exec vitest run --project unit packages/lexical-y packages/lexical-code packages/lexical-hashtag packages/lexical-link Tests 8166 passed | 56 expected fail | 5 skipped (8227) ``` The randomized notes-app test: 2000 of 2000 seeds pass. `Fuzz.test.ts` with `FUZZ_SEED_START=1 FUZZ_SEEDS=3000` and `FUZZ_SEED_START=50000 FUZZ_SEEDS=2000`: the same 8 seeds fail as on main, every other seed passes. `pnpm run ci-check` passes.
There was a problem hiding this comment.
Thanks for the follow-ups. The fixes are well covered: against main, the new TabLeak.test.ts fails the 12 tab, token/segmented and two-tab cases, and they pass on this branch. Deferring the hashtag, code highlight and token/segmented "after" cases to #9586 as test.fails makes sense.
Please fix: wrapper items of another node's opening write are now owned
opensNodeRun (packages/lexical-y/src/internal/Cursor.ts:274-293) counts every item of the write that opened a node's run, wrapper items included, and minimizeFormatChanges (line 495) stops stepping over them. A node's write lists its wrapper items first (packages/lexical-y/src/internal/Inline.ts:181), so content inserted inside a link right before a hashtag, tab or token that opened that link in the same write gets a w:link item of its own. That item survives the link author's undo and a collaborator's concurrent unlink, so the link stays on the inserted content on every peer, where main removes it (three peers, both client id orders):
B inserts after ab |
A inserts inside the link, before B's first child | Then | main |
this branch |
|---|---|---|---|---|
link(#BB " more") |
text XY |
B undoes | no link | link("XY") |
link(#BB) |
text XY |
C unlinks | no link | link("XY") |
link(tab "zz") |
a tab | C unlinks | no link | link(tab) |
This contradicts the changeset's Verify: line ("A link or format a collaborator removes around a tab at the same time stays removed"). Typing, Tab and paste at a link's start put the content before the link, so it takes a node-level insert such as insertBefore inside the link, but once written it persists. For a typed node (tab, hashtag, code highlight token), wrapper items are the only ones opensNodeRun newly protects, since its t item comes right after them and was already never stepped past, so letting the cursor step over wrapper items keeps the token/segmented fix:
isFormat(r) &&
this.unitGives(r, formats) &&
(keyOf(r).startsWith(WRAPPER_PREFIX) || !opensNodeRun(r))With that change the three cases end without a link, as on main, and the tests in TabLeak.test.ts still pass. Please add the three cases to TabLeak.test.ts, for example:
t.b.update(() => {
const [left] = ($block([0]).getFirstChild() as TextNode).splitText(2);
left.insertAfter(
$createLinkNode('https://example.com').append($createTabNode(), $t('zz')),
);
});
await t.sync();
t.offline();
t.a.update(() => {
const tab = $block([0]).getAllTextNodes().find(n => n.getType() === 'tab')!;
tab.insertBefore($createTabNode());
});
t.c.update(() => {
const link = $block([0]).getChildren().find($isLinkNode)!;
for (const child of link.getChildren()) {
link.insertBefore(child);
}
link.remove();
});
await t.sync();
// every peer: no link left in the lineSmaller suggestions
.changeset/lexical-y-own-segment-run.md:5: hashtags and code highlight tokens written right before a collaborator's node already kept their type onmain(their write opens with thetitem the cursor never stepped past, and their "before" cases pass onmain); only token and segmented text lost their mode there. CONTRIBUTING.md asks for the changelog entry to be written for someone who uses Lexical, so naming token and segmented text only keeps it accurate.- The Description's opening still describes the first commit's rule ("items of its own for every key it has", "a node written before a collaborator's was already safe"). AGENTS.md asks the Description to say what the change makes the code do; folding the two follow-up sections into it keeps that accurate.
- Consecutive Tab presses by one person now store two more format items each: eight presses at the end of a line store 26 live format items against 18 on
main. A client's own undo removes its later write first, so its own run needs no owned type; returningnullfromownSegmentKeyswhen the segment opener is the local client's (opener.id.client === this.tr.doc.clientID) brings that back to 18 while the new tests still pass.
| isFormat(r) && | ||
| this.unitGives(r, formats) && | ||
| !isNodeTypeOpener(r.content as ContentFormat) | ||
| !opensNodeRun(r) |
There was a problem hiding this comment.
This also stops the cursor from stepping over wrapper items (w:*) of another node's opening write. A node's write lists its wrapper items first (Inline.ts:181), so content inserted inside a link right before a hashtag, tab or token that opened the link in the same write gets a w:link item of its own, which survives the link author's undo and a collaborator's concurrent unlink: the link stays on that content on every peer, where main removes it. For a typed node the wrapper items are the only ones this newly protects (its t item comes right after them and was never stepped past), so stepping over them keeps the token/segmented fix:
(keyOf(r).startsWith(WRAPPER_PREFIX) || !opensNodeRun(r))| '@lexical/y': patch | ||
| --- | ||
|
|
||
| [fix] Keep a tab a tab when it was written right next to a collaborator's tab that they undo, and keep a hashtag, code highlight token or token or segmented text written right before a collaborator's node of the same kind what it is when they undo theirs |
There was a problem hiding this comment.
Hashtags and code highlight tokens written right before a collaborator's node already kept their type on main (their write opens with the t item the cursor never stepped past, and their before cases in TabLeak.test.ts pass on main); only token and segmented text lost their mode there. Since this line becomes the CHANGELOG.md entry (CONTRIBUTING.md), naming only token and segmented text keeps it accurate.
| return null; | ||
| } | ||
| const opener = this.valueSource(SEGMENT_KEY); | ||
| if (opener === null) { |
There was a problem hiding this comment.
Nit: this also owns the type inside a run the local client opened, so consecutive Tab presses store two more format items each (eight presses: 26 live format items against 18 on main). A client's own undo removes its later write first, so returning null when opener.id.client === this.tr.doc.clientID keeps the fix for other clients' runs and the old size for one's own.
Description
Text nodes that Lexical keeps apart from an equal neighbour (a tab, text in
token or segmented mode, and every
TextNodesubclass such as a hashtag ora code highlight token) are stored as runs with a segment key, opened and
closed by format items their writer adds. Two cases lost such a node:
inside that node's run and wrote only the items whose values differed (its
segment key), reading its type and formats from the collaborator's opener.
When the collaborator undid their node, the undo deleted that opener and
its closers, and the node was read as something else: a tab as a
"\t"inthe text before it, a hashtag or a code highlight token as plain text.
The writer now gives content that starts a segment other than the one in
effect at the cursor items of its own for every key it has, so it never
depends on another node's run. A node written before a collaborator's was
already safe (the writer never steps past another node's type opener).
"\t"under one tab opener (left by the case above throughundo and redo, in documents written before this fix) were read as one tab
node: one tab was not shown, and the binding's self-check reported the
line (
text "code\t\txx" != "code\txx"). Each"\t"is now read as a tabnode of its own.
Reported downstream (a collaborative notes app's randomized test with undo).
Changeset
.changeset/lexical-y-own-segment-run.md(@lexical/ypatch,[fix]withVerify:).Test plan
New tests in
packages/lexical-y/src/__tests__/unit/TabLeak.test.ts:to an existing one, A adds one right after (or before) B's, B undoes (or
removes) its node, for a tab, token text, segmented text, bold token text
and a hashtag in a paragraph and a tab and a code highlight token in a code
block, with both client id orders. Every peer keeps A's node as written,
no tab character is left outside a tab node, and the peers converge with a
fresh reader.
"\t"written inside atab's run (paragraph and code block) reads as two tab nodes, and both
peers still edit the line and converge.
Before
A randomized three-peer test over a notes app's node set (lists, tables,
code blocks, links, tabs; typing, splits, merges, range edits, undo and
redo, reloads, self-check on), 2000 seeds:
Tests 5 failed | 1995 passed (2000)(seeds 1070, 1288, 1529, 1760, 1870, each a self-check warningtext "code\t\txx" != "code\txx").After
The randomized test:
Tests 2000 passed (2000).Fuzz.test.tswithFUZZ_SEED_START=1 FUZZ_SEEDS=3000andFUZZ_SEED_START=50000 FUZZ_SEEDS=2000: the same 8 seeds fail with andwithout this change (2277, 50180, 50231, 50372, 51439, 51507, 51972, 51976;
the same messages on main), and every other seed passes.
pnpm run ci-checkpasses. Browsers were not exercised (headless and jsdomunit tests only).
Review follow-up (
0f080d9)TabLeak.test.ts: 22 of 103 fail on main, 16 onee30f21, 103 pass on0f080d9.@lexical/y, code, hashtag and link unit suites: 8176 passed, 46 expected fail. The randomized notes-app test: 2000 of 2000.Fuzz.test.tsextended seeds: the same 8 seeds fail as on main.pnpm run ci-checkpasses. Eight Tab presses at the end of a line store 26 format items (18 on main).Verify:now mentions that a stored line where two tabs were shown as one shows both.Second follow-up (
904c01c)Only a tab's type is owned inside another node's run. With every type owned, a code highlight token typed into a code line kept its own type item when a collaborator made the line a paragraph at the same time, and the self-check reported the line (found by a randomized session run; main is clean on that session). A tab is a tab in any line, so owning its type cannot be undone by a line or range change.
Other types and modes follow the run they are written in, as on main: written right after a collaborator's node of the same kind that they undo, a hashtag, code highlight token or token/segmented text still becomes plain text (Bug: [lexical-y] A hashtag, code highlight token or token/segmented text written right after a collaborator's node of the same kind becomes plain text when the collaborator undoes theirs #9586). Those 10 cases stay in
TabLeak.test.tsas expected failures naming Bug: [lexical-y] A hashtag, code highlight token or token/segmented text written right after a collaborator's node of the same kind becomes plain text when the collaborator undoes theirs #9586. Written right before, every kind keeps its type and mode.TabLeak.test.ts: 93 passed, 10 expected fail. Unit suites (y, code, hashtag, link): 8166 passed, 56 expected fail. The notes-app randomized test 2000/2000; 150 sessions of the randomized tab-heavy lane: same flag counts as main.Fuzz.test.tsextended seeds: same 8 seeds as main.pnpm run ci-checkpasses.With [lexical-y] Bug Fix: Keep the run ends of text an undo removes, and end a value an undo ends past other writes' run ends #9561, the token, segmented and bold-token cases of Bug: [lexical-y] A hashtag, code highlight token or token/segmented text written right after a collaborator's node of the same kind becomes plain text when the collaborator undoes theirs #9586 pass ([lexical-y] Bug Fix: Keep the run ends of text an undo removes, and end a value an undo ends past other writes' run ends #9561 keeps their mode and format); flip those
test.failswhen both have landed. The hashtag and code highlight cases still fail with [lexical-y] Bug Fix: Keep the run ends of text an undo removes, and end a value an undo ends past other writes' run ends #9561.