From c3f44dbf59c84f32f8c7125a08bc73e09c0f98d2 Mon Sep 17 00:00:00 2001 From: abd-msyukyu-odoo Date: Thu, 23 Feb 2023 12:16:19 +0000 Subject: [PATCH 1/7] [FIX] web_editor: notify current step when joining a collaborator which already synced How to reproduce (scenario): - Open 3 windows with 3 different sessions in Odoo user_1 -> in CRM, My Pipeline user_2 -> in Knowledge on article "target" user_3 -> in Knowledge on article "target" -> user_2 and user_3 are in a collaborative session -> user_1 "insert view in article" "target" (via Favorites) -> user_1 is redirected to knowledge to the target article -> RTC_DATA_CHANNEL_OPEN between user_1 and user_2 happens -> user_1 reset the html_field value from the history steps of user_2 -> the embedded view "My Pipeline" is inserted via `execCommand` and create a step in user_1 editor -> user_2 is notified via OE_HISTORY_STEP and receive the new nodes (view) -> RTC_DATA_CHANNEL_OPEN between user_1 and user_3 happens -> since user_3 was already synced with user_2, there is no `historyResetFromSteps` -> user_3 does not get the step from user_1 with the embedded view nodes now -> user_3 makes multiple steps (i.e. write some text in a paragraph) Current Behavior: -> user_2 and user_1 are notified of those steps and add them to their histories -> user_3 stil did not get the step with the embedded view nodes -> now if user_1 and user_2 make some steps, user_3 will be notified, but those steps "parents" are the steps from user_3, so user_3 may not request a full history comparison and may never get the missing step Fix: -> when a RTC_DATA_CHANNEL_OPEN event happens, the connected client is notified of the last registered step so it can call GET_MISSING_STEPS if it does not have it Note: This fix solves the specific case mentioned above, but other cases of desynchronization may happen, see Task-3208277 for a more complete fix. Task-2821374 Part-of: odoo/odoo#104680 --- addons/web_editor/static/src/js/wysiwyg/wysiwyg.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js index 0852d3e673c..e608fc95394 100644 --- a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js +++ b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js @@ -566,10 +566,12 @@ const Wysiwyg = Widget.extend({ } historySyncFinished = true; } else { + const currentStep = this.odooEditor._historySteps[this.odooEditor._historySteps.length - 1]; const remoteSelection = await this.ptp.requestClient(fromClientId, 'get_collaborative_selection', undefined, { transport: 'rtc' }); if (remoteSelection) { this.odooEditor.onExternalMultiselectionUpdate(remoteSelection); } + this.ptp.notifyClient(fromClientId, 'oe_history_step', currentStep, { transport: 'rtc' }); } break; } From 4e115baaadbcd9ea8cb284e608b3c7a0d5f7ea2f Mon Sep 17 00:00:00 2001 From: abd-msyukyu-odoo Date: Sat, 25 Feb 2023 15:02:03 +0000 Subject: [PATCH 2/7] [IMP] web_editor: fully implement oeProtected and oeTransientContent With the introduction of Knowledge Behavior Component, came a need to create html nodes which would have limited interactions with the editor. i.e. an Odoo view already has everything it needs to function properly, and when it is inserted in the editor, any manipulation on the selection or on the style that could be done with it should be prevented. Another example would be the /template block (will be renamed /clipboard in the future) that has a non-editable part (buttons which have a definite action in Odoo, and which should not be interacted with) as well as an editable part inside of it). To solve this use case, this commit proposes to mark specific html nodes with a `data-oe-protected` attribute which could have one of three values: - "true" - Only mutations of type "attributes" can be registered on the node itself which has the `data-oe-protected="true"` attribute - Prevent mutations of children (and sub-children) from being registered by the mutationObserver of the editor - Prevent the selection handling when its anchor is inside a `data-oe-protected="true"` element, even if it is `contenteditable="false"` - Prevent the command hint - Prevent the usage of the wysiwyg toolbar - Prevent the dblClick tooltip - Prevent the editor sanitization `Sanitize.js` - "false" - Designed to be contained inside a node with `data-oe-protected="true"` - Re-enable all features disabled by a parent node with `data-oe-protected="true" for the children of a node with `data-oe-protected="false" - ("") - This is considered equivalent to have the `data-oe-protected` attribute set to "true" (like other html attributes). Another attribute is added: `data-oe-transient-content`, with the following values: - "true" - Prevent the serialization of the children of the node, so they are not shared during a collaboration. - Transient nodes will be removed during `cleanForSave`, meaning that they will never be part of the html_field value in the database - ("") - equivalent to "true" The use case is an embedded view: there is a large quantity of nodes that are not relevant to share nor to save, since it will be recreated with the lastest data from the database, with the information relevant to the currently active user each time it has to be rendered. Note: This commit does not handle the dynamic switch from a specific value for `data-oe-protected` to another (i.e. switching from "false" to "" or "true"). This could cause a number of problems like: - some mutations from when the value was "true" are not yet handled when the switch (to "false") happens => those mutations will be registered as if they were always under the "false" value, even though it is not the case. - in collaborative, some nodes with oids that were not relevant (under the value "true") won't necessarily have the same oids in between collaborators. Therefore we cannot suddently listen to their mutations and expect the changes to be shared by switching to "false". In conclusion: the `data-oe-protected` attribute value should stay the same during the entire edition. Task-2821374 Part-of: odoo/odoo#104680 --- .../js/editor/odoo-editor/src/OdooEditor.js | 54 +++- .../editor/odoo-editor/src/utils/sanitize.js | 10 +- .../editor/odoo-editor/src/utils/serialize.js | 11 +- .../js/editor/odoo-editor/src/utils/utils.js | 18 ++ .../odoo-editor/test/spec/collab.test.js | 82 +++++- .../odoo-editor/test/spec/editor.test.js | 236 ++++++++++++++---- .../static/src/js/wysiwyg/wysiwyg.js | 5 +- odoo/tools/mail.py | 3 +- 8 files changed, 349 insertions(+), 70 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 0cf80615eb4..073cf27ba93 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 @@ -69,6 +69,7 @@ import { splitTextNode, isEditorTab, isMacOS, + isProtected, isVoidElement, cleanZWS, isZWS, @@ -794,7 +795,9 @@ export class OdooEditor extends EventTarget { } } observerFlush() { - this.observerApply(this.filterMutationRecords(this.observer.takeRecords())); + const records = this.observer.takeRecords(); + this.observerIdSet(records); + this.observerApply(this.filterMutationRecords(records)); } observerActive(label) { this._observerUnactiveLabels.delete(label); @@ -802,6 +805,7 @@ export class OdooEditor extends EventTarget { if (!this.observer) { this.observer = new MutationObserver(records => { + this.observerIdSet(records); records = this.filterMutationRecords(records); if (!records.length) return; this.dispatchEvent(new Event('contentChanged')); @@ -826,6 +830,14 @@ export class OdooEditor extends EventTarget { this.dispatchEvent(new Event('observerActive')); } + observerIdSet(records) { + for (const record of records) { + if (record.type === 'childList') { + this.idSet(record.target); + } + } + } + observerApply(records) { // There is a case where node A is added and node B is a descendant of // node A where node B was not in the observed tree) then node B is @@ -956,11 +968,29 @@ export class OdooEditor extends EventTarget { continue; } } - if (record.target && [Node.TEXT_NODE, Node.ELEMENT_NODE].includes(record.target.nodeType)) { - const closestProtected = closestElement(record.target, '[data-oe-protected="true"]'); - if (closestProtected && closestProtected.nodeType === Node.ELEMENT_NODE && - record.target !== closestProtected) { - continue; + const closestProtectedCandidate = closestElement(record.target, '[data-oe-protected]'); + if (closestProtectedCandidate) { + const protectedValue = closestProtectedCandidate.dataset.oeProtected; + switch (protectedValue) { + case "true": + case "": + if ( + record.type !== "attributes" || + record.target !== closestProtectedCandidate || + isProtected(closestProtectedCandidate.parentElement) + ) { + continue; + } + break; + case "false": + if ( + record.type === "attributes" && + record.target === closestProtectedCandidate && + isProtected(closestProtectedCandidate.parentElement) + ) { + continue; + } + break; } } filteredRecords.push(record); @@ -2271,7 +2301,7 @@ export class OdooEditor extends EventTarget { const selection = this.document.getSelection(); // Selection could be gone if the document comes from an iframe that has been removed. const anchorNode = selection && selection.rangeCount && selection.getRangeAt(0) && selection.anchorNode; - if (anchorNode && (closestElement(anchorNode, '[data-oe-protected="true"]') || !ancestors(anchorNode).includes(this.editable))) { + if (isProtected(anchorNode) || !ancestors(anchorNode).includes(this.editable)) { return false; } this.deselectTable(); @@ -3642,7 +3672,7 @@ export class OdooEditor extends EventTarget { return; } const anchorNode = selection.anchorNode; - if (anchorNode && closestElement(anchorNode, '[data-oe-protected="true"]')) { + if (isProtected(anchorNode)) { return; } @@ -3838,8 +3868,8 @@ export class OdooEditor extends EventTarget { } } - // Clean all protected nodes because they are not sanitized - const protectedNodes = element.querySelectorAll('[data-oe-protected="true"]'); + // Clean all transient nodes + const protectedNodes = element.querySelectorAll('[data-oe-transient-content="true"], [data-oe-transient-content=""]'); for (const node of protectedNodes) { node.replaceChildren(); } @@ -3875,7 +3905,7 @@ export class OdooEditor extends EventTarget { _handleCommandHint() { const selection = this.document.getSelection(); const anchorNode = selection.anchorNode; - if (anchorNode && closestElement(anchorNode, '[data-oe-protected="true"]')) { + if (isProtected(anchorNode)) { return; } @@ -3950,7 +3980,7 @@ export class OdooEditor extends EventTarget { _fixSelectionOnContenteditableFalse() { const selection = this.document.getSelection(); const anchorNode = selection.anchorNode; - if (anchorNode && closestElement(anchorNode, '[data-oe-protected="true"]')) { + if (isProtected(anchorNode)) { return; } // When the browser set the selection inside a node that is diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/sanitize.js b/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/sanitize.js index fa0a9449411..b17ed6f52e3 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/sanitize.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/sanitize.js @@ -14,6 +14,7 @@ import { getDeepRange, isUnbreakable, isEditorTab, + isProtected, isZWS, getUrlsInfosInString, isVoidElement, @@ -124,9 +125,12 @@ class Sanitize { _parse(node) { while (node) { - const closestProtected = closestElement(node, '[data-oe-protected="true"]'); - if (closestProtected && node !== closestProtected) { - return; + if (isProtected(node)) { + for (const unprotected of node.querySelectorAll('[data-oe-protected="false"]')) { + this._parse(unprotected.firstChild); + } + node = node.nextSibling; + continue; } // Merge identical elements together. while ( diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/serialize.js b/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/serialize.js index d651b977433..cf3efff53df 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/serialize.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/serialize.js @@ -18,11 +18,14 @@ export function serializeNode(node, nodesToStripFromChildren = new Set()) { result.attributes[node.attributes[i].name] = node.attributes[i].value; } let child = node.firstChild; - while (child) { - if (!nodesToStripFromChildren.has(child.oid)) { - result.children.push(serializeNode(child, nodesToStripFromChildren)); + // Don't serialize transient nodes + if (!["true", ""].includes(node.dataset.oeTransientContent)) { + while (child) { + if (!nodesToStripFromChildren.has(child.oid)) { + result.children.push(serializeNode(child, nodesToStripFromChildren)); + } + child = child.nextSibling; } - child = child.nextSibling; } } return result; diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/utils.js b/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/utils.js index 9041fe55814..28e9283d900 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/utils.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/utils.js @@ -1338,6 +1338,24 @@ export function isMediaElement(node) { (node.classList.contains('o_image') || node.classList.contains('media_iframe_video'))) ); } +/** + * A "protected" node will have its mutations filtered and not be registered + * in an history step. Some editor features like selection handling, command + * hint, toolbar, tooltip, etc. are also disabled. Protected roots have their + * data-oe-protected attribute set to either "" or "true". If the closest parent + * with a data-oe-protected attribute has the value "false", it is not + * protected. Unknown values are ignored. + * + * @param {Node} node + * @returns {boolean} + */ +export function isProtected(node) { + const closestProtectedElement = closestElement(node, '[data-oe-protected]'); + if (closestProtectedElement) { + return ["", "true"].includes(closestProtectedElement.dataset.oeProtected); + } + return false; +} export function isVoidElement(node) { return isMediaElement(node) || node.tagName === 'HR'; } diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js index 4a9f2d9952a..fb1453ca967 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js @@ -1,4 +1,4 @@ -import { OdooEditor } from '../../src/OdooEditor.js'; +import { OdooEditor, parseHTML, setSelection } from '../../src/OdooEditor.js'; import { insertCharsAt, parseMultipleTextualSelection, @@ -6,6 +6,7 @@ import { setTestSelection, targetDeepest, undo, + unformat, } from '../utils.js'; const overridenDomClass = [ @@ -586,4 +587,83 @@ describe('Collaboration', () => { }); }); }); + describe('data-oe-protected', () => { + it('should not share protected mutations and share unprotected ones', () => { + testMultiEditor({ + clientIds: ['c1', 'c2'], + contentBefore: '

[c1}{c1][c2}{c2]

', + afterCreate: clientInfos => { + clientInfos.c1.editor.editable.prepend(...parseHTML(unformat(` +
+


+
+


+
+
+ `)).children); + clientInfos.c1.editor.historyStep(); + const pTrue = clientInfos.c1.editor.editable.querySelector('#true'); + setSelection(pTrue, 0); + clientInfos.c1.editor.execCommand('insert', 'a'); + const pFalse = clientInfos.c1.editor.editable.querySelector('#false'); + setSelection(pFalse, 0); + clientInfos.c1.editor.execCommand('insert', 'a'); + clientInfos.c2.editor.onExternalHistorySteps(clientInfos.c1.editor._historySteps); + testSameHistory(clientInfos); + }, + afterCursorInserted: clientInfos => { + chai.expect(clientInfos.c1.editable.innerHTML).to.equal(unformat(` +
+

a

+
+

a[c1}{c1]

+
+
+

[c2}{c2]

+ `)); + chai.expect(clientInfos.c2.editable.innerHTML).to.equal(unformat(` +
+


+
+

a[c1}{c1]

+
+
+

[c2}{c2]

+ `)); + }, + }); + }); + }); + describe('data-oe-transient-content', () => { + it('should send an empty transient-content element', () => { + testMultiEditor({ + clientIds: ['c1', 'c2'], + contentBefore: '

[c1}{c1][c2}{c2]

', + afterCreate: clientInfos => { + clientInfos.c1.editor.editable.prepend(...parseHTML(unformat(` +
+

secret

+
+ `)).children); + clientInfos.c1.editor.historyStep(); + clientInfos.c2.editor.onExternalHistorySteps( + clientInfos.c1.editor._historySteps + ); + testSameHistory(clientInfos); + }, + afterCursorInserted: clientInfos => { + chai.expect(clientInfos.c1.editable.innerHTML).to.equal(unformat(` +
+

secret

+
+

[c1}{c1][c2}{c2]

+ `)); + chai.expect(clientInfos.c2.editable.innerHTML).to.equal(unformat(` +
+

[c1}{c1][c2}{c2]

+ `)); + }, + }); + }); + }); }); 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 25f23bd8d56..e368faaf5b1 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 @@ -5881,56 +5881,200 @@ X[] }); }); - describe('oe-protected', () => { - it('should ignore protected elements children mutations', async () => { - await testEditor(BasicEditor, { - contentBefore: unformat(` -

a[]

-

a

- `), - stepFunction: async editor => { - await insertText(editor, 'bc'); - const protectedParagraph = editor.editable.querySelector('[data-oe-protected="true"] > p'); - setSelection(protectedParagraph, 1); - await insertText(editor, 'b'); - editor.historyUndo(); - }, - contentAfterEdit: unformat(` -

ab[]

-

ab

- `), + describe('data-oe-protected', () => { + describe('true', () => { + it('should ignore protected elements children mutations', async () => { + await testEditor(BasicEditor, { + contentBefore: unformat(` +

a[]

+

a

+ `), + stepFunction: async editor => { + await insertText(editor, 'bc'); + const protectedParagraph = editor.editable.querySelector('[data-oe-protected="true"] > p'); + setSelection(protectedParagraph, 1); + await insertText(editor, 'b'); + editor.historyUndo(); + }, + contentAfterEdit: unformat(` +

ab[]

+

ab

+ `), + }); + }); + it('should not sanitize (sanitize.js) protected elements children', async () => { + await testEditor(BasicEditor, { + contentBefore: unformat(` +
+

+

+
+
+

+

+
+ `), + stepFunction: async editor => editor.sanitize(), + contentAfterEdit: unformat(` +
+

\u200B

+

+
+
+

+

+
+ `), + }); + }); + it('should not fix selection in contenteditable="false" protected elements children', async () => { + await testEditor(BasicEditor, { + contentBefore: unformat(` +


+
+

[very important text that needs to be selected]

+
+


+ `), + stepFunction: async editor => editor._fixSelectionOnContenteditableFalse(), + contentAfter: unformat(` +


+
+

[very important text that needs to be selected]

+
+


+ `), + }); + }); + it('should not handle table selection in protected elements children', async () => { + await testEditor(BasicEditor, { + contentBefore: unformat(` +
+

a[bc

a]bcdef
+
+ `), + contentAfterEdit: unformat(` +
+

a[bc

a]bcdef
+
+ `), + }); }); }); - it('should not sanitize protected elements children', async () => { - await testEditor(BasicEditor, { - contentBefore: unformat(` -
-

-

-
-
-

-

-
- `), - stepFunction: async editor => editor.sanitize(), - contentAfterEdit: unformat(` -
-

\u200B

-

-
-
-

-

-
- `), + describe('false', () => { + it('should not ignore unprotected elements children mutations', async () => { + await testEditor(BasicEditor, { + contentBefore: unformat(` +

a[]

+

a

+ `), + stepFunction: async editor => { + await insertText(editor, 'bc'); + const unProtectedParagraph = editor.editable.querySelector('[data-oe-protected="false"] > p'); + setSelection(unProtectedParagraph, 1); + await insertText(editor, 'bc'); + editor.historyUndo(); + }, + contentAfterEdit: unformat(` +

abc

+

ab[]

+ `), + }); }); - }); - it('should remove protected elements children during cleaning', async () => { - await testEditor(BasicEditor, { - contentBefore: '

a[]

a

', - contentAfter: '

a[]

', + it('should sanitize (sanitize.js) unprotected elements children', async () => { + await testEditor(BasicEditor, { + contentBefore: unformat(` +
+

+

+
+

+

+
+
+ `), + stepFunction: async editor => editor.sanitize(), + contentAfterEdit: unformat(` +
+

+

+
+

\u200B

+

+
+
+ `), + }); + }); + it('should fix selection in contenteditable="false" unprotected elements children', async () => { + await testEditor(BasicEditor, { + contentBefore: unformat(` +


+
+
+

[editable text which edition is temporarily disabled]

+
+
+


+ `), + stepFunction: async editor => editor._fixSelectionOnContenteditableFalse(), + contentAfter: unformat(` +

[]

+
+
+

editable text which edition is temporarily disabled

+
+
+


+ `), + }); + }); + it('should handle table selection in unprotected elements children', async () => { + await testEditor(BasicEditor, { + contentBefore: unformat(` +
+
+

a[bc

a]bcdef
+
+
+ `), + contentAfterEdit: unformat(` +
+
+

a[bc

+ + + + +
a]bcdef
+
+
+ `), + }); }); }); }); + describe('data-oe-transient-content', () => { + it('should remove transient elements children during cleaning', async () => { + await testEditor(BasicEditor, { + contentBefore: '

a

a

', + contentAfter: '

a

', + }); + }); + it('should ignore transient elements children during serialization', async () => { + await testEditor(BasicEditor, { + contentBefore: '

a

a

', + stepFunction: async editor => { + const elements = []; + for (const element of [...editor.editable.children]) { + elements.push(editor.unserializeNode(editor.serializeNode(element))); + } + const container = document.createElement('DIV'); + container.append(...elements); + editor.resetContent(container.innerHTML) + }, + contentAfter: '

a

', + }); + }) + }); }); diff --git a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js index e608fc95394..5039d251ac8 100644 --- a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js +++ b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js @@ -29,6 +29,7 @@ const QWeb = core.qweb; const OdooEditor = OdooEditorLib.OdooEditor; const getDeepRange = OdooEditorLib.getDeepRange; const getInSelection = OdooEditorLib.getInSelection; +const isProtected = OdooEditorLib.isProtected; const isBlock = OdooEditorLib.isBlock; const rgbToHex = OdooEditorLib.rgbToHex; const preserveCursor = OdooEditorLib.preserveCursor; @@ -303,7 +304,7 @@ const Wysiwyg = Widget.extend({ const selection = self.odooEditor.document.getSelection(); const anchorNode = selection.anchorNode; - if (anchorNode && closestElement(anchorNode, '[data-oe-protected="true"]')) { + if (isProtected(anchorNode)) { return; } @@ -1852,7 +1853,7 @@ const Wysiwyg = Widget.extend({ _updateEditorUI: function (e) { let selection = this.odooEditor.document.getSelection(); const anchorNode = selection.anchorNode; - if (anchorNode && closestElement(anchorNode, '[data-oe-protected="true"]')) { + if (isProtected(anchorNode)) { return; } diff --git a/odoo/tools/mail.py b/odoo/tools/mail.py index 53f2f43a34a..3f0bc5e6756 100644 --- a/odoo/tools/mail.py +++ b/odoo/tools/mail.py @@ -32,11 +32,10 @@ safe_attrs = clean.defs.safe_attrs | frozenset( ['style', 'data-o-mail-quote', # quote detection 'data-oe-model', 'data-oe-id', 'data-oe-field', 'data-oe-type', 'data-oe-expression', 'data-oe-translation-initial-sha', 'data-oe-nodeid', - 'data-last-history-steps', + 'data-last-history-steps', 'data-oe-protected', 'data-oe-transient-content', 'data-publish', 'data-id', 'data-res_id', 'data-interval', 'data-member_id', 'data-scroll-background-ratio', 'data-view-id', 'data-class', 'data-mimetype', 'data-original-src', 'data-original-id', 'data-gl-filter', 'data-quality', 'data-resize-width', 'data-shape', 'data-shape-colors', 'data-file-name', 'data-original-mimetype', - 'data-oe-protected', # editor 'data-behavior-props', 'data-prop-name', # knowledge commands ]) SANITIZE_TAGS = { From 0eac6dd552812757e9300b74164d7983f1d73ba5 Mon Sep 17 00:00:00 2001 From: abd-msyukyu-odoo Date: Fri, 14 Apr 2023 14:36:55 +0000 Subject: [PATCH 3/7] [IMP] web_editor: prevent 'insert' command unwrapping in root Currently, the 'insert' command unpacks nodes from a lone paragraph, or from the first and last node if there are multiple nodes. In knowledge, to append an embedded view to the editable, there is a need to insert two sibling nodes at once (without unpacking them). With the current behavior of 'insert', it is possible to do so by inserting a paragraph containing the 2 nodes that need to be inserted. The result will be that the 'insert' command unpacks the parent paragraph and correctly insert the 2 nodes. This commit prevents the 'insert' command from unwrapping nodes if the anchorNode of the selection is the editable itself. Task-2821374 Part-of: odoo/odoo#104680 --- .../odoo-editor/src/commands/commands.js | 45 ++++++++++--------- .../odoo-editor/test/spec/insert.test.js | 18 ++++++++ 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/src/commands/commands.js b/addons/web_editor/static/src/js/editor/odoo-editor/src/commands/commands.js index 3e032b0b244..1f0fffc1814 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/src/commands/commands.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/src/commands/commands.js @@ -178,27 +178,30 @@ export const editorCommands = { } startNode = startNode || editor.document.getSelection().anchorNode; - - // In case the html inserted is all contained in a single root

or

  • - // tag, we take the all content of the

    or

  • and avoid inserting the - //

    or

  • . The same is true for a
     inside a 
    .
    -        if (container.childElementCount === 1 && (
    -            container.firstChild.nodeName === 'P' ||
    -            container.firstChild.nodeName === 'LI' ||
    -            container.firstChild.nodeName === 'PRE' && closestElement(startNode, 'pre')
    -        )) {
    -            const p = container.firstElementChild;
    -            container.replaceChildren(...p.childNodes);
    -        } else if (container.childElementCount > 1) {
    -            // Grab the content of the first child block and isolate it.
    -            if (isBlock(container.firstChild) && !['TABLE', 'UL', 'OL'].includes(container.firstChild.nodeName)) {
    -                containerFirstChild.replaceChildren(...container.firstElementChild.childNodes);
    -                container.firstElementChild.remove();
    -            }
    -            // Grab the content of the last child block and isolate it.
    -            if (isBlock(container.lastChild) && !['TABLE', 'UL', 'OL'].includes(container.lastChild.nodeName)) {
    -                containerLastChild.replaceChildren(...container.lastElementChild.childNodes);
    -                container.lastElementChild.remove();
    +        // If the selection anchorNode is the editable itself, the content
    +        // should not be unwrapped.
    +        if (selection.anchorNode.oid !== 'root') {
    +            // In case the html inserted is all contained in a single root 

    or

  • + // tag, we take the all content of the

    or

  • and avoid inserting the + //

    or

  • . The same is true for a
     inside a 
    .
    +            if (container.childElementCount === 1 && (
    +                container.firstChild.nodeName === 'P' ||
    +                container.firstChild.nodeName === 'LI' ||
    +                container.firstChild.nodeName === 'PRE' && closestElement(startNode, 'pre')
    +            )) {
    +                const p = container.firstElementChild;
    +                container.replaceChildren(...p.childNodes);
    +            } else if (container.childElementCount > 1) {
    +                // Grab the content of the first child block and isolate it.
    +                if (isBlock(container.firstChild) && !['TABLE', 'UL', 'OL'].includes(container.firstChild.nodeName)) {
    +                    containerFirstChild.replaceChildren(...container.firstElementChild.childNodes);
    +                    container.firstElementChild.remove();
    +                }
    +                // Grab the content of the last child block and isolate it.
    +                if (isBlock(container.lastChild) && !['TABLE', 'UL', 'OL'].includes(container.lastChild.nodeName)) {
    +                    containerLastChild.replaceChildren(...container.lastElementChild.childNodes);
    +                    container.lastElementChild.remove();
    +                }
                 }
             }
     
    diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/insert.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/insert.test.js
    index d5e2e9552e3..08e1b83b099 100644
    --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/insert.test.js
    +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/insert.test.js
    @@ -90,6 +90,24 @@ describe('insert HTML', () => {
                     contentAfter: '
    abcdef[]
    ghi
    ', }); }); + it('should not unwrap single node if the selection anchorNode is the editable', async () => { + await testEditor(BasicEditor, { + contentBefore: '

    content

    []', + stepFunction: async editor => { + await editor.execCommand('insert', parseHTML('

    def

    ')); + }, + contentAfter: '

    content

    def[]

    ', + }); + }); + it('should not unwrap nodes if the selection anchorNode is the editable', async () => { + await testEditor(BasicEditor, { + contentBefore: '

    content

    []', + stepFunction: async editor => { + await editor.execCommand('insert', parseHTML('
    abc

    def

    ')); + }, + contentAfter: '

    content

    abc

    def[]

    ', + }); + }); }); describe('not collapsed selection', () => { it('should delete selection and insert html in its place', async () => { From 918d0109f49e4b26b8465d142949c39ba7bfad32 Mon Sep 17 00:00:00 2001 From: abd-msyukyu-odoo Date: Thu, 9 Mar 2023 09:02:46 +0000 Subject: [PATCH 4/7] [IMP] web_editor: add a hook for historyResetFromSteps When the current user connects to a collaborative peer, the content inside his editor will be reset. When opening a record, there is no way to know if such a connection will happen. In Knowledge, when one wants to insert an embedded view in an article body from elsewhere in Odoo, it is inserted programatically once the article is opened. At that time the peerToPeer connection is not established. Then, if a collaborative session is started with someone already editing the html_field, the content is reset, and at that point, the insertedd element is removed, so it should be inserted again. This commit introduces a hook that can be used to execute some specific action if the content of the editor is reset through `historyResetFromSteps` during the collaboration first synchronization. It can be provided as an option of the `wysiwyg`. Task-2821374 Part-of: odoo/odoo#104680 --- addons/web_editor/static/src/js/wysiwyg/wysiwyg.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js index 5039d251ac8..55eda0de6a1 100644 --- a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js +++ b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js @@ -75,6 +75,7 @@ const Wysiwyg = Widget.extend({ allowCommandImage: true, allowCommandLink: true, insertParagraphAfterColumns: true, + onHistoryResetFromSteps: () => {}, autostart: true, }, init: function (parent, options) { @@ -559,6 +560,7 @@ const Wysiwyg = Widget.extend({ if (remoteSelection) { this.odooEditor.onExternalMultiselectionUpdate(remoteSelection); } + this.options.onHistoryResetFromSteps(); } // In case there are steps received in the meantime, process them. if (historyStepsBuffer.length) { From 40b762a69f98cc8eb3b621856ee98d23769030d9 Mon Sep 17 00:00:00 2001 From: abd-msyukyu-odoo Date: Wed, 8 Feb 2023 12:35:55 +0000 Subject: [PATCH 5/7] [IMP] web_editor: force load collaborative on form view Add an option to the html_field (collaborative_trigger) to allow to choose if the peerTopeer connection of the collaborative mode should happen when the html_field is loaded ('start') or when the field has the focus ('focus'). Will be used in Knowledge since the html_field of an article is the main focus when opening the form in edit mode. The collaborative mode should be loaded as fast as possible. Task-2821374 Part-of: odoo/odoo#104680 --- .../static/src/js/backend/html_field.js | 4 ++++ .../web_editor/static/src/js/wysiwyg/wysiwyg.js | 16 ++++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/addons/web_editor/static/src/js/backend/html_field.js b/addons/web_editor/static/src/js/backend/html_field.js index 676fc314321..0f002da5bb5 100644 --- a/addons/web_editor/static/src/js/backend/html_field.js +++ b/addons/web_editor/static/src/js/backend/html_field.js @@ -668,6 +668,10 @@ export const htmlField = { }; if ('collaborative' in options) { wysiwygOptions.collaborative = options.collaborative; + // Two supported triggers: + // 'start': Join the peerToPeer connection immediately + // 'focus': Join when the editable has focus + wysiwygOptions.collaborativeTrigger = options.collaborative_trigger || 'focus'; } if ('allowCommandImage' in options) { // Set the option only if it is explicitly set in the view so a default diff --git a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js index 55eda0de6a1..6548fcf75db 100644 --- a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js +++ b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js @@ -143,8 +143,12 @@ const Wysiwyg = Widget.extend({ this.getSession()['notification_type'] ) { editorCollaborationOptions = this.setupCollaboration(options.collaborationChannel); - // Wait until editor is focused to join the peer to peer network. - this.$editable[0].addEventListener('focus', this._joinPeerToPeer); + if (this.options.collaborativeTrigger === 'start') { + this._joinPeerToPeer(); + } else if (this.options.collaborativeTrigger === 'focus') { + // Wait until editor is focused to join the peer to peer network. + this.$editable[0].addEventListener('focus', this._joinPeerToPeer); + } } const getYoutubeVideoElement = async (url) => { @@ -2610,8 +2614,12 @@ const Wysiwyg = Widget.extend({ this.setValue(value); this.odooEditor.historyReset(); this.setupCollaboration(collaborationChannel); - // Wait until editor is focused to join the peer to peer network. - this.$editable[0].addEventListener('focus', this._joinPeerToPeer); + if (this.options.collaborativeTrigger === 'start') { + this._joinPeerToPeer(); + } else if (this.options.collaborativeTrigger === 'focus') { + // Wait until editor is focused to join the peer to peer network. + this.$editable[0].addEventListener('focus', this._joinPeerToPeer); + } const initialHistoryId = value && this._getInitialHistoryId(value); if (initialHistoryId) { this.odooEditor.historySetInitialId(initialHistoryId); From f04b8191c8a91c0bce778a27a930d83b0f3feeb9 Mon Sep 17 00:00:00 2001 From: abd-msyukyu-odoo Date: Tue, 7 Mar 2023 15:04:42 +0000 Subject: [PATCH 6/7] [IMP] web_editor: make mocha tests odoo modules In order to be able to use web dependencies for mocha tests, convert every existing test to an `@odoo-module` and make a new asset `web_editor.mocha_tests` that will be used for tests in `/web_editor/tests` Task-2821374 Part-of: odoo/odoo#104680 --- addons/web_editor/__manifest__.py | 9 ++++ .../js/editor/odoo-editor/src/qweb_sample.js | 2 + .../js/editor/odoo-editor/test/editor-test.js | 54 ++++++++++--------- .../odoo-editor/test/spec/align.test.js | 2 + .../odoo-editor/test/spec/autostep.test.js | 2 + .../odoo-editor/test/spec/collab.test.js | 2 + .../odoo-editor/test/spec/color.test.js | 2 + .../odoo-editor/test/spec/copyPaste.test.js | 2 + .../odoo-editor/test/spec/editor.test.js | 2 + .../odoo-editor/test/spec/fontAwesome.test.js | 2 + .../odoo-editor/test/spec/format.test.js | 2 + .../odoo-editor/test/spec/insert.test.js | 2 + .../editor/odoo-editor/test/spec/link.test.js | 2 + .../editor/odoo-editor/test/spec/list.test.js | 2 + .../odoo-editor/test/spec/odooFields.test.js | 2 + .../odoo-editor/test/spec/powerbox.test.js | 2 + .../editor/odoo-editor/test/spec/tabs.test.js | 2 + .../odoo-editor/test/spec/urlRegex.test.js | 2 + .../odoo-editor/test/spec/utils.test.js | 2 + addons/web_editor/views/editor.xml | 2 +- 20 files changed, 73 insertions(+), 26 deletions(-) diff --git a/addons/web_editor/__manifest__.py b/addons/web_editor/__manifest__.py index cf0d09a5009..3bc1544c8db 100644 --- a/addons/web_editor/__manifest__.py +++ b/addons/web_editor/__manifest__.py @@ -171,6 +171,15 @@ Odoo Web Editor widget. 'web_editor/static/tests/**/*', 'web_editor/static/src/js/editor/odoo-editor/test/utils.js' ], + 'web_editor.mocha_tests': [ + 'web/static/src/legacy/js/promise_extension.js', + 'web/static/src/boot.js', + # insert module dependencies here + + 'web_editor/static/src/js/editor/odoo-editor/src/**/*js', + 'web_editor/static/src/js/editor/odoo-editor/test/spec/*js', + 'web_editor/static/src/js/editor/odoo-editor/test/*js', + ], }, 'auto_install': True, 'license': 'LGPL-3', diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/src/qweb_sample.js b/addons/web_editor/static/src/js/editor/odoo-editor/src/qweb_sample.js index 7b6ddec23f8..cbdcdaa4ba0 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/src/qweb_sample.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/src/qweb_sample.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + export const qwebSample = /* xml */ `

    Qweb examples

    diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/editor-test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/editor-test.js index 3ec76aed5f7..a518e6b5ca8 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/editor-test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/editor-test.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + import './spec/utils.test.js'; import './spec/align.test.js'; import './spec/color.test.js'; @@ -16,31 +18,33 @@ import './spec/odooFields.test.js'; import './spec/powerbox.test.js'; /* global mocha */ -mocha.run(failures => { - if (failures) { - for (const faillureElement of [...document.querySelectorAll('.test.fail')]) { - const clonedFaillureElement = faillureElement.cloneNode(true); - clonedFaillureElement.querySelector('a').remove(); - console.error( - [ - clonedFaillureElement.querySelector('h2').innerText, - clonedFaillureElement.querySelector('.error').innerText, - ].join('\n\n'), - ); +window.addEventListener("load", () => { + mocha.run(failures => { + if (failures) { + for (const faillureElement of [...document.querySelectorAll('.test.fail')]) { + const clonedFaillureElement = faillureElement.cloneNode(true); + clonedFaillureElement.querySelector('a').remove(); + console.error( + [ + clonedFaillureElement.querySelector('h2').innerText, + clonedFaillureElement.querySelector('.error').innerText, + ].join('\n\n'), + ); + } + + // Better visualisation of invisible (ZWS & TABS) character in test + // report. + const report = document.querySelector("#mocha-report"); + const allErrors = report.querySelectorAll('.test.fail .error'); + allErrors.forEach((errorEl) => { + let errorElHtml = errorEl.outerHTML + errorElHtml = errorElHtml.replaceAll('//zws//', 'zws'); + errorElHtml = errorElHtml.replaceAll('//TAB//', 'Tab'); + errorEl.outerHTML = errorElHtml; + }); + } else { + console.log('test successful'); } - // Better visualisation of invisible (ZWS & TABS) character in test - // report. - const report = document.querySelector("#mocha-report"); - const allErrors = report.querySelectorAll('.test.fail .error'); - allErrors.forEach((errorEl) => { - let errorElHtml = errorEl.outerHTML - errorElHtml = errorElHtml.replaceAll('//zws//', 'zws'); - errorElHtml = errorElHtml.replaceAll('//TAB//', 'Tab'); - errorEl.outerHTML = errorElHtml; - }); - } else { - console.log('test successful'); - } - + }); }); diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/align.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/align.test.js index fa35cfb2a42..bab895ad440 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/align.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/align.test.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { BasicEditor, testEditor } from '../utils.js'; const justifyLeft = async function (editor) { diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/autostep.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/autostep.test.js index ec69e6b7e6e..f42ee1ce729 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/autostep.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/autostep.test.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { BasicEditor, testEditor } from '../utils.js'; const timeoutPromise = ms => diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js index fb1453ca967..fbccfbfe268 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { OdooEditor, parseHTML, setSelection } from '../../src/OdooEditor.js'; import { insertCharsAt, diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/color.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/color.test.js index 20220caaacf..aebd678e0af 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/color.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/color.test.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { BasicEditor, testEditor } from '../utils.js'; const setColor = (color, mode) => { 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 3dbd4c189e9..d07876b5ebb 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 @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { BasicEditor, testEditor, 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 e368faaf5b1..b173428ecb9 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 @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { OdooEditor } from '../../src/OdooEditor.js'; import { getTraversedNodes, diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/fontAwesome.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/fontAwesome.test.js index 40522c23166..7089921bdd3 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/fontAwesome.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/fontAwesome.test.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { BasicEditor, insertText, testEditor, deleteForward, deleteBackward } from '../utils.js'; describe('FontAwesome', () => { diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/format.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/format.test.js index d4563e92fb9..f086d40c1ba 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/format.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/format.test.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { isSelectionFormat } from '../../src/utils/utils.js'; import { BasicEditor, testEditor, setTestSelection, Direction, unformat } from '../utils.js'; diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/insert.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/insert.test.js index 08e1b83b099..e6438d5ac1a 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/insert.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/insert.test.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { parseHTML } from '../../src/utils/utils.js'; import { BasicEditor, testEditor, unformat, insertText, deleteBackward } from '../utils.js'; 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 50ce2b31ca3..819b7e5e2fc 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 @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { URL_REGEX, URL_REGEX_WITH_INFOS } from '../../src/OdooEditor.js'; import { BasicEditor, diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/list.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/list.test.js index dbb42fba1ab..5f456382c6b 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/list.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/list.test.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { BasicEditor, click, diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/odooFields.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/odooFields.test.js index f2af54e924a..b3ab2dcc6be 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/odooFields.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/odooFields.test.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { BasicEditor, testEditor, unformat } from '../utils.js'; describe('Odoo fields', () => { diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/powerbox.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/powerbox.test.js index a0d79ae9b81..4efaad731ed 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/powerbox.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/powerbox.test.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { setSelection } from '../../src/OdooEditor.js'; import { Powerbox } from '../../src/powerbox/Powerbox.js'; import { BasicEditor, insertText, testEditor, triggerEvent } from '../utils.js'; diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/tabs.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/tabs.test.js index 7210b6f25e8..fe765bdcd3b 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/tabs.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/tabs.test.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { BasicEditor, deleteBackward, diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/urlRegex.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/urlRegex.test.js index a111d80de21..2d0dc6e80f7 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/urlRegex.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/urlRegex.test.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { URL_REGEX, URL_REGEX_WITH_INFOS } from '../../src/OdooEditor.js'; describe('urlRegex', () => { diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/utils.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/utils.test.js index 3cc3c5c4aeb..a10b2989724 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/utils.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/utils.test.js @@ -1,3 +1,5 @@ +/** @odoo-module */ + import { ancestors, boundariesIn, diff --git a/addons/web_editor/views/editor.xml b/addons/web_editor/views/editor.xml index 8d84cf93ccb..952d37390b0 100644 --- a/addons/web_editor/views/editor.xml +++ b/addons/web_editor/views/editor.xml @@ -187,7 +187,7 @@ - +
    From e00a33b17ac1d01cfda6a95d9a40f3af07629062 Mon Sep 17 00:00:00 2001 From: abd-msyukyu-odoo Date: Tue, 4 Apr 2023 09:58:54 +0000 Subject: [PATCH 7/7] [IMP] web_editor: buffer external steps during Component rendering Add a mechanism to buffer external steps when some asynchronous rendering needs to be done (i.e. OWL rendering). This is to prevent external steps occuring inside the rendered result from being applied when the rendering is currently ongoing. Remove `historyResetFromSteps` and `onExternalHistorySteps` as they were only introduced to trigger an update of Knowledge Behaviors, but now that update is done through `checkForExtraRendering`. Modify collaborative tests of the editor so that steps `afterCreate` and `afterCursorInserted` can be asynchronous. Task-2821374 Part-of: odoo/odoo#104680 --- addons/web_editor/__manifest__.py | 1 + .../js/editor/odoo-editor/src/OdooEditor.js | 48 ++++++- .../odoo-editor/test/spec/collab.test.js | 135 +++++++++++++----- .../static/src/js/wysiwyg/wysiwyg.js | 3 + 4 files changed, 147 insertions(+), 40 deletions(-) diff --git a/addons/web_editor/__manifest__.py b/addons/web_editor/__manifest__.py index 3bc1544c8db..569bc1b9575 100644 --- a/addons/web_editor/__manifest__.py +++ b/addons/web_editor/__manifest__.py @@ -175,6 +175,7 @@ Odoo Web Editor widget. 'web/static/src/legacy/js/promise_extension.js', 'web/static/src/boot.js', # insert module dependencies here + 'web/static/src/core/utils/concurrency.js', 'web_editor/static/src/js/editor/odoo-editor/src/**/*js', 'web_editor/static/src/js/editor/odoo-editor/test/spec/*js', 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 073cf27ba93..ad928a1a79b 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 @@ -239,6 +239,23 @@ export class OdooEditor extends EventTarget { preHistoryUndo: () => {}, isHintBlacklisted: () => false, filterMutationRecords: (records) => records, + /** + * In case an external asynchronous post processing has to be + * applied on some nodes after an external step (i.e. render + * an OWL Component), the owner of the post-processing will + * return a Promise through this hook resolved when it is done. + * Further collaborative external steps will be buffered as + * long as that promise is not resolved, to avoid a situation + * where the editor tries to apply mutations inside a node that + * is currently being rendered (not ready). + * + * @param {Element} editable + * @returns {Promise|null} Promise that will be resolved when + * the rendering is done, or null if there is no + * rendering to do. The editor will buffer new external + * steps (collaborative) until the promise is resolved. + */ + postProcessExternalSteps: () => null, onPostSanitize: () => {}, direction: 'ltr', _t: string => string, @@ -325,6 +342,11 @@ export class OdooEditor extends EventTarget { this._collabSelectionsContainer.classList.add('oe-collaboration-selections-container'); this.editable.before(this._collabSelectionsContainer); + // Promise for extra rendering, collaborative external steps will be + // buffered (delayed) until it is resolved. + this._postProcessExternalStepsPromise = null; + this._externalStepsBuffer = []; + this.idSet(editable); this._historyStepsActive = true; this.historyReset(); @@ -1071,10 +1093,11 @@ export class OdooEditor extends EventTarget { this._historySnapshots = [{ step: steps[0] }]; this._historySteps = steps; + this._postProcessExternalStepsPromise = this.options.postProcessExternalSteps(this.editable); + this._handleCommandHint(); this.multiselectionRefresh(); this.observerActive(); - this.dispatchEvent(new Event('historyResetFromSteps')); } historyGetMissingSteps({fromStepId, toStepId}) { const fromIndex = this._historySteps.findIndex(x => x.id === fromStepId); @@ -1535,19 +1558,40 @@ export class OdooEditor extends EventTarget { this._collabClientId = id; } + /** + * Apply external steps coming from the collaboration. Buffer them if + * _postProcessExternalStepsPromise is not null until it is resolved (since + * steps could potentially concern elements currently being rendered + * asynchronously). + * + * @param {Object} newSteps External steps to be applied + */ onExternalHistorySteps(newSteps) { + if (this._postProcessExternalStepsPromise) { + this._externalStepsBuffer.push(...newSteps); + } this.observerUnactive(); this._computeHistorySelection(); + let stepIndex = 0; for (const newStep of newSteps) { this._historyAddExternalStep(newStep); + stepIndex++; + this._postProcessExternalStepsPromise = this.options.postProcessExternalSteps(this.editable); + if (this._postProcessExternalStepsPromise) { + this._postProcessExternalStepsPromise.then(() => { + this._postProcessExternalStepsPromise = undefined; + this.onExternalHistorySteps(this._externalStepsBuffer); + }); + this._externalStepsBuffer = newSteps.slice(stepIndex); + break; + } } this.observerActive(); this.historyResetLatestComputedSelection(); this._handleCommandHint(); this.multiselectionRefresh(); - this.dispatchEvent(new Event('onExternalHistorySteps')); } // Multi selection diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js index fbccfbfe268..415e3a1db02 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js @@ -10,6 +10,7 @@ import { undo, unformat, } from '../utils.js'; +import { Deferred } from "@web/core/utils/concurrency"; const overridenDomClass = [ 'HTMLBRElement', @@ -73,7 +74,7 @@ const testSameHistory = clientInfos => { } }; -const testMultiEditor = spec => { +const testMultiEditor = async spec => { const clientInfos = {}; const concurentActions = spec.concurentActions || []; const clientIds = spec.clientIds || Object.keys(concurentActions); @@ -124,7 +125,7 @@ const testMultiEditor = spec => { let nextId = 1; OdooEditor.prototype._generateId = () => 'fake_id_' + nextId++; - clientInfo.editor = new OdooEditor(clientInfo.editable, { + clientInfo.editor = new OdooEditor(clientInfo.editable, Object.assign({ toSanitize: false, document: iframeDocument, collaborationClientId: clientInfo.clientId, @@ -143,7 +144,7 @@ const testMultiEditor = spec => { } clientInfo.editor.onExternalHistorySteps(missingSteps.concat([step])); }, - }); + }, spec.options || {})); clientInfo.editor.keyboardType = 'PHYSICAL'; const selection = selections[clientInfo.clientId]; if (selection) { @@ -163,7 +164,7 @@ const testMultiEditor = spec => { OdooEditor.prototype._generateId = () => 'fake_concurent_id_' + concurentNextId++; if (spec.afterCreate) { - spec.afterCreate(clientInfos); + await spec.afterCreate(clientInfos); } shouldListenSteps = false; @@ -223,7 +224,7 @@ const testMultiEditor = spec => { } } if (spec.afterCursorInserted) { - spec.afterCursorInserted(clientInfos); + await spec.afterCursorInserted(clientInfos); } for (const clientInfo of clientInfosList) { clientInfo.editor.destroy(); @@ -233,8 +234,8 @@ const testMultiEditor = spec => { describe('Collaboration', () => { describe('Conflict resolution', () => { - it('all client steps should be on the same order', () => { - testMultiEditor({ + it('all client steps should be on the same order', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2', 'c3'], contentBefore: '

    a[c1}{c1]e[c2}{c2]i[c3}{c3]

    ', afterCreate: clientInfos => { @@ -261,8 +262,8 @@ describe('Collaboration', () => { contentAfter: '

    abcd[c1}{c1]efgh[c2}{c2]ijkl[c3}{c3]

    ', }); }); - it('should 2 client insertText in 2 different paragraph', () => { - testMultiEditor({ + it('should 2 client insertText in 2 different paragraph', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2'], contentBefore: '

    ab[c1}{c1]

    cd[c2}{c2]

    ', afterCreate: clientInfos => { @@ -280,8 +281,8 @@ describe('Collaboration', () => { contentAfter: '

    abe[c1}{c1]

    cdf[c2}{c2]

    ', }); }); - it('should 2 client insertText twice in 2 different paragraph', () => { - testMultiEditor({ + it('should 2 client insertText twice in 2 different paragraph', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2'], contentBefore: '

    ab[c1}{c1]

    cd[c2}{c2]

    ', afterCreate: clientInfos => { @@ -301,8 +302,8 @@ describe('Collaboration', () => { contentAfter: '

    abef[c1}{c1]

    cdgh[c2}{c2]

    ', }); }); - it('should insertText with client 1 and deleteBackward with client 2', () => { - testMultiEditor({ + it('should insertText with client 1 and deleteBackward with client 2', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2'], contentBefore: '

    ab[c1}{c1][c2}{c2]c

    ', afterCreate: clientInfos => { @@ -320,8 +321,8 @@ describe('Collaboration', () => { contentAfter: '

    a[c2}{c2]c[c1}{c1]dc

    ', }); }); - it('should insertText twice with client 1 and deleteBackward twice with client 2', () => { - testMultiEditor({ + it('should insertText twice with client 1 and deleteBackward twice with client 2', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2'], contentBefore: '

    ab[c1}{c1][c2}{c2]c

    ', afterCreate: clientInfos => { @@ -342,8 +343,8 @@ describe('Collaboration', () => { }); }); }); - it('should reset from snapshot', () => { - testMultiEditor({ + it('should reset from snapshot', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2'], contentBefore: '

    a[c1}{c1]

    ', afterCreate: clientInfos => { @@ -365,8 +366,8 @@ describe('Collaboration', () => { }); }); describe('steps whith no parent in history', () => { - it('should be able to retreive steps when disconnected from clients that has send step', () => { - testMultiEditor({ + it('should be able to retreive steps when disconnected from clients that has send step', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2', 'c3'], contentBefore: '

    a[c1}{c1]b[c2}{c2]c[c3}{c3]

    ', afterCreate: clientInfos => { @@ -390,8 +391,8 @@ describe('Collaboration', () => { contentAfter: '

    ad[c1}{c1]be[c2}{c2]c[c3}{c3]

    ', }); }); - it('should receive steps where parent was not received', () => { - testMultiEditor({ + it('should receive steps where parent was not received', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2', 'c3'], contentBefore: '

    a[c1}{c1]b[c2}{c2]

    ', afterCreate: clientInfos => { @@ -433,8 +434,8 @@ describe('Collaboration', () => { }); }); describe('sanitize', () => { - it('should sanitize when adding a node', () => { - testMultiEditor({ + it('should sanitize when adding a node', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2'], contentBefore: '

    a

    ', afterCreate: clientInfos => { @@ -453,7 +454,7 @@ describe('Collaboration', () => { }); }); it('should sanitize when adding a script as descendant', async () => { - testMultiEditor({ + await testMultiEditor({ clientIds: ['c1', 'c2'], contentBefore: '

    a[c1}{c1][c2}{c2]

    ', afterCreate: clientInfos => { @@ -472,8 +473,8 @@ describe('Collaboration', () => { }, }); }); - it('should sanitize when changing an attribute', () => { - testMultiEditor({ + it('should sanitize when changing an attribute', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2'], contentBefore: '

    a

    ', afterCreate: clientInfos => { @@ -494,8 +495,8 @@ describe('Collaboration', () => { }); }); - it('should sanitize when undo is adding a script node', () => { - testMultiEditor({ + it('should sanitize when undo is adding a script node', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2'], contentBefore: '

    a

    ', afterCreate: clientInfos => { @@ -520,8 +521,8 @@ describe('Collaboration', () => { }, }); }); - it('should sanitize when undo is adding a descendant script node', () => { - testMultiEditor({ + it('should sanitize when undo is adding a descendant script node', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2'], contentBefore: '

    a

    ', afterCreate: clientInfos => { @@ -546,8 +547,8 @@ describe('Collaboration', () => { }, }); }); - it('should sanitize when undo is changing an attribute', () => { - testMultiEditor({ + it('should sanitize when undo is changing an attribute', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2'], contentBefore: '

    a

    ', afterCreate: clientInfos => { @@ -573,8 +574,8 @@ describe('Collaboration', () => { }, }); }); - it('should not sanitize contenteditable attribute (check DOMPurify DEFAULT_ALLOWED_ATTR)', () => { - testMultiEditor({ + it('should not sanitize contenteditable attribute (check DOMPurify DEFAULT_ALLOWED_ATTR)', async () => { + await testMultiEditor({ clientIds: ['c1'], contentBefore: '
    [c1}{c1]
    ', afterCreate: clientInfos => { @@ -590,8 +591,8 @@ describe('Collaboration', () => { }); }); describe('data-oe-protected', () => { - it('should not share protected mutations and share unprotected ones', () => { - testMultiEditor({ + it('should not share protected mutations and share unprotected ones', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2'], contentBefore: '

    [c1}{c1][c2}{c2]

    ', afterCreate: clientInfos => { @@ -637,8 +638,8 @@ describe('Collaboration', () => { }); }); describe('data-oe-transient-content', () => { - it('should send an empty transient-content element', () => { - testMultiEditor({ + it('should send an empty transient-content element', async () => { + await testMultiEditor({ clientIds: ['c1', 'c2'], contentBefore: '

    [c1}{c1][c2}{c2]

    ', afterCreate: clientInfos => { @@ -668,4 +669,62 @@ describe('Collaboration', () => { }); }); }); + describe('post process external steps', () => { + it('should properly await a processing promise before accepting new external steps.', async () => { + const deferredPromise = new Deferred(); + const postProcessExternalSteps = (element) => { + if (element.querySelector('.process')) { + setTimeout(() => { + deferredPromise.resolve(); + }); + return deferredPromise; + } + return null; + }; + await testMultiEditor({ + options: { + postProcessExternalSteps: postProcessExternalSteps, + }, + clientIds: ['c1', 'c2'], + contentBefore: '

    [c1}{c1][c2}{c2]

    ', + afterCreate: async clientInfos => { + clientInfos.c1.editor.editable.append(...parseHTML(unformat(` +
    +

    secret

    +
    + `)).children); + clientInfos.c1.editor.historyStep(); + clientInfos.c1.editor.editable.append(...parseHTML(unformat(` +

    post-process

    + `)).children); + clientInfos.c1.editor.historyStep(); + clientInfos.c2.editor.onExternalHistorySteps( + clientInfos.c1.editor._historySteps + ); + chai.expect(clientInfos.c1.editable.innerHTML).to.equal(unformat(` +

    +
    +

    secret

    +
    +

    post-process

    + `)); + chai.expect(clientInfos.c2.editable.innerHTML).to.equal(unformat(` +

    +
    +

    secret

    +
    + `)); + await deferredPromise; + chai.expect(clientInfos.c2.editable.innerHTML).to.equal(unformat(` +

    +
    +

    secret

    +
    +

    post-process

    + `)); + testSameHistory(clientInfos); + }, + }); + }); + }) }); diff --git a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js index 6548fcf75db..c9578a2ddcb 100644 --- a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js +++ b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js @@ -694,6 +694,9 @@ const Wysiwyg = Widget.extend({ this.ptp && this.odooEditor.onExternalHistorySteps(missingSteps.concat([step])); }, }; + if (this.options.postProcessExternalSteps) { + editorCollaborationOptions.postProcessExternalSteps = this.options.postProcessExternalSteps; + } return editorCollaborationOptions; }, /**