Create getSafeIdSelector and replace all Id selectors with it. (#2747)

* init

* Add other occurence

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) {

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 + ' *');
                 }

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++;
     }
 

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'

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;

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,

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}`;
+    }
+}

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';