setNodeSelection(non-leaf first node in doc with text) can create a NodeSelection for text

[kivikakk]

What’s the bug you are facing?

If you try to setNodeSelection() on the very first node in the document, and that node is non-leaf, if there's a text selection able to be made within that node, Tiptap will end up creating a NodeSelection for the first text node within that.

The text NodeSelection doesn't behave well, and causes an error to be thrown in its deselect when trying to select anything else.

Which browser was this experienced in? Are any special extensions installed?

Chrome 104.0.5112.79 (Official Build) (arm64), unlikely to be browser-specific.

How can we reproduce the bug on our side?

See this CodeSandbox which reproduces this in onCreate: https://codesandbox.io/s/tiptap-setnodeselection-repro-mtht1t?file=/src/App.js

This is a contrived example using paragraphs for simplicity, but it happens with more complex custom node types which are both selectable themselves and contain editable content.

Can you provide a CodeSandbox?

No response

What did you expect to happen?

The first node should be selected (if possible) OR no selection made at all. An invalid selection shouldn't be made, at any rate.

Anything to add? (optional)

This happens because setNodeSelection clamps the position parameter to selectable ranges in the document:

tiptap/packages/core/src/commands/setNodeSelection.ts

Lines 20 to 23 in 57a9e86

	const minPos = Selection.atStart(doc).from
	const maxPos = Selection.atEnd(doc).to
	const resolvedPos = minMax(position, minPos, maxPos)
	const selection = NodeSelection.create(doc, resolvedPos)

This wasn't always the case; the change to clamp the parameter this way happened in 9425e72. Before that commit, it was just to the bounds of the whole document:

tiptap/packages/core/src/commands/setNodeSelection.ts

Lines 19 to 20 in f57466c

	const from = minMax(position, 0, doc.content.size)
	const selection = NodeSelection.create(doc, from)

9425e72 made the same change (using Selection.atStart(doc).from and Selection.atEnd(doc).to) to both setTextSelection and setNodeSelection, but I'm inclined to believe that only the setTextSelection change was necessary to address #1588, and that furthermore the setNodeSelection change was incorrect.

Selection.atStart proceeds to find the first cursor position within the node (because it's not an atom) and returns a TextSelection at position 1, which in turns becomes minPos:

  /// Find the cursor or leaf node selection closest to the start of
  /// the given document. Will return an
  /// [`AllSelection`](#state.AllSelection) if no valid position
  /// exists.
  static atStart(doc: Node): Selection {
    return findSelectionIn(doc, doc, 0, 0, 1) || new AllSelection(doc)
  }

I think we should just revert setNodeSelection to bounding within [0, doc.content.size] again.

Did you update your dependencies?

• Yes, I’ve updated my dependencies to use the latest version of all packages.

Are you sponsoring us?

• Yes, I’m a sponsor. 💖