[FIX] web: properly destroy field widgets in some cases

Before this commit, the web client did not properly destroy field
widgets when updating a one2many after an onchange.  This is rarely an
issue, because in general, there are no other field widgets besides the
row currently in edition.  However, if we specify explicitely a widget,
or if we use a custom widget, it may be a problem.

addons/web/static/src/js/views/list/list_editable_renderer.js

@@ -127,7 +127,7 @@ ListRenderer.include({
      */
     confirmUpdate: function (state, id, fields, ev) {
         var self = this;
-        var oldRowIndex = _.findIndex(this.state.data, {id: id});
+        var oldData = this.state.data;
         this.state = state;
         return this.confirmChange(state, id, fields, ev).then(function () {
             // If no record with 'id' can be found in the state, the
@@ -137,9 +137,15 @@ ListRenderer.include({
             if (!record) {
                 return;
             }
+            var oldRowIndex = _.findIndex(oldData, {id: id});
             var $row = self.$('.o_data_row:nth(' + oldRowIndex + ')');
             $row.nextAll('.o_data_row').remove();
             $row.prevAll().remove();
+            _.each(oldData, function (rec) {
+                if (rec.id !== id) {
+                    self._destroyFieldWidgets(rec.id);
+                }
+            });
             var newRowIndex = _.findIndex(state.data, {id: id});
             _.each(state.data, function (record, index) {
                 if (index === newRowIndex) {
@@ -186,7 +192,6 @@ ListRenderer.include({
      * @param {string} recordID
      */
     removeLine: function (state, recordID) {
-        var self = this;
         var rowIndex = _.findIndex(this.state.data, {id: recordID});
         this.state = state;
         if (rowIndex === -1) {
@@ -204,11 +209,7 @@ ListRenderer.include({
             $row.replaceWith(this._renderEmptyRow());
         }
 
-        // destroy the removed row's widgets
-        var widgetsToDestroy = this.allFieldWidgets[recordID].slice();
-        _.each(widgetsToDestroy, function (widget) {
-            self._destroyFieldWidget(recordID, widget);
-        });
+        this._destroyFieldWidgets(recordID);
     },
     /**
      * Updates the already rendered row associated to the given recordID so that
@@ -354,6 +355,19 @@ ListRenderer.include({
     // Private
     //--------------------------------------------------------------------------
 
+    /**
+     * Destroy all field widgets corresponding to a record.  Useful when we are
+     * removing a useless row.
+     *
+     * @param {string} recordID
+     */
+    _destroyFieldWidgets: function (recordID) {
+        if (recordID in this.allFieldWidgets) {
+            var widgetsToDestroy = this.allFieldWidgets[recordID].slice();
+            _.each(widgetsToDestroy, this._destroyFieldWidget.bind(this, recordID));
+            delete this.allFieldWidgets[recordID];
+        }
+    },
     /**
      * Returns the current number of columns.  The editable renderer may add a
      * trash icon on the right of a record, so we need to take this into account

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

@@ -1,6 +1,7 @@
 odoo.define('web.relational_fields_tests', function (require) {
 "use strict";
 
+var AbstractField = require('web.AbstractField');
 var BasicModel = require('web.BasicModel');
 var concurrency = require('web.concurrency');
 var FormView = require('web.FormView');
@@ -3537,7 +3538,19 @@ QUnit.module('relational_fields', {
     });
 
     QUnit.test('one2many with CREATE onchanges correctly refreshed', function (assert) {
-        assert.expect(4);
+        assert.expect(5);
+
+        var delta = 0;
+        var oldInit = AbstractField.prototype.init;
+        var oldDestroy = AbstractField.prototype.destroy;
+        AbstractField.prototype.init = function () {
+            delta++;
+            oldInit.apply(this, arguments);
+        };
+        AbstractField.prototype.destroy = function () {
+            delta--;
+            oldDestroy.apply(this, arguments);
+        };
 
         var deactiveOnchange = true;
 
@@ -3584,7 +3597,7 @@ QUnit.module('relational_fields', {
                     '<field name="foo"/>' +
                     '<field name="turtles">' +
                         '<tree editable="bottom">' +
-                            '<field name="display_name"/>' +
+                            '<field name="display_name" widget="char"/>' +
                             '<field name="turtle_int"/>' +
                         '</tree>' +
                     '</field>' +
@@ -3596,7 +3609,7 @@ QUnit.module('relational_fields', {
         });
 
         assert.strictEqual(form.$('.o_data_row').length, 0,
-            "o2m shouldnn't contain any row");
+            "o2m shouldn't contain any row");
 
         form.$('.o_field_x2many_list_row_add a').click();
         // trigger the first onchange
@@ -3615,12 +3628,17 @@ QUnit.module('relational_fields', {
         assert.strictEqual(form.$('.o_data_row').text(), "first20second-20",
             "should correctly refresh the records");
 
+        assert.strictEqual(form.$('.o_field_widget').length, delta,
+            "all (non visible) field widgets should have been destroyed");
+
         form.$buttons.find('.o_form_button_save').click();
 
         assert.strictEqual(form.$('.o_data_row').text(), "first20second-20",
             "should correctly refresh the records after save");
 
         form.destroy();
+        AbstractField.prototype.init = oldInit;
+        AbstractField.prototype.destroy = oldDestroy;
     });
 
     QUnit.test('one2many list (editable): readonly domain is evaluated', function (assert) {