TINY-10840: Fixed errors thrown by ForceBlocks and element selections

.changes/unreleased/tinymce-TINY-10840-2024-05-07.yaml

@@ -0,0 +1,6 @@
+project: tinymce
+kind: Fixed
+body: Deleting in a `div` with preceeding `br` elements would sometimes throw errors.
+time: 2024-05-07T11:18:49.114387+02:00
+custom:
+  Issue: TINY-10840

modules/tinymce/src/core/main/ts/ForceBlocks.ts

@@ -1,15 +1,15 @@
-import { Arr, Fun, Obj } from '@ephox/katamari';
+import { Arr, Obj } from '@ephox/katamari';
 import { Insert, SugarElement } from '@ephox/sugar';
 
 import Editor from './api/Editor';
 import Schema, { SchemaMap } from './api/html/Schema';
 import * as Options from './api/Options';
 import * as Bookmarks from './bookmark/Bookmarks';
+import * as StructureBookmark from './bookmark/StructureBookmark';
 import * as TransparentElements from './content/TransparentElements';
 import * as NodeType from './dom/NodeType';
 import * as PaddingBr from './dom/PaddingBr';
 import * as Parents from './dom/Parents';
-import * as EditorFocus from './focus/EditorFocus';
 import * as Namespace from './html/Namespace';
 
 /**
@@ -62,7 +62,7 @@ const addRootBlocks = (editor: Editor) => {
   const rootNode = editor.getBody();
   let rootBlockNode: Node | undefined | null;
   let tempNode: Node;
-  let wrapped = false;
+  let bm: StructureBookmark.StructureBookmark | null = null;
 
   const forcedRootBlock = Options.getForcedRootBlock(editor);
   if (!startNode || !NodeType.isElement(startNode)) {
@@ -74,10 +74,17 @@ const addRootBlocks = (editor: Editor) => {
     return;
   }
 
-  // Get current selection
-  const rng = selection.getRng();
-  const { startContainer, startOffset, endContainer, endOffset } = rng;
-  const restoreSelection = EditorFocus.hasFocus(editor);
+  // Firefox will automatically remove the last BR if you insert nodes next to it and add a BR back if you remove those siblings
+  // and since the bookmark code inserts temporary nodes an new BR will be constantly removed and added and triggering a selection
+  // change causing an infinite recursion. So we treat this special case on it's own.
+  if (rootNode.firstChild === rootNode.lastChild && NodeType.isBr(rootNode.firstChild)) {
+    rootBlockNode = createRootBlock(editor);
+    rootBlockNode.appendChild(PaddingBr.createPaddingBr().dom);
+    rootNode.replaceChild(rootBlockNode, rootNode.firstChild);
+    editor.selection.setCursorLocation(rootBlockNode, 0);
+    editor.nodeChanged();
+    return;
+  }
 
   // Wrap non block elements and text nodes
   let node = rootNode.firstChild;
@@ -96,9 +103,19 @@ const addRootBlocks = (editor: Editor) => {
       }
 
       if (!rootBlockNode) {
+        if (!bm && editor.hasFocus()) {
+          bm = StructureBookmark.getBookmark(editor.selection.getRng(), () => document.createElement('span'));
+        }
+
+        // Firefox will remove the last BR element if you insert nodes next to it using DOM APIs like insertBefore
+        // so for that weird edge case we stop processing.
+        if (!node.parentNode) {
+          node = null;
+          break;
+        }
+
         rootBlockNode = createRootBlock(editor);
         rootNode.insertBefore(rootBlockNode, node);
-        wrapped = true;
       }
 
       tempNode = node;
@@ -110,10 +127,8 @@ const addRootBlocks = (editor: Editor) => {
     }
   }
 
-  if (wrapped && restoreSelection) {
-    rng.setStart(startContainer, startOffset);
-    rng.setEnd(endContainer, endOffset);
-    selection.setRng(rng);
+  if (bm) {
+    editor.selection.setRng(StructureBookmark.resolveBookmark(bm));
     editor.nodeChanged();
   }
 };
@@ -132,10 +147,11 @@ const insertEmptyLine = (editor: Editor, root: SugarElement<HTMLElement>, insert
 };
 
 const setup = (editor: Editor): void => {
-  editor.on('NodeChange', Fun.curry(addRootBlocks, editor));
+  editor.on('NodeChange', () => addRootBlocks(editor));
 };
 
 export {
   insertEmptyLine,
   setup
 };
+

modules/tinymce/src/core/main/ts/bookmark/StructureBookmark.ts

@@ -0,0 +1,77 @@
+import * as NodeType from '../dom/NodeType';
+
+export interface ElementBookmark {
+  marker: Node;
+  before: boolean;
+}
+
+export interface TextBookmark {
+  text: Text;
+  offset: number;
+}
+
+type BookmarkEndpoint = ElementBookmark | TextBookmark;
+
+export interface StructureBookmark {
+  start: BookmarkEndpoint;
+  end: BookmarkEndpoint;
+}
+
+const isTextEndpoint = (endpoint: BookmarkEndpoint): endpoint is TextBookmark => endpoint.hasOwnProperty('text');
+const isElementEndpoint = (endpoint: BookmarkEndpoint): endpoint is ElementBookmark => endpoint.hasOwnProperty('marker');
+
+export const getBookmark = (range: Range, createMarker: () => Node): StructureBookmark => {
+  const getEndpoint = (container: Node, offset: number): ElementBookmark | TextBookmark => {
+    if (NodeType.isText(container)) {
+      return { text: container, offset };
+    } else {
+      const marker = createMarker();
+      const children = container.childNodes;
+      if (offset < children.length) {
+        container.insertBefore(marker, children[offset]);
+        return { marker, before: true };
+      } else {
+        container.appendChild(marker);
+        return { marker, before: false };
+      }
+    }
+  };
+
+  const end = getEndpoint(range.endContainer, range.endOffset);
+  const start = getEndpoint(range.startContainer, range.startOffset);
+
+  return { start, end };
+};
+
+export const resolveBookmark = (bm: StructureBookmark): Range => {
+  const { start, end } = bm;
+  const rng = new window.Range();
+
+  if (isTextEndpoint(start)) {
+    rng.setStart(start.text, start.offset);
+  } else {
+    if (isElementEndpoint(start)) {
+      if (start.before) {
+        rng.setStartBefore(start.marker);
+      } else {
+        rng.setStartAfter(start.marker);
+      }
+      start.marker.parentNode?.removeChild(start.marker);
+    }
+  }
+
+  if (isTextEndpoint(end)) {
+    rng.setEnd(end.text, end.offset);
+  } else {
+    if (isElementEndpoint(end)) {
+      if (end.before) {
+        rng.setEndBefore(end.marker);
+      } else {
+        rng.setEndAfter(end.marker);
+      }
+      end.marker.parentNode?.removeChild(end.marker);
+    }
+  }
+
+  return rng;
+};

modules/tinymce/src/core/main/ts/delete/DeleteCommands.ts

@@ -6,6 +6,7 @@ import * as BlockRangeDelete from './BlockRangeDelete';
 import * as CaretBoundaryDelete from './CaretBoundaryDelete';
 import * as CefDelete from './CefDelete';
 import * as DeleteUtils from './DeleteUtils';
+import * as DivDelete from './DivDelete';
 import * as ImageBlockDelete from './ImageBlockDelete';
 import * as InlineBoundaryDelete from './InlineBoundaryDelete';
 import * as InlineFormatDelete from './InlineFormatDelete';
@@ -25,6 +26,7 @@ const findAction = (editor: Editor, caret: Cell<Text | null>, forward: boolean)
     MediaDelete.backspaceDelete,
     BlockRangeDelete.backspaceDelete,
     InlineFormatDelete.backspaceDelete,
+    DivDelete.backspaceDelete
   ], (item) => item(editor, forward))
     .filter((_) => editor.selection.isEditable());
 

modules/tinymce/src/core/main/ts/delete/DivDelete.ts

@@ -0,0 +1,39 @@
+import { Optional } from '@ephox/katamari';
+
+import Editor from '../api/Editor';
+import * as StructureBookmark from '../bookmark/StructureBookmark';
+import { execNativeDeleteCommand, execNativeForwardDeleteCommand } from './DeleteUtils';
+
+export const backspaceDelete = (editor: Editor, forward: boolean): Optional<() => void> => {
+  const dom = editor.dom;
+  const startBlock = dom.getParent(editor.selection.getStart(), dom.isBlock);
+  const endBlock = dom.getParent(editor.selection.getEnd(), dom.isBlock);
+  const body = editor.getBody();
+  const startBlockName = startBlock?.nodeName?.toLowerCase();
+
+  // Only act on single root div that is not empty
+  if (startBlockName === 'div' && startBlock && endBlock && startBlock === body.firstChild && endBlock === body.lastChild && !dom.isEmpty(body)) {
+    const wrapper = startBlock.cloneNode(false) as HTMLElement;
+
+    const deleteAction = () => {
+      if (forward) {
+        execNativeForwardDeleteCommand(editor);
+      } else {
+        execNativeDeleteCommand(editor);
+      }
+
+      // Div was deleted by delete operation then lets restore it
+      if (body.firstChild !== startBlock) {
+        const bookmark = StructureBookmark.getBookmark(editor.selection.getRng(), () => document.createElement('span'));
+        Array.from(body.childNodes).forEach((node) => wrapper.appendChild(node));
+        body.appendChild(wrapper);
+        editor.selection.setRng(StructureBookmark.resolveBookmark(bookmark));
+      }
+    };
+
+    return Optional.some(deleteAction);
+  }
+
+  return Optional.none();
+};
+

modules/tinymce/src/core/main/ts/keyboard/DeleteBackspaceKeys.ts

@@ -9,6 +9,7 @@ import * as BlockRangeDelete from '../delete/BlockRangeDelete';
 import * as CaretBoundaryDelete from '../delete/CaretBoundaryDelete';
 import * as CefDelete from '../delete/CefDelete';
 import * as DetailsDelete from '../delete/DetailsDelete';
+import * as DivDelete from '../delete/DivDelete';
 import * as ImageBlockDelete from '../delete/ImageBlockDelete';
 import * as InlineBoundaryDelete from '../delete/InlineBoundaryDelete';
 import * as InlineFormatDelete from '../delete/InlineFormatDelete';
@@ -66,7 +67,9 @@ const executeKeydownOverride = (editor: Editor, caret: Cell<Text | null>, evt: K
     { keyCode: VK.BACKSPACE, action: MatchKeys.action(BlockBoundaryDelete.backspaceDelete, editor, false) },
     { keyCode: VK.DELETE, action: MatchKeys.action(BlockBoundaryDelete.backspaceDelete, editor, true) },
     { keyCode: VK.BACKSPACE, action: MatchKeys.action(InlineFormatDelete.backspaceDelete, editor, false) },
-    { keyCode: VK.DELETE, action: MatchKeys.action(InlineFormatDelete.backspaceDelete, editor, true) }
+    { keyCode: VK.DELETE, action: MatchKeys.action(InlineFormatDelete.backspaceDelete, editor, true) },
+    { keyCode: VK.BACKSPACE, action: MatchKeys.action(DivDelete.backspaceDelete, editor, false) },
+    { keyCode: VK.DELETE, action: MatchKeys.action(DivDelete.backspaceDelete, editor, true) }
   ], evt)
     .filter((_) => editor.selection.isEditable())
     .each((applyAction) => {

modules/tinymce/src/core/test/ts/browser/ForceBlocksTest.ts

@@ -119,6 +119,17 @@ describe('browser.tinymce.core.ForceBlocksTest', () => {
     assert.equal(HtmlUtils.cleanHtml(editor.getBody().innerHTML), '<span data-mce-type="bookmark">a</span>');
   });
 
+  it('TINY-10840: Wrapping should not throw error on element indexes', () => {
+    const editor = hook.editor();
+    editor.getBody().innerHTML = '<br><br>';
+    assert.doesNotThrow(() => {
+      TinySelections.setCursor(editor, [ ], 2);
+      editor.nodeChanged(); // This is needed since on some browsers selection change is async
+    });
+    TinyAssertions.assertContent(editor, '<p><br><br></p>');
+    TinyAssertions.assertCursor(editor, [ 0 ], 2);
+  });
+
   context('Transparent elements', () => {
     it('TINY-9172: Do not wrap root level transparent elements if they blocks inside', () => {
       const editor = hook.editor();