From a7bdb437ee980ab01c62ae27a3ae66722d235f45 Mon Sep 17 00:00:00 2001 From: Jiuqing Song Date: Mon, 4 Dec 2023 14:06:48 -0800 Subject: [PATCH] Content Model: Fix overwrite table cell bug (#2240) --- .../lib/domToModel/utils/addSelectionMarker.ts | 8 ++++++-- .../roosterjs-content-model-dom/lib/index.ts | 1 - .../lib/modelApi/common/addSegment.ts | 6 ++++-- .../lib/modelApi/common/ensureParagraph.ts | 7 +++++-- .../domToModel/processors/childProcessorTest.ts | 1 + .../domToModel/utils/addSelectionMarkerTest.ts | 1 + .../lib/edit/keyboardInput.ts | 3 +++ .../test/edit/keyboardInputTest.ts | 14 ++++++++++++++ 8 files changed, 34 insertions(+), 7 deletions(-) diff --git a/packages-content-model/roosterjs-content-model-dom/lib/domToModel/utils/addSelectionMarker.ts b/packages-content-model/roosterjs-content-model-dom/lib/domToModel/utils/addSelectionMarker.ts index 86234b6faab..6e173de9f32 100644 --- a/packages-content-model/roosterjs-content-model-dom/lib/domToModel/utils/addSelectionMarker.ts +++ b/packages-content-model/roosterjs-content-model-dom/lib/domToModel/utils/addSelectionMarker.ts @@ -7,9 +7,13 @@ import type { ContentModelBlockGroup, DomToModelContext } from 'roosterjs-conten * @internal */ export function addSelectionMarker(group: ContentModelBlockGroup, context: DomToModelContext) { - const marker = createSelectionMarker(context.segmentFormat); + const segmentFormat = { + ...context.defaultFormat, + ...context.segmentFormat, + }; + const marker = createSelectionMarker(segmentFormat); addDecorators(marker, context); - addSegment(group, marker, context.blockFormat); + addSegment(group, marker, context.blockFormat, segmentFormat); } diff --git a/packages-content-model/roosterjs-content-model-dom/lib/index.ts b/packages-content-model/roosterjs-content-model-dom/lib/index.ts index 27cb62585d7..1df3dc46f67 100644 --- a/packages-content-model/roosterjs-content-model-dom/lib/index.ts +++ b/packages-content-model/roosterjs-content-model-dom/lib/index.ts @@ -49,7 +49,6 @@ export { createListLevel } from './modelApi/creators/createListLevel'; export { addBlock } from './modelApi/common/addBlock'; export { addCode } from './modelApi/common/addDecorators'; export { addLink } from './modelApi/common/addDecorators'; -export { ensureParagraph } from './modelApi/common/ensureParagraph'; export { normalizeContentModel } from './modelApi/common/normalizeContentModel'; export { isGeneralSegment } from './modelApi/common/isGeneralSegment'; diff --git a/packages-content-model/roosterjs-content-model-dom/lib/modelApi/common/addSegment.ts b/packages-content-model/roosterjs-content-model-dom/lib/modelApi/common/addSegment.ts index 35389ed6e25..4680693861c 100644 --- a/packages-content-model/roosterjs-content-model-dom/lib/modelApi/common/addSegment.ts +++ b/packages-content-model/roosterjs-content-model-dom/lib/modelApi/common/addSegment.ts @@ -4,6 +4,7 @@ import type { ContentModelBlockGroup, ContentModelParagraph, ContentModelSegment, + ContentModelSegmentFormat, } from 'roosterjs-content-model-types'; /** @@ -16,9 +17,10 @@ import type { export function addSegment( group: ContentModelBlockGroup, newSegment: ContentModelSegment, - blockFormat?: ContentModelBlockFormat + blockFormat?: ContentModelBlockFormat, + segmentFormat?: ContentModelSegmentFormat ): ContentModelParagraph { - const paragraph = ensureParagraph(group, blockFormat); + const paragraph = ensureParagraph(group, blockFormat, segmentFormat); const lastSegment = paragraph.segments[paragraph.segments.length - 1]; if (newSegment.segmentType == 'SelectionMarker') { diff --git a/packages-content-model/roosterjs-content-model-dom/lib/modelApi/common/ensureParagraph.ts b/packages-content-model/roosterjs-content-model-dom/lib/modelApi/common/ensureParagraph.ts index 9e22bda96c2..e638f1920ce 100644 --- a/packages-content-model/roosterjs-content-model-dom/lib/modelApi/common/ensureParagraph.ts +++ b/packages-content-model/roosterjs-content-model-dom/lib/modelApi/common/ensureParagraph.ts @@ -4,23 +4,26 @@ import type { ContentModelBlockFormat, ContentModelBlockGroup, ContentModelParagraph, + ContentModelSegmentFormat, } from 'roosterjs-content-model-types'; /** + * @internal * Ensure there is a Paragraph that can insert segments in a Content Model Block Group * @param group The parent block group of the target paragraph * @param blockFormat Format of the paragraph. This is only used if we need to create a new paragraph */ export function ensureParagraph( group: ContentModelBlockGroup, - blockFormat?: ContentModelBlockFormat + blockFormat?: ContentModelBlockFormat, + segmentFormat?: ContentModelSegmentFormat ): ContentModelParagraph { const lastBlock = group.blocks[group.blocks.length - 1]; if (lastBlock?.blockType == 'Paragraph') { return lastBlock; } else { - const paragraph = createParagraph(true, blockFormat); + const paragraph = createParagraph(true, blockFormat, segmentFormat); addBlock(group, paragraph); return paragraph; diff --git a/packages-content-model/roosterjs-content-model-dom/test/domToModel/processors/childProcessorTest.ts b/packages-content-model/roosterjs-content-model-dom/test/domToModel/processors/childProcessorTest.ts index 4be880b0ffa..623dc10a878 100644 --- a/packages-content-model/roosterjs-content-model-dom/test/domToModel/processors/childProcessorTest.ts +++ b/packages-content-model/roosterjs-content-model-dom/test/domToModel/processors/childProcessorTest.ts @@ -325,6 +325,7 @@ describe('childProcessor', () => { { segmentType: 'SelectionMarker', format: { a: 'b' } as any, isSelected: true }, ], isImplicit: true, + segmentFormat: { a: 'b' } as any, format: {}, }); }); diff --git a/packages-content-model/roosterjs-content-model-dom/test/domToModel/utils/addSelectionMarkerTest.ts b/packages-content-model/roosterjs-content-model-dom/test/domToModel/utils/addSelectionMarkerTest.ts index b1cb0ec6ca5..c84bfa3d12d 100644 --- a/packages-content-model/roosterjs-content-model-dom/test/domToModel/utils/addSelectionMarkerTest.ts +++ b/packages-content-model/roosterjs-content-model-dom/test/domToModel/utils/addSelectionMarkerTest.ts @@ -52,6 +52,7 @@ describe('addSelectionMarker', () => { format: { fontWeight: 'bold' }, }, ], + segmentFormat: { fontWeight: 'bold' }, }, ], }); diff --git a/packages-content-model/roosterjs-content-model-plugins/lib/edit/keyboardInput.ts b/packages-content-model/roosterjs-content-model-plugins/lib/edit/keyboardInput.ts index 3680f3c67d5..794b2df12ed 100644 --- a/packages-content-model/roosterjs-content-model-plugins/lib/edit/keyboardInput.ts +++ b/packages-content-model/roosterjs-content-model-plugins/lib/edit/keyboardInput.ts @@ -1,4 +1,5 @@ import { deleteSelection, isModifierKey } from 'roosterjs-content-model-core'; +import { normalizeContentModel } from 'roosterjs-content-model-dom'; import type { IContentModelEditor } from 'roosterjs-content-model-editor'; import type { DOMSelection } from 'roosterjs-content-model-types'; @@ -26,6 +27,8 @@ export function keyboardInput(editor: IContentModelEditor, rawEvent: KeyboardEve // We have deleted something, next input should inherit the segment format from deleted content, so set pending format here context.newPendingFormat = result.insertPoint?.marker.format; + normalizeContentModel(model); + // Do not preventDefault since we still want browser to handle the final input for now return true; } else { diff --git a/packages-content-model/roosterjs-content-model-plugins/test/edit/keyboardInputTest.ts b/packages-content-model/roosterjs-content-model-plugins/test/edit/keyboardInputTest.ts index 93fb2ff88be..ec0864a6c55 100644 --- a/packages-content-model/roosterjs-content-model-plugins/test/edit/keyboardInputTest.ts +++ b/packages-content-model/roosterjs-content-model-plugins/test/edit/keyboardInputTest.ts @@ -1,4 +1,5 @@ import * as deleteSelection from 'roosterjs-content-model-core/lib/publicApi/selection/deleteSelection'; +import * as normalizeContentModel from 'roosterjs-content-model-dom/lib/modelApi/common/normalizeContentModel'; import { IContentModelEditor } from 'roosterjs-content-model-editor'; import { keyboardInput } from '../../lib/edit/keyboardInput'; import { @@ -14,6 +15,7 @@ describe('keyboardInput', () => { let getDOMSelectionSpy: jasmine.Spy; let deleteSelectionSpy: jasmine.Spy; let mockedModel: ContentModelDocument; + let normalizeContentModelSpy: jasmine.Spy; let mockedContext: FormatWithContentModelContext; let formatResult: boolean | undefined; @@ -34,6 +36,7 @@ describe('keyboardInput', () => { }); getDOMSelectionSpy = jasmine.createSpy('getDOMSelection'); deleteSelectionSpy = spyOn(deleteSelection, 'deleteSelection'); + normalizeContentModelSpy = spyOn(normalizeContentModel, 'normalizeContentModel'); editor = { getDOMSelection: getDOMSelectionSpy, @@ -69,6 +72,7 @@ describe('keyboardInput', () => { newEntities: [], newImages: [], }); + expect(normalizeContentModelSpy).not.toHaveBeenCalled(); }); it('Letter input, expanded selection, no modifier key, deleteSelection returns not deleted', () => { @@ -100,6 +104,7 @@ describe('keyboardInput', () => { clearModelCache: true, skipUndoSnapshot: true, }); + expect(normalizeContentModelSpy).not.toHaveBeenCalled(); }); it('Letter input, expanded selection, no modifier key, deleteSelection returns range', () => { @@ -132,6 +137,7 @@ describe('keyboardInput', () => { skipUndoSnapshot: true, newPendingFormat: undefined, }); + expect(normalizeContentModelSpy).toHaveBeenCalledWith(mockedModel); }); it('Letter input, table selection, no modifier key, deleteSelection returns range', () => { @@ -161,6 +167,7 @@ describe('keyboardInput', () => { skipUndoSnapshot: true, newPendingFormat: undefined, }); + expect(normalizeContentModelSpy).toHaveBeenCalledWith(mockedModel); }); it('Letter input, image selection, no modifier key, deleteSelection returns range', () => { @@ -190,6 +197,7 @@ describe('keyboardInput', () => { skipUndoSnapshot: true, newPendingFormat: undefined, }); + expect(normalizeContentModelSpy).toHaveBeenCalledWith(mockedModel); }); it('Letter input, no selection, no modifier key, deleteSelection returns range', () => { @@ -214,6 +222,7 @@ describe('keyboardInput', () => { newEntities: [], newImages: [], }); + expect(normalizeContentModelSpy).not.toHaveBeenCalled(); }); it('Letter input, expanded selection, has modifier key, deleteSelection returns range', () => { @@ -244,6 +253,7 @@ describe('keyboardInput', () => { newEntities: [], newImages: [], }); + expect(normalizeContentModelSpy).not.toHaveBeenCalled(); }); it('Space input, table selection, no modifier key, deleteSelection returns range', () => { @@ -273,6 +283,7 @@ describe('keyboardInput', () => { skipUndoSnapshot: true, newPendingFormat: undefined, }); + expect(normalizeContentModelSpy).toHaveBeenCalledWith(mockedModel); }); it('Backspace input, table selection, no modifier key, deleteSelection returns range', () => { @@ -299,6 +310,7 @@ describe('keyboardInput', () => { newEntities: [], newImages: [], }); + expect(normalizeContentModelSpy).not.toHaveBeenCalled(); }); it('Enter input, table selection, no modifier key, deleteSelection returns range', () => { @@ -328,6 +340,7 @@ describe('keyboardInput', () => { skipUndoSnapshot: true, newPendingFormat: undefined, }); + expect(normalizeContentModelSpy).toHaveBeenCalledWith(mockedModel); }); it('Letter input, expanded selection, no modifier key, deleteSelection returns range, has segment format', () => { @@ -366,5 +379,6 @@ describe('keyboardInput', () => { skipUndoSnapshot: true, newPendingFormat: mockedFormat, }); + expect(normalizeContentModelSpy).toHaveBeenCalledWith(mockedModel); }); });