From 4a57f9c54fb2b15de1a26373ea1c4fd5ebf8256e Mon Sep 17 00:00:00 2001 From: Antoine Guenet Date: Fri, 3 Feb 2023 09:50:21 +0000 Subject: [PATCH] [FIX] web_editor: prevent infinite loop on using checkboxes In order to be able to check checklist items and color stars even when in readonly mode, we need to add unique ids to the checkboxes and stars. Since these ids were random and in order to ensure uniqueness after DOM manipulations, we updated all these ids every time the document was sanitized. Since the `sanitize` function is called every time the `cleanForSave` method is called, and since `wysiwyg`'s `getValue` method calls `cleanForSave`, this meant that the ids were modified every time we were getting the value of the wysiwyg. This meant that every call to `getValue` returned a different value, so the editable was always marked as dirty, sometimes causing an infinite loop (see steps below). This commit makes the IDs iterative rather than random, so they are predictable (practical for testing), and ensures that the ids are only updated when necessary, so that upon two consecutive calls to the `sanitize` function, only the first call may change the HTML. STEPS: * create a sale order * add checkboxes to Terms & Conditions fields * edit a sale order line * click outside the edited sale order line opw-3116430 X-original-commit: 86ff07d9c201668e372377f9ac41d3801aa223e9 Part-of: odoo/odoo#111871 --- .../editor/odoo-editor/src/utils/sanitize.js | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) 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 0d4458e2189..2b7e64d2a20 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 @@ -82,19 +82,35 @@ export function areSimilarElements(node, node2) { class Sanitize { constructor(root) { this.root = root; + this.parse(root); + // Handle unique ids. const rootClosestBlock = closestBlock(root); - if (rootClosestBlock) { - // Remove unique ids from checklists and stars. These will be - // renewed afterwards. - for (const node of rootClosestBlock.querySelectorAll('[id^=checkId-]')) { - node.removeAttribute('id'); - } - } - this.parse(root); if (rootClosestBlock) { // Ensure unique ids on checklists and stars. - for (const node of rootClosestBlock.querySelectorAll('.o_checklist > li, .o_stars')) { - node.setAttribute('id', `checkId-${Math.floor(new Date() * Math.random())}`); + const elementsWithId = [...rootClosestBlock.querySelectorAll('[id^=checkId-]')]; + const maxId = Math.max(...[0, ...elementsWithId.map(node => +node.getAttribute('id').substring(8))]); + let nextId = maxId + 1; + const ids = []; + for (const node of rootClosestBlock.querySelectorAll('[id^=checkId-], .o_checklist > li, .o_stars')) { + if ( + !node.classList.contains('o_stars') && ( + !node.parentElement.classList.contains('o_checklist') || + [...node.children].some(child => ['UL', 'OL'].includes(child.nodeName)) + )) { + // Remove unique ids from checklists and stars from elements + // that are no longer checklist items or stars, and from + // parents of nested lists. + node.removeAttribute('id') + } else { + // Add/change IDs where needed, and ensure they're unique. + let id = node.getAttribute('id'); + if (!id || ids.includes(id)) { + id = `checkId-${nextId}`; + nextId++; + node.setAttribute('id', id); + } + ids.push(id); + } } } }