[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>

4 files changed, 40 insertions, 8 deletions

diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js
index c3d25f37d09..7a428923608 100644
--- a/addons/web/static/src/js/fields/relational_fields.js
+++ b/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;
     },
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 197892f3361..33267283a15 100644
--- a/addons/web/static/src/js/views/basic/basic_renderer.js
+++ b/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
diff --git a/addons/web/static/src/js/views/form/form_controller.js b/addons/web/static/src/js/views/form/form_controller.js
index 5f43b04f7cd..7586dcc4bab 100644
--- a/addons/web/static/src/js/views/form/form_controller.js
+++ b/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);
     },
     /**
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 ac8ff541544..190fbe88ad2 100644
--- a/addons/web/static/src/js/views/form/form_renderer.js
+++ b/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
@@ -191,6 +192,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
      *
      * @todo make sure this method is called