Improve trailing space to nbsp conversion with noFollowingTextSegmentOrLast

Refactor handleParagraph to track whether a text segment is the last in the
paragraph or has no following text segment (excluding SelectionMarkers).
This ensures trailing spaces are converted to   not only for the very
last segment, but also when the next non-marker segment is not a Text segment.

- Convert forEach to for loop in handleParagraph for segment iteration
- Extract hasTextSegmentAfter helper to check for upcoming text segments
- Add noFollowingTextSegmentOrLast property to ModelToDomSegmentContext
- Update handleText to use the new property name
- Fix stale isLastSegment references in handleTextTest
- Add comprehensive tests for noFollowingTextSegmentOrLast in handleParagraphTest

Author: BryanValverdeU

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

@@ -7,6 +7,7 @@ import { unwrap } from '../../domUtils/unwrap';
 import type {
     ContentModelBlockHandler,
     ContentModelParagraph,
+    ContentModelSegment,
     ModelToDomContext,
     ModelToDomSegmentContext,
 } from 'roosterjs-content-model-types';
@@ -75,21 +76,12 @@ export const handleParagraph: ContentModelBlockHandler<ContentModelParagraph> =
                         );
                     }
 
-                    // Find the last logical inline segment to be treated as the "last" one.
-                    // Skip SelectionMarker since it is not rendered as visible content.
-                    let lastSegmentIndex = -1;
+                    for (let i = 0; i < paragraph.segments.length; i++) {
+                        const segment = paragraph.segments[i];
 
-                    for (let i = paragraph.segments.length - 1; i >= 0; i--) {
-                        const seg = paragraph.segments[i];
-
-                        if (seg.segmentType != 'SelectionMarker') {
-                            lastSegmentIndex = i;
-                            break;
-                        }
-                    }
-
-                    paragraph.segments.forEach((segment, index) => {
-                        segmentContext.isLastSegment = index === lastSegmentIndex;
+                        segmentContext.noFollowingTextSegmentOrLast =
+                            i === paragraph.segments.length - 1 ||
+                            !hasTextSegmentAfter(paragraph.segments, i);
 
                         const newSegments: Node[] = [];
                         context.modelHandlers.segment(
@@ -100,12 +92,12 @@ export const handleParagraph: ContentModelBlockHandler<ContentModelParagraph> =
                             newSegments
                         );
 
-                        newSegments.forEach(node => {
+                        for (const node of newSegments) {
                             context.domIndexer?.onSegment(node, paragraph, [segment]);
-                        });
-                    });
+                        }
+                    }
 
-                    delete segmentContext.isLastSegment;
+                    delete segmentContext.noFollowingTextSegmentOrLast;
                 }
             };
 
@@ -156,3 +148,18 @@ export const handleParagraph: ContentModelBlockHandler<ContentModelParagraph> =
 
     return refNode;
 };
+
+function hasTextSegmentAfter(segments: ReadonlyArray<ContentModelSegment>, index: number): boolean {
+    for (let i = index + 1; i < segments.length; i++) {
+        const type = segments[i].segmentType;
+        if (type === 'SelectionMarker') {
+            continue;
+        }
+        if (type === 'Text') {
+            return true;
+        } else {
+            return false;
+        }
+    }
+    return false;
+}

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

@@ -14,7 +14,7 @@ export const handleText: ContentModelSegmentHandler<ContentModelText> = (
     segmentNodes
 ) => {
     const textContent =
-        context.isLastSegment && segment.text.endsWith(' ')
+        context.noFollowingTextSegmentOrLast && segment.text.endsWith(' ')
             ? segment.text.slice(0, -1) + nonBreakingSpace
             : segment.text;
     const txt = doc.createTextNode(textContent);

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

@@ -13,6 +13,7 @@ import {
     ContentModelSegment,
     ContentModelSegmentHandler,
     ModelToDomContext,
+    ModelToDomSegmentContext,
 } from 'roosterjs-content-model-types';
 
 describe('handleParagraph', () => {
@@ -1170,4 +1171,231 @@ describe('Handle paragraph and adjust selections', () => {
             context.rewriteFromModel
         );
     });
+
+    describe('noFollowingTextSegmentOrLast', () => {
+        let parent: HTMLElement;
+        let context: ModelToDomContext;
+        let handleSegment: jasmine.Spy<ContentModelSegmentHandler<ContentModelSegment>>;
+
+        beforeEach(() => {
+            parent = document.createElement('div');
+            handleSegment = jasmine.createSpy('handleSegment');
+            context = createModelToDomContext(
+                {
+                    allowCacheElement: true,
+                },
+                {
+                    modelHandlerOverride: {
+                        segment: handleSegment,
+                    },
+                }
+            );
+        });
+
+        it('should be true for the only text segment', () => {
+            const captured: (boolean | undefined)[] = [];
+            handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
+                captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
+            });
+
+            const paragraph: ContentModelParagraph = {
+                blockType: 'Paragraph',
+                segments: [{ segmentType: 'Text', text: 'hello', format: {} }],
+                format: {},
+            };
+            handleParagraph(document, parent, paragraph, context, null);
+
+            expect(captured).toEqual([true]);
+        });
+
+        it('should be false for text before another text segment', () => {
+            const captured: (boolean | undefined)[] = [];
+            handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
+                captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
+            });
+
+            const paragraph: ContentModelParagraph = {
+                blockType: 'Paragraph',
+                segments: [
+                    { segmentType: 'Text', text: 'first', format: {} },
+                    { segmentType: 'Text', text: 'second', format: {} },
+                ],
+                format: {},
+            };
+            handleParagraph(document, parent, paragraph, context, null);
+
+            expect(captured).toEqual([false, true]);
+        });
+
+        it('should be true for text followed by a Br segment', () => {
+            const captured: (boolean | undefined)[] = [];
+            handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
+                captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
+            });
+
+            const paragraph: ContentModelParagraph = {
+                blockType: 'Paragraph',
+                segments: [
+                    { segmentType: 'Text', text: 'hello', format: {} },
+                    { segmentType: 'Br', format: {} },
+                ],
+                format: {},
+            };
+            handleParagraph(document, parent, paragraph, context, null);
+
+            // Text is followed by Br (non-text), so noFollowingTextSegmentOrLast is true for text; also true for Br
+            expect(captured).toEqual([true, true]);
+        });
+
+        it('should be true for text followed by an Image segment', () => {
+            const captured: (boolean | undefined)[] = [];
+            handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
+                captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
+            });
+
+            const paragraph: ContentModelParagraph = {
+                blockType: 'Paragraph',
+                segments: [
+                    { segmentType: 'Text', text: 'hello', format: {} },
+                    { segmentType: 'Image', src: 'test.png', format: {}, dataset: {} },
+                ],
+                format: {},
+            };
+            handleParagraph(document, parent, paragraph, context, null);
+
+            expect(captured).toEqual([true, true]);
+        });
+
+        it('should skip SelectionMarker when determining next text segment', () => {
+            const captured: (boolean | undefined)[] = [];
+            handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
+                captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
+            });
+
+            const paragraph: ContentModelParagraph = {
+                blockType: 'Paragraph',
+                segments: [
+                    { segmentType: 'Text', text: 'first', format: {} },
+                    { segmentType: 'SelectionMarker', isSelected: true, format: {} },
+                    { segmentType: 'Text', text: 'second', format: {} },
+                ],
+                format: {},
+            };
+            handleParagraph(document, parent, paragraph, context, null);
+
+            // first text -> marker -> second text: first=false, marker=false, second=true
+            expect(captured).toEqual([false, false, true]);
+        });
+
+        it('should be true when text is followed by SelectionMarker only', () => {
+            const captured: (boolean | undefined)[] = [];
+            handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
+                captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
+            });
+
+            const paragraph: ContentModelParagraph = {
+                blockType: 'Paragraph',
+                segments: [
+                    { segmentType: 'Text', text: 'hello', format: {} },
+                    { segmentType: 'SelectionMarker', isSelected: true, format: {} },
+                ],
+                format: {},
+            };
+            handleParagraph(document, parent, paragraph, context, null);
+
+            // text -> marker (no text after): both should be true
+            expect(captured).toEqual([true, true]);
+        });
+
+        it('should be true for text followed by Entity segment', () => {
+            const captured: (boolean | undefined)[] = [];
+            handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
+                captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
+            });
+
+            const wrapper = document.createElement('span');
+            const paragraph: ContentModelParagraph = {
+                blockType: 'Paragraph',
+                segments: [
+                    { segmentType: 'Text', text: 'hello', format: {} },
+                    {
+                        segmentType: 'Entity',
+                        blockType: 'Entity',
+                        entityFormat: { entityType: 'test', id: 'e1', isReadonly: true },
+                        format: {},
+                        wrapper,
+                    },
+                ],
+                format: {},
+            };
+            handleParagraph(document, parent, paragraph, context, null);
+
+            expect(captured).toEqual([true, true]);
+        });
+
+        it('should handle text followed by General segment', () => {
+            const captured: (boolean | undefined)[] = [];
+            handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
+                captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
+            });
+
+            const paragraph: ContentModelParagraph = {
+                blockType: 'Paragraph',
+                segments: [
+                    { segmentType: 'Text', text: 'hello', format: {} },
+                    {
+                        segmentType: 'General',
+                        blockType: 'BlockGroup',
+                        blockGroupType: 'General',
+                        blocks: [],
+                        element: document.createElement('span'),
+                        format: {},
+                    },
+                ],
+                format: {},
+            };
+            handleParagraph(document, parent, paragraph, context, null);
+
+            expect(captured).toEqual([true, true]);
+        });
+
+        it('should be false for text when text comes after non-text segment', () => {
+            const captured: (boolean | undefined)[] = [];
+            handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
+                captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
+            });
+
+            const paragraph: ContentModelParagraph = {
+                blockType: 'Paragraph',
+                segments: [
+                    { segmentType: 'Text', text: 'first', format: {} },
+                    { segmentType: 'Br', format: {} },
+                    { segmentType: 'Text', text: 'second', format: {} },
+                ],
+                format: {},
+            };
+            handleParagraph(document, parent, paragraph, context, null);
+
+            // first text: Br is next non-marker -> false (Br breaks, no text immediately), but actually
+            // hasTextSegmentAfter looks past Br to find Text, Br is not Text so returns false at index 0
+            // Br at index 1: next is Text -> false? No, hasTextSegmentAfter checks if type === 'Text'
+            // Actually: index 0 -> next is Br (not SelectionMarker, not Text) -> false -> noFollowingTextSegmentOrLast=true
+            // index 1 (Br) -> next is Text -> true -> noFollowingTextSegmentOrLast=false
+            // index 2 (Text) -> no more -> noFollowingTextSegmentOrLast=true
+            expect(captured).toEqual([true, false, true]);
+        });
+
+        it('should be cleaned up after processing paragraph', () => {
+            handleSegment.and.callFake(() => {});
+
+            const paragraph: ContentModelParagraph = {
+                blockType: 'Paragraph',
+                segments: [{ segmentType: 'Text', text: 'hello', format: {} }],
+                format: {},
+            };
+            handleParagraph(document, parent, paragraph, context, null);
+
+            expect((context as ModelToDomSegmentContext).noFollowingTextSegmentOrLast).toBeUndefined();
+        });
+    });
 });

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

@@ -9,7 +9,7 @@ describe('handleText', () => {
 
     beforeEach(() => {
         parent = document.createElement('div');
-        context = { ...createModelToDomContext(), isLastSegment: false };
+        context = { ...createModelToDomContext() };
     });
 
     it('Text segment', () => {
@@ -165,7 +165,7 @@ describe('handleText', () => {
             format: {},
         };
 
-        context.isLastSegment = true;
+        context.noFollowingTextSegmentOrLast = true;
 
         handleText(document, parent, text, context, []);
 
@@ -179,7 +179,7 @@ describe('handleText', () => {
             format: {},
         };
 
-        context.isLastSegment = true;
+        context.noFollowingTextSegmentOrLast = true;
 
         handleText(document, parent, text, context, []);
 
@@ -193,7 +193,19 @@ describe('handleText', () => {
             format: {},
         };
 
-        context.isLastSegment = false;
+        context.noFollowingTextSegmentOrLast = false;
+
+        handleText(document, parent, text, context, []);
+
+        expect(parent.innerHTML).toBe('<span>hello </span>');
+    });
+
+    it('Text ending with space without noFollowingTextSegmentOrLast set keeps space', () => {
+        const text: ContentModelText = {
+            segmentType: 'Text',
+            text: 'hello ',
+            format: {},
+        };
 
         handleText(document, parent, text, context, []);
 

packages/roosterjs-content-model-types/lib/context/ModelToDomContext.ts

@@ -19,7 +19,9 @@ export interface ModelToDomContext
  */
 export interface ModelToDomSegmentContext extends ModelToDomContext {
     /**
-     * Whether the current segment being processed is the last segment in its paragraph
+     * Whether the current segment is the last segment in the paragraph,
+     * or there are no more Text segments after it (excluding SelectionMarkers).
+     * When true, trailing spaces should be converted to  .
      */
-    isLastSegment?: boolean;
+    noFollowingTextSegmentOrLast?: boolean;
 }