Skip to content

Commit 093adf1

Browse files
christianhgchristianhg
committed
fix: skip leafless subtrees when normalizing DOM points
`getEditableChildAndIndex` treated any element with children as content, so a leafless subtree (a table's <colgroup> being the canonical case) became a dead-end: `normalizeDOMPoint` descended into it, the resulting point on a <col> resolved to nothing in `toSelectionPoint`, `suppressThrow` swallowed the failure, and the entire selection sync silently skipped, leaving the model stale while the screen showed e.g. a whole-table selection after a triple-click. The child-seeking loop now also skips elements whose subtree holds no editable content (no text and no rendered block/inline object), backtracking to a sibling exactly as it already does for `contenteditable="false"` elements. Zero-width leaves keep matching via their text content, and void objects keep matching via their rendered attributes, so void-adjacent positions are unaffected. Pinned by two tests on a container table rendered with a plain <colgroup>: an element-level range spanning the table maps to first-cell start -> last-cell end, and a collapsed element-level point before the colgroup maps into the first cell (both fail without the fix: one maps wrong, one maps to null).
1 parent 9a98074 commit 093adf1

3 files changed

Lines changed: 208 additions & 3 deletions

File tree

‎.changeset/normalize-dom-point-leafless-subtrees.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@portabletext/editor': patch
3+
---
4+
5+
fix: selection mapping no longer dead-ends in leafless subtrees
6+
7+
Selecting across a table rendered with a `<colgroup>` (for example via triple-click, or clicking near the table's edges) could silently fail to update the editor's selection, leaving it stale or collapsed while the screen showed a larger selection. DOM positions that normalize into an element holding no editable content now skip to the nearest sibling with content instead, so element-level selections map to what they visually cover.

‎packages/editor/src/engine/dom/utils/dom.ts‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,12 +160,16 @@ const getEditableChildAndIndex = (
160160
let triedForward = false
161161
let triedBackward = false
162162

163-
// While the child is a comment node, or an element node with no children,
164-
// keep iterating to find a sibling non-void, non-comment node.
163+
// While the child is a comment node, an element node with no children, or
164+
// an element whose subtree holds no editable content (a leafless subtree
165+
// like a table's <colgroup> would otherwise dead-end the descent), keep
166+
// iterating to find a sibling with editable content.
165167
while (
166168
isDOMComment(child) ||
167169
(isDOMElement(child) && child.childNodes.length === 0) ||
168-
(isDOMElement(child) && child.getAttribute('contenteditable') === 'false')
170+
(isDOMElement(child) &&
171+
child.getAttribute('contenteditable') === 'false') ||
172+
(isDOMElement(child) && !hasEditableContent(child))
169173
) {
170174
if (triedForward && triedBackward) {
171175
break
@@ -207,6 +211,21 @@ const getEditableChild = (
207211
return child
208212
}
209213

214+
/**
215+
* Whether an element's subtree holds anything a DOM point can resolve to:
216+
* text, or a rendered block/inline object.
217+
*/
218+
const hasEditableContent = (element: DOMElement): boolean => {
219+
if (element.textContent !== '') {
220+
return true
221+
}
222+
return (
223+
element.querySelector(
224+
'[data-pt-block="object"], [data-pt-inline="object"]',
225+
) !== null
226+
)
227+
}
228+
210229
/**
211230
* Get the dom selection from Shadow Root if possible, otherwise from the document
212231
*/

‎packages/editor/tests/selection-sync-leafless-subtree.test.tsx‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
import {defineSchema} from '@portabletext/schema'
2+
import {createTestKeyGenerator} from '@portabletext/test'
3+
import {describe, expect, test, vi} from 'vitest'
4+
import {userEvent} from 'vitest/browser'
5+
import {NodePlugin} from '../src/plugins/plugin.node'
6+
import {defineContainer} from '../src/renderers/renderer.types'
7+
import {createTestEditor} from '../src/test/vitest'
8+
9+
const schemaDefinition = defineSchema({
10+
blockObjects: [
11+
{
12+
name: 'table',
13+
fields: [
14+
{
15+
name: 'rows',
16+
type: 'array',
17+
of: [
18+
{
19+
type: 'object',
20+
name: 'row',
21+
fields: [
22+
{
23+
name: 'cells',
24+
type: 'array',
25+
of: [
26+
{
27+
type: 'object',
28+
name: 'cell',
29+
fields: [
30+
{name: 'value', type: 'array', of: [{type: 'block'}]},
31+
],
32+
},
33+
],
34+
},
35+
],
36+
},
37+
],
38+
},
39+
],
40+
},
41+
],
42+
})
43+
44+
// The <colgroup> renders no text and no leaves: a DOM point normalized into
45+
// it can resolve to nothing.
46+
const tableContainer = defineContainer({
47+
type: 'table',
48+
arrayField: 'rows',
49+
render: ({attributes, children}) => (
50+
<table data-testid="table" {...attributes}>
51+
<colgroup>
52+
<col />
53+
<col />
54+
</colgroup>
55+
<tbody>{children}</tbody>
56+
</table>
57+
),
58+
of: [
59+
defineContainer({
60+
type: 'row',
61+
arrayField: 'cells',
62+
render: ({attributes, children}) => <tr {...attributes}>{children}</tr>,
63+
of: [
64+
defineContainer({
65+
type: 'cell',
66+
arrayField: 'value',
67+
render: ({attributes, children}) => (
68+
<td {...attributes}>{children}</td>
69+
),
70+
}),
71+
],
72+
}),
73+
],
74+
})
75+
76+
const initialValue = [
77+
{
78+
_type: 'table',
79+
_key: 't0',
80+
rows: [
81+
{
82+
_type: 'row',
83+
_key: 'r0',
84+
cells: [
85+
{
86+
_type: 'cell',
87+
_key: 'c0',
88+
value: [
89+
{
90+
_type: 'block',
91+
_key: 'b0',
92+
style: 'normal',
93+
markDefs: [],
94+
children: [{_type: 'span', _key: 's0', text: 'A', marks: []}],
95+
},
96+
],
97+
},
98+
{
99+
_type: 'cell',
100+
_key: 'c1',
101+
value: [
102+
{
103+
_type: 'block',
104+
_key: 'b1',
105+
style: 'normal',
106+
markDefs: [],
107+
children: [{_type: 'span', _key: 's1', text: 'B', marks: []}],
108+
},
109+
],
110+
},
111+
],
112+
},
113+
],
114+
},
115+
]
116+
117+
function cellSpanPath(cellKey: string, blockKey: string, spanKey: string) {
118+
return [
119+
{_key: 't0'},
120+
'rows',
121+
{_key: 'r0'},
122+
'cells',
123+
{_key: cellKey},
124+
'value',
125+
{_key: blockKey},
126+
'children',
127+
{_key: spanKey},
128+
]
129+
}
130+
131+
describe('selection sync across leafless subtrees', () => {
132+
test('an element-level range over the table maps to a full-table selection', async () => {
133+
const {editor, locator} = await createTestEditor({
134+
keyGenerator: createTestKeyGenerator(),
135+
schemaDefinition,
136+
initialValue,
137+
children: <NodePlugin nodes={[tableContainer]} />,
138+
})
139+
await userEvent.click(locator)
140+
141+
// The range a triple-click produces: element-level endpoints spanning the
142+
// whole <table>, offset 0 sits before the <colgroup>.
143+
const table = locator.element().querySelector('[data-testid="table"]')
144+
expect(table).not.toBeNull()
145+
window
146+
.getSelection()
147+
?.setBaseAndExtent(table!, 0, table!, table!.childNodes.length)
148+
149+
await vi.waitFor(() => {
150+
expect(editor.getSnapshot().context.selection).toEqual({
151+
anchor: {path: cellSpanPath('c0', 'b0', 's0'), offset: 0},
152+
focus: {path: cellSpanPath('c1', 'b1', 's1'), offset: 1},
153+
backward: false,
154+
})
155+
})
156+
})
157+
158+
test('a collapsed element-level point before the colgroup maps to the first cell', async () => {
159+
const {editor, locator} = await createTestEditor({
160+
keyGenerator: createTestKeyGenerator(),
161+
schemaDefinition,
162+
initialValue,
163+
children: <NodePlugin nodes={[tableContainer]} />,
164+
})
165+
await userEvent.click(locator)
166+
167+
const table = locator.element().querySelector('[data-testid="table"]')
168+
expect(table).not.toBeNull()
169+
window.getSelection()?.setBaseAndExtent(table!, 0, table!, 0)
170+
171+
await vi.waitFor(() => {
172+
expect(editor.getSnapshot().context.selection).toEqual({
173+
anchor: {path: cellSpanPath('c0', 'b0', 's0'), offset: 0},
174+
focus: {path: cellSpanPath('c0', 'b0', 's0'), offset: 0},
175+
backward: false,
176+
})
177+
})
178+
})
179+
})

0 commit comments

Comments
 (0)