From f574992549b7fb2ec6d4c478018ff094dc35436a Mon Sep 17 00:00:00 2001 From: Bryan Valverde U Date: Tue, 21 Jan 2025 08:54:58 -0600 Subject: [PATCH 1/3] Fix unhandled exception when using Regex in long Excel content and additional approach to get table from clipboard (#2926) * Try fic * Refactor processPastedContentFromExcel to destructure event properties --- .../Excel/processPastedContentFromExcel.ts | 81 ++++++++++++++----- .../test/paste/validateExcelFragmentTest.ts | 48 +++++++++++ 2 files changed, 110 insertions(+), 19 deletions(-) create mode 100644 packages/roosterjs-content-model-plugins/test/paste/validateExcelFragmentTest.ts diff --git a/packages/roosterjs-content-model-plugins/lib/paste/Excel/processPastedContentFromExcel.ts b/packages/roosterjs-content-model-plugins/lib/paste/Excel/processPastedContentFromExcel.ts index 5bbc167fe76..08a6e61a69f 100644 --- a/packages/roosterjs-content-model-plugins/lib/paste/Excel/processPastedContentFromExcel.ts +++ b/packages/roosterjs-content-model-plugins/lib/paste/Excel/processPastedContentFromExcel.ts @@ -1,13 +1,19 @@ import { addParser } from '../utils/addParser'; import { isNodeOfType, moveChildNodes } from 'roosterjs-content-model-dom'; import { setProcessor } from '../utils/setProcessor'; -import type { BeforePasteEvent, DOMCreator, ElementProcessor } from 'roosterjs-content-model-types'; +import type { + BeforePasteEvent, + ClipboardData, + DOMCreator, + ElementProcessor, +} from 'roosterjs-content-model-types'; const LAST_TD_END_REGEX = /<\/\s*td\s*>((?!<\/\s*tr\s*>)[\s\S])*$/i; const LAST_TR_END_REGEX = /<\/\s*tr\s*>((?!<\/\s*table\s*>)[\s\S])*$/i; const LAST_TR_REGEX = /]*>[^<]*/i; const LAST_TABLE_REGEX = /]*>[^<]*/i; const DEFAULT_BORDER_STYLE = 'solid 1px #d4d4d4'; +const TABLE_SELECTOR = 'table'; /** * @internal @@ -20,13 +26,9 @@ export function processPastedContentFromExcel( domCreator: DOMCreator, allowExcelNoBorderTable?: boolean ) { - const { fragment, htmlBefore, clipboardData } = event; - const html = clipboardData.html ? excelHandler(clipboardData.html, htmlBefore) : undefined; + const { fragment, htmlBefore, htmlAfter, clipboardData } = event; - if (html && clipboardData.html != html) { - const doc = domCreator.htmlToDOM(html); - moveChildNodes(fragment, doc?.body); - } + validateExcelFragment(fragment, domCreator, htmlBefore, clipboardData, htmlAfter); // For Excel Online const firstChild = fragment.firstChild; @@ -86,22 +88,63 @@ export const childProcessor: ElementProcessor = (group, element, con } }; +/** + * @internal + * Exported only for unit test + */ +export function validateExcelFragment( + fragment: DocumentFragment, + domCreator: DOMCreator, + htmlBefore: string, + clipboardData: ClipboardData, + htmlAfter: string +) { + // Clipboard content of Excel may contain the and EndFragment comment tags inside the table + // + // @example + // + // + // ... + // + //
+ // + // This causes that the fragment is not properly created and the table is not extracted. + // The content that is before the StartFragment is htmlBefore and the content that is after the EndFragment is htmlAfter. + // So attempt to create a new document fragment with the content of htmlBefore + clipboardData.html + htmlAfter + // If a table is found, replace the fragment with the new fragment + const result = + !fragment.querySelector(TABLE_SELECTOR) && + domCreator.htmlToDOM(htmlBefore + clipboardData.html + htmlAfter); + if (result && result.querySelector(TABLE_SELECTOR)) { + moveChildNodes(fragment, result?.body); + } else { + // If the table is still not found, try to extract the table from the clipboard data using Regex + const html = clipboardData.html ? excelHandler(clipboardData.html, htmlBefore) : undefined; + + if (html && clipboardData.html != html) { + const doc = domCreator.htmlToDOM(html); + moveChildNodes(fragment, doc?.body); + } + } +} + /** * @internal Export for test only * @param html Source html */ - export function excelHandler(html: string, htmlBefore: string): string { - if (html.match(LAST_TD_END_REGEX)) { - const trMatch = htmlBefore.match(LAST_TR_REGEX); - const tr = trMatch ? trMatch[0] : ''; - html = tr + html + ''; - } - if (html.match(LAST_TR_END_REGEX)) { - const tableMatch = htmlBefore.match(LAST_TABLE_REGEX); - const table = tableMatch ? tableMatch[0] : ''; - html = table + html + '
'; + try { + if (html.match(LAST_TD_END_REGEX)) { + const trMatch = htmlBefore.match(LAST_TR_REGEX); + const tr = trMatch ? trMatch[0] : ''; + html = tr + html + ''; + } + if (html.match(LAST_TR_END_REGEX)) { + const tableMatch = htmlBefore.match(LAST_TABLE_REGEX); + const table = tableMatch ? tableMatch[0] : ''; + html = table + html + '
'; + } + } finally { + return html; } - - return html; } diff --git a/packages/roosterjs-content-model-plugins/test/paste/validateExcelFragmentTest.ts b/packages/roosterjs-content-model-plugins/test/paste/validateExcelFragmentTest.ts new file mode 100644 index 00000000000..8bb38d34982 --- /dev/null +++ b/packages/roosterjs-content-model-plugins/test/paste/validateExcelFragmentTest.ts @@ -0,0 +1,48 @@ +import { ClipboardData, DOMCreator } from 'roosterjs-content-model-types'; +import { validateExcelFragment } from '../../lib/paste/Excel/processPastedContentFromExcel'; + +describe('validateExcelFragment', () => { + let domCreator: DOMCreator; + let fragment: DocumentFragment; + let clipboardData: ClipboardData; + let htmlToDomSpy: jasmine.Spy; + + beforeEach(() => { + htmlToDomSpy = jasmine.createSpy(); + htmlToDomSpy.and.callFake((html: string) => { + return new DOMParser().parseFromString(html, 'text/html'); + }); + + domCreator = { + htmlToDOM: htmlToDomSpy, + }; + fragment = document.createDocumentFragment(); + clipboardData = { + html: '', + } as ClipboardData; + }); + + it('should replace fragment with new fragment containing table from combined htmlBefore, clipboardData.html, and htmlAfter', () => { + const htmlBefore = ''; + const htmlAfter = '
'; + clipboardData.html = 'Test'; + + validateExcelFragment(fragment, domCreator, htmlBefore, clipboardData, htmlAfter); + + expect(fragment.querySelector('table')).not.toBeNull(); + expect(domCreator.htmlToDOM).toHaveBeenCalledWith( + htmlBefore + clipboardData.html + htmlAfter + ); + }); + + it('should use excelHandler to extract table from clipboard data if not found initially', () => { + const htmlBefore = ''; + const htmlAfter = ''; + clipboardData.html = 'Test'; + + validateExcelFragment(fragment, domCreator, htmlBefore, clipboardData, htmlAfter); + + expect(fragment.querySelector('table')).not.toBeNull(); + expect(domCreator.htmlToDOM).toHaveBeenCalledTimes(2); + }); +}); From 54c7dbe316cc2a6ccc986490390a2dcc954dfece Mon Sep 17 00:00:00 2001 From: Rain-Zheng <67583056+Rain-Zheng@users.noreply.github.com> Date: Wed, 22 Jan 2025 02:14:05 +0800 Subject: [PATCH 2/3] Skip CM handling when deleting expanded selection within text node (#2925) * handling expanded selection on Backspace/Delete key * nit * update --------- Co-authored-by: Jiuqing Song --- .../lib/edit/EditPlugin.ts | 17 +++++-- .../lib/edit/keyboardDelete.ts | 20 ++++++-- .../test/edit/EditPluginTest.ts | 28 ++++++++--- .../test/edit/keyboardDeleteTest.ts | 50 +++++++++++++++++++ 4 files changed, 100 insertions(+), 15 deletions(-) diff --git a/packages/roosterjs-content-model-plugins/lib/edit/EditPlugin.ts b/packages/roosterjs-content-model-plugins/lib/edit/EditPlugin.ts index d0c1263a2c1..481fdabfe16 100644 --- a/packages/roosterjs-content-model-plugins/lib/edit/EditPlugin.ts +++ b/packages/roosterjs-content-model-plugins/lib/edit/EditPlugin.ts @@ -19,6 +19,12 @@ export type EditOptions = { * Whether to handle Tab key in keyboard. @default true */ handleTabKey?: boolean; + + /** + * Whether expanded selection within a text node should be handled by CM when pressing Backspace/Delete key. + * @default true + */ + handleExpandedSelectionOnDelete?: boolean; }; const BACKSPACE_KEY = 8; @@ -33,6 +39,7 @@ const DEAD_KEY = 229; const DefaultOptions: Partial = { handleTabKey: true, + handleExpandedSelectionOnDelete: true, }; /** @@ -164,7 +171,7 @@ export class EditPlugin implements EditorPlugin { case 'Backspace': // Use our API to handle BACKSPACE/DELETE key. // No need to clear cache here since if we rely on browser's behavior, there will be Input event and its handler will reconcile cache - keyboardDelete(editor, rawEvent); + keyboardDelete(editor, rawEvent, this.options.handleExpandedSelectionOnDelete); break; case 'Delete': @@ -172,7 +179,7 @@ export class EditPlugin implements EditorPlugin { // No need to clear cache here since if we rely on browser's behavior, there will be Input event and its handler will reconcile cache // And leave it to browser when shift key is pressed so that browser will trigger cut event if (!event.rawEvent.shiftKey) { - keyboardDelete(editor, rawEvent); + keyboardDelete(editor, rawEvent, this.options.handleExpandedSelectionOnDelete); } break; @@ -225,7 +232,8 @@ export class EditPlugin implements EditorPlugin { key: 'Backspace', keyCode: BACKSPACE_KEY, which: BACKSPACE_KEY, - }) + }), + this.options.handleExpandedSelectionOnDelete ); break; case 'deleteContentForward': @@ -235,7 +243,8 @@ export class EditPlugin implements EditorPlugin { key: 'Delete', keyCode: DELETE_KEY, which: DELETE_KEY, - }) + }), + this.options.handleExpandedSelectionOnDelete ); break; } diff --git a/packages/roosterjs-content-model-plugins/lib/edit/keyboardDelete.ts b/packages/roosterjs-content-model-plugins/lib/edit/keyboardDelete.ts index 44c73db93aa..544c980c543 100644 --- a/packages/roosterjs-content-model-plugins/lib/edit/keyboardDelete.ts +++ b/packages/roosterjs-content-model-plugins/lib/edit/keyboardDelete.ts @@ -27,13 +27,14 @@ import type { DOMSelection, DeleteSelectionStep, IEditor } from 'roosterjs-conte * Do keyboard event handling for DELETE/BACKSPACE key * @param editor The editor object * @param rawEvent DOM keyboard event + * @param handleExpandedSelection Whether to handle expanded selection within a text node by CM * @returns True if the event is handled by content model, otherwise false */ -export function keyboardDelete(editor: IEditor, rawEvent: KeyboardEvent) { +export function keyboardDelete(editor: IEditor, rawEvent: KeyboardEvent, handleExpandedSelection: boolean = true) { let handled = false; const selection = editor.getDOMSelection(); - if (shouldDeleteWithContentModel(selection, rawEvent)) { + if (shouldDeleteWithContentModel(selection, rawEvent, handleExpandedSelection)) { editor.formatContentModel( (model, context) => { const result = deleteSelection( @@ -80,11 +81,20 @@ function getDeleteSteps(rawEvent: KeyboardEvent, isMac: boolean): (DeleteSelecti ]; } -function shouldDeleteWithContentModel(selection: DOMSelection | null, rawEvent: KeyboardEvent) { +function shouldDeleteWithContentModel(selection: DOMSelection | null, rawEvent: KeyboardEvent, handleExpandedSelection: boolean) { if (!selection) { return false; // Nothing to delete - } else if (selection.type != 'range' || !selection.range.collapsed) { - return true; // Selection is not collapsed, need to delete all selections + } else if (selection.type != 'range') { + return true; + } else if (!selection.range.collapsed) { + if (handleExpandedSelection) { + return true; // Selection is not collapsed, need to delete all selections + } + + const range = selection.range; + const { startContainer, endContainer } = selection.range; + const isInSameTextNode = startContainer === endContainer && isNodeOfType(startContainer, 'TEXT_NODE'); + return !(isInSameTextNode && !isModifierKey(rawEvent) && range.endOffset - range.startOffset < (startContainer.nodeValue?.length ?? 0)); } else { const range = selection.range; diff --git a/packages/roosterjs-content-model-plugins/test/edit/EditPluginTest.ts b/packages/roosterjs-content-model-plugins/test/edit/EditPluginTest.ts index 7e94b272ad0..31ac523d30a 100644 --- a/packages/roosterjs-content-model-plugins/test/edit/EditPluginTest.ts +++ b/packages/roosterjs-content-model-plugins/test/edit/EditPluginTest.ts @@ -66,7 +66,7 @@ describe('EditPlugin', () => { rawEvent, }); - expect(keyboardDeleteSpy).toHaveBeenCalledWith(editor, rawEvent); + expect(keyboardDeleteSpy).toHaveBeenCalledWith(editor, rawEvent, true); expect(keyboardInputSpy).not.toHaveBeenCalled(); expect(keyboardEnterSpy).not.toHaveBeenCalled(); expect(keyboardTabSpy).not.toHaveBeenCalled(); @@ -83,7 +83,7 @@ describe('EditPlugin', () => { rawEvent, }); - expect(keyboardDeleteSpy).toHaveBeenCalledWith(editor, rawEvent); + expect(keyboardDeleteSpy).toHaveBeenCalledWith(editor, rawEvent, true); expect(keyboardInputSpy).not.toHaveBeenCalled(); expect(keyboardEnterSpy).not.toHaveBeenCalled(); expect(keyboardTabSpy).not.toHaveBeenCalled(); @@ -106,6 +106,20 @@ describe('EditPlugin', () => { expect(keyboardTabSpy).not.toHaveBeenCalled(); }); + it('handleExpandedSelectionOnDelete disabled', () => { + plugin = new EditPlugin({ handleExpandedSelectionOnDelete: false }); + const rawEvent = { key: 'Delete' } as any; + + plugin.initialize(editor); + + plugin.onPluginEvent({ + eventType: 'keyDown', + rawEvent, + }); + + expect(keyboardDeleteSpy).toHaveBeenCalledWith(editor, rawEvent, false); + }); + it('Tab', () => { plugin = new EditPlugin(); const rawEvent = { key: 'Tab' } as any; @@ -261,7 +275,7 @@ describe('EditPlugin', () => { expect(keyboardDeleteSpy).toHaveBeenCalledWith(editor, { key: 'Delete', - } as any); + } as any, true); plugin.onPluginEvent({ eventType: 'keyDown', @@ -271,7 +285,7 @@ describe('EditPlugin', () => { expect(keyboardDeleteSpy).toHaveBeenCalledTimes(2); expect(keyboardDeleteSpy).toHaveBeenCalledWith(editor, { key: 'Delete', - } as any); + } as any, true); expect(keyboardInputSpy).not.toHaveBeenCalled(); expect(keyboardEnterSpy).not.toHaveBeenCalled(); expect(keyboardTabSpy).not.toHaveBeenCalled(); @@ -309,7 +323,8 @@ describe('EditPlugin', () => { key: 'Backspace', keyCode: 8, which: 8, - }) + }), + true ); }); @@ -337,7 +352,8 @@ describe('EditPlugin', () => { key: 'Delete', keyCode: 46, which: 46, - }) + }), + true ); }); }); diff --git a/packages/roosterjs-content-model-plugins/test/edit/keyboardDeleteTest.ts b/packages/roosterjs-content-model-plugins/test/edit/keyboardDeleteTest.ts index 9678665979c..ed2e975a2a8 100644 --- a/packages/roosterjs-content-model-plugins/test/edit/keyboardDeleteTest.ts +++ b/packages/roosterjs-content-model-plugins/test/edit/keyboardDeleteTest.ts @@ -580,6 +580,31 @@ describe('keyboardDelete', () => { expect(formatWithContentModelSpy).not.toHaveBeenCalled(); }); + it('No need to delete - handleExpandedSelection disabled', () => { + const rawEvent = { key: 'Backspace' } as any; + const formatWithContentModelSpy = jasmine.createSpy('formatContentModel'); + const node = document.createTextNode('test'); + const range: DOMSelection = { + type: 'range', + range: ({ + collapsed: false, + startContainer: node, + endContainer: node, + startOffset: 1, + endOffset: 3, + } as any) as Range, + isReverted: false, + }; + const editor = { + formatContentModel: formatWithContentModelSpy, + getDOMSelection: () => range, + } as any; + + keyboardDelete(editor, rawEvent, false /* handleExpandedSelectionOnDelete */); + + expect(formatWithContentModelSpy).not.toHaveBeenCalled(); + }); + it('Backspace from the beginning', () => { const rawEvent = { key: 'Backspace' } as any; const formatWithContentModelSpy = jasmine.createSpy('formatContentModel'); @@ -625,4 +650,29 @@ describe('keyboardDelete', () => { expect(formatWithContentModelSpy).toHaveBeenCalledTimes(1); }); + + it('Delete all the content of text node - handleExpandedSelection disabled', () => { + const rawEvent = { key: 'Backspace' } as any; + const formatWithContentModelSpy = jasmine.createSpy('formatContentModel'); + const node = document.createTextNode('test'); + const range: DOMSelection = { + type: 'range', + range: ({ + collapsed: false, + startContainer: node, + endContainer: node, + startOffset: 0, + endOffset: 4, + } as any) as Range, + isReverted: false, + }; + const editor = { + formatContentModel: formatWithContentModelSpy, + getDOMSelection: () => range, + } as any; + + keyboardDelete(editor, rawEvent, false /* handleExpandedSelectionOnDelete */); + + expect(formatWithContentModelSpy).toHaveBeenCalledTimes(1); + }); }); From 6816792dd9c3e292ecf1521697330155ae9df09e Mon Sep 17 00:00:00 2001 From: Vi Nguyen <74168693+vinguyen12@users.noreply.github.com> Date: Fri, 24 Jan 2025 12:22:27 -0800 Subject: [PATCH 3/3] fix and change version --- versions.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/versions.json b/versions.json index 45c49bb7916..880a37dfb58 100644 --- a/versions.json +++ b/versions.json @@ -1,6 +1,6 @@ { "react": "9.0.1", - "main": "9.17.0", - "legacyAdapter": "8.63.0", + "main": "9.18.0", + "legacyAdapter": "8.63.1", "overrides": {} }