From 8f9362809e91ff7c2876ab464017abb13a4e0974 Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Tue, 25 Apr 2017 13:52:21 +0200
Subject: [PATCH] [FIX] web: FormView: notebook: activate first visible tab

In a notebook, tabs may have an invisible attrs. The first visible
tab should be active in any case. This wasn't working at the first
rendering of the view, because the invisible attrs was evaluated
for each tab, before processing the next ones, so the it couldn't
find a next tab to activate.

This fix ensures to process all tabs (i.e. generate their DOM
element) before evaluating their invisible attrs, and trying to
find which tab to activate.
---
 .../static/src/js/views/form/form_renderer.js | 26 +++++++++++-----
 addons/web/static/tests/views/form_tests.js   | 31 +++++++++++++++++++
 2 files changed, 49 insertions(+), 8 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 6955ca47c5c..82d2453c24e 100644
--- a/addons/web/static/src/js/views/form/form_renderer.js
+++ b/addons/web/static/src/js/views/form/form_renderer.js
@@ -335,8 +335,6 @@ var FormRenderer = BasicRenderer.extend({
             });
             this._registerModifiers(node, this.state, $label, {
                 callback: function (element, modifiers, record) {
-                    var recordWidgets = self.allFieldWidgets[record.id];
-                    var widget = _.findWhere(recordWidgets, {name: fieldName});
                     element.$el.toggleClass('o_form_label_empty', !!(
                         record.data.id
                         && (modifiers.readonly || self.mode === 'readonly')
@@ -623,7 +621,11 @@ var FormRenderer = BasicRenderer.extend({
         var self = this;
         var $headers = $('<ul class="nav nav-tabs">');
         var $pages = $('<div class="tab-content nav nav-tabs">');
-        _.each(node.children, function (child, index) {
+        // renderedTabs is used to aggregate the generated $headers and $pages
+        // alongside their node, so that their modifiers can be registered once
+        // all tabs have been rendered, to ensure that the first visible tab
+        // is correctly activated
+        var renderedTabs = _.map(node.children, function (child, index) {
             var pageID = _.uniqueId('notebook_page_');
             var $header = self._renderTabHeader(child, pageID);
             var $page = self._renderTabPage(child, pageID);
@@ -632,20 +634,28 @@ var FormRenderer = BasicRenderer.extend({
                 $page.addClass('active');
             }
             self._handleAttributes($header, child);
-            self._registerModifiers(child, self.state, $header, {
-                callback: function (element, modifiers, record) {
+            $headers.append($header);
+            $pages.append($page);
+            return {
+                $header: $header,
+                $page: $page,
+                node: child,
+            };
+        });
+        // register the modifiers for each tab
+        _.each(renderedTabs, function (tab) {
+            self._registerModifiers(tab.node, self.state, tab.$header, {
+                callback: function (element, modifiers) {
                     // if the active tab is invisible, activate the first visible tab instead
                     if (modifiers.invisible && element.$el.hasClass('active')) {
                         element.$el.removeClass('active');
-                        $page.removeClass('active');
+                        tab.$page.removeClass('active');
                         var $firstVisibleTab = $headers.find('li:not(.o_form_invisible):first()');
                         $firstVisibleTab.addClass('active');
                         $pages.find($firstVisibleTab.find('a').attr('href')).addClass('active');
                     }
                 },
             });
-            $headers.append($header);
-            $pages.append($page);
         });
         return $('<div class="o_notebook">')
                 .data('name', node.attrs.name || '_default_')
diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js
index 3118937d3ee..2c6d01fab6b 100644
--- a/addons/web/static/tests/views/form_tests.js
+++ b/addons/web/static/tests/views/form_tests.js
@@ -340,6 +340,37 @@ QUnit.module('Views', {
         form.destroy();
     });
 
+    QUnit.test('first notebook page invisible', function (assert) {
+        assert.expect(2);
+
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form string="Partners">' +
+                    '<sheet>' +
+                        '<field name="product_id"/>' +
+                        '<notebook>' +
+                            '<page string="Foo" invisible="1">' +
+                                '<field name="foo"/>' +
+                            '</page>' +
+                            '<page string="Bar">' +
+                                '<field name="bar"/>' +
+                            '</page>' +
+                        '</notebook>' +
+                    '</sheet>' +
+                '</form>',
+            res_id: 1,
+        });
+
+        assert.ok(form.$('.o_notebook .nav li:first()').hasClass('o_form_invisible'),
+            'first tab should be invisible');
+        assert.ok(form.$('.o_notebook .nav li:nth(1)').hasClass('active'),
+            'second tab should be active');
+
+        form.destroy();
+    });
+
     QUnit.test('invisible attrs on group are re-evaluated on field change', function (assert) {
         assert.expect(2);