From f4d72dd1ed869e0900cccd04ed4d2e41a334c261 Mon Sep 17 00:00:00 2001 From: abd-msyukyu-odoo Date: Mon, 14 Mar 2022 18:01:56 +0000 Subject: [PATCH] [IMP] web_editor: prevent command refocus when unnecessary Currently, the focus is forced on this.editable when using a powerbox command. Knowledge /template will introduce blocks which are contentEditable=False, but contains sub-elements with contentEditable=True, which are allowed to use powerbox commands. Those sub-elements have another editable context as this.editable. As such, forcing a refocus on this.editable will remove the caret from the sub-element. Avoid doing the refocus if the currently focused element is a child of this.editable and is contentEditable=true. Task-id: 2794990 Part-of: odoo/odoo#86489 --- .../static/lib/odoo-editor/src/OdooEditor.js | 3 +- .../static/lib/odoo-editor/src/utils/utils.js | 18 ++++ .../lib/odoo-editor/test/spec/editor.test.js | 4 +- .../lib/odoo-editor/test/spec/utils.test.js | 95 ++++++++++++++++++- .../static/lib/odoo-editor/test/utils.js | 10 ++ 5 files changed, 127 insertions(+), 3 deletions(-) diff --git a/addons/web_editor/static/lib/odoo-editor/src/OdooEditor.js b/addons/web_editor/static/lib/odoo-editor/src/OdooEditor.js index b716bbbcafd..36c929c540f 100644 --- a/addons/web_editor/static/lib/odoo-editor/src/OdooEditor.js +++ b/addons/web_editor/static/lib/odoo-editor/src/OdooEditor.js @@ -18,6 +18,7 @@ import { containsUnremovable, DIRECTIONS, endPos, + ensureFocus, getCursorDirection, getListMode, getOuid, @@ -1893,7 +1894,7 @@ export class OdooEditor extends EventTarget { this.historyStep(true); this._historyStepsStates.set(peek(this._historySteps).id, 'consumed'); setTimeout(() => { - this.editable.focus(); + ensureFocus(this.editable); getDeepRange(this.editable, { select: true }); }); }, diff --git a/addons/web_editor/static/lib/odoo-editor/src/utils/utils.js b/addons/web_editor/static/lib/odoo-editor/src/utils/utils.js index 1e1db7fd575..28bcc8e643b 100644 --- a/addons/web_editor/static/lib/odoo-editor/src/utils/utils.js +++ b/addons/web_editor/static/lib/odoo-editor/src/utils/utils.js @@ -462,6 +462,24 @@ export function getNormalizedCursorPosition(node, offset, full = true) { return [node, offset]; } +/** + * Guarantee that the focus is on element or one of its children. + * + * A simple call to element.focus will change the editable context + * if one of the parents of the current activeElement is not editable, + * and the caret position will not be preserved, even if activeElement is + * one of the subchildren of element. This is why the (re)focus is + * only called when the current activeElement is not one of the + * (sub)children of element. + * + * @param {Element} element should have the focus or a child with the focus + */ + export function ensureFocus(element) { + const activeElement = element.ownerDocument.activeElement; + if (activeElement !== element && (!element.contains(activeElement) || !activeElement.isContentEditable)) { + element.focus(); + } +} /** * @param {Node} anchorNode * @param {number} anchorOffset diff --git a/addons/web_editor/static/lib/odoo-editor/test/spec/editor.test.js b/addons/web_editor/static/lib/odoo-editor/test/spec/editor.test.js index 7fb041b37ab..439b9dcd7a5 100644 --- a/addons/web_editor/static/lib/odoo-editor/test/spec/editor.test.js +++ b/addons/web_editor/static/lib/odoo-editor/test/spec/editor.test.js @@ -1,6 +1,6 @@ import { applyInlineStyle } from '../../src/commands/commands.js'; import { OdooEditor } from '../../src/OdooEditor.js'; -import { getTraversedNodes } from '../../src/utils/utils.js'; +import { getTraversedNodes, setCursorStart } from '../../src/utils/utils.js'; import { BasicEditor, deleteBackward, @@ -9,6 +9,8 @@ import { insertParagraphBreak, insertText, keydown, + keyup, + nextTickFrame, redo, testEditor, undo, diff --git a/addons/web_editor/static/lib/odoo-editor/test/spec/utils.test.js b/addons/web_editor/static/lib/odoo-editor/test/spec/utils.test.js index 308335b70e7..b506479f6ac 100644 --- a/addons/web_editor/static/lib/odoo-editor/test/spec/utils.test.js +++ b/addons/web_editor/static/lib/odoo-editor/test/spec/utils.test.js @@ -7,6 +7,7 @@ import { closestElement, descendants, endPos, + ensureFocus, firstLeaf, getAdjacentPreviousSiblings, getAdjacentNextSiblings, @@ -33,7 +34,15 @@ import { DIRECTIONS, isBlock, } from '../../src/utils/utils.js'; -import { BasicEditor, testEditor } from '../utils.js'; +import { + BasicEditor, + insertText, + keydown, + keyup, + nextTickFrame, + testEditor, + unformat, +} from '../utils.js'; const cleanTestHtml = () => { const testElements = document.querySelectorAll('body>div[contenteditable=true]'); @@ -722,6 +731,90 @@ describe('Utils', () => { // Cursor management //------------------------------------------------------------------------------ + describe('ensureFocus', () => { + it('should preserve the focus on the child of this.editable when executing a powerbox command even if it is enclosed in a contenteditable=false', async () => { + await testEditor(BasicEditor, { + contentBefore: unformat(` +
+

[]

+
+


`), + stepFunction: async editor => { + const sel = document.getSelection(); + const element = sel.anchorNode; + await keydown(editor.editable, '/'); + await insertText(editor, '/'); + await keyup(editor.editable, '/'); + await insertText(editor, 'h2'); + await keyup(element, '2', { bubbles: true }); + await keydown(editor.editable, 'Enter'); + const activeElement = document.activeElement; + setCursorStart(activeElement.lastElementChild); + await nextTickFrame(); + }, + contentAfter: unformat(` +
+

[]

+
+


`), + }); + }); + it('should preserve the focus on the child of this.editable even if it is enclosed in a contenteditable=false', async () => { + await testEditor(BasicEditor, { + contentBefore: unformat(` +
+

[]

+
+


`), + stepFunction: async editor => { + ensureFocus(editor.editable); + await nextTickFrame(); + let activeElement = document.activeElement; + setCursorStart(activeElement.lastElementChild); + await insertText(editor, 'focusWasConserved'); + // Proof that a simple call to Element.focus would change + // the focus in this case. + editor.editable.focus(); + await nextTickFrame(); + activeElement = document.activeElement; + setCursorStart(activeElement.lastElementChild); + await nextTickFrame(); + }, + contentAfter: unformat(` +
+

focusWasConserved

+
+

[]

`), + }); + }); + it('should update the focus when the active element is not the focus target', async () => { + await testEditor(BasicEditor, { + contentBefore: unformat(` +
+

[]

+
+
+


+
`), + stepFunction: async editor => { + const sel = document.getSelection(); + const element = editor.editable.querySelector('#target'); + ensureFocus(element); + await nextTickFrame(); + const activeElement = document.activeElement; + setCursorStart(activeElement.lastElementChild); + await nextTickFrame(); + }, + contentAfter: unformat(` +
+


+
+
+

[]

+
`), + }); + }); + }); describe('getNormalizedCursorPosition', () => { // TODO: test more. it('should move the cursor from after a to within it', () => { diff --git a/addons/web_editor/static/lib/odoo-editor/test/utils.js b/addons/web_editor/static/lib/odoo-editor/test/utils.js index 503ce197ca4..abad4c14e81 100644 --- a/addons/web_editor/static/lib/odoo-editor/test/utils.js +++ b/addons/web_editor/static/lib/odoo-editor/test/utils.js @@ -422,6 +422,16 @@ export async function keydown(editable, key, options = {}) { await nextTickFrame(); } +export async function keyup(editable, key, options = {}) { + options = Object.assign( + { key: key }, + options + ); + const ev = new KeyboardEvent('keyup', options); + editable.dispatchEvent(ev); + await nextTickFrame(); +} + export async function deleteForward(editor) { editor.execCommand('oDeleteForward'); }