From e00a33b17ac1d01cfda6a95d9a40f3af07629062 Mon Sep 17 00:00:00 2001 From: abd-msyukyu-odoo Date: Tue, 4 Apr 2023 09:58:54 +0000 Subject: [PATCH] [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; }, /**