From 9f7af9f9b742d08ca69a4d4d0b963f625c4b4bba Mon Sep 17 00:00:00 2001
From: Khoi Nguyen <bng@odoo.com>
Date: Wed, 11 Oct 2017 10:00:29 +0200
Subject: [PATCH] [FIX] web: ticking a checkbox in an x2many after onchange

This commits fixes a traceback that appears when ticking a checkbox in
an x2many which triggers an onchange. The root cause of the issue is
twofold:

- confirmUpdate requires currentFieldIndex to be defined, but might be
  needed when setting currentFieldIndex;
- confirmUpdate attempts to restore the selection range even if we are
  not dealing with a char field (e.g. checkbox).

We solve the issue by ensuring that _selectCell gives a temporary value
to currentFieldIndex. This value is chosen so that confirmUpdate calls
_selectCell with the right parameters. Moreover, we only restore the
selection range when the range is defined.
---
 .../static/src/js/views/form/form_renderer.js |  1 +
 .../js/views/list/list_editable_renderer.js   | 14 +++++++--
 .../tests/fields/relational_fields_tests.js   | 29 +++++++++++++++++++
 3 files changed, 42 insertions(+), 2 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 ab5e4beb6b5..0652ee6bc4c 100644
--- a/addons/web/static/src/js/views/form/form_renderer.js
+++ b/addons/web/static/src/js/views/form/form_renderer.js
@@ -855,6 +855,7 @@ var FormRenderer = BasicRenderer.extend({
         if (ev.data.direction === "next") {
             index = this.allFieldWidgets[this.state.id].indexOf(ev.data.target);
             this._activateNextFieldWidget(this.state, index);
+            debugger;
         } else if (ev.data.direction === "previous") {
             index = this.allFieldWidgets[this.state.id].indexOf(ev.data.target);
             this._activatePreviousFieldWidget(this.state, index);
diff --git a/addons/web/static/src/js/views/list/list_editable_renderer.js b/addons/web/static/src/js/views/list/list_editable_renderer.js
index 9027fc3c984..b0ddbdcd385 100644
--- a/addons/web/static/src/js/views/list/list_editable_renderer.js
+++ b/addons/web/static/src/js/views/list/list_editable_renderer.js
@@ -136,7 +136,9 @@ ListRenderer.include({
             currentRowID = this.state.data[this.currentRow].id;
             currentWidget = this.allFieldWidgets[currentRowID][this.currentFieldIndex];
             focusedElement = currentWidget.getFocusableElement().get(0);
-            selectionRange = dom.getSelectionRange(focusedElement);
+            if (currentWidget.formatType !== 'boolean') {
+                selectionRange = dom.getSelectionRange(focusedElement);
+            }
         }
 
         var oldData = this.state.data;
@@ -179,7 +181,9 @@ ListRenderer.include({
                     currentRowID = self.state.data[newRowIndex].id;
                     currentWidget = self.allFieldWidgets[currentRowID][self.currentFieldIndex];
                     focusedElement = currentWidget.getFocusableElement().get(0);
-                    dom.setSelectionRange(focusedElement, selectionRange);
+                    if (selectionRange) {
+                        dom.setSelectionRange(focusedElement, selectionRange);
+                    }
                 });
             }
         });
@@ -585,12 +589,18 @@ ListRenderer.include({
             if (fieldIndex >= (self.allFieldWidgets[record.id] || []).length) {
                 return $.Deferred().reject();
             }
+            // _activateFieldWidget might trigger an onchange,
+            // which requires currentFieldIndex to be set
+            // so that the cursor can be restored
+            var oldFieldIndex = self.currentFieldIndex;
+            self.currentFieldIndex = fieldIndex;
             fieldIndex = self._activateFieldWidget(record, fieldIndex, {
                 inc: 1,
                 wrap: wrap,
                 event: options && options.event,
             });
             if (fieldIndex < 0) {
+                self.currentFieldIndex = oldFieldIndex;
                 return $.Deferred().reject();
             }
             self.currentFieldIndex = fieldIndex;
diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js
index 641546b3fc1..824b68c520d 100644
--- a/addons/web/static/tests/fields/relational_fields_tests.js
+++ b/addons/web/static/tests/fields/relational_fields_tests.js
@@ -6248,6 +6248,35 @@ QUnit.module('relational_fields', {
         form.destroy();
     });
 
+    QUnit.test('checkbox in an x2many that triggers an onchange', function (assert) {
+        assert.expect(1);
+
+        this.data.partner.onchanges = {
+            bar: function () {}
+        };
+
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form string="Partners">' +
+                    '<field name="p">' +
+                        '<tree editable="bottom">' +
+                            '<field name="bar"/>' +
+                        '</tree>' +
+                    '</field>' +
+                '</form>',
+        });
+
+        form.$('.o_field_x2many_list_row_add a').click();
+
+        form.$('.o_field_widget[name=bar] input').click();
+        assert.notOk(form.$('.o_field_widget[name=bar] input').prop('checked'),
+            "the checkbox should be unticked");
+
+        form.destroy();
+    });
+
     QUnit.test('one2many with default value: edit line to make it invalid', function (assert) {
         assert.expect(3);