Skip to content

Commit 1d54415

Browse files
BryanValverdeUclaudeCopilot
authored
Preserve trailing space in last paragraph segment by converting to nbsp (#3235) (#3287)
* Preserve trailing space in last paragraph segment by converting to nbsp When the last text segment in a paragraph ends with a regular space, browsers collapse it during rendering. This change detects that case in handleText and replaces the trailing space with a non-breaking space (\u00A0) so it is preserved in the output. To support this, a new ModelToDomSegmentContext interface is introduced that extends ModelToDomContext with an isLastSegment flag. handleParagraph sets this flag for each segment before dispatching, and ContentModelSegmentHandler is updated to use ModelToDomSegmentContext as its context type, eliminating the need for type casts in the handlers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * 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 &nbsp; 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 --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent abb61fb commit 1d54415

File tree

7 files changed

+346
-13
lines changed

7 files changed

+346
-13
lines changed

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

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ import { unwrap } from '../../domUtils/unwrap';
77
import type {
88
ContentModelBlockHandler,
99
ContentModelParagraph,
10+
ContentModelSegment,
1011
ModelToDomContext,
12+
ModelToDomSegmentContext,
1113
} from 'roosterjs-content-model-types';
1214

1315
const DefaultParagraphTag = 'div';
@@ -55,6 +57,8 @@ export const handleParagraph: ContentModelBlockHandler<ContentModelParagraph> =
5557
if (parent) {
5658
const firstSegment = paragraph.segments[0];
5759

60+
const segmentContext: ModelToDomSegmentContext = context;
61+
5862
if (firstSegment?.segmentType == 'SelectionMarker') {
5963
// Make sure there is a segment created before selection marker.
6064
// If selection marker is the first selected segment in a paragraph, create a dummy text node,
@@ -67,19 +71,33 @@ export const handleParagraph: ContentModelBlockHandler<ContentModelParagraph> =
6771
segmentType: 'Text',
6872
text: '',
6973
},
70-
context,
74+
segmentContext,
7175
[]
7276
);
7377
}
7478

75-
paragraph.segments.forEach(segment => {
79+
for (let i = 0; i < paragraph.segments.length; i++) {
80+
const segment = paragraph.segments[i];
81+
82+
segmentContext.noFollowingTextSegmentOrLast =
83+
i === paragraph.segments.length - 1 ||
84+
!hasTextSegmentAfter(paragraph.segments, i);
85+
7686
const newSegments: Node[] = [];
77-
context.modelHandlers.segment(doc, parent, segment, context, newSegments);
87+
context.modelHandlers.segment(
88+
doc,
89+
parent,
90+
segment,
91+
segmentContext,
92+
newSegments
93+
);
7894

79-
newSegments.forEach(node => {
95+
for (const node of newSegments) {
8096
context.domIndexer?.onSegment(node, paragraph, [segment]);
81-
});
82-
});
97+
}
98+
}
99+
100+
delete segmentContext.noFollowingTextSegmentOrLast;
83101
}
84102
};
85103

@@ -130,3 +148,18 @@ export const handleParagraph: ContentModelBlockHandler<ContentModelParagraph> =
130148

131149
return refNode;
132150
};
151+
152+
function hasTextSegmentAfter(segments: ReadonlyArray<ContentModelSegment>, index: number): boolean {
153+
for (let i = index + 1; i < segments.length; i++) {
154+
const type = segments[i].segmentType;
155+
if (type === 'SelectionMarker') {
156+
continue;
157+
}
158+
if (type === 'Text') {
159+
return true;
160+
} else {
161+
return false;
162+
}
163+
}
164+
return false;
165+
}

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { handleSegmentCommon } from '../utils/handleSegmentCommon';
22
import type { ContentModelSegmentHandler, ContentModelText } from 'roosterjs-content-model-types';
33

4+
const nonBreakingSpace = '\u00A0';
5+
46
/**
57
* @internal
68
*/
@@ -11,7 +13,11 @@ export const handleText: ContentModelSegmentHandler<ContentModelText> = (
1113
context,
1214
segmentNodes
1315
) => {
14-
const txt = doc.createTextNode(segment.text);
16+
const textContent =
17+
context.noFollowingTextSegmentOrLast && segment.text.endsWith(' ')
18+
? segment.text.slice(0, -1) + nonBreakingSpace
19+
: segment.text;
20+
const txt = doc.createTextNode(textContent);
1521
const element = doc.createElement('span');
1622

1723
parent.appendChild(element);

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

Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
ContentModelSegment,
1414
ContentModelSegmentHandler,
1515
ModelToDomContext,
16+
ModelToDomSegmentContext,
1617
} from 'roosterjs-content-model-types';
1718

1819
describe('handleParagraph', () => {
@@ -1170,4 +1171,231 @@ describe('Handle paragraph and adjust selections', () => {
11701171
context.rewriteFromModel
11711172
);
11721173
});
1174+
1175+
describe('noFollowingTextSegmentOrLast', () => {
1176+
let parent: HTMLElement;
1177+
let context: ModelToDomContext;
1178+
let handleSegment: jasmine.Spy<ContentModelSegmentHandler<ContentModelSegment>>;
1179+
1180+
beforeEach(() => {
1181+
parent = document.createElement('div');
1182+
handleSegment = jasmine.createSpy('handleSegment');
1183+
context = createModelToDomContext(
1184+
{
1185+
allowCacheElement: true,
1186+
},
1187+
{
1188+
modelHandlerOverride: {
1189+
segment: handleSegment,
1190+
},
1191+
}
1192+
);
1193+
});
1194+
1195+
it('should be true for the only text segment', () => {
1196+
const captured: (boolean | undefined)[] = [];
1197+
handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
1198+
captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
1199+
});
1200+
1201+
const paragraph: ContentModelParagraph = {
1202+
blockType: 'Paragraph',
1203+
segments: [{ segmentType: 'Text', text: 'hello', format: {} }],
1204+
format: {},
1205+
};
1206+
handleParagraph(document, parent, paragraph, context, null);
1207+
1208+
expect(captured).toEqual([true]);
1209+
});
1210+
1211+
it('should be false for text before another text segment', () => {
1212+
const captured: (boolean | undefined)[] = [];
1213+
handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
1214+
captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
1215+
});
1216+
1217+
const paragraph: ContentModelParagraph = {
1218+
blockType: 'Paragraph',
1219+
segments: [
1220+
{ segmentType: 'Text', text: 'first', format: {} },
1221+
{ segmentType: 'Text', text: 'second', format: {} },
1222+
],
1223+
format: {},
1224+
};
1225+
handleParagraph(document, parent, paragraph, context, null);
1226+
1227+
expect(captured).toEqual([false, true]);
1228+
});
1229+
1230+
it('should be true for text followed by a Br segment', () => {
1231+
const captured: (boolean | undefined)[] = [];
1232+
handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
1233+
captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
1234+
});
1235+
1236+
const paragraph: ContentModelParagraph = {
1237+
blockType: 'Paragraph',
1238+
segments: [
1239+
{ segmentType: 'Text', text: 'hello', format: {} },
1240+
{ segmentType: 'Br', format: {} },
1241+
],
1242+
format: {},
1243+
};
1244+
handleParagraph(document, parent, paragraph, context, null);
1245+
1246+
// Text is followed by Br (non-text), so noFollowingTextSegmentOrLast is true for text; also true for Br
1247+
expect(captured).toEqual([true, true]);
1248+
});
1249+
1250+
it('should be true for text followed by an Image segment', () => {
1251+
const captured: (boolean | undefined)[] = [];
1252+
handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
1253+
captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
1254+
});
1255+
1256+
const paragraph: ContentModelParagraph = {
1257+
blockType: 'Paragraph',
1258+
segments: [
1259+
{ segmentType: 'Text', text: 'hello', format: {} },
1260+
{ segmentType: 'Image', src: 'test.png', format: {}, dataset: {} },
1261+
],
1262+
format: {},
1263+
};
1264+
handleParagraph(document, parent, paragraph, context, null);
1265+
1266+
expect(captured).toEqual([true, true]);
1267+
});
1268+
1269+
it('should skip SelectionMarker when determining next text segment', () => {
1270+
const captured: (boolean | undefined)[] = [];
1271+
handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
1272+
captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
1273+
});
1274+
1275+
const paragraph: ContentModelParagraph = {
1276+
blockType: 'Paragraph',
1277+
segments: [
1278+
{ segmentType: 'Text', text: 'first', format: {} },
1279+
{ segmentType: 'SelectionMarker', isSelected: true, format: {} },
1280+
{ segmentType: 'Text', text: 'second', format: {} },
1281+
],
1282+
format: {},
1283+
};
1284+
handleParagraph(document, parent, paragraph, context, null);
1285+
1286+
// first text -> marker -> second text: first=false, marker=false, second=true
1287+
expect(captured).toEqual([false, false, true]);
1288+
});
1289+
1290+
it('should be true when text is followed by SelectionMarker only', () => {
1291+
const captured: (boolean | undefined)[] = [];
1292+
handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
1293+
captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
1294+
});
1295+
1296+
const paragraph: ContentModelParagraph = {
1297+
blockType: 'Paragraph',
1298+
segments: [
1299+
{ segmentType: 'Text', text: 'hello', format: {} },
1300+
{ segmentType: 'SelectionMarker', isSelected: true, format: {} },
1301+
],
1302+
format: {},
1303+
};
1304+
handleParagraph(document, parent, paragraph, context, null);
1305+
1306+
// text -> marker (no text after): both should be true
1307+
expect(captured).toEqual([true, true]);
1308+
});
1309+
1310+
it('should be true for text followed by Entity segment', () => {
1311+
const captured: (boolean | undefined)[] = [];
1312+
handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
1313+
captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
1314+
});
1315+
1316+
const wrapper = document.createElement('span');
1317+
const paragraph: ContentModelParagraph = {
1318+
blockType: 'Paragraph',
1319+
segments: [
1320+
{ segmentType: 'Text', text: 'hello', format: {} },
1321+
{
1322+
segmentType: 'Entity',
1323+
blockType: 'Entity',
1324+
entityFormat: { entityType: 'test', id: 'e1', isReadonly: true },
1325+
format: {},
1326+
wrapper,
1327+
},
1328+
],
1329+
format: {},
1330+
};
1331+
handleParagraph(document, parent, paragraph, context, null);
1332+
1333+
expect(captured).toEqual([true, true]);
1334+
});
1335+
1336+
it('should handle text followed by General segment', () => {
1337+
const captured: (boolean | undefined)[] = [];
1338+
handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
1339+
captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
1340+
});
1341+
1342+
const paragraph: ContentModelParagraph = {
1343+
blockType: 'Paragraph',
1344+
segments: [
1345+
{ segmentType: 'Text', text: 'hello', format: {} },
1346+
{
1347+
segmentType: 'General',
1348+
blockType: 'BlockGroup',
1349+
blockGroupType: 'General',
1350+
blocks: [],
1351+
element: document.createElement('span'),
1352+
format: {},
1353+
},
1354+
],
1355+
format: {},
1356+
};
1357+
handleParagraph(document, parent, paragraph, context, null);
1358+
1359+
expect(captured).toEqual([true, true]);
1360+
});
1361+
1362+
it('should be false for text when text comes after non-text segment', () => {
1363+
const captured: (boolean | undefined)[] = [];
1364+
handleSegment.and.callFake((_doc, _parent, _seg, ctx) => {
1365+
captured.push((ctx as ModelToDomSegmentContext).noFollowingTextSegmentOrLast);
1366+
});
1367+
1368+
const paragraph: ContentModelParagraph = {
1369+
blockType: 'Paragraph',
1370+
segments: [
1371+
{ segmentType: 'Text', text: 'first', format: {} },
1372+
{ segmentType: 'Br', format: {} },
1373+
{ segmentType: 'Text', text: 'second', format: {} },
1374+
],
1375+
format: {},
1376+
};
1377+
handleParagraph(document, parent, paragraph, context, null);
1378+
1379+
// first text: Br is next non-marker -> false (Br breaks, no text immediately), but actually
1380+
// hasTextSegmentAfter looks past Br to find Text, Br is not Text so returns false at index 0
1381+
// Br at index 1: next is Text -> false? No, hasTextSegmentAfter checks if type === 'Text'
1382+
// Actually: index 0 -> next is Br (not SelectionMarker, not Text) -> false -> noFollowingTextSegmentOrLast=true
1383+
// index 1 (Br) -> next is Text -> true -> noFollowingTextSegmentOrLast=false
1384+
// index 2 (Text) -> no more -> noFollowingTextSegmentOrLast=true
1385+
expect(captured).toEqual([true, false, true]);
1386+
});
1387+
1388+
it('should be cleaned up after processing paragraph', () => {
1389+
handleSegment.and.callFake(() => {});
1390+
1391+
const paragraph: ContentModelParagraph = {
1392+
blockType: 'Paragraph',
1393+
segments: [{ segmentType: 'Text', text: 'hello', format: {} }],
1394+
format: {},
1395+
};
1396+
handleParagraph(document, parent, paragraph, context, null);
1397+
1398+
expect((context as ModelToDomSegmentContext).noFollowingTextSegmentOrLast).toBeUndefined();
1399+
});
1400+
});
11731401
});

0 commit comments

Comments
 (0)