From b18a8ab05d6100c667dea34e5fb8e4f2ab5e1eeb Mon Sep 17 00:00:00 2001 From: Antoine Guenet Date: Fri, 26 May 2023 11:32:41 +0200 Subject: [PATCH] [FIX] web_editor: handle selection at edges of links This replaces the link isolation mechanism with a more robust one that doesn't require setting the `contenteditable` attribute on the editable to `false` (which is rife with issues). In so doing, we also improve the handling of selection at the edges of links, making a clear distinction between the selection being inside the link or outside of it. Note: we only do this for collapsed selections, as it's not clear what should happen when the selection is not collapsed. task-3103566 Part-of: odoo/odoo#141303 --- .../js/editor/odoo-editor/src/OdooEditor.js | 222 +++++++++++------- .../odoo-editor/test/spec/copyPaste.test.js | 1 + .../odoo-editor/test/spec/editor.test.js | 1 + .../editor/odoo-editor/test/spec/link.test.js | 22 +- .../src/js/editor/odoo-editor/test/utils.js | 7 + .../static/src/js/wysiwyg/wysiwyg.js | 6 +- 6 files changed, 159 insertions(+), 100 deletions(-) diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js b/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js index 13767c10b2c..650c79cf0b3 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js @@ -338,6 +338,8 @@ export class OdooEditor extends EventTarget { // Set contenteditable before clone as FF updates the content at this point. this._activateContenteditable(); + this._setLinkZws(); + this._collabClientId = this.options.collaborationClientId; this._collabClientAvatarUrl = this.options.collaborationClientAvatarUrl; @@ -1275,6 +1277,7 @@ export class OdooEditor extends EventTarget { if (!this._historyStepsActive) { return; } + this._setLinkZws(); this.sanitize(); // check that not two unBreakables modified if (this._toRollback) { @@ -1517,9 +1520,6 @@ export class OdooEditor extends EventTarget { } } if (sideEffect) { - if (!this._fixLinkMutatedElements) { - this._activateContenteditable(); - } this.historySetSelection(step); } } @@ -1977,26 +1977,66 @@ export class OdooEditor extends EventTarget { } } - setContenteditableLink(link) { - const editableChildren = link.querySelectorAll('[contenteditable=true]'); - this._fixLinkMutatedElements = { - link, - wasContenteditableTrue: [...editableChildren], - wasContenteditableFalse: [], - wasContenteditableNull: [], - }; - this._stopContenteditable(); - - const contentEditableAttribute = link.getAttribute('contenteditable'); - if (contentEditableAttribute === 'true') { - this._fixLinkMutatedElements.wasContenteditableTrue.push(link); - } else if (contentEditableAttribute === 'false') { - this._fixLinkMutatedElements.wasContenteditableFalse.push(link); - } else { - this._fixLinkMutatedElements.wasContenteditableNull.push(link); + _setLinkZws() { + this._resetLinkZws(); + const selection = this.document.getSelection(); + if (!selection.isCollapsed) { + return; + } + const linkInSelection = getInSelection(this.document, 'a'); + const isLinkSelection = selection.anchorNode === linkInSelection; + let commonAncestorContainer = selection.rangeCount && selection.getRangeAt(0).commonAncestorContainer; + if (commonAncestorContainer) { + // Consider all the links in the closest block that contains the + // whole selection, limiting to the editable. + if (!this.editable.contains(commonAncestorContainer)) { + commonAncestorContainer = this.editable; + } + let block = closestBlock(commonAncestorContainer); + if (!block || !this.editable.contains(block)) { + block = this.editable; + } + let links = [...block.querySelectorAll('a')]; + // Consider the links at the edges of the sibling blocks, limiting + // to the editable. + if (this.editable.contains(block)) { + links.push( + closestElement(previousLeaf(block, this.editable, true), 'a'), + closestElement(nextLeaf(block, this.editable, true), 'a'), + ); + } + const offset = selection.anchorOffset; + let didAddZwsInLinkInSelection = false; + for (const link of links) { + if ( + link && + !isBlock(link) && + link.textContent !== '' && + !( + // Ignore links wrapped around a single image. + link.children.length === 1 && + link.firstElementChild.nodeName === 'IMG' + ) + ) { + link.prepend(this._createLinkZws('start')); + // Only add the ZWS at the end if the link is in selection. + if (link === linkInSelection) { + link.append(this._createLinkZws('end')); + didAddZwsInLinkInSelection = true; + } + const zwsAfter = this._createLinkZws('after'); + link.after(zwsAfter); + if (!zwsAfter.parentElement || !zwsAfter.parentElement.isContentEditable) { + zwsAfter.remove(); + } + } + } + if (isLinkSelection && offset && didAddZwsInLinkInSelection) { + // Correct the offset if the link is in selection, to account + // for the added ZWS. + setSelection(linkInSelection, Math.min(offset + 1, linkInSelection.childNodes.length)); + } } - - [...editableChildren, link].forEach(node => node.setAttribute('contenteditable', true)); } /** @@ -2405,6 +2445,7 @@ export class OdooEditor extends EventTarget { // Do not apply commands out of the editable area. return false; } + this._resetLinkZws(); if (!sel.isCollapsed && BACKSPACE_FIRST_COMMANDS.includes(method)) { let range = getDeepRange(this.editable, {sel, splitText: true, select: true, correctTripleClick: true}); if (range && @@ -2426,19 +2467,7 @@ export class OdooEditor extends EventTarget { } } if (editorCommands[method]) { - // Make sure to restore the content editable before applying an - // editor command, as it might have been temporarily disabled for - // browser behaviors which should not concern editor commands. - const link = this._fixLinkMutatedElements && this._fixLinkMutatedElements.link; - if (this._fixLinkMutatedElements) { - this.resetContenteditableLink(); - this._activateContenteditable(); - } - const returnValue = editorCommands[method](this, ...args); - if (link) { - this.setContenteditableLink(link); - } - return returnValue; + return editorCommands[method](this, ...args); } if (method.startsWith('justify')) { const mode = method.split('justify').join('').toLocaleLowerCase(); @@ -2488,19 +2517,8 @@ export class OdooEditor extends EventTarget { } } } - resetContenteditableLink() { - if (this._fixLinkMutatedElements) { - for (const element of this._fixLinkMutatedElements.wasContenteditableTrue) { - element.setAttribute('contenteditable', 'true'); - } - for (const element of this._fixLinkMutatedElements.wasContenteditableFalse) { - element.setAttribute('contenteditable', 'false'); - } - for (const element of this._fixLinkMutatedElements.wasContenteditableNull) { - element.removeAttribute('contenteditable'); - } - delete this._fixLinkMutatedElements; - } + _resetLinkZws(element = this.editable) { + element.querySelectorAll('[data-o-link-zws]').forEach(zws => zws.remove()); } _activateContenteditable() { this.observerUnactive('_activateContenteditable'); @@ -3467,6 +3485,15 @@ export class OdooEditor extends EventTarget { } this.observer.takeRecords(); } + _createLinkZws(side) { + const span = document.createElement('span'); + span.setAttribute('data-o-link-zws', side); + if (side !== 'end') { + span.setAttribute('contenteditable', 'false'); + } + span.textContent = '\u200B'; + return span; + } disableAvatarForElement(element) { this.enableAvatars(); @@ -3526,6 +3553,7 @@ export class OdooEditor extends EventTarget { ev.inputType === 'insertText' && ev.data === null && this._lastBeforeInputType === 'insertParagraph'; + this._resetLinkZws(); if (this.keyboardType === KEYBOARD_TYPES.PHYSICAL || !wasCollapsed) { if (ev.inputType === 'deleteContentBackward') { this._compositionStep(); @@ -3543,19 +3571,18 @@ export class OdooEditor extends EventTarget { ev.preventDefault(); this._handleAutomaticLinkInsertion(); if (this._applyCommand('oEnter') === UNBREAKABLE_ROLLBACK_CODE) { - const brs = this._applyCommand('oShiftEnter'); + const brs = this._applyRawCommand('oShiftEnter'); const anchor = brs[0].parentElement; if (anchor.nodeName === 'A') { if (brs.includes(anchor.firstChild)) { brs.forEach(br => anchor.before(br)); setSelection(...rightPos(brs[brs.length - 1])); - this.historyStep(); } else if (brs.includes(anchor.lastChild)) { brs.forEach(br => anchor.after(br)); setSelection(...rightPos(brs[0])); - this.historyStep(); } } + this.historyStep(); } } else if (['insertText', 'insertCompositionText'].includes(ev.inputType)) { // insertCompositionText, courtesy of Samsung keyboard. @@ -3750,7 +3777,9 @@ export class OdooEditor extends EventTarget { if (/^.$/u.test(ev.key) && !ev.ctrlKey && !ev.metaKey && (isMacOS() || !ev.altKey)) { const selection = this.document.getSelection(); if (selection && !selection.isCollapsed) { + this._resetLinkZws(); this.deleteRange(selection); + this._setLinkZws(); } } if (ev.key === 'Backspace') { @@ -3904,13 +3933,15 @@ export class OdooEditor extends EventTarget { ev.stopPropagation(); this.execCommand('strikeThrough'); } else if (IS_KEYBOARD_EVENT_LEFT_ARROW(ev)) { - getDeepRange(this.editable); - const selection = this.document.getSelection(); - // Find previous character. - let { focusNode, focusOffset } = selection; + if (ev.shiftKey) { + this._resetLinkZws(); + } + getDeepRange(this.editable, { select: true }); + let { anchorNode, anchorOffset, focusNode, focusOffset } = this.document.getSelection(); if (!focusNode) { return; } + // Find previous character. let previousCharacter = focusOffset > 0 && focusNode.textContent[focusOffset - 1]; if (!previousCharacter) { focusNode = previousLeaf(focusNode); @@ -3918,24 +3949,26 @@ export class OdooEditor extends EventTarget { previousCharacter = focusNode.textContent[focusOffset - 1]; } // Move selection if previous character is zero-width space - if (previousCharacter === '\u200B') { + if (previousCharacter === '\u200B' && !focusNode.parentElement.hasAttribute('data-o-link-zws')) { focusOffset -= 1; while (focusNode && (focusOffset < 0 || !focusNode.textContent[focusOffset])) { focusNode = nextLeaf(focusNode); focusOffset = focusNode && nodeSize(focusNode); } - const startContainer = ev.shiftKey ? selection.anchorNode : focusNode; - const startOffset = ev.shiftKey ? selection.anchorOffset : focusOffset; + const startContainer = ev.shiftKey ? anchorNode : focusNode; + const startOffset = ev.shiftKey ? anchorOffset : focusOffset; setSelection(startContainer, startOffset, focusNode, focusOffset); } } else if (IS_KEYBOARD_EVENT_RIGHT_ARROW(ev)) { - getDeepRange(this.editable); - const selection = this.document.getSelection(); - // Find next character. - let { focusNode, focusOffset } = selection; + if (ev.shiftKey) { + this._resetLinkZws(); + } + getDeepRange(this.editable, { select: true }); + let { anchorNode, anchorOffset, focusNode, focusOffset } = this.document.getSelection(); if (!focusNode) { return; } + // Find next character. let nextCharacter = focusNode.textContent[focusOffset]; if (!nextCharacter) { focusNode = nextLeaf(focusNode); @@ -3943,7 +3976,7 @@ export class OdooEditor extends EventTarget { nextCharacter = focusNode.textContent[focusOffset]; } // Move selection if next character is zero-width space - if (nextCharacter === '\u200B') { + if (nextCharacter === '\u200B' && !focusNode.parentElement.hasAttribute('data-o-link-zws')) { focusOffset += 1; let newFocusNode = focusNode; while (newFocusNode && (!newFocusNode.textContent[focusOffset] || !closestElement(newFocusNode).isContentEditable)) { @@ -3954,8 +3987,8 @@ export class OdooEditor extends EventTarget { newFocusNode = focusNode; // Do not move selection to next block. focusOffset = nodeSize(focusNode); } - const startContainer = ev.shiftKey ? selection.anchorNode : newFocusNode; - const startOffset = ev.shiftKey ? selection.anchorOffset : focusOffset; + const startContainer = ev.shiftKey ? anchorNode : newFocusNode; + const startOffset = ev.shiftKey ? anchorOffset : focusOffset; setSelection(startContainer, startOffset, newFocusNode, focusOffset); } } @@ -3987,6 +4020,41 @@ export class OdooEditor extends EventTarget { let appliedCustomSelection = false; if (selection.rangeCount && selection.getRangeAt(0)) { appliedCustomSelection = this._handleSelectionInTable(); + + // Handle selection/navigation at the edges of links. + const link = getInSelection(this.document, 'a'); + if (link && selection.isCollapsed) { + // 1. If the selection starts or ends at the end of a link + // (after the end zws), move the selection after the "after" + // zws. This ensures that the cursor is visibly outside the + // link. We want to do this only if the link has an end zws + // to prevent ejecting the selection when moving in from the + // right. + const endZws = link.querySelector('[data-o-link-zws="end"]'); + const isAtEndOfLink = ( + // The selection is at the end of the link, ie. at offset + // max of the link, with no next leaf that is in the link. + endZws && selection.anchorOffset === nodeSize(selection.anchorNode) && + closestElement(selection.anchorNode, 'a') === link && + closestElement(nextLeaf(selection.anchorNode, this.editable), 'a') !== link + ); + if (isAtEndOfLink) { + let afterZws = link.nextElementSibling; + if (!afterZws) { + afterZws = this._createLinkZws('after'); + link.after(afterZws); + } + setSelection( + afterZws.nextSibling || afterZws.parentElement, + afterZws.nextSibling ? 0 : nodeSize(afterZws.parentElement), + ); + return; // The selection is changed and will therefore re-trigger the _onSelectionChange. + } + } + // 2. Make sure the link has the required zws if the selection + // wasn't changed. + this._setLinkZws(); + if (this.options.onCollaborativeSelectionChange) { this.options.onCollaborativeSelectionChange(this.getCurrentCollaborativeSelection()); } @@ -4090,7 +4158,6 @@ export class OdooEditor extends EventTarget { clean() { this.observerUnactive(); - this.resetContenteditableLink(); this.cleanForSave(); this.observerActive(); } @@ -4157,6 +4224,9 @@ export class OdooEditor extends EventTarget { } this._pluginCall('cleanForSave', [element]); + // Remove all link ZWS. + this._resetLinkZws(element); + // Clean the zero-width spaces added by the `fillEmpty` function // (flagged with the "data-oe-zws-empty-inline" attributes). Reverse the // list to start from the deepest elements (for emptiness checks). @@ -4361,21 +4431,8 @@ export class OdooEditor extends EventTarget { this._currentMouseState = ev.type; this._lastMouseClickPosition = [ev.x, ev.y]; - // When selecting all the text within a link then triggering delete or - // inserting a character, the cursor and insertion is outside the link. - // To avoid this problem, we make all editable zone become uneditable - // except the link. Then when cliking outside the link, reset the - // editable zones. - const link = closestElement(ev.target, 'a'); - this.resetContenteditableLink(); this._activateContenteditable(); - if ( - link && link.isContentEditable && - !link.querySelector('div') && - !closestElement(ev.target, '.o_not_editable') - ) { - this.setContenteditableLink(link); - } + // Ignore any changes that might have happened before this point. this.observer.takeRecords(); @@ -4631,11 +4688,6 @@ export class OdooEditor extends EventTarget { const link = closestElement(sel.anchorNode, 'a'); if (link && sel.toString().replace(/\u200B/g, '') === link.innerText.replace(/\u200B/g, '')) { const start = leftPos(link); - // Exit link isolation since we're removing the link and editing outside of it. - if (this._fixLinkMutatedElements && this._fixLinkMutatedElements.link === link) { - this.resetContenteditableLink(); - this._activateContenteditable(); - } link.remove(); setSelection(...start, ...start, false); } diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/copyPaste.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/copyPaste.test.js index 121eb082ad4..a1237944243 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/copyPaste.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/copyPaste.test.js @@ -1997,6 +1997,7 @@ describe('Paste', () => { // Pick the second command (Paste as URL) triggerEvent(editor.editable, 'keydown', { key: 'ArrowDown' }); triggerEvent(editor.editable, 'keydown', { key: 'Enter' }); + await nextTick(); }, contentAfter: `

${url}[]

`, }); diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/editor.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/editor.test.js index 29f47770c8d..30aa4fbfb4a 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/editor.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/editor.test.js @@ -3692,6 +3692,7 @@ X[] }); it('should insert line breaks outside the edges of an anchor', async () => { const pressEnter = editor => { + editor._resetLinkZws(); // Any interaction causing insertParagraph should trigger this. editor.document.execCommand('insertParagraph'); }; await testEditor(BasicEditor, { diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/link.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/link.test.js index b8e5841da76..a69024be6b7 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/link.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/link.test.js @@ -664,9 +664,14 @@ describe('Link', () => { console.log(a.closest('.odoo-editor-editable').outerHTML); await deleteBackward(editor); console.log(a.closest('.odoo-editor-editable').outerHTML); - window.chai.expect(a.parentElement.isContentEditable).to.be.equal(false); }, - contentAfterEdit: '

a[]\u200Bc

', + contentAfterEdit: '

a' + + '\u200B' + // start zws + '[]\u200B' + // content: empty inline zws + '\u200B' + // end zws + '' + + '\u200B' + // after zws + 'c

', contentAfter: '

a[]c

', }); }); @@ -674,16 +679,11 @@ describe('Link', () => { await testEditor(BasicEditor, { contentBefore: '

ab[]c

', stepFunction: async editor => { - const a = await clickOnLink(editor); - window.chai.expect(a.parentElement.isContentEditable).to.be.equal(false); + await clickOnLink(editor); await deleteBackward(editor); - window.chai.expect(a.parentElement.isContentEditable).to.be.equal(false); await insertText(editor, 'a'); - window.chai.expect(a.parentElement.isContentEditable).to.be.equal(false); await insertText(editor, 'b'); - window.chai.expect(a.parentElement.isContentEditable).to.be.equal(false); await insertText(editor, 'c'); - window.chai.expect(a.parentElement.isContentEditable).to.be.equal(false); }, contentAfter: '

aabc[]c

', }); @@ -692,14 +692,10 @@ describe('Link', () => { await testEditor(BasicEditor, { contentBefore: '

abc[]abc

', stepFunction: async editor => { - const a = await clickOnLink(editor); - window.chai.expect(a.parentElement.isContentEditable).to.be.equal(false); + await clickOnLink(editor); await deleteBackward(editor); - window.chai.expect(a.parentElement.isContentEditable).to.be.equal(false); await deleteBackward(editor); - window.chai.expect(a.parentElement.isContentEditable).to.be.equal(false); await deleteBackward(editor); - window.chai.expect(a.parentElement.isContentEditable).to.be.equal(false); await deleteBackward(editor); }, contentAfter: '

[]abc

', diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/utils.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/utils.js index 464a038a209..f86e5a9a83e 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/utils.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/utils.js @@ -90,6 +90,13 @@ export function parseTextualSelection(testContainer) { node = next; } if (anchorNode && focusNode) { + // Correct for the addition of the link ZWS start characters. + if (anchorNode.nodeName === 'A' && anchorOffset) { + anchorOffset += 1; + } + if (focusNode.nodeName === 'A' && focusOffset) { + focusOffset += 1; + } return { anchorNode: anchorNode, anchorOffset: anchorOffset, diff --git a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js index 3f47a6128f9..ae6ef1b60bf 100644 --- a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js +++ b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js @@ -631,7 +631,10 @@ export class Wysiwyg extends Component { $target.data('popover-widget-initialized', this.linkPopover); })(); } - $target.focus(); + // Setting the focus on the closest contenteditable element + // resets the selection inside that element if no selection + // exists. + $target.closest('[contenteditable=true]').focus(); if ($target.closest('#wrapwrap').length && this.snippetsMenu) { this.toggleLinkTools({ forceOpen: true, @@ -1416,7 +1419,6 @@ export class Wysiwyg extends Component { this.odooEditor.historyUnpauseSteps(); this.odooEditor.historyStep(); const link = data.linkDialog.$link[0]; - this.odooEditor.setContenteditableLink(link); setSelection(link, 0, link, link.childNodes.length, false); link.focus(); },