From 869cc1523242c9c22cf02c327a832bc7cd34e10c Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Tue, 2 Jul 2019 13:09:16 +0000
Subject: [PATCH] [FIX] web: form: combine field and renderer classnames

The rendering of field widgets is asynchronous (as it returns a
native Promise). However, the '_renderFieldWidget' method of basic
renderers is sync, and returns a fake div that will be replaced by
the widget'$el afterwards.

The form renderer sometimes needs to add classNames on the rendered
nodes (e.g. each child of an outer group as class 'o_group_col_x').

To handle this, once the field widget is ready, we copied all
attributes of the fake node to the widget's node. Unfortunately, we
were erasing the attr value (e.g. className) by the new one. As a
consequence, the className defined in the widget were lost. This
could be observed for the 'note' field in 'sale.order'.

As this was only necessary for classNames, this rev. only retrieves
the class attribute of the fake node, and adds it to the classNames
of the widget's $el.

closes odoo/odoo#34528

Signed-off-by: VincentSchippefilt <VincentSchippefilt@users.noreply.github.com>
---
 .../src/js/views/basic/basic_renderer.js      |  4 +-
 addons/web/static/tests/views/form_tests.js   | 37 ++++++++++++++++++-
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/addons/web/static/src/js/views/basic/basic_renderer.js b/addons/web/static/src/js/views/basic/basic_renderer.js
index 4c03d9b77a1..d01bf40669e 100644
--- a/addons/web/static/src/js/views/basic/basic_renderer.js
+++ b/addons/web/static/src/js/views/basic/basic_renderer.js
@@ -590,8 +590,8 @@ var BasicRenderer = AbstractRenderer.extend({
             // this.renderFieldWidget(...).addClass(...), the class is added on
             // the temporary div and not on the actual element that will be
             // rendered. As we do not return a promise and some callers cannot
-            // wait for this.defs, we copy those attributes to the final element.
-            widget.$el.attr($el.getAttributes());
+            // wait for this.defs, we copy those classnames to the final element.
+            widget.$el.addClass($el.attr('class'));
 
             $el.replaceWith(widget.$el);
             self._registerModifiers(node, record, widget, {
diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js
index 5b13568192b..f1c12414fcf 100644
--- a/addons/web/static/tests/views/form_tests.js
+++ b/addons/web/static/tests/views/form_tests.js
@@ -209,7 +209,7 @@ QUnit.module('Views', {
         assert.hasClass(form.$('div.o_group span[name="int_field"]:eq(0)'),'o_readonly_modifier', "second int_field widget should be readonly");
         form.destroy();
     });
-   
+
     QUnit.test('duplicate fields rendered properly (one2many)', async function (assert) {
         assert.expect(7);
         this.data.partner.records.push({
@@ -398,6 +398,39 @@ QUnit.module('Views', {
 
         form.destroy();
     });
+
+    QUnit.test('group containing both a field and a group', async function (assert) {
+        // The purpose of this test is to check that classnames defined in a
+        // field widget and those added by the form renderer are correctly
+        // combined. For instance, the renderer adds className 'o_group_col_x'
+        // on outer group's children (an outer group being a group that contains
+        // at least a group).
+        assert.expect(4);
+
+        var form = await createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form>' +
+                    '<group>' +
+                        '<field name="foo"/>' +
+                        '<group>' +
+                            '<field name="int_field"/>' +
+                        '</group>' +
+                    '</group>' +
+                '</form>',
+            res_id: 1,
+        });
+
+        assert.containsOnce(form, '.o_group .o_field_widget[name=foo]');
+        assert.containsOnce(form, '.o_group .o_inner_group .o_field_widget[name=int_field]');
+
+        assert.hasClass(form.$('.o_field_widget[name=foo]'), 'o_field_char');
+        assert.hasClass(form.$('.o_field_widget[name=foo]'), 'o_group_col_6');
+
+        form.destroy();
+    });
+
     QUnit.test('Form and subview with _view_ref contexts', async function (assert) {
         assert.expect(2);
 
@@ -470,6 +503,7 @@ QUnit.module('Views', {
         form.destroy();
         actionManager.destroy();
     });
+
     QUnit.test('invisible fields are properly hidden', async function (assert) {
         assert.expect(4);
 
@@ -6857,7 +6891,6 @@ QUnit.module('Views', {
         delete widgetRegistry.map.test;
     });
 
-
     QUnit.test('bounce edit button in readonly mode', async function (assert) {
         assert.expect(3);