mirror of
https://github.com/penpot/penpot.git
synced 2026-09-11 14:39:35 +00:00
🐛 Fix TypeError when deleting text at edge spans in text editor
The text editor's SelectionController threw 'TypeError: Invalid text node' when: - Pressing Backspace to delete the only character of the **first** text span in a paragraph that contains multiple spans. - Pressing Delete to delete the only character of the **last** text span in the same situation. - Pressing a word-backward shortcut that empties the first span of a multi-span paragraph. In all three cases the tree-iterator (previousNode / nextNode) returned null because no sibling text node existed in that direction, and that null was subsequently passed to getTextNodeLength() which calls isTextNode() — which unconditionally throws when given a falsy value. Fix: use the null-coalescing fallback to the first/last remaining child's text node of the paragraph before calling collapse() / getTextNodeLength().
This commit is contained in:
parent
92de9ed258
commit
5a2c09f246
@ -1238,7 +1238,11 @@ export class SelectionController extends EventTarget {
|
|||||||
textSpan.childNodes.length === 0
|
textSpan.childNodes.length === 0
|
||||||
) {
|
) {
|
||||||
textSpan.remove();
|
textSpan.remove();
|
||||||
return this.collapse(nextTextNode, 0);
|
// nextTextNode can be null when deleting the last text node in the last
|
||||||
|
// span of the paragraph; fall back to the last text node of the
|
||||||
|
// preceding sibling span so the cursor stays within the paragraph.
|
||||||
|
const forwardTarget = nextTextNode ?? paragraph.lastChild?.lastChild;
|
||||||
|
return this.collapse(forwardTarget, 0);
|
||||||
}
|
}
|
||||||
return this.collapse(this.focusNode, this.focusOffset);
|
return this.collapse(this.focusNode, this.focusOffset);
|
||||||
}
|
}
|
||||||
@ -1284,9 +1288,14 @@ export class SelectionController extends EventTarget {
|
|||||||
textSpan.childNodes.length === 0
|
textSpan.childNodes.length === 0
|
||||||
) {
|
) {
|
||||||
textSpan.remove();
|
textSpan.remove();
|
||||||
|
// previousTextNode can be null when deleting the first text node in
|
||||||
|
// the paragraph (no preceding sibling text node exists). Fall back
|
||||||
|
// to the first text node of the now-first remaining span so the
|
||||||
|
// cursor stays within the paragraph.
|
||||||
|
const backwardTarget = previousTextNode ?? paragraph.firstChild?.firstChild;
|
||||||
return this.collapse(
|
return this.collapse(
|
||||||
previousTextNode,
|
backwardTarget,
|
||||||
getTextNodeLength(previousTextNode),
|
getTextNodeLength(backwardTarget),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -1358,9 +1367,13 @@ export class SelectionController extends EventTarget {
|
|||||||
textSpan.childNodes.length === 0
|
textSpan.childNodes.length === 0
|
||||||
) {
|
) {
|
||||||
textSpan.remove();
|
textSpan.remove();
|
||||||
|
// previousTextNode can be null when the deleted node was the first
|
||||||
|
// in the paragraph. Fall back to the first text node of the
|
||||||
|
// now-first remaining span.
|
||||||
|
const backwardTarget = previousTextNode ?? paragraph.firstChild?.firstChild;
|
||||||
return this.collapse(
|
return this.collapse(
|
||||||
previousTextNode,
|
backwardTarget,
|
||||||
getTextNodeLength(previousTextNode),
|
getTextNodeLength(backwardTarget),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -581,6 +581,27 @@ describe("SelectionController", () => {
|
|||||||
expect(textEditorMock.root.textContent).toBe("");
|
expect(textEditorMock.root.textContent).toBe("");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("`removeBackwardText` should not throw when deleting the first character of the first span in a multi-span paragraph", () => {
|
||||||
|
// Regression test: previousNode() returns null when the cursor is at the
|
||||||
|
// very first text node; passing null to getTextNodeLength used to throw
|
||||||
|
// "TypeError: Invalid text node".
|
||||||
|
const textEditorMock = TextEditorMock.createTextEditorMockWith([
|
||||||
|
["A", "B"],
|
||||||
|
]);
|
||||||
|
const root = textEditorMock.root;
|
||||||
|
const selection = document.getSelection();
|
||||||
|
const selectionController = new SelectionController(
|
||||||
|
textEditorMock,
|
||||||
|
selection,
|
||||||
|
);
|
||||||
|
// Focus at offset 1 of the first span's text node ("A"), then delete backward
|
||||||
|
const firstTextNode = root.firstChild.firstChild.firstChild;
|
||||||
|
focus(selection, textEditorMock, firstTextNode, 1);
|
||||||
|
expect(() => selectionController.removeBackwardText()).not.toThrow();
|
||||||
|
// "A" is removed; the paragraph should keep the second span with "B"
|
||||||
|
expect(textEditorMock.root.textContent).toBe("B");
|
||||||
|
});
|
||||||
|
|
||||||
test("`insertParagraph` should insert a new paragraph in an empty editor", () => {
|
test("`insertParagraph` should insert a new paragraph in an empty editor", () => {
|
||||||
const textEditorMock = TextEditorMock.createTextEditorMockEmpty();
|
const textEditorMock = TextEditorMock.createTextEditorMockEmpty();
|
||||||
const root = textEditorMock.root;
|
const root = textEditorMock.root;
|
||||||
@ -875,6 +896,26 @@ describe("SelectionController", () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("`removeForwardText` should not throw when deleting the last character of the last span in a multi-span paragraph", () => {
|
||||||
|
// Regression test: nextNode() returns null when the cursor is at the
|
||||||
|
// very last text node; passing null to collapse used to crash.
|
||||||
|
const textEditorMock = TextEditorMock.createTextEditorMockWith([
|
||||||
|
["A", "B"],
|
||||||
|
]);
|
||||||
|
const root = textEditorMock.root;
|
||||||
|
const selection = document.getSelection();
|
||||||
|
const selectionController = new SelectionController(
|
||||||
|
textEditorMock,
|
||||||
|
selection,
|
||||||
|
);
|
||||||
|
// Focus at offset 0 of the second span's text node ("B"), then delete forward
|
||||||
|
const secondTextNode = root.firstChild.lastChild.firstChild;
|
||||||
|
focus(selection, textEditorMock, secondTextNode, 0);
|
||||||
|
expect(() => selectionController.removeForwardText()).not.toThrow();
|
||||||
|
// "B" is removed; the paragraph should keep the first span with "A"
|
||||||
|
expect(textEditorMock.root.textContent).toBe("A");
|
||||||
|
});
|
||||||
|
|
||||||
test("`replaceText` should replace the selected text", () => {
|
test("`replaceText` should replace the selected text", () => {
|
||||||
const textEditorMock =
|
const textEditorMock =
|
||||||
TextEditorMock.createTextEditorMockWithText("Hello, World!");
|
TextEditorMock.createTextEditorMockWithText("Hello, World!");
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user