Skip to content

fix(collaborative-prosemirror): never throw restoring the selection on a remote update - #1048

Open
mmamedel wants to merge 1 commit into
streamich:masterfrom
mmamedel:fix/prosemirror-facade-selection-restore
Open

fix(collaborative-prosemirror): never throw restoring the selection on a remote update#1048
mmamedel wants to merge 1 commit into
streamich:masterfrom
mmamedel:fix/prosemirror-facade-selection-restore

Conversation

@mmamedel

Copy link
Copy Markdown

Problem

ProseMirrorFacade.set() maps the previous selection through the applied transaction and reinstates it with TextSelection.create():

const newAnchor = tr.mapping.map(selection.anchor);
const newHead = tr.mapping.map(selection.head);
tr.setSelection(TextSelection.create(tr.doc, newAnchor, newHead));

When a remote update changes the document such that the mapped position no longer points into inline content — e.g. it removes the trailing blocks that held the local caretTextSelection.create() throws:

TextSelection endpoint not pointing into a node with inline content (doc)

The consequence is worse than a caret glitch: set() also runs inside PeritextBinding.bind()'s initial syncFromModel(), so the exception aborts the entire binding. The editor stays permanently unwired, and every subsequent local edit is applied to ProseMirror only and silently lost — nothing throws afterwards, which makes it very hard to diagnose.

We hit this in production in a collaborative screenplay editor built on these packages: a client resuming a session (server snapshot replacing the body while the caret sat in later content) lost all subsequent typing on reload.

Fix

Use TextSelection.between(), ProseMirror's non-throwing constructor, which clamps to the nearest valid text selection. The same hazard exists in setSelection() (a CRDT-space selection can resolve around block boundaries mid-sync) — guarded identically.

Tests

New spec ProseMirrorFacade.selection-restore.spec.ts:

  • reproduces the exact failure (caret in a trailing paragraph + a remote set() that removes it) — fails with the quoted error before this change, passes after;
  • pins that in-range mapped selections are still restored exactly (no behavior change for the normal case).

All 13 existing collaborative-prosemirror suites pass unchanged (719 tests).

…n a remote update

ProseMirrorFacade.set() maps the previous selection through the applied
transaction and reinstates it with TextSelection.create(), which throws
("TextSelection endpoint not pointing into a node with inline content") when
the mapped position no longer points into inline content — e.g. when a remote
update removes the trailing blocks that held the local caret.

Because set() also runs inside PeritextBinding.bind()'s initial syncFromModel(),
that exception aborts the entire binding: the editor stays permanently unwired
and every subsequent local edit is silently lost (nothing throws afterwards).

Use TextSelection.between(), which clamps to the nearest valid text selection
instead of throwing. The same hazard exists in setSelection() (remote
presence/CRDT-space selections around block boundaries) — guarded identically.

The new spec reproduces the exact production failure: a caret in a trailing
paragraph + a remote set() that removes it fails with the quoted error before
this change and passes after; a second case pins that in-range selections are
still restored exactly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant