Skip to content

Commit 6460054

Browse files
JiuqingSongclaude
andauthored
Add skipFormatContainerFallbackCheck to keep persisted Content Model path valid (#3358)
When formatInsertPointWithContentModel persists the Content Model group path during DOM to Model conversion, a FormatContainer that falls back to a plain paragraph would be removed from the model, leaving the persisted path invalid. Add a skipFormatContainerFallbackCheck option that keeps the FormatContainer in that scenario so the formatting callback can still rely on the path. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 48760de commit 6460054

9 files changed

Lines changed: 180 additions & 2 deletions

File tree

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,15 @@ export function formatInsertPointWithContentModel(
6060
textWithSelection: getShadowTextProcessor(bundle),
6161
},
6262
tryGetFromCache: false,
63+
// When an element carries "container level" styles such as margin or padding, we first
64+
// wrap it in a FormatContainer. After all its child nodes are processed, we decide whether
65+
// to keep the FormatContainer or fall back to a plain paragraph when it only wraps a single
66+
// paragraph. However, formatInsertPointWithContentModel persists the Content Model group path
67+
// during processing so the later formatting callback can still use it (see the
68+
// DomToModelContextWithPath interface below). If the FormatContainer falls back to a paragraph,
69+
// it is removed from the model and the persisted path becomes invalid. To keep the path valid,
70+
// we skip the fallback check here and always keep the FormatContainer when one is needed.
71+
skipFormatContainerFallbackCheck: true,
6372
}
6473
);
6574
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ describe('formatInsertPointWithContentModel', () => {
4747
textWithSelection: jasmine.anything() as any,
4848
},
4949
tryGetFromCache: false,
50+
skipFormatContainerFallbackCheck: true,
5051
}
5152
);
5253

@@ -112,6 +113,7 @@ describe('formatInsertPointWithContentModel', () => {
112113
textWithSelection: jasmine.anything() as any,
113114
},
114115
tryGetFromCache: false,
116+
skipFormatContainerFallbackCheck: true,
115117
}
116118
);
117119

packages/roosterjs-content-model-core/lib/coreApi/createContentModel/createContentModel.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ export const createContentModel: CreateContentModel = (core, option, selectionOv
4545
? createDomToModelContext(editorContext, settings.builtIn, settings.customized, option)
4646
: createDomToModelContextWithConfig(settings.calculated, editorContext);
4747

48+
if (option?.skipFormatContainerFallbackCheck) {
49+
domToModelContext.skipFormatContainerFallbackCheck = true;
50+
}
51+
4852
if (selection) {
4953
domToModelContext.selection = selection;
5054
}

packages/roosterjs-content-model-core/test/coreApi/createContentModel/createContentModelTest.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,30 @@ describe('createContentModel', () => {
107107
expect(domToContentModelSpy).toHaveBeenCalledWith(mockedDiv, currentContext);
108108
expect(model).toBe(mockedModel);
109109
});
110+
111+
it('Pass skipFormatContainerFallbackCheck option to context', () => {
112+
const currentContext = { ...originalContext } as DomToModelContext;
113+
114+
spyOn(createDomToModelContext, 'createDomToModelContext').and.returnValue(currentContext);
115+
116+
createContentModel(core, {
117+
tryGetFromCache: false,
118+
skipFormatContainerFallbackCheck: true,
119+
});
120+
121+
expect(domToContentModelSpy).toHaveBeenCalledWith(mockedDiv, currentContext);
122+
expect(currentContext.skipFormatContainerFallbackCheck).toBe(true);
123+
});
124+
125+
it('Do not set skipFormatContainerFallbackCheck when option not passed', () => {
126+
const currentContext = { ...originalContext } as DomToModelContext;
127+
128+
spyOn(createDomToModelContext, 'createDomToModelContext').and.returnValue(currentContext);
129+
130+
createContentModel(core, { tryGetFromCache: false });
131+
132+
expect(currentContext.skipFormatContainerFallbackCheck).toBeUndefined();
133+
});
110134
});
111135

112136
describe('createContentModel with selection', () => {

packages/roosterjs-content-model-dom/lib/domToModel/context/createDomToModelContext.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export function createDomToModelContext(
3939
export function createDomToModelContextWithConfig(
4040
config: DomToModelSettings,
4141
editorContext?: EditorContext
42-
) {
42+
): DomToModelContext {
4343
return Object.assign(
4444
{},
4545
editorContext,

packages/roosterjs-content-model-dom/lib/domToModel/processors/formatContainerProcessor.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,11 @@ const formatContainerProcessorInternal = (
8686
formatContainer.zeroFontSize = true;
8787
}
8888

89-
if (shouldFallbackToParagraph(formatContainer) && !forceFormatContainer) {
89+
if (
90+
!context.skipFormatContainerFallbackCheck &&
91+
shouldFallbackToParagraph(formatContainer) &&
92+
!forceFormatContainer
93+
) {
9094
// For DIV container that only has one paragraph child, container style can be merged into paragraph
9195
// and no need to have this container
9296
const paragraph = formatContainer.blocks[0] as ContentModelParagraph;

packages/roosterjs-content-model-dom/test/domToModel/processors/formatContainerProcessorTest.ts

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,3 +641,122 @@ describe('forceFormatContainerProcessor', () => {
641641
});
642642
});
643643
});
644+
645+
describe('formatContainerProcessor with skipFormatContainerFallbackCheck', () => {
646+
let context: DomToModelContext;
647+
648+
beforeEach(() => {
649+
context = createDomToModelContext();
650+
context.skipFormatContainerFallbackCheck = true;
651+
});
652+
653+
it('div with single paragraph child should NOT fallback to paragraph', () => {
654+
const group = createContentModelDocument();
655+
const div = document.createElement('div');
656+
657+
div.appendChild(document.createTextNode('test'));
658+
659+
formatContainerProcessor(group, div, context);
660+
661+
expect(group).toEqual({
662+
blockGroupType: 'Document',
663+
blocks: [
664+
{
665+
blockType: 'BlockGroup',
666+
blockGroupType: 'FormatContainer',
667+
tagName: 'div',
668+
blocks: [
669+
{
670+
blockType: 'Paragraph',
671+
segments: [
672+
{
673+
segmentType: 'Text',
674+
text: 'test',
675+
format: {},
676+
},
677+
],
678+
format: {},
679+
isImplicit: true,
680+
},
681+
],
682+
format: {},
683+
},
684+
{ blockType: 'Paragraph', segments: [], format: {}, isImplicit: true },
685+
],
686+
});
687+
});
688+
689+
it('div with id and single paragraph child should NOT fallback to paragraph', () => {
690+
const group = createContentModelDocument();
691+
const div = document.createElement('div');
692+
693+
div.id = 'testId';
694+
div.appendChild(document.createTextNode('test'));
695+
696+
formatContainerProcessor(group, div, context);
697+
698+
expect(group).toEqual({
699+
blockGroupType: 'Document',
700+
blocks: [
701+
{
702+
blockType: 'BlockGroup',
703+
blockGroupType: 'FormatContainer',
704+
tagName: 'div',
705+
blocks: [
706+
{
707+
blockType: 'Paragraph',
708+
segments: [
709+
{
710+
segmentType: 'Text',
711+
text: 'test',
712+
format: {},
713+
},
714+
],
715+
format: {},
716+
isImplicit: true,
717+
},
718+
],
719+
format: {
720+
id: 'testId',
721+
},
722+
},
723+
{ blockType: 'Paragraph', segments: [], format: {}, isImplicit: true },
724+
],
725+
});
726+
});
727+
728+
it('blockquote (non-div) is unaffected and still kept as FormatContainer', () => {
729+
const group = createContentModelDocument();
730+
const quote = document.createElement('blockquote');
731+
732+
quote.appendChild(document.createTextNode('test'));
733+
734+
formatContainerProcessor(group, quote, context);
735+
736+
expect(group).toEqual({
737+
blockGroupType: 'Document',
738+
blocks: [
739+
{
740+
blockType: 'BlockGroup',
741+
blockGroupType: 'FormatContainer',
742+
tagName: 'blockquote',
743+
blocks: [
744+
{
745+
blockType: 'Paragraph',
746+
segments: [{ segmentType: 'Text', text: 'test', format: {} }],
747+
format: {},
748+
isImplicit: true,
749+
},
750+
],
751+
format: {
752+
marginTop: '1em',
753+
marginBottom: '1em',
754+
marginRight: '40px',
755+
marginLeft: '40px',
756+
},
757+
},
758+
{ blockType: 'Paragraph', segments: [], format: {}, isImplicit: true },
759+
],
760+
});
761+
});
762+
});

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,14 @@ export interface DomToModelOptionForCreateModel extends DomToModelOption {
4848
* When this option is passed, "tryGetFromCache" will be ignored.
4949
*/
5050
recalculateTableSize?: boolean | 'all' | 'selected' | 'none';
51+
52+
/**
53+
* When set to true, if a container element could be represented by a FormatContainer, always keep the
54+
* FormatContainer and never fall back to a paragraph, even when it only has a single child.
55+
* Set this when the intermediate FormatContainer is persisted during DOM to Content Model conversion
56+
* and is later used during formatting.
57+
*/
58+
skipFormatContainerFallbackCheck?: boolean;
5159
}
5260

5361
/**

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,4 +152,12 @@ export interface DomToModelSettings {
152152
* If true elements that has display:none style will be processed
153153
*/
154154
processNonVisibleElements?: boolean;
155+
156+
/**
157+
* When set to true, if a container element could be represented by a FormatContainer, always keep the
158+
* FormatContainer and never fall back to a paragraph, even when it only has a single child.
159+
* Set this when the intermediate FormatContainer is persisted during DOM to Content Model conversion
160+
* and is later used during formatting.
161+
*/
162+
skipFormatContainerFallbackCheck?: boolean;
155163
}

0 commit comments

Comments
 (0)