Fix #2202 again (#2450)

microsoft · Feb 29, 2024 · 743de92 · 743de92
1 parent cb6c80b
commit 743de92
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 7 deletions.
diff --git a/packages-content-model/roosterjs-content-model-core/lib/publicApi/selection/deleteBlock.ts b/packages-content-model/roosterjs-content-model-core/lib/publicApi/selection/deleteBlock.ts
@@ -38,9 +38,6 @@ export function deleteBlock(
                 : undefined;
 
             if (operation !== undefined) {
-                const wrapper = blockToDelete.wrapper;
-
-                wrapper.parentNode?.removeChild(wrapper);
                 replacement ? blocks.splice(index, 1, replacement) : blocks.splice(index, 1);
                 context?.deletedEntities.push({
                     entity: blockToDelete,

diff --git a/packages-content-model/roosterjs-content-model-core/lib/publicApi/selection/deleteSegment.ts b/packages-content-model/roosterjs-content-model-core/lib/publicApi/selection/deleteSegment.ts
@@ -48,9 +48,6 @@ export function deleteSegment(
                 ? 'removeFromEnd'
                 : undefined;
             if (operation !== undefined) {
-                const wrapper = segmentToDelete.wrapper;
-
-                wrapper.parentNode?.removeChild(wrapper);
                 segments.splice(index, 1);
                 context?.deletedEntities.push({
                     entity: segmentToDelete,

diff --git a/packages-content-model/roosterjs-content-model-dom/lib/domUtils/reuseCachedElement.ts b/packages-content-model/roosterjs-content-model-dom/lib/domUtils/reuseCachedElement.ts
@@ -11,10 +11,12 @@ import { isEntityElement } from './entityUtils';
  */
 export function reuseCachedElement(parent: Node, element: Node, refNode: Node | null): Node | null {
     if (element.parentNode == parent) {
+        const isEntity = isEntityElement(element);
+
         // Remove nodes before the one we are hitting since they don't appear in Content Model at this position.
         // But we don't want to touch entity since it would better to keep entity at its place unless it is removed
         // In that case we will remove it after we have handled all other nodes
-        while (refNode && refNode != element && !isEntityElement(refNode)) {
+        while (refNode && refNode != element && (isEntity || !isEntityElement(refNode))) {
             const next = refNode.nextSibling;
 
             refNode.parentNode?.removeChild(refNode);

diff --git a/packages-content-model/roosterjs-content-model-dom/test/domUtils/reuseCachedElementTest.ts b/packages-content-model/roosterjs-content-model-dom/test/domUtils/reuseCachedElementTest.ts
@@ -66,6 +66,7 @@ describe('reuseCachedElement', () => {
         const refNode = document.createElement('div');
         const element = document.createElement('span');
         const nextNode = document.createElement('br');
+        const removeChildSpy = spyOn(Node.prototype, 'removeChild').and.callThrough();
 
         parent.appendChild(refNode);
         parent.appendChild(element);
@@ -75,11 +76,39 @@ describe('reuseCachedElement', () => {
 
         const result = reuseCachedElement(parent, element, refNode);
 
+        expect(removeChildSpy).not.toHaveBeenCalled();
         expect(parent.outerHTML).toBe(
             '<div><span></span><div class="_Entity _EType_TestEntity _EReadonly_1" contenteditable="false"></div><br></div>'
         );
         expect(parent.firstChild).toBe(element);
         expect(parent.firstChild?.nextSibling).toBe(refNode);
         expect(result).toBe(refNode);
     });
+
+    it('RefNode is entity, current element is entity', () => {
+        const parent = document.createElement('div');
+        const refNode = document.createElement('div');
+        const element = document.createElement('span');
+        const nextNode = document.createElement('br');
+        const removeChildSpy = spyOn(Node.prototype, 'removeChild').and.callThrough();
+
+        parent.appendChild(refNode);
+        parent.appendChild(element);
+        parent.appendChild(nextNode);
+
+        setEntityElementClasses(refNode, 'TestEntity', true);
+        setEntityElementClasses(element, 'TestEntity2', true);
+
+        const result = reuseCachedElement(parent, element, refNode);
+
+        expect(removeChildSpy).toHaveBeenCalledTimes(1);
+        expect(removeChildSpy).toHaveBeenCalledWith(refNode);
+
+        expect(parent.outerHTML).toBe(
+            '<div><span class="_Entity _EType_TestEntity2 _EReadonly_1" contenteditable="false"></span><br></div>'
+        );
+        expect(parent.firstChild).toBe(element);
+        expect(parent.firstChild?.nextSibling).toBe(nextNode);
+        expect(result).toBe(nextNode);
+    });
 });