diff --git a/packages/roosterjs-content-model-core/lib/coreApi/restoreUndoSnapshot/restoreSnapshotSelection.ts b/packages/roosterjs-content-model-core/lib/coreApi/restoreUndoSnapshot/restoreSnapshotSelection.ts index 266400b67ff..b3c9c9bf732 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/restoreUndoSnapshot/restoreSnapshotSelection.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/restoreUndoSnapshot/restoreSnapshotSelection.ts @@ -1,5 +1,6 @@ -import type { DOMSelection, EditorCore, Snapshot } from 'roosterjs-content-model-types'; import { getPositionFromPath } from './getPositionFromPath'; +import { getSafeIdSelector } from 'roosterjs-content-model-dom'; +import type { DOMSelection, EditorCore, Snapshot } from 'roosterjs-content-model-types'; /** * @internal @@ -29,7 +30,7 @@ export function restoreSnapshotSelection(core: EditorCore, snapshot: Snapshot) { break; case 'table': const table = physicalRoot.querySelector( - '#' + snapshotSelection.tableId + getSafeIdSelector(snapshotSelection.tableId) ) as HTMLTableElement; if (table) { @@ -45,7 +46,7 @@ export function restoreSnapshotSelection(core: EditorCore, snapshot: Snapshot) { break; case 'image': const image = physicalRoot.querySelector( - '#' + snapshotSelection.imageId + getSafeIdSelector(snapshotSelection.imageId) ) as HTMLImageElement; if (image) { diff --git a/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts b/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts index 39cdd9828be..4b08be3e861 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts @@ -3,7 +3,12 @@ import { areSameSelections } from '../../corePlugin/cache/areSameSelections'; import { ensureUniqueId } from '../setEditorStyle/ensureUniqueId'; import { findLastedCoInMergedCell } from './findLastedCoInMergedCell'; import { findTableCellElement } from './findTableCellElement'; -import { isNodeOfType, parseTableCells, toArray } from 'roosterjs-content-model-dom'; +import { + getSafeIdSelector, + isNodeOfType, + parseTableCells, + toArray, +} from 'roosterjs-content-model-dom'; import type { ParsedTable, SelectionChangedEvent, @@ -59,7 +64,7 @@ export const setDOMSelection: SetDOMSelection = (core, selection, skipSelectionC `outline-style:solid!important; outline-color:${ imageSelectionColor || DEFAULT_SELECTION_BORDER_COLOR }!important;`, - [`#${ensureUniqueId(image, IMAGE_ID)}`] + [getSafeIdSelector(ensureUniqueId(image, IMAGE_ID))] ); core.api.setEditorStyle( core, @@ -105,13 +110,21 @@ export const setDOMSelection: SetDOMSelection = (core, selection, skipSelectionC }; const tableId = ensureUniqueId(table, TABLE_ID); + const tableSelector = getSafeIdSelector(tableId); + const tableSelectors = firstCell.row == 0 && firstCell.col == 0 && lastCell.row == parsedTable.length - 1 && lastCell.col == (parsedTable[lastCell.row]?.length ?? 0) - 1 - ? [`#${tableId}`, `#${tableId} *`] - : handleTableSelected(parsedTable, tableId, table, firstCell, lastCell); + ? [tableSelector, `${tableSelector} *`] + : handleTableSelected( + parsedTable, + tableSelector, + table, + firstCell, + lastCell + ); core.selection.selection = selection; @@ -163,7 +176,7 @@ export const setDOMSelection: SetDOMSelection = (core, selection, skipSelectionC function handleTableSelected( parsedTable: ParsedTable, - tableId: string, + tableSelector: string, table: HTMLTableElement, firstCell: TableCellCoordinate, lastCell: TableCellCoordinate @@ -214,7 +227,7 @@ function handleTableSelected( cellIndex >= firstCell.col && cellIndex <= lastCell.col ) { - const selector = `#${tableId}${middleElSelector} tr:nth-child(${currentRow})>${cell.tagName}:nth-child(${tdCount})`; + const selector = `${tableSelector}${middleElSelector} tr:nth-child(${currentRow})>${cell.tagName}:nth-child(${tdCount})`; selectors.push(selector, selector + ' *'); } diff --git a/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/ensureUniqueId.ts b/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/ensureUniqueId.ts index c52e9f0bc47..00bb8aaeabb 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/ensureUniqueId.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/ensureUniqueId.ts @@ -1,3 +1,5 @@ +import { getSafeIdSelector } from 'roosterjs-content-model-dom'; + /** * @internal */ @@ -7,7 +9,7 @@ export function ensureUniqueId(element: HTMLElement, idPrefix: string): string { const doc = element.ownerDocument; let i = 0; - while (!element.id || doc.querySelectorAll(`[id="${element.id}"]`).length > 1) { + while (!element.id || doc.querySelectorAll(getSafeIdSelector(element.id)).length > 1) { element.id = idPrefix + '_' + i++; } diff --git a/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/setEditorStyle.ts b/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/setEditorStyle.ts index a26a905e1f4..23255fa19b7 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/setEditorStyle.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/setEditorStyle.ts @@ -1,4 +1,5 @@ import { ensureUniqueId } from './ensureUniqueId'; +import { getSafeIdSelector } from 'roosterjs-content-model-dom'; import type { SetEditorStyle } from 'roosterjs-content-model-types'; const MAX_RULE_SELECTOR_LENGTH = 9000; @@ -34,7 +35,9 @@ export const setEditorStyle: SetEditorStyle = ( } if (cssRule) { - const rootSelector = '#' + ensureUniqueId(core.physicalRoot, CONTENT_DIV_ID); + const rootSelector = getSafeIdSelector( + ensureUniqueId(core.physicalRoot, CONTENT_DIV_ID) + ); const selectors = !subSelectors ? [rootSelector] : typeof subSelectors === 'string' diff --git a/packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts b/packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts index 26f8537f3c9..04da3ead0eb 100644 --- a/packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts +++ b/packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts @@ -529,7 +529,7 @@ describe('setDOMSelection', () => { createRangeSpy.and.returnValue(mockedRange); querySelectorAllSpy.and.callFake(selector => { - return selector == '[id="image_0"]' ? ['', ''] : ['']; + return selector == '#image_0' ? ['', ''] : ['']; }); hasFocusSpy.and.returnValue(false); @@ -573,6 +573,127 @@ describe('setDOMSelection', () => { ['*::selection'] ); }); + + it('image selection with duplicated and unsupported id', () => { + const mockedSelection = { + type: 'image', + image: mockedImage, + } as any; + const selectNodeSpy = jasmine.createSpy('selectNode'); + const collapseSpy = jasmine.createSpy('collapse'); + const mockedRange = { + selectNode: selectNodeSpy, + collapse: collapseSpy, + }; + + mockedImage.id = '0_image_0'; + createRangeSpy.and.returnValue(mockedRange); + + querySelectorAllSpy.and.callFake(selector => { + return selector == '[id="0_image_0"]' ? ['', ''] : ['']; + }); + hasFocusSpy.and.returnValue(false); + + setDOMSelection(core, mockedSelection); + + expect(core.selection).toEqual({ + skipReselectOnFocus: undefined, + selection: mockedSelection, + imageSelectionBorderColor: DEFAULT_SELECTION_BORDER_COLOR, + tableCellSelectionBackgroundColor: DEFAULT_TABLE_CELL_SELECTION_BACKGROUND_COLOR, + } as any); + expect(triggerEventSpy).toHaveBeenCalledWith( + core, + { + eventType: 'selectionChanged', + newSelection: mockedSelection, + }, + true + ); + expect(selectNodeSpy).toHaveBeenCalledWith(mockedImage); + expect(collapseSpy).not.toHaveBeenCalledWith(); + expect(addRangeToSelectionSpy).toHaveBeenCalledWith(doc, mockedRange, undefined); + expect(setEditorStyleSpy).toHaveBeenCalledTimes(5); + expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelection', null); + expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelectionHideCursor', null); + expect(setEditorStyleSpy).toHaveBeenCalledWith( + core, + '_DOMSelectionHideSelection', + null + ); + expect(setEditorStyleSpy).toHaveBeenCalledWith( + core, + '_DOMSelection', + 'outline-style:solid!important; outline-color:#DB626C!important;', + ['[id="0_image_0_0"]'] + ); + expect(setEditorStyleSpy).toHaveBeenCalledWith( + core, + '_DOMSelectionHideSelection', + 'background-color: transparent !important;', + ['*::selection'] + ); + }); + + it('image selection with unsupported id', () => { + mockedImage.id = '0'; + const mockedSelection = { + type: 'image', + image: mockedImage, + } as any; + const selectNodeSpy = jasmine.createSpy('selectNode'); + const collapseSpy = jasmine.createSpy('collapse'); + const mockedRange = { + selectNode: selectNodeSpy, + collapse: collapseSpy, + }; + + createRangeSpy.and.returnValue(mockedRange); + + querySelectorAllSpy.and.returnValue([]); + hasFocusSpy.and.returnValue(false); + + setDOMSelection(core, mockedSelection); + + expect(core.selection).toEqual({ + skipReselectOnFocus: undefined, + selection: mockedSelection, + imageSelectionBorderColor: DEFAULT_SELECTION_BORDER_COLOR, + tableCellSelectionBackgroundColor: DEFAULT_TABLE_CELL_SELECTION_BACKGROUND_COLOR, + } as any); + expect(triggerEventSpy).toHaveBeenCalledWith( + core, + { + eventType: 'selectionChanged', + newSelection: mockedSelection, + }, + true + ); + expect(selectNodeSpy).toHaveBeenCalledWith(mockedImage); + expect(collapseSpy).not.toHaveBeenCalledWith(); + expect(addRangeToSelectionSpy).toHaveBeenCalledWith(doc, mockedRange, undefined); + expect(mockedImage.id).toBe('0'); + expect(setEditorStyleSpy).toHaveBeenCalledTimes(5); + expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelection', null); + expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelectionHideCursor', null); + expect(setEditorStyleSpy).toHaveBeenCalledWith( + core, + '_DOMSelectionHideSelection', + null + ); + expect(setEditorStyleSpy).toHaveBeenCalledWith( + core, + '_DOMSelection', + 'outline-style:solid!important; outline-color:#DB626C!important;', + ['[id="0"]'] + ); + expect(setEditorStyleSpy).toHaveBeenCalledWith( + core, + '_DOMSelectionHideSelection', + 'background-color: transparent !important;', + ['*::selection'] + ); + }); }); describe('Table selection', () => { @@ -634,7 +755,8 @@ describe('setDOMSelection', () => { lastRow: number, result: string[], selectionColor?: string, - expectedDarkSelectionColor?: string + expectedDarkSelectionColor?: string, + expectedId?: string ) { const mockedSelection = { type: 'table', @@ -676,7 +798,7 @@ describe('setDOMSelection', () => { }, true ); - expect(mockedTable.id).toBe('table_0'); + expect(mockedTable.id).toBe(expectedId || 'table_0'); expect(setEditorStyleSpy).toHaveBeenCalledTimes(5); expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelection', null); expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelectionHideCursor', null); @@ -711,6 +833,27 @@ describe('setDOMSelection', () => { ]); }); + it('Select Table Cells TR under Table Tag with unsupportedId', () => { + const table = buildTable(true); + table.id = '0'; + runTest( + table, + 1, + 0, + 1, + 1, + [ + '[id="0"]>TBODY> tr:nth-child(1)>TD:nth-child(2)', + '[id="0"]>TBODY> tr:nth-child(1)>TD:nth-child(2) *', + '[id="0"]>TBODY> tr:nth-child(2)>TD:nth-child(2)', + '[id="0"]>TBODY> tr:nth-child(2)>TD:nth-child(2) *', + ], + undefined /* selectionColor */, + undefined /* expectedDarkSelectionColor */, + '0' + ); + }); + it('Select Table Cells TBODY', () => { runTest(buildTable(false), 0, 0, 0, 1, [ '#table_0> tr:nth-child(1)>TD:nth-child(1)', @@ -891,6 +1034,22 @@ describe('setDOMSelection', () => { ]); }); + it('Select All with unsupported Id', () => { + const table = buildTable(true /* tbody */, false, false); + table.id = '0'; + runTest( + table, + 0, + 0, + 1, + 1, + ['[id="0"]', '[id="0"] *'], + undefined /* selectionColor */, + undefined /* expectedDarkSelectionColor */, + '0' + ); + }); + it('Select All with custom selection color', () => { const selectionColor = 'red'; core.selection.tableCellSelectionBackgroundColor = selectionColor; diff --git a/packages/roosterjs-content-model-core/test/coreApi/setEditorStyle/ensureUniqueIdTest.ts b/packages/roosterjs-content-model-core/test/coreApi/setEditorStyle/ensureUniqueIdTest.ts index 0237af7ef8e..499ceee73fb 100644 --- a/packages/roosterjs-content-model-core/test/coreApi/setEditorStyle/ensureUniqueIdTest.ts +++ b/packages/roosterjs-content-model-core/test/coreApi/setEditorStyle/ensureUniqueIdTest.ts @@ -38,13 +38,26 @@ describe('ensureUniqueId', () => { id: 'dup', } as any; querySelectorAllSpy.and.callFake((selector: string) => - selector == '[id="dup"]' ? [{}, {}] : [] + selector == '#dup' ? [{}, {}] : [] ); const result = ensureUniqueId(element, 'prefix'); expect(result).toBe('dup_0'); }); + it('Has duplicated and unsupported id', () => { + const element = { + ownerDocument: doc, + id: '0dup', + } as any; + querySelectorAllSpy.and.callFake((selector: string) => + selector == '[id="0dup"]' ? [{}, {}] : [] + ); + const result = ensureUniqueId(element, 'prefix'); + + expect(result).toBe('0dup_0'); + }); + it('Should not throw when element id starts with number', () => { const element = { ownerDocument: doc, diff --git a/packages/roosterjs-content-model-dom/lib/domUtils/getSafeIdSelector.ts b/packages/roosterjs-content-model-dom/lib/domUtils/getSafeIdSelector.ts new file mode 100644 index 00000000000..941c42f1c75 --- /dev/null +++ b/packages/roosterjs-content-model-dom/lib/domUtils/getSafeIdSelector.ts @@ -0,0 +1,20 @@ +const StartsWithUnsupportedCharacter = /^[.|\-|_|\d]/; + +/** + * Returns a safe Id to use in Native APIs. + * IDs that start with number or hyphen can throw errors if used. + * @see https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id + * @param id + * @returns + */ +export function getSafeIdSelector(id: string) { + if (!id) { + return id; + } + + if (id.match(StartsWithUnsupportedCharacter)) { + return `[id="${id}"]`; + } else { + return `#${id}`; + } +} diff --git a/packages/roosterjs-content-model-dom/lib/index.ts b/packages/roosterjs-content-model-dom/lib/index.ts index 10dc2a755ef..12077de7df0 100644 --- a/packages/roosterjs-content-model-dom/lib/index.ts +++ b/packages/roosterjs-content-model-dom/lib/index.ts @@ -19,6 +19,7 @@ export { updateMetadata, getMetadata, hasMetadata } from './modelApi/metadata/up export { isNodeOfType } from './domUtils/isNodeOfType'; export { isElementOfType } from './domUtils/isElementOfType'; export { getObjectKeys } from './domUtils/getObjectKeys'; +export { getSafeIdSelector } from './domUtils/getSafeIdSelector'; export { toArray } from './domUtils/toArray'; export { moveChildNodes, wrapAllChildNodes } from './domUtils/moveChildNodes'; export { wrap } from './domUtils/wrap'; diff --git a/packages/roosterjs-content-model-dom/test/domUtils/getSafeIdSelectorTest.ts b/packages/roosterjs-content-model-dom/test/domUtils/getSafeIdSelectorTest.ts new file mode 100644 index 00000000000..34afbbb072f --- /dev/null +++ b/packages/roosterjs-content-model-dom/test/domUtils/getSafeIdSelectorTest.ts @@ -0,0 +1,29 @@ +import { getSafeIdSelector } from '../../lib/domUtils/getSafeIdSelector'; + +describe('getSafeIdSelector', () => { + it('emptyString', () => { + const result = getSafeIdSelector(''); + + expect(result).toEqual(''); + }); + it('Valid', () => { + const result = getSafeIdSelector('Test'); + + expect(result).toEqual('#Test'); + }); + it('Invalid with number', () => { + const result = getSafeIdSelector('0_Test'); + + expect(result).toEqual('[id="0_Test"]'); + }); + it('Invalid with dash', () => { + const result = getSafeIdSelector('-Test'); + + expect(result).toEqual('[id="-Test"]'); + }); + it('Invalid with underscore', () => { + const result = getSafeIdSelector('_Test'); + + expect(result).toEqual('[id="_Test"]'); + }); +}); diff --git a/packages/roosterjs-content-model-plugins/lib/imageEdit/ImageEditPlugin.ts b/packages/roosterjs-content-model-plugins/lib/imageEdit/ImageEditPlugin.ts index 35cef77486a..22d57145b1d 100644 --- a/packages/roosterjs-content-model-plugins/lib/imageEdit/ImageEditPlugin.ts +++ b/packages/roosterjs-content-model-plugins/lib/imageEdit/ImageEditPlugin.ts @@ -14,8 +14,8 @@ import { Resizer } from './Resizer/resizerContext'; import { Rotator } from './Rotator/rotatorContext'; import { updateRotateHandle } from './Rotator/updateRotateHandle'; import { updateWrapper } from './utils/updateWrapper'; - import { + getSafeIdSelector, isElementOfType, isModifierKey, isNodeOfType, @@ -347,7 +347,7 @@ export class ImageEditPlugin implements ImageEditor, EditorPlugin { this.zoomScale = editor.getDOMHelper().calculateZoomScale(); editor.setEditorStyle('imageEdit', `outline-style:none!important;`, [ - `span:has(>img#${this.selectedImage.id})`, + `span:has(>img${getSafeIdSelector(this.selectedImage.id)})`, ]); } diff --git a/packages/roosterjs-content-model-plugins/test/imageEdit/ImageEditPluginTest.ts b/packages/roosterjs-content-model-plugins/test/imageEdit/ImageEditPluginTest.ts index 8ab5af40299..fd16083deef 100644 --- a/packages/roosterjs-content-model-plugins/test/imageEdit/ImageEditPluginTest.ts +++ b/packages/roosterjs-content-model-plugins/test/imageEdit/ImageEditPluginTest.ts @@ -298,4 +298,55 @@ describe('ImageEditPlugin', () => { expect(dataset).toBeTruthy(); plugin.dispose(); }); + + it('flip with unsupportedId', () => { + const modelWithUnsupportedId: ContentModelDocument = { + blockGroupType: 'Document', + blocks: [ + { + blockType: 'Paragraph', + segments: [ + { + segmentType: 'Image', + src: 'test', + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + id: '0', + maxWidth: '1800px', + }, + dataset: {}, + isSelectedAsImageSelection: true, + isSelected: true, + }, + ], + format: {}, + segmentFormat: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + }, + }, + ], + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: '#000000', + }, + }; + const plugin = new ImageEditPlugin(); + const editor = initEditor('image_edit', [plugin], modelWithUnsupportedId); + spyOn(editor, 'setEditorStyle').and.callThrough(); + + plugin.initialize(editor); + plugin.flipImage('horizontal'); + plugin.dispose(); + + expect(editor.setEditorStyle).toHaveBeenCalledWith( + 'imageEdit', + 'outline-style:none!important;', + ['span:has(>img[id="0"])'] + ); + }); });