[IMP] web: form: labels of several occurrences of a field

Before this commit, when the same field occurred multiple times in
a form view, all occurrences were associated with the same id, and
all occurrences of the corresponding label (if any) had the same
'for' attribute. As a consequence, besides the fact that there were
warnings in the chrome console, clicking on any occurrence of the
duplicated label focuses the first occurrence of the field.

This commit generates a unique id for each pair label/field, when
possible. However, there is a case that can't be handled
automatically: when the label isn't automatically generated (field
located outside a group, or with nolabel attribute set to "1").
Typically, in this case, there is a <label> node with "for"
attribute referencing the associated field. In this situation, the
id must be defined in the arch, and used in the "for" attribute:

  <form>
    <label for="phone"/><field name="phone"/>
    <label for="phone_2"/><field name="phone" id="phone_2"/>
  </form>

Task 2261697

2 files changed, 68 insertions, 30 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 483db4b2b09..d35012bfc39 100644
--- a/addons/web/static/src/js/views/form/form_renderer.js
+++ b/addons/web/static/src/js/views/form/form_renderer.js
@@ -11,6 +11,9 @@ var viewUtils = require('web.viewUtils');
 var _t = core._t;
 var qweb = core.qweb;
 
+// symbol used as key to set the <field> node id on its widget
+const symbol = Symbol('form');
+
 var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
     className: "o_form_view",
     events: _.extend({}, BasicRenderer.prototype.events, {
@@ -28,9 +31,12 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
 
     /**
      * @override
+     * @param {Object} params.fieldIdsToNames maps <field> node ids to field names
+     *   (useful when there are several occurrences of the same field in the arch)
      */
-    init: function () {
+    init: function (parent, state, params) {
         this._super.apply(this, arguments);
+        this.fieldIdsToNames = params.fieldIdsToNames;
         this.idsForLabels = {};
         this.lastActivatedFieldIndex = -1;
         this.alertFields = {};
@@ -113,20 +119,21 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
      * @returns {string[]}
      */
     canBeSaved: function () {
-        var self = this;
         var fieldNames = this._super.apply(this, arguments);
 
         var $labels = this.$('label');
         $labels.removeClass('o_field_invalid');
 
-        _.each(fieldNames, function (fieldName) {
-            var idForLabel = self.idsForLabels[fieldName];
+        const allWidgets = this.allFieldWidgets[this.state.id] || [];
+        const widgets = allWidgets.filter(w => fieldNames.includes(w.name));
+        for (const widget of widgets) {
+            const idForLabel = this.idsForLabels[widget[symbol]];
             if (idForLabel) {
                 $labels
                     .filter('[for=' + idForLabel + ']')
                     .addClass('o_field_invalid');
             }
-        });
+        }
         return fieldNames;
     },
     /*
@@ -171,7 +178,7 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
         var self = this;
         return this._super.apply(this, arguments).then(function (resetWidgets) {
             _.each(resetWidgets, function (widget) {
-                self._setIDForLabel(widget, self.idsForLabels[widget.name]);
+                self._setIDForLabel(widget, self.idsForLabels[widget[symbol]]);
             });
             if (self.$('.o_field_invalid').length) {
                 self.canBeSaved(self.state.id);
@@ -372,16 +379,14 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
     },
     /**
      * @private
-     * @param {string} name
+     * @param {string} uid a <field> node id
      * @returns {string}
      */
-    _getIDForLabel: function (name) {
-        var idForLabel = this.idsForLabels[name];
-        if (!idForLabel) {
-            idForLabel = _.uniqueId('o_field_input_');
-            this.idsForLabels[name] = idForLabel;
+    _getIDForLabel: function (uid) {
+        if (!this.idsForLabels[uid]) {
+            this.idsForLabels[uid] = _.uniqueId('o_field_input_');
         }
-        return idForLabel;
+        return this.idsForLabels[uid];
     },
     /**
      * @override
@@ -396,7 +401,9 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
      */
     _postProcessField: function (widget, node) {
         this._super.apply(this, arguments);
-        this._setIDForLabel(widget, this._getIDForLabel(node.attrs.name));
+        // set the node id on the widget, as it might be necessary later (tooltips, confirmChange...)
+        widget[symbol] = node.attrs.id;
+        this._setIDForLabel(widget, this._getIDForLabel(node.attrs.id));
         if (JSON.parse(node.attrs.default_focus || "0")) {
             this.defaultFocusField = widget;
         }
@@ -656,12 +663,12 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
     },
     /**
      * @private
-     * @param {string} label
+     * @param {Object} node
      * @returns {jQueryElement}
      */
-    _renderInnerGroupLabel: function (label) {
+    _renderInnerGroupLabel: function (node) {
         return $('<td/>', {class: 'o_td_label'})
-            .append(this._renderTagLabel(label));
+            .append(this._renderTagLabel(node));
     },
     /**
      * Render a node, from the arch of the view. It is a generic method, that
@@ -846,17 +853,22 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
     _renderTagLabel: function (node) {
         var self = this;
         var text;
-        var fieldName = node.tag === 'label' ? node.attrs.for : node.attrs.name;
+        let fieldName;
+        if (node.tag === 'label') {
+            fieldName = this.fieldIdsToNames[node.attrs.for]; // 'for' references a <field> node id
+        } else {
+            fieldName = node.attrs.name;
+        }
         if ('string' in node.attrs) { // allow empty string
             text = node.attrs.string;
         } else if (fieldName) {
             text = this.state.fields[fieldName].string;
-        } else  {
+        } else {
             return this._renderGenericTag(node);
         }
         var $result = $('<label>', {
             class: 'o_form_label',
-            for: this._getIDForLabel(fieldName),
+            for: this._getIDForLabel(node.tag === 'label' ? node.attrs.for : node.attrs.id),
             text: text,
         });
         if (node.tag === 'label') {
@@ -1039,16 +1051,8 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
 
         // Attach the tooltips on the fields' label
         _.each(this.allFieldWidgets[this.state.id], function (widget) {
-            var idForLabel = self.idsForLabels[widget.name];
-            // We usually don't support multiple widgets for the same field on the
-            // same view but it is the case with the new settings view on V11.0.
-            // Therefore, we need to retrieve the correct label since it could be
-            // displayed multiple times on the view, otherwise, for example the
-            // enterprise label will be displayed as many times as the field
-            // exists on settings.
-            var $widgets = self.$('.o_field_widget[name=' + widget.name + ']');
+            const idForLabel = self.idsForLabels[widget[symbol]];
             var $label = idForLabel ? self.$('.o_form_label[for=' + idForLabel + ']') : $();
-            $label = $label.eq($widgets.index(widget.$el));
             self._addFieldTooltip(widget, $label);
             if (widget.attrs.widget === 'upgrade_boolean') {
                 // this widget needs a reference to its $label to be correctly
diff --git a/addons/web/static/src/js/views/form/form_view.js b/addons/web/static/src/js/views/form/form_view.js
index 37cca1a0917..e3ec5177df3 100644
--- a/addons/web/static/src/js/views/form/form_view.js
+++ b/addons/web/static/src/js/views/form/form_view.js
@@ -2,11 +2,11 @@ odoo.define('web.FormView', function (require) {
 "use strict";
 
 var BasicView = require('web.BasicView');
-var config = require('web.config');
 var Context = require('web.Context');
 var core = require('web.core');
 var FormController = require('web.FormController');
 var FormRenderer = require('web.FormRenderer');
+const { generateID } = require('web.utils');
 
 var _lt = core._lt;
 
@@ -48,6 +48,7 @@ var FormView = BasicView.extend({
 
         this.rendererParams.mode = mode;
         this.rendererParams.isFromFormViewDialog = params.isFromFormViewDialog;
+        this.rendererParams.fieldIdsToNames = this.fieldsView.fieldIdsToNames;
     },
 
     //--------------------------------------------------------------------------
@@ -149,6 +150,39 @@ var FormView = BasicView.extend({
         return Promise.all(defs);
     },
     /**
+     * @override
+     */
+    _processArch(arch, fv) {
+        fv.fieldIdsToNames = {}; // maps field ids (identifying <field> nodes) to field names
+        return this._super(...arguments);
+    },
+    /**
+     * Override to populate the 'fieldIdsToNames' dict mapping <field> node ids
+     * to field names. Those ids are computed as follows:
+     *   - if set on the node, we use the 'id' attribute
+     *   - otherwise
+     *       - if this is the first occurrence of the field in the arch, we use
+     *         its name as id ('name' attribute)
+     *       - otherwise we generate an id by concatenating the field name with
+     *         a unique id
+     *       - in both cases, we set the id we generated in the attrs, as it
+     *         will be used by the renderer.
+     *
+     * @override
+     */
+    _processNode(node, fv) {
+        if (node.tag === 'field') {
+            const name = node.attrs.name;
+            let uid = node.attrs.id;
+            if (!uid) {
+                uid = name in fv.fieldIdsToNames ? `${name}__${generateID()}__` : name;
+                node.attrs.id = uid;
+            }
+            fv.fieldIdsToNames[uid] = name;
+        }
+        return this._super(...arguments);
+    },
+    /**
      * We set here the limit for the number of records fetched (in one page).
      * This method is only called for subviews, not for main views.
      *