[FIX] web: improve automatic creation of new lines in x2many in forms

This commit fixes a few issues with the previous implementation of the
automatic addition of new lines in x2many inside forms

1. Before this commit, when there is a change of mode in a formview
`edit` -> `readonly` -> `edit`, there may be some cases that
mistakenly and automatically opens a modal.

This case occurs in a form view with a x2many field whose 1st inner
focusable element opens a modal (e.g. "Add an item").

In `edit` mode:
	1. Open this modal from the x2many
	2. Close the modal
	3. Change mode to `readonly`
	4. Change mode to `edit`
> the same modal is automatically open.

This is due to the tracking of the last activated field that persists
after a change of mode. Since the last activated field in the 1st
`edit` is the x2many, it is granted the focus at the beginning of the
2nd `edit`, which automatically opens the modal.

This commit fixes the issue by resetting the tracking of the last
activated field when there is a change of mode. As a result, at the
beginning of the 2nd `edit`, the focus should always be on the 1st
field.

2. Before this commit, if the focus was moved out of the form (to a modal
for instance) then back, the form would re-activate the last field, which
is the x2many and would re-add a new line. This behavior is not correct
and now going back from a modal to a form does not add a new line.

Co-authored-by: Alexandre Kühn <aku@odoo.com>

addons/web/static/src/js/fields/relational_fields.js

@@ -906,8 +906,10 @@ var FieldX2Many = AbstractField.extend({
 
     /**
      * @override
+     * @param {Object|undefined} [options={}]
+     * @param {boolean} [options.noAutomaticCreate=false]
      */
-    activate: function () {
+    activate: function (options) {
         if (!this.activeActions.create || this.isReadonly || !this.$el.is(":visible")) {
             return false;
         }
@@ -915,7 +917,11 @@ var FieldX2Many = AbstractField.extend({
             this.$buttons.find(".o-kanban-button-new").focus();
         }
         if (this.view.arch.tag === 'tree') {
-            this.renderer.$('.o_field_x2many_list_row_add a:first').focus();
+            if (options && options.noAutomaticCreate) {
+                this.renderer.$('.o_field_x2many_list_row_add a:first').focus();
+            } else {
+                this.renderer.$('.o_field_x2many_list_row_add a:first').click();
+            }
         }
         return true;
     },

addons/web/static/src/js/views/basic/basic_renderer.js

@@ -168,9 +168,10 @@ var BasicRenderer = AbstractRenderer.extend({
      * @private
      * @param {Object} record
      * @param {integer} currentIndex
-     * @param {Object} [options]
+     * @param {Object} [options={}]
      * @param {integer} [options.inc=1] - the increment to use when searching for the
      *   "next" possible one
+     * @param {boolean} [options.noAutomaticCreate=false]
      * @param {boolean} [options.wrap=false] if true, when we arrive at the end of the
      *   list of widget, we wrap around and try to activate widgets starting at
      *   the beginning. Otherwise, we just stop trying and return -1
@@ -184,7 +185,11 @@ var BasicRenderer = AbstractRenderer.extend({
 
         var recordWidgets = this.allFieldWidgets[record.id] || [];
         for (var i = 0 ; i < recordWidgets.length ; i++) {
-            var activated = recordWidgets[currentIndex].activate({event: options.event});
+            var activated = recordWidgets[currentIndex].activate(
+                {
+                    event: options.event,
+                    noAutomaticCreate: options.noAutomaticCreate || false
+                });
             if (activated) {
                 return currentIndex;
             }
@@ -213,11 +218,12 @@ var BasicRenderer = AbstractRenderer.extend({
      * @private
      * @param {Object} record
      * @param {integer} currentIndex
+     * @param {Object|undefined} options
      * @return {integer}
      */
-    _activateNextFieldWidget: function (record, currentIndex) {
+    _activateNextFieldWidget: function (record, currentIndex, options) {
         currentIndex = (currentIndex + 1) % (this.allFieldWidgets[record.id] || []).length;
-        return this._activateFieldWidget(record, currentIndex, {inc: 1});
+        return this._activateFieldWidget(record, currentIndex, _.extend({inc: 1}, options));
     },
     /**
      * This is a wrapper of the {@see _activateFieldWidget} function to select

addons/web/static/src/js/views/form/form_controller.js

@@ -361,6 +361,9 @@ var FormController = BasicController.extend({
     },
     /**
      * Calls unfreezeOrder when changing the mode.
+     * Also, when there is a change of mode, the tracking of last activated
+     * field is reset, so that the following field activation process starts
+     * with the 1st field.
      *
      * @override
      */
@@ -368,6 +371,9 @@ var FormController = BasicController.extend({
         if ((recordID || this.handle) === this.handle) {
             this.model.unfreezeOrder(this.handle);
         }
+        if (this.mode !== mode) {
+            this.renderer.resetLastActivatedField();
+        }
         return this._super.apply(this, arguments);
     },
     /**

addons/web/static/src/js/views/form/form_renderer.js

@@ -74,7 +74,7 @@ var FormRenderer = BasicRenderer.extend({
             }
         }
         if (focusWidget) {
-            return focusWidget.activate({noselect: true});
+            return focusWidget.activate({noselect: true, noAutomaticCreate: true});
         }
     },
     /**
@@ -165,7 +165,8 @@ var FormRenderer = BasicRenderer.extend({
      * field.
      */
     focusLastActivatedWidget: function () {
-        this._activateNextFieldWidget(this.state, this.lastActivatedFieldIndex - 1);
+        this._activateNextFieldWidget(this.state, this.lastActivatedFieldIndex - 1,
+            { noAutomaticCreate: true });
     },
     /**
      * returns the active tab pages for each notebook
@@ -190,6 +191,19 @@ var FormRenderer = BasicRenderer.extend({
         });
         return state;
     },
+    /**
+     * Reset the tracking of the last activated field. The fast entry with
+     * keyboard navigation needs to track the last activated field in order to
+     * set the focus.
+     *
+     * In particular, when there are changes of mode (e.g. edit -> readonly ->
+     * edit), we do not want to auto-set the focus on the previously last
+     * activated field. To avoid this issue, this method should be called
+     * whenever there is a change of mode.
+     */
+    resetLastActivatedField: function () {
+        this.lastActivatedFieldIndex = -1;
+    },
     /**
      * restore active tab pages for each notebook
      *

addons/web/static/tests/fields/relational_fields_tests.js

@@ -13404,8 +13404,7 @@ QUnit.module('relational_fields', {
     });
 
     QUnit.module('TabNavigation');
-
-    QUnit.test('when Navigating to a many to one with tabs, it receives the focus on the add new line link', function (assert) {
+    QUnit.test('when Navigating to a many2one with tabs, it receives the focus and adds a new line', function (assert) {
          assert.expect(3);
 
         var form = createView({
@@ -13441,9 +13440,9 @@ QUnit.module('relational_fields', {
             which: $.ui.keyCode.TAB,
             keyCode: $.ui.keyCode.TAB,
         }));
-        assert.strictEqual(assert.strictEqual(form.$el.find('.o_field_x2many_list_row_add>a')[0],
+        assert.strictEqual(assert.strictEqual(form.$el.find('input[name="turtle_foo"]')[0],
                             document.activeElement,
-                            "after tab, the focus should be on the many2one on the add new line"));
+                            "after tab, the focus should be on the many2one on the first input of the newly added line"));
 
         form.destroy();
     });
@@ -13551,7 +13550,7 @@ QUnit.module('relational_fields', {
     });
 
     QUnit.test('when Navigating to a many to one with tabs, editing in a popup, the popup should receive the focus then give it back', function (assert) {
-        assert.expect(4);
+        assert.expect(3);
 
         this.data.partner.records[0].turtles = [];
 
@@ -13596,18 +13595,9 @@ QUnit.module('relational_fields', {
             keyCode: $.ui.keyCode.TAB,
         }));
 
-        assert.strictEqual(form.$el.find('.o_field_x2many_list_row_add a')[0],
-            document.activeElement,
-            "after tab, the focus should be on the add new line link");
-
-        form.$el.find('.o_field_x2many_list_row_add a').trigger($.Event('keydown', {
-            which: $.ui.keyCode.ENTER,
-            keyCode: $.ui.keyCode.ENTER,
-        }));
-
         assert.strictEqual($.find('input[name="turtle_foo"]')[0],
             document.activeElement,
-            "after enter, the focus should be in the popup, in the first input field");
+            "when the one2many received the focus, the popup should open because it automatically adds a new line");
 
         $('input[name="turtle_foo"]').trigger($.Event('keydown', {
             which: $.ui.keyCode.ESCAPE,
@@ -13618,7 +13608,7 @@ QUnit.module('relational_fields', {
             document.activeElement,
             "after escape, the focus should be back on the add new line link");
 
-        form.destroy();
+       form.destroy();
     });
 
     QUnit.test('navigating through an editable list with custom controls [REQUIRE FOCUS]', function (assert) {