From 4e115baaadbcd9ea8cb284e608b3c7a0d5f7ea2f Mon Sep 17 00:00:00 2001 From: abd-msyukyu-odoo Date: Sat, 25 Feb 2023 15:02:03 +0000 Subject: [PATCH] [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 = {