Fix #2880 optimize() causes segment cache to be wrong when merging <A> (

#2889)

packages/roosterjs-content-model-api/lib/publicApi/utils/formatTextSegmentBeforeSelectionMarker.ts

@@ -48,6 +48,11 @@ export function formatTextSegmentBeforeSelectionMarker(
 
                     if (previousSegment && previousSegment.segmentType === 'Text') {
                         result = true;
+
+                        // Preserve pending format if any when format text segment, so if there is pending format (e.g. from paste)
+                        // and some auto action happens after paste, the pending format will still take effect
+                        context.newPendingFormat = 'preserve';
+
                         rewrite = callback(
                             model,
                             previousSegment,

packages/roosterjs-content-model-api/test/publicApi/utils/formatTextSegmentBeforeSelectionMarkerTest.ts

@@ -23,13 +23,16 @@ describe('formatTextSegmentBeforeSelectionMarker', () => {
         const formatWithContentModelSpy = jasmine
             .createSpy('formatWithContentModel')
             .and.callFake((callback, options) => {
-                const result = callback(input, {
+                const context: FormatContentModelContext = {
                     newEntities: [],
                     deletedEntities: [],
                     newImages: [],
                     canUndoByBackspace: true,
-                });
+                };
+                const result = callback(input, context);
+
                 expect(result).toBe(expectedResult);
+                expect(context.newPendingFormat).toBe(expectedResult ? 'preserve' : undefined);
             });
 
         formatTextSegmentBeforeSelectionMarker(

packages/roosterjs-content-model-core/lib/corePlugin/cache/domIndexerImpl.ts

@@ -592,14 +592,14 @@ export class DomIndexerImpl implements DomIndexer {
         const { previousSibling, nextSibling } = node;
 
         if (
-            (segmentItem = getIndexedSegmentItem(previousSibling)) &&
+            (segmentItem = getIndexedSegmentItem(getLastLeaf(previousSibling))) &&
             (existingSegment = segmentItem.segments[segmentItem.segments.length - 1]) &&
             (index = segmentItem.paragraph.segments.indexOf(existingSegment)) >= 0
         ) {
             // When we can find indexed segment before current one, use it as the insert index
             this.indexNode(segmentItem.paragraph, index + 1, node, existingSegment.format);
         } else if (
-            (segmentItem = getIndexedSegmentItem(nextSibling)) &&
+            (segmentItem = getIndexedSegmentItem(getFirstLeaf(nextSibling))) &&
             (existingSegment = segmentItem.segments[0]) &&
             (index = segmentItem.paragraph.segments.indexOf(existingSegment)) >= 0
         ) {
@@ -691,3 +691,19 @@ export class DomIndexerImpl implements DomIndexer {
         this.onSegment(textNode, paragraph, [text]);
     }
 }
+
+function getLastLeaf(node: Node | null): Node | null {
+    while (node?.lastChild) {
+        node = node.lastChild;
+    }
+
+    return node;
+}
+
+function getFirstLeaf(node: Node | null): Node | null {
+    while (node?.firstChild) {
+        node = node.firstChild;
+    }
+
+    return node;
+}

packages/roosterjs-content-model-core/test/corePlugin/cache/domIndexerImplTest.ts

@@ -17,6 +17,7 @@ import {
     DOMSelection,
 } from 'roosterjs-content-model-types';
 import {
+    addLink,
     createBr,
     createContentModelDocument,
     createEntity,
@@ -1301,6 +1302,77 @@ describe('domIndexerImpl.reconcileChildList', () => {
             segments: [segment],
         });
     });
+
+    it('Added Text after link that contains image and text', () => {
+        const domIndexer = new DomIndexerImpl(true);
+        const a = document.createElement('a');
+        const img = document.createElement('img');
+        const text = document.createTextNode('test');
+        const newText = document.createTextNode('a');
+        const div = document.createElement('div');
+
+        a.appendChild(img);
+        a.appendChild(text);
+        div.appendChild(a);
+        div.appendChild(newText);
+
+        const paragraph = createParagraph();
+        const segmentImg = createImage('src');
+        const segmentText = createText('test');
+
+        addLink(segmentImg, {
+            format: { href: 'test' },
+            dataset: {},
+        });
+        addLink(segmentText, {
+            format: { href: 'test' },
+            dataset: {},
+        });
+
+        paragraph.segments.push(segmentImg, segmentText);
+
+        ((img as Node) as IndexedSegmentNode).__roosterjsContentModel = {
+            paragraph: paragraph,
+            segments: [segmentImg],
+        };
+        ((text as Node) as IndexedSegmentNode).__roosterjsContentModel = {
+            paragraph: paragraph,
+            segments: [segmentText],
+        };
+
+        const result = domIndexer.reconcileChildList([newText], []);
+
+        expect(result).toBeTrue();
+        expect(paragraph).toEqual({
+            blockType: 'Paragraph',
+            segments: [
+                {
+                    segmentType: 'Image',
+                    src: 'src',
+                    format: {},
+                    dataset: {},
+                    link: { format: { href: 'test' }, dataset: {} },
+                },
+                {
+                    segmentType: 'Text',
+                    text: 'test',
+                    format: {},
+                    link: { format: { href: 'test' }, dataset: {} },
+                },
+                { segmentType: 'Text', text: 'a', format: {} },
+            ],
+            format: {},
+        });
+        expect(((newText as Node) as IndexedSegmentNode).__roosterjsContentModel.paragraph).toBe(
+            paragraph
+        );
+        expect(
+            ((newText as Node) as IndexedSegmentNode).__roosterjsContentModel.segments.length
+        ).toBe(1);
+        expect(((newText as Node) as IndexedSegmentNode).__roosterjsContentModel.segments[0]).toBe(
+            paragraph.segments[2]
+        );
+    });
 });
 
 describe('domIndexerImpl.reconcileElementId', () => {

packages/roosterjs-content-model-dom/lib/modelToDom/handlers/handleSegmentDecorator.ts

@@ -14,8 +14,7 @@ export const handleSegmentDecorator: ContentModelSegmentHandler<ContentModelSegm
     _,
     parent,
     segment,
-    context,
-    segmentNodes
+    context
 ) => {
     const { code, link } = segment;
 
@@ -27,7 +26,6 @@ export const handleSegmentDecorator: ContentModelSegmentHandler<ContentModelSegm
                 applyFormat(a, context.formatAppliers.link, link.format, context);
                 applyFormat(a, context.formatAppliers.dataset, link.dataset, context);
 
-                segmentNodes?.push(a);
                 context.onNodeCreated?.(link, a);
             });
         }
@@ -38,7 +36,6 @@ export const handleSegmentDecorator: ContentModelSegmentHandler<ContentModelSegm
 
                 applyFormat(codeNode, context.formatAppliers.code, code.format, context);
 
-                segmentNodes?.push(codeNode);
                 context.onNodeCreated?.(code, codeNode);
             });
         }

packages/roosterjs-content-model-dom/test/modelToDom/handlers/handleBrTest.ts

@@ -78,8 +78,7 @@ describe('handleSegment', () => {
         handleBr(document, parent, br, context, newSegments);
 
         expect(parent.innerHTML).toBe('<span><a href="/test"><br></a></span>');
-        expect(newSegments.length).toBe(2);
+        expect(newSegments.length).toBe(1);
         expect((newSegments[0] as HTMLElement).outerHTML).toBe('<br>');
-        expect((newSegments[1] as HTMLElement).outerHTML).toBe('<a href="/test"><br></a>');
     });
 });

packages/roosterjs-content-model-dom/test/modelToDom/handlers/handleSegmentDecoratorTest.ts

@@ -1,5 +1,4 @@
 import { createModelToDomContext } from '../../../lib/modelToDom/context/createModelToDomContext';
-import { expectHtml } from '../../testUtils';
 import { handleSegmentDecorator } from '../../../lib/modelToDom/handlers/handleSegmentDecorator';
 import {
     ContentModelCode,
@@ -20,8 +19,7 @@ describe('handleSegmentDecorator', () => {
     function runTest(
         link: ContentModelLink | undefined,
         code: ContentModelCode | undefined,
-        expectedInnerHTML: string,
-        expectedSegmentNodesHTML: (string | string[])[]
+        expectedInnerHTML: string
     ) {
         parent = document.createElement('span');
         parent.textContent = 'test';
@@ -37,10 +35,7 @@ describe('handleSegmentDecorator', () => {
         handleSegmentDecorator(document, parent, segment, context, segmentNodes);
 
         expect(parent.innerHTML).toBe(expectedInnerHTML);
-        expect(segmentNodes.length).toBe(expectedSegmentNodesHTML.length);
-        expectedSegmentNodesHTML.forEach((expectedHTML, i) => {
-            expectHtml((segmentNodes[i] as HTMLElement).outerHTML, expectedHTML);
-        });
+        expect(segmentNodes.length).toBe(0);
     }
 
     it('simple link', () => {
@@ -52,9 +47,7 @@ describe('handleSegmentDecorator', () => {
             dataset: {},
         };
 
-        runTest(link, undefined, '<a href="http://test.com/test">test</a>', [
-            '<a href="http://test.com/test">test</a>',
-        ]);
+        runTest(link, undefined, '<a href="http://test.com/test">test</a>');
     });
 
     it('link with color', () => {
@@ -67,9 +60,7 @@ describe('handleSegmentDecorator', () => {
             dataset: {},
         };
 
-        runTest(link, undefined, '<a href="http://test.com/test" style="color: red;">test</a>', [
-            '<a href="http://test.com/test" style="color: red;">test</a>',
-        ]);
+        runTest(link, undefined, '<a href="http://test.com/test" style="color: red;">test</a>');
     });
 
     it('link without underline', () => {
@@ -84,8 +75,7 @@ describe('handleSegmentDecorator', () => {
         runTest(
             link,
             undefined,
-            '<a href="http://test.com/test" style="text-decoration: none;">test</a>',
-            ['<a href="http://test.com/test" style="text-decoration: none;">test</a>']
+            '<a href="http://test.com/test" style="text-decoration: none;">test</a>'
         );
     });
 
@@ -101,9 +91,7 @@ describe('handleSegmentDecorator', () => {
             },
         };
 
-        runTest(link, undefined, '<a href="http://test.com/test" data-a="b" data-c="d">test</a>', [
-            '<a href="http://test.com/test" data-a="b" data-c="d">test</a>',
-        ]);
+        runTest(link, undefined, '<a href="http://test.com/test" data-a="b" data-c="d">test</a>');
     });
 
     it('simple code', () => {
@@ -113,7 +101,7 @@ describe('handleSegmentDecorator', () => {
             },
         };
 
-        runTest(undefined, code, '<code>test</code>', ['<code>test</code>']);
+        runTest(undefined, code, '<code>test</code>');
     });
 
     it('code with font', () => {
@@ -123,9 +111,7 @@ describe('handleSegmentDecorator', () => {
             },
         };
 
-        runTest(undefined, code, '<code style="font-family: Arial;">test</code>', [
-            '<code style="font-family: Arial;">test</code>',
-        ]);
+        runTest(undefined, code, '<code style="font-family: Arial;">test</code>');
     });
 
     it('link and code', () => {
@@ -142,10 +128,7 @@ describe('handleSegmentDecorator', () => {
             },
         };
 
-        runTest(link, code, '<code><a href="http://test.com/test">test</a></code>', [
-            '<a href="http://test.com/test">test</a>',
-            '<code><a href="http://test.com/test">test</a></code>',
-        ]);
+        runTest(link, code, '<code><a href="http://test.com/test">test</a></code>');
     });
 
     it('Link with onNodeCreated', () => {
@@ -194,12 +177,7 @@ describe('handleSegmentDecorator', () => {
             dataset: {},
         };
 
-        runTest(
-            link,
-            undefined,
-            '<a href="http://test.com/test" style="display: block;">test</a>',
-            ['<a href="http://test.com/test" style="display: block;">test</a>']
-        );
+        runTest(link, undefined, '<a href="http://test.com/test" style="display: block;">test</a>');
     });
 
     it('code with display: block', () => {
@@ -210,9 +188,7 @@ describe('handleSegmentDecorator', () => {
             },
         };
 
-        runTest(undefined, code, '<code style="display: block;">test</code>', [
-            '<code style="display: block;">test</code>',
-        ]);
+        runTest(undefined, code, '<code style="display: block;">test</code>');
     });
 
     it('link with background color', () => {
@@ -228,8 +204,7 @@ describe('handleSegmentDecorator', () => {
         runTest(
             link,
             undefined,
-            '<a href="http://test.com/test" style="background-color: red;">test</a>',
-            ['<a href="http://test.com/test" style="background-color: red;">test</a>']
+            '<a href="http://test.com/test" style="background-color: red;">test</a>'
         );
     });
 });

packages/roosterjs-content-model-dom/test/modelToDom/utils/handleSegmentCommonTest.ts

@@ -33,9 +33,8 @@ describe('handleSegmentCommon', () => {
             '<span style="font-size: 10pt; color: red; line-height: 2;"><b><a href="href">test</a></b></span>'
         );
         expect(onNodeCreated).toHaveBeenCalledWith(segment, txt);
-        expect(segmentNodes.length).toBe(2);
+        expect(segmentNodes.length).toBe(1);
         expect(segmentNodes[0]).toBe(txt);
-        expect(segmentNodes[1]).toBe(txt.parentNode!);
     });
 
     it('element with child', () => {