From e4e69d54fbbf743d89242e03df768bc4f5f40c57 Mon Sep 17 00:00:00 2001
From: qsm-odoo <qsm@odoo.com>
Date: Mon, 24 Jan 2022 18:14:54 +0000
Subject: [PATCH] [FIX] website, web_editor: prevent traceback on popup removal

Before this commit, a traceback occured when trying to remove a popup
snippet, only if content inside was clicked on before (base case but
that last part was not covered by our main test which drags and drops +
removes all snippets).

The problem is complex and annoying to solve in stable. The cause is the
combination of [1] and [2]. Indeed, with [2] we solved the snippet
activation flow to ensure the onFocus and onBlur methods are called when
needed... but that new correct implementation relies on the fact that
the number of snippet editor instances does not change for the whole
async operation of activating a snippet. That should naturally be the
case but is not because of [1]: when we remove a snippet we first
trigger a snippet activation then destroy the snippet (and its editor
instance)... but the snippet activation is not awaited at the moment.
Thus making the snippet editor instance be destroyed *during* the
snippet activation flow. Of course a better solution than this commit
must be found but this fixes the traceback with a very minimal change
for the 15.0 stable version while waiting for that miraculous solution.

[1]: https://github.com/odoo/odoo/commit/ae219ec06baf0280c76ae09b1453cc1481eece30
[2]: https://github.com/odoo/odoo/commit/0acc5e784b15d9c963660da3781763448503f33e

task-2735663

closes odoo/odoo#83378

X-original-commit: da7c1b5757a7780897c5736ddecc014a7fb50bdb
Signed-off-by: Romain Derie (rde) <rde@odoo.com>
---
 .../static/src/js/editor/snippets.editor.js   | 13 ++++++--
 .../tests/tours/snippet_popup_add_remove.js   | 30 +++++++++++++++++++
 addons/website/tests/test_snippets.py         |  3 ++
 3 files changed, 43 insertions(+), 3 deletions(-)
 create mode 100644 addons/website/static/tests/tours/snippet_popup_add_remove.js

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 645b797fd84..79b86a0301b 100644
--- a/addons/web_editor/static/src/js/editor/snippets.editor.js
+++ b/addons/web_editor/static/src/js/editor/snippets.editor.js
@@ -470,8 +470,11 @@ var SnippetEditor = Widget.extend({
             // body. If the editable has options, we do not want to show them.
             parent = $(parent).closest('body');
         }
-        this.trigger_up('activate_snippet', {
-            $snippet: $(previousSibling || nextSibling || parent)
+        const activateSnippetProm = new Promise(resolve => {
+            this.trigger_up('activate_snippet', {
+                $snippet: $(previousSibling || nextSibling || parent),
+                onSuccess: resolve,
+            });
         });
 
         // Actually remove the snippet and its option UI.
@@ -523,7 +526,11 @@ var SnippetEditor = Widget.extend({
         this.$body.find('.o_table_handler').remove();
 
         this.trigger_up('snippet_removed');
-        this.destroy();
+        // FIXME that whole Promise should be awaited before the DOM removal etc
+        // as explained above where it is defined. However, it is critical to at
+        // least await it before destroying the snippet editor instance
+        // otherwise the logic of activateSnippet gets messed up.
+        activateSnippetProm.then(() => this.destroy());
         $parent.trigger('content_changed');
 
         // TODO Page content changed, some elements may need to be adapted
diff --git a/addons/website/static/tests/tours/snippet_popup_add_remove.js b/addons/website/static/tests/tours/snippet_popup_add_remove.js
new file mode 100644
index 00000000000..fc457e8635f
--- /dev/null
+++ b/addons/website/static/tests/tours/snippet_popup_add_remove.js
@@ -0,0 +1,30 @@
+/** @odoo-module */
+
+import tour from 'web_tour.tour';
+
+tour.register('snippet_popup_add_remove', {
+    test: true,
+    url: '/?enable_editor=1',
+}, [{
+    content: 'Drop s_popup snippet',
+    trigger: '#oe_snippets .oe_snippet:has( > [data-snippet="s_popup"]) .oe_snippet_thumbnail',
+    run: "drag_and_drop #wrap",
+}, {
+    content: 'Edit s_popup snippet',
+    in_modal: false,
+    trigger: '#wrap.o_editable [data-snippet="s_popup"] .row > div', // Click deep in the snippet structure
+}, {
+    content: 'Check s_popup setting are loaded, wait panel is visible',
+    in_modal: false,
+    trigger: '.o_we_customize_panel',
+    run: () => null,
+}, {
+    content: `Remove the s_popup snippet`,
+    in_modal: false,
+    trigger: '.o_we_customize_panel we-button.oe_snippet_remove:first',
+}, {
+    content: 'Check the s_popup was removed',
+    in_modal: false,
+    trigger: '#wrap.o_editable:not(:has([data-snippet="s_popup"]))',
+    run: () => null,
+}]);
diff --git a/addons/website/tests/test_snippets.py b/addons/website/tests/test_snippets.py
index e651d04a32c..557a7df6d3e 100644
--- a/addons/website/tests/test_snippets.py
+++ b/addons/website/tests/test_snippets.py
@@ -38,3 +38,6 @@ class TestSnippets(HttpCase):
             'https://instagram.com/odoo.official/',
             'Social media should have been updated'
         )
+
+    def test_06_snippet_popup_add_remove(self):
+        self.start_tour('/?enable_editor=1', 'snippet_popup_add_remove', login='admin')