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 2d199b1889d..a89fee21134 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts @@ -2,7 +2,13 @@ import { addRangeToSelection } from './addRangeToSelection'; import { ensureUniqueId } from '../setEditorStyle/ensureUniqueId'; import { findLastedCoInMergedCell } from './findLastedCoInMergedCell'; import { findTableCellElement } from './findTableCellElement'; -import { isNodeOfType, parseTableCells, toArray } from 'roosterjs-content-model-dom'; +import { + isElementOfType, + isNodeOfType, + parseTableCells, + toArray, + wrap, +} from 'roosterjs-content-model-dom'; import type { ParsedTable, SelectionChangedEvent, @@ -18,7 +24,7 @@ const TABLE_ID = 'table'; const DEFAULT_SELECTION_BORDER_COLOR = '#DB626C'; const TABLE_CSS_RULE = 'background-color:#C6C6C6!important;'; const CARET_CSS_RULE = 'caret-color: transparent'; -const TRANSPARENT_SELECTION_CSS_RULE = 'background-color: transparent !important'; +const TRANSPARENT_SELECTION_CSS_RULE = 'background-color: transparent !important;'; const SELECTION_SELECTOR = '*::selection'; /** @@ -39,16 +45,19 @@ export const setDOMSelection: SetDOMSelection = (core, selection, skipSelectionC try { switch (selection?.type) { case 'image': - const image = selection.image; + const image = ensureImageHasSpanParent(selection.image); - core.selection.selection = selection; + core.selection.selection = { + type: 'image', + image, + }; core.api.setEditorStyle( core, DOM_SELECTION_CSS_KEY, `outline-style:auto!important; outline-color:${ core.selection.imageSelectionBorderColor || DEFAULT_SELECTION_BORDER_COLOR }!important;`, - [`#${ensureUniqueId(image, IMAGE_ID)}`] + [`span:has(>img#${ensureUniqueId(image, IMAGE_ID)})`] ); core.api.setEditorStyle( core, @@ -231,3 +240,20 @@ function setRangeSelection(doc: Document, element: HTMLElement | undefined, coll addRangeToSelection(doc, range, isReverted); } } + +function ensureImageHasSpanParent(image: HTMLImageElement): HTMLImageElement { + const parent = image.parentElement; + + if ( + parent && + isNodeOfType(parent, 'ELEMENT_NODE') && + isElementOfType(parent, 'span') && + parent.firstChild == image && + parent.lastChild == image + ) { + return image; + } + + wrap(image.ownerDocument, image, 'span'); + return image; +} 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 a2dd2548a3c..f1db9866d87 100644 --- a/packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts +++ b/packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts @@ -14,6 +14,8 @@ describe('setDOMSelection', () => { let doc: Document; let contentDiv: HTMLDivElement; let mockedRange = 'RANGE' as any; + let createElementSpy: jasmine.Spy; + let appendChildSpy: jasmine.Spy; beforeEach(() => { querySelectorAllSpy = jasmine.createSpy('querySelectorAll'); @@ -27,11 +29,16 @@ describe('setDOMSelection', () => { createRangeSpy = jasmine.createSpy('createRange'); setEditorStyleSpy = jasmine.createSpy('setEditorStyle'); containsSpy = jasmine.createSpy('contains').and.returnValue(true); + appendChildSpy = jasmine.createSpy('appendChild'); + createElementSpy = jasmine.createSpy('createElement').and.returnValue({ + appendChild: appendChildSpy, + }); doc = { querySelectorAll: querySelectorAllSpy, createRange: createRangeSpy, contains: containsSpy, + createElement: createElementSpy, } as any; contentDiv = { ownerDocument: doc, @@ -221,9 +228,14 @@ describe('setDOMSelection', () => { describe('Image selection', () => { let mockedImage: HTMLImageElement; - beforeEach(() => { mockedImage = { + parentElement: { + ownerDocument: doc, + firstElementChild: mockedImage, + lastElementChild: mockedImage, + appendChild: appendChildSpy, + }, ownerDocument: doc, } as any; }); @@ -265,6 +277,7 @@ describe('setDOMSelection', () => { expect(mockedImage.id).toBe('image_0'); expect(setEditorStyleSpy).toHaveBeenCalledTimes(5); expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelection', null); + expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelectionHideCursor', null); expect(setEditorStyleSpy).toHaveBeenCalledWith( core, '_DOMSelectionHideSelection', @@ -274,11 +287,17 @@ describe('setDOMSelection', () => { core, '_DOMSelection', 'outline-style:auto!important; outline-color:#DB626C!important;', - ['#image_0'] + ['span:has(>img#image_0)'] + ); + expect(setEditorStyleSpy).toHaveBeenCalledWith( + core, + '_DOMSelectionHideSelection', + 'background-color: transparent !important;', + ['*::selection'] ); }); - it('image selection with duplicated id', () => { + it('image selection with customized selection border color', () => { const mockedSelection = { type: 'image', image: mockedImage, @@ -290,12 +309,11 @@ describe('setDOMSelection', () => { collapse: collapseSpy, }; - mockedImage.id = 'image_0'; + core.selection.imageSelectionBorderColor = 'red'; + createRangeSpy.and.returnValue(mockedRange); - querySelectorAllSpy.and.callFake(selector => { - return selector == '#image_0' ? ['', ''] : ['']; - }); + querySelectorAllSpy.and.returnValue([]); hasFocusSpy.and.returnValue(false); setDOMSelection(core, mockedSelection); @@ -303,6 +321,7 @@ describe('setDOMSelection', () => { expect(core.selection).toEqual({ skipReselectOnFocus: undefined, selection: mockedSelection, + imageSelectionBorderColor: 'red', } as any); expect(triggerEventSpy).toHaveBeenCalledWith( core, @@ -317,6 +336,7 @@ describe('setDOMSelection', () => { 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', @@ -325,18 +345,18 @@ describe('setDOMSelection', () => { expect(setEditorStyleSpy).toHaveBeenCalledWith( core, '_DOMSelection', - 'outline-style:auto!important; outline-color:#DB626C!important;', - ['#image_0_0'] + 'outline-style:auto!important; outline-color:red!important;', + ['span:has(>img#image_0)'] ); expect(setEditorStyleSpy).toHaveBeenCalledWith( core, '_DOMSelectionHideSelection', - 'background-color: transparent !important', + 'background-color: transparent !important;', ['*::selection'] ); }); - it('image selection with customized selection border color', () => { + it('do not select if node is out of document', () => { const mockedSelection = { type: 'image', image: mockedImage, @@ -348,7 +368,7 @@ describe('setDOMSelection', () => { collapse: collapseSpy, }; - core.selection.imageSelectionBorderColor = 'red'; + doc.contains = () => false; createRangeSpy.and.returnValue(mockedRange); @@ -360,7 +380,6 @@ describe('setDOMSelection', () => { expect(core.selection).toEqual({ skipReselectOnFocus: undefined, selection: mockedSelection, - imageSelectionBorderColor: 'red', } as any); expect(triggerEventSpy).toHaveBeenCalledWith( core, @@ -370,9 +389,10 @@ describe('setDOMSelection', () => { }, true ); - expect(selectNodeSpy).toHaveBeenCalledWith(mockedImage); - expect(collapseSpy).not.toHaveBeenCalledWith(); - expect(addRangeToSelectionSpy).toHaveBeenCalledWith(doc, mockedRange, undefined); + expect(selectNodeSpy).not.toHaveBeenCalled(); + expect(collapseSpy).not.toHaveBeenCalled(); + expect(addRangeToSelectionSpy).not.toHaveBeenCalled(); + expect(mockedImage.id).toBe('image_0'); expect(setEditorStyleSpy).toHaveBeenCalledTimes(5); expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelection', null); expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelectionHideCursor', null); @@ -384,18 +404,18 @@ describe('setDOMSelection', () => { expect(setEditorStyleSpy).toHaveBeenCalledWith( core, '_DOMSelection', - 'outline-style:auto!important; outline-color:red!important;', - ['#image_0'] + 'outline-style:auto!important; outline-color:#DB626C!important;', + ['span:has(>img#image_0)'] ); expect(setEditorStyleSpy).toHaveBeenCalledWith( core, '_DOMSelectionHideSelection', - 'background-color: transparent !important', + 'background-color: transparent !important;', ['*::selection'] ); }); - it('do not select if node is out of document', () => { + it('image selection with duplicated id', () => { const mockedSelection = { type: 'image', image: mockedImage, @@ -407,11 +427,12 @@ describe('setDOMSelection', () => { collapse: collapseSpy, }; - doc.contains = () => false; - + mockedImage.id = 'image_0'; createRangeSpy.and.returnValue(mockedRange); - querySelectorAllSpy.and.returnValue([]); + querySelectorAllSpy.and.callFake(selector => { + return selector == '#image_0' ? ['', ''] : ['']; + }); hasFocusSpy.and.returnValue(false); setDOMSelection(core, mockedSelection); @@ -428,23 +449,27 @@ describe('setDOMSelection', () => { }, true ); - expect(selectNodeSpy).not.toHaveBeenCalled(); - expect(collapseSpy).not.toHaveBeenCalled(); - expect(addRangeToSelectionSpy).not.toHaveBeenCalled(); - expect(mockedImage.id).toBe('image_0'); + 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:auto!important; outline-color:#DB626C!important;', - ['#image_0'] + ['span:has(>img#image_0_0)'] ); expect(setEditorStyleSpy).toHaveBeenCalledWith( core, '_DOMSelectionHideSelection', - 'background-color: transparent !important', + 'background-color: transparent !important;', ['*::selection'] ); }); diff --git a/packages/roosterjs-content-model-dom/lib/modelToDom/optimizers/removeUnnecessarySpan.ts b/packages/roosterjs-content-model-dom/lib/modelToDom/optimizers/removeUnnecessarySpan.ts index ee048a395bb..3b0ccb11b88 100644 --- a/packages/roosterjs-content-model-dom/lib/modelToDom/optimizers/removeUnnecessarySpan.ts +++ b/packages/roosterjs-content-model-dom/lib/modelToDom/optimizers/removeUnnecessarySpan.ts @@ -8,7 +8,8 @@ export function removeUnnecessarySpan(root: Node) { if ( isNodeOfType(child, 'ELEMENT_NODE') && child.tagName == 'SPAN' && - child.attributes.length == 0 + child.attributes.length == 0 && + !isImageSpan(child) ) { const node = child; let refNode = child.nextSibling; @@ -26,3 +27,11 @@ export function removeUnnecessarySpan(root: Node) { } } } + +const isImageSpan = (child: HTMLElement) => { + return ( + isNodeOfType(child.firstChild, 'ELEMENT_NODE') && + child.firstChild.tagName == 'IMG' && + child.firstChild == child.lastChild + ); +}; diff --git a/packages/roosterjs-content-model-dom/test/modelToDom/optimizers/removeUnnecessarySpanTest.ts b/packages/roosterjs-content-model-dom/test/modelToDom/optimizers/removeUnnecessarySpanTest.ts index fd79446c9af..f2aab1c27e2 100644 --- a/packages/roosterjs-content-model-dom/test/modelToDom/optimizers/removeUnnecessarySpanTest.ts +++ b/packages/roosterjs-content-model-dom/test/modelToDom/optimizers/removeUnnecessarySpanTest.ts @@ -53,4 +53,13 @@ describe('removeUnnecessarySpan', () => { expect(div.innerHTML).toBe('test1test2test3'); }); + + it('Do not remove image span', () => { + const div = document.createElement('div'); + div.innerHTML = ''; + + removeUnnecessarySpan(div); + + expect(div.innerHTML).toBe(''); + }); });