From 1ed32e3db7e35da706143233d3b262b3dbb737cb Mon Sep 17 00:00:00 2001 From: Neil Jenkins Date: Mon, 9 Oct 2023 11:52:58 +1100 Subject: [PATCH] Squire: Fix crash removing formatting The fixer node may get removed when merging adjacent inlines, so move the cleanup to before this. For safety, also properly check for non-null nodes so we don't crash if it's still elided for some reason. --- source/Editor.ts | 28 +++++++++++++++------------- source/node/Whitespace.ts | 2 +- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/source/Editor.ts b/source/Editor.ts index 0b3fa64..94b5859 100644 --- a/source/Editor.ts +++ b/source/Editor.ts @@ -1524,7 +1524,7 @@ class Squire { // We need a node in the selection to break the surrounding // formatted text. - let fixer: Node | Text | undefined; + let fixer: Node | Text | null | undefined; if (range.collapsed) { if (cantFocusEmptyTextNodes) { fixer = document.createTextNode(ZWS); @@ -1615,6 +1615,20 @@ class Squire { replaceWith(el, empty(el)); }); + if (cantFocusEmptyTextNodes && fixer) { + // Clean up any previous ZWS in this block. They are not needed, + // and this works around a Chrome bug where it doesn't render the + // text in some situations with multiple ZWS(!) + fixer = fixer.parentNode; + let block = fixer; + while (block && isInline(block)) { + block = block.parentNode; + } + if (block) { + removeZWS(block, fixer); + } + } + // Merge adjacent inlines: this._getRangeAndRemoveBookmark(range); if (fixer) { @@ -1622,18 +1636,6 @@ class Squire { } mergeInlines(root, range); - if (cantFocusEmptyTextNodes && fixer) { - // Clean up any previous ZWS in this block. They are not needed, - // and this works around a Chrome bug where it doesn't render the - // text in some situations with multiple ZWS(!) - fixer = fixer.parentNode!; - let block = fixer; - while (isInline(block)) { - block = block.parentNode!; - } - removeZWS(block, fixer); - } - return range; } diff --git a/source/node/Whitespace.ts b/source/node/Whitespace.ts index 38e4ea7..b46326f 100644 --- a/source/node/Whitespace.ts +++ b/source/node/Whitespace.ts @@ -34,7 +34,7 @@ const isLineBreak = (br: Element, isLBIfEmptyBlock: boolean): boolean => { // contained ZWS space then remove it too. We may want to keep one ZWS node at // the bottom of the tree so the block can be selected. Define that node as the // keepNode. -const removeZWS = (root: Node, keepNode?: Node): void => { +const removeZWS = (root: Node, keepNode?: Node | null): void => { const walker = new TreeIterator(root, SHOW_TEXT); let textNode: Text | null; let index: number;