From 2dc940693289f05d44fc988f8092a3aff6e83682 Mon Sep 17 00:00:00 2001 From: David Monjoie Date: Wed, 29 Jun 2022 10:33:13 +0000 Subject: [PATCH] [FIX] web_editor: less sanitize _parse recursive calls The _parse function of sanitize used to be recursive both along the length and depth of the DOM tree, making it exceed maximum stack size limit for long documents. This updated version is now only recursive in the depth of the tree, not its length. It would be possible to write a fully non-recursive version of this function but it is out of scope of this task and is unlikely to happen anyway as DOM trees are way more unlikely to exceed stack limits in depth rather than length. task-2895671 closes odoo/odoo#94967 X-original-commit: c711a2c4106da0c2d5c35ee7a367ba86a9dcbdb7 Signed-off-by: David Monjoie (dmo) --- .../lib/odoo-editor/src/utils/sanitize.js | 160 +++++++++--------- 1 file changed, 82 insertions(+), 78 deletions(-) diff --git a/addons/web_editor/static/lib/odoo-editor/src/utils/sanitize.js b/addons/web_editor/static/lib/odoo-editor/src/utils/sanitize.js index 8ea53d29b1e..498f83bd6e7 100644 --- a/addons/web_editor/static/lib/odoo-editor/src/utils/sanitize.js +++ b/addons/web_editor/static/lib/odoo-editor/src/utils/sanitize.js @@ -104,93 +104,97 @@ class Sanitize { } _parse(node) { - if (!node) { - return; - } - - // Merge identical elements together - while (areSimilarElements(node, node.previousSibling) && !isUnbreakable(node)) { - getDeepRange(this.root, { select: true }); - const restoreCursor = node.isConnected && preserveCursor(this.root.ownerDocument); - const nodeP = node.previousSibling; - moveNodes(...endPos(node.previousSibling), node); - if (restoreCursor) { - restoreCursor(); - }; - node = nodeP; - } - - // Remove zero-width spaces added by `fillEmpty` when there is content - // and the selection is not next to it. - const anchor = this.root.ownerDocument.getSelection().anchorNode; - if ( - node.nodeType === Node.TEXT_NODE && - node.textContent.includes('\u200B') && - ( - node.textContent.length > 1 || - // There can be multiple ajacent text nodes, in which case the - // zero-width space is not needed either, despite being alone - // (length === 1) in its own text node. - Array.from(node.parentNode.childNodes).find( - sibling => - sibling !== node && - sibling.nodeType === Node.TEXT_NODE && - sibling.length > 0 - ) - ) && - !isBlock(node.parentElement) && - anchor !== node - ) { - const restoreCursor = node.isConnected && preserveCursor(this.root.ownerDocument); - node.textContent = node.textContent.replace('\u200B', ''); - node.parentElement.removeAttribute("data-oe-zws-empty-inline"); - if (restoreCursor) { - restoreCursor(); - }; - } - - // Remove empty blocks in
  • - if (node.nodeName === 'P' && node.parentElement.tagName === 'LI') { - const next = node.nextSibling; - const pnode = node.parentElement; - if (isEmptyBlock(node)) { - const restoreCursor = node.isConnected && preserveCursor(this.root.ownerDocument); - node.remove(); - fillEmpty(pnode); - this._parse(next); + while (node) { + // Merge identical elements together + while ( + areSimilarElements(node, node.previousSibling) && + !isUnbreakable(node) + ) { + getDeepRange(this.root, { select: true }); + const restoreCursor = node.isConnected && + preserveCursor(this.root.ownerDocument); + const nodeP = node.previousSibling; + moveNodes(...endPos(node.previousSibling), node); if (restoreCursor) { - restoreCursor(new Map([[node, pnode]])); - }; - return; + restoreCursor(); + } + node = nodeP; + } + + // Remove zero-width spaces added by `fillEmpty` when there is + // content and the selection is not next to it. + const anchor = this.root.ownerDocument.getSelection().anchorNode; + if ( + node.nodeType === Node.TEXT_NODE && + node.textContent.includes('\u200B') && + ( + node.textContent.length > 1 || + // There can be multiple ajacent text nodes, in which case + // the zero-width space is not needed either, despite being + // alone (length === 1) in its own text node. + Array.from(node.parentNode.childNodes).find( + sibling => + sibling !== node && + sibling.nodeType === Node.TEXT_NODE && + sibling.length > 0 + ) + ) && + !isBlock(node.parentElement) && + anchor !== node + ) { + const restoreCursor = node.isConnected && + preserveCursor(this.root.ownerDocument); + node.textContent = node.textContent.replace('\u200B', ''); + node.parentElement.removeAttribute("data-oe-zws-empty-inline"); + if (restoreCursor) { + restoreCursor(); + } + } + + // Remove empty blocks in
  • + if ( + node.nodeName === 'P' && + node.parentElement.tagName === 'LI' && + isEmptyBlock(node) + ) { + const parent = node.parentElement; + const restoreCursor = node.isConnected && + preserveCursor(this.root.ownerDocument); + node.remove(); + fillEmpty(parent); + if (restoreCursor) { + restoreCursor(new Map([[node, parent]])); + } + } + + // Transform
  • into

    if they are not in a

      /
        + if (node.nodeName === 'LI' && !node.closest('ul, ol')) { + const paragraph = document.createElement("p"); + paragraph.replaceChildren(...node.childNodes); + node.replaceWith(paragraph); + node = paragraph; } - } - // Transform
      1. into

        if they are not in a

          /
            - if (node.nodeName === 'LI' && !node.closest('ul, ol')) { - const p = document.createElement("p"); - p.replaceChildren(...node.childNodes); - node.replaceWith(p); - node = p; - } - // Sanitize font awesome elements - if (isFontAwesome(node)) { // Ensure a zero width space is present inside the FA element. - if (node.innerHTML !== '\u200B') node.innerHTML = '​'; - } + if (isFontAwesome(node) && node.textContent !== '\u200B') { + node.textContent = '\u200B'; + } - // Sanitize media elements - if (isMediaElement(node) || node.tagName === 'HR') { - // Ensure all media elements are tagged contenteditable=false. - // we cannot use the node.isContentEditable because it can wrongly return false - // when the editor is starting up ( first sanitize ) - if (node.getAttribute('contenteditable') !== 'false') { + // Ensure elements which should not contain any content are tagged + // contenteditable=false to avoid any hiccup. + if ( + (isMediaElement(node) || node.tagName === 'HR') && + node.getAttribute('contenteditable') !== 'false' + ) { node.setAttribute('contenteditable', 'false'); } + + if (node.firstChild) { + this._parse(node.firstChild); + } + node = node.nextSibling; } - // FIXME not parse out of editable zone... - this._parse(node.firstChild); - this._parse(node.nextSibling); } }