From fdaa9c8088e81bde2618784b42027be44598d11c Mon Sep 17 00:00:00 2001 From: Joe Anderson Date: Wed, 13 Aug 2025 22:28:16 +0100 Subject: [PATCH] Fix handling of `editor.isSelectable` in `Editor.positions` (#5929) * Fix handling of `editor.isSelectable` in `Editor.positions` * Clean-up --- .changeset/eleven-clocks-begin.md | 7 +++++ packages/slate/src/editor/positions.ts | 29 +++++++++++++++---- .../after/non-selectable-block-last.tsx | 17 +++++++++++ .../Editor/after/non-selectable-block.tsx | 18 ++++++++++++ .../after/non-selectable-inline-last.tsx | 20 +++++++++++++ .../before/non-selectable-block-first.tsx | 17 +++++++++++ .../Editor/before/non-selectable-block.tsx | 18 ++++++++++++ .../before/non-selectable-inline-first.tsx | 20 +++++++++++++ 8 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 .changeset/eleven-clocks-begin.md create mode 100644 packages/slate/test/interfaces/Editor/after/non-selectable-block-last.tsx create mode 100644 packages/slate/test/interfaces/Editor/after/non-selectable-block.tsx create mode 100644 packages/slate/test/interfaces/Editor/after/non-selectable-inline-last.tsx create mode 100644 packages/slate/test/interfaces/Editor/before/non-selectable-block-first.tsx create mode 100644 packages/slate/test/interfaces/Editor/before/non-selectable-block.tsx create mode 100644 packages/slate/test/interfaces/Editor/before/non-selectable-inline-first.tsx diff --git a/.changeset/eleven-clocks-begin.md b/.changeset/eleven-clocks-begin.md new file mode 100644 index 000000000..4bffc64ac --- /dev/null +++ b/.changeset/eleven-clocks-begin.md @@ -0,0 +1,7 @@ +--- +'slate': patch +--- + +- Fix error when a non-selectable node has no next or previous node +- Do not return points from `Editor.positions` that are inside non-selectable nodes + - Previously, `editor.isSelectable` was handled incorrectly inside `Editor.positions`. When encountering a non-selectable node, it would immediately return the point before or after it (depending on `reverse`), but it would not skip returning points inside the non-selectable node if more than one point was consumed from `Editor.positions`. diff --git a/packages/slate/src/editor/positions.ts b/packages/slate/src/editor/positions.ts index d2c43dacd..1a8fe577e 100644 --- a/packages/slate/src/editor/positions.ts +++ b/packages/slate/src/editor/positions.ts @@ -51,6 +51,7 @@ export function* positions( let distance = 0 // Distance for leafText to catch up to blockText. let leafTextRemaining = 0 let leafTextOffset = 0 + const skippedPaths: Path[] = [] // Iterate through all nodes in range, grabbing entire textual content // of block nodes in blockText, and text nodes in leafText. @@ -63,19 +64,35 @@ export function* positions( reverse, voids, })) { + // If the node is inside a skipped ancestor, do not return any points, but + // still process its content so that the iteration state remains correct. + const hasSkippedAncestor = skippedPaths.some(p => Path.isAncestor(p, path)) + + function* maybeYield(point: Point) { + if (!hasSkippedAncestor) { + yield point + } + } + /* * ELEMENT NODE - Yield position(s) for voids, collect blockText for blocks */ if (Element.isElement(node)) { if (!editor.isSelectable(node)) { /** - * If the node is not selectable, skip it + * If the node is not selectable, skip it and its descendants */ + skippedPaths.push(path) if (reverse) { - yield Editor.end(editor, Path.previous(path)) + if (Path.hasPrevious(path)) { + yield* maybeYield(Editor.end(editor, Path.previous(path))) + } continue } else { - yield Editor.start(editor, Path.next(path)) + const nextPath = Path.next(path) + if (Editor.hasPath(editor, nextPath)) { + yield* maybeYield(Editor.start(editor, nextPath)) + } continue } } @@ -84,7 +101,7 @@ export function* positions( // yield their first point. If the `voids` option is set to true, // then we will iterate over their content. if (!voids && (editor.isVoid(node) || editor.isElementReadOnly(node))) { - yield Editor.start(editor, path) + yield* maybeYield(Editor.start(editor, path)) continue } @@ -143,7 +160,7 @@ export function* positions( // Yield position at the start of node (potentially). if (isFirst || isNewBlock || unit === 'offset') { - yield { path, offset: leafTextOffset } + yield* maybeYield({ path, offset: leafTextOffset }) isNewBlock = false } @@ -178,7 +195,7 @@ export function* positions( // to catch up with `blockText`, so we can reset `distance` // and yield this position in this node. distance = 0 - yield { path, offset: leafTextOffset } + yield* maybeYield({ path, offset: leafTextOffset }) } } } diff --git a/packages/slate/test/interfaces/Editor/after/non-selectable-block-last.tsx b/packages/slate/test/interfaces/Editor/after/non-selectable-block-last.tsx new file mode 100644 index 000000000..0872aa92b --- /dev/null +++ b/packages/slate/test/interfaces/Editor/after/non-selectable-block-last.tsx @@ -0,0 +1,17 @@ +/** @jsx jsx */ + +import { Editor } from 'slate' +import { jsx } from '../../..' + +export const input = ( + + one + two + +) + +export const test = editor => { + return Editor.after(editor, { path: [0, 0], offset: 3 }) +} + +export const output = undefined diff --git a/packages/slate/test/interfaces/Editor/after/non-selectable-block.tsx b/packages/slate/test/interfaces/Editor/after/non-selectable-block.tsx new file mode 100644 index 000000000..710264b03 --- /dev/null +++ b/packages/slate/test/interfaces/Editor/after/non-selectable-block.tsx @@ -0,0 +1,18 @@ +/** @jsx jsx */ + +import { Editor } from 'slate' +import { jsx } from '../../..' + +export const input = ( + + one + two + three + +) + +export const test = editor => { + return Editor.after(editor, { path: [0, 0], offset: 3 }) +} + +export const output = { path: [2, 0], offset: 0 } diff --git a/packages/slate/test/interfaces/Editor/after/non-selectable-inline-last.tsx b/packages/slate/test/interfaces/Editor/after/non-selectable-inline-last.tsx new file mode 100644 index 000000000..7067bd6ec --- /dev/null +++ b/packages/slate/test/interfaces/Editor/after/non-selectable-inline-last.tsx @@ -0,0 +1,20 @@ +/** @jsx jsx */ + +import { Editor } from 'slate' +import { jsx } from '../../..' + +// This is invalid due to the lack of a text node after the inline, but this +// case can arise prior to normalization so it needs to be handled anyway. +export const input = ( + + + onetwo + + +) + +export const test = editor => { + return Editor.after(editor, { path: [0, 0], offset: 3 }) +} + +export const output = undefined diff --git a/packages/slate/test/interfaces/Editor/before/non-selectable-block-first.tsx b/packages/slate/test/interfaces/Editor/before/non-selectable-block-first.tsx new file mode 100644 index 000000000..561e3f6dc --- /dev/null +++ b/packages/slate/test/interfaces/Editor/before/non-selectable-block-first.tsx @@ -0,0 +1,17 @@ +/** @jsx jsx */ + +import { Editor } from 'slate' +import { jsx } from '../../..' + +export const input = ( + + two + three + +) + +export const test = editor => { + return Editor.before(editor, { path: [1, 0], offset: 0 }) +} + +export const output = undefined diff --git a/packages/slate/test/interfaces/Editor/before/non-selectable-block.tsx b/packages/slate/test/interfaces/Editor/before/non-selectable-block.tsx new file mode 100644 index 000000000..112bb24dc --- /dev/null +++ b/packages/slate/test/interfaces/Editor/before/non-selectable-block.tsx @@ -0,0 +1,18 @@ +/** @jsx jsx */ + +import { Editor } from 'slate' +import { jsx } from '../../..' + +export const input = ( + + one + two + three + +) + +export const test = editor => { + return Editor.before(editor, { path: [2, 0], offset: 0 }) +} + +export const output = { path: [0, 0], offset: 3 } diff --git a/packages/slate/test/interfaces/Editor/before/non-selectable-inline-first.tsx b/packages/slate/test/interfaces/Editor/before/non-selectable-inline-first.tsx new file mode 100644 index 000000000..71394ac18 --- /dev/null +++ b/packages/slate/test/interfaces/Editor/before/non-selectable-inline-first.tsx @@ -0,0 +1,20 @@ +/** @jsx jsx */ + +import { Editor } from 'slate' +import { jsx } from '../../..' + +// This is invalid due to the lack of a text node before the inline, but this +// case can arise prior to normalization so it needs to be handled anyway. +export const input = ( + + + twothree + + +) + +export const test = editor => { + return Editor.before(editor, { path: [0, 1], offset: 0 }) +} + +export const output = undefined