From f7a8f42113d96e95236afffb70bfc9f35dc78173 Mon Sep 17 00:00:00 2001
From: Julien Mougenot <jum@odoo.com>
Date: Wed, 15 Apr 2020 10:13:33 +0000
Subject: [PATCH] [FIX] web: export/import form state with pure JS

Before this commit, restoring the previously active tab on a form view was done
through programatically clicking on the tab and letting Bootstrap handle the switch
itself. This caused some cases of race condition since the click handler is asynchronous.

Now, classes are applied manually and should not cause any concurrency issue.

closes odoo/odoo#49668

X-original-commit: 214b842cba030f451430ff0d8c3fc1868be515e9
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
Signed-off-by: Julien Mougenot (jum) <jum@odoo.com>
---
 .../static/src/js/views/form/form_renderer.js | 58 +++++++++++--------
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/addons/web/static/src/js/views/form/form_renderer.js b/addons/web/static/src/js/views/form/form_renderer.js
index 18a5695005f..08819f79f57 100644
--- a/addons/web/static/src/js/views/form/form_renderer.js
+++ b/addons/web/static/src/js/views/form/form_renderer.js
@@ -209,18 +209,14 @@ var FormRenderer = BasicRenderer.extend({
      * @returns {Object} a map from notebook name to the active tab index
      */
     getLocalState: function () {
-        var state = {};
-        this.$('div.o_notebook').each(function () {
-            var $notebook = $(this);
-            var name = $notebook.data('name');
-            var index = -1;
-            $notebook.find('.nav-link').each(function (i) {
-                if ($(this).hasClass('active')) {
-                    index = i;
-                }
-            });
-            state[name] = index;
-        });
+        const state = {};
+        for (const notebook of this.el.querySelectorAll(':scope div.o_notebook')) {
+            const name = notebook.dataset.name;
+            const navs = notebook.querySelectorAll(':scope .o_notebook_headers .nav-item > .nav-link');
+            state[name] = [...navs].findIndex(
+                nav => nav.classList.contains('active')
+            );
+        }
         return state;
     },
     /**
@@ -237,23 +233,36 @@ var FormRenderer = BasicRenderer.extend({
         this.lastActivatedFieldIndex = -1;
     },
     /**
-     * restore active tab pages for each notebook
-     *
-     * @todo make sure this method is called
+     * Restore active tab pages for each notebook. It relies on the implicit fact
+     * that each nav header corresponds to a tab page.
      *
      * @param {Object} state the result from a getLocalState call
      */
     setLocalState: function (state) {
-        this.$('div.o_notebook').each(function () {
-            var $notebook = $(this);
-            var name = $notebook.data('name');
+        for (const notebook of this.el.querySelectorAll(':scope div.o_notebook')) {
+            const name = notebook.dataset.name;
             if (name in state) {
-                var $page = $notebook.find('> .o_notebook_headers > .nav-tabs > .nav-item').eq(state[name]);
-                if (!$page.hasClass('o_invisible_modifier')) {
-                    $page.find('a[data-toggle="tab"]').click();
+                const navs = notebook.querySelectorAll(':scope .o_notebook_headers .nav-item');
+                const pages = notebook.querySelectorAll(':scope .tab-content > .tab-pane');
+                // We can't base the amount on the 'navs' length since some overrides
+                // are adding pageless nav items.
+                const validTabsAmount = pages.length;
+                let activeIndex = state[name];
+                if (navs[activeIndex].classList.contains('o_invisible_modifier')) {
+                    activeIndex = [...navs].findIndex(
+                        nav => !nav.classList.contains('o_invisible_modifier')
+                    );
                 }
+                if (activeIndex <= 0) {
+                    continue; // No visible tab OR first tab = active tab (no change to make).
+                }
+                for (let i = 0; i < validTabsAmount; i++) {
+                    navs[i].querySelector('.nav-link').classList.toggle('active', activeIndex === i);
+                    pages[i].classList.toggle('active', activeIndex === i);
+                }
+                core.bus.trigger('DOM_updated');
             }
-        });
+        }
     },
     /**
      * @override method from AbstractRenderer
@@ -915,9 +924,8 @@ var FormRenderer = BasicRenderer.extend({
             });
         });
         var $notebookHeaders = $('<div class="o_notebook_headers">').append($headers);
-        var $notebook = $('<div class="o_notebook">')
-                .data('name', node.attrs.name || '_default_')
-                .append($notebookHeaders, $pages);
+        var $notebook = $('<div class="o_notebook">').append($notebookHeaders, $pages);
+        $notebook[0].dataset.name = node.attrs.name || '_default_';
         this._registerModifiers(node, this.state, $notebook);
         this._handleAttributes($notebook, node);
         return $notebook;