From 40de57cbcaf94fc9299bb2a54e9556e0383f6c1c Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Wed, 30 Aug 2023 15:38:49 +0200 Subject: [PATCH] [FIX] web_editor: prevent crash because of saved snippets tooltips Since [1], many tickets were created because of a crash preventing any action in edit mode. This actually occurred in most databases were snippets were saved previously (which probably is most old-enough ones). Steps to reproduce on a fresh DB: 1. Enter edit mode 2. Drag and drop a snippet which contains an image, like "Text - Image" 3. Click on the image once (a tooltip asks to double click) 4. Save the snippet (not the page, floppy disk icon in the right panel) => You cannot do anything in edit mode ever again. This is actually a combination of 2 bugs: A. On step (3) in the steps above, the image receives the "aria-describedby" attribute related to the tooltip... and it never disappears (if the page is saved, the attribute is saved with it). When saving the snippets we should probably clean such problematic attributes to be sure anyway too. B. Because of [1], the snippets saved by the customers and which contain the problematic "aria-describedby" attribute are now the source of a crash: we try to hide the related tooltips... even though there is none, which causes a crash. This commit focuses on fixing (B). (A) will need to be fixed later, probably in an earlier version anyway. [1]: https://github.com/odoo/odoo/commit/095721a9cace476c3eae445c7abd8f8b458aea36 opw-3475051 opw-3481944 opw-3482170 opw-3483220 opw-3483829 opw-3483837 opw-3484069 opw-3484647 opw-3484950 opw-3485343 opw-3485612 opw-3485623 opw-3485704 opw-3486096 opw-3486425 ... and many more closes odoo/odoo#133707 X-original-commit: 824be46dded7efacf2a351b78e723aa9e205b60d Signed-off-by: Romain Derie (rde) Signed-off-by: Quentin Smetz (qsm) --- .../static/src/js/editor/snippets.editor.js | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/addons/web_editor/static/src/js/editor/snippets.editor.js b/addons/web_editor/static/src/js/editor/snippets.editor.js index 9059f6945cd..59c25ccc5b3 100644 --- a/addons/web_editor/static/src/js/editor/snippets.editor.js +++ b/addons/web_editor/static/src/js/editor/snippets.editor.js @@ -3454,7 +3454,7 @@ var SnippetsMenu = Widget.extend({ * @param {this.tabs.VALUE} [tab='blocks'] - the tab to select */ _updateRightPanelContent: function ({content, tab, ...options}) { - this._hideActiveTooltip(); + this._hideTooltips(); this._closeWidgets(); this._currentTab = tab || this.tabs.BLOCKS; @@ -3582,22 +3582,27 @@ var SnippetsMenu = Widget.extend({ }); }, /** - * Hides the active tooltip. + * Hides the active tooltips. + * + * The BS documentation says that "Tooltips that use delegation (which are + * created using the selector option) cannot be individually destroyed on + * descendant trigger elements". So this function should be useful to remove + * the active tooltips manually. + * For instance, without this, clicking on "Hide in Desktop" on a snippet + * will leave the tooltip "forever" visible even if the "Hide in Desktop" + * button is gone. * * @private */ - _hideActiveTooltip() { - // The BS documentation says that "Tooltips that use delegation (which - // are created using the selector option) cannot be individually - // destroyed on descendant trigger elements". So we remove the active - // tooltips manually. - // For instance, without this, clicking on "Hide in Desktop" on a - // snippet will leave the tooltip "forever" visible even if the "Hide in - // Desktop" button is gone. - const tooltipClass = 'aria-describedby'; - const tooltippedEl = this.el.querySelector(`[${tooltipClass}^="tooltip"]`); - if (tooltippedEl) { - Tooltip.getInstance(tooltippedEl).hide(); + _hideTooltips() { + // While functionally there is probably no way to have multiple active + // tooltips, it is possible that the panel contains multiple tooltip + // descriptions (we do not know what is in customers' own saved snippets + // for example). In any case, it does not hurt to technically consider + // the case anyway. + const tooltipTargetEls = this.el.querySelectorAll('[aria-describedby^="tooltip"]'); + for (const el of tooltipTargetEls) { + Tooltip.getInstance(el)?.hide(); } }, @@ -4472,9 +4477,9 @@ var SnippetsMenu = Widget.extend({ } this._buttonAction = true; let removeLoadingEffect; - // Remove the tooltip now, because the button will be disabled and so, + // Remove the tooltips now, because the button will be disabled and so, // the tooltip will not be removable (see BS doc). - this._hideActiveTooltip(); + this._hideTooltips(); if (addLoadingEffect) { removeLoadingEffect = dom.addButtonLoadingEffect(button); }