[REF] web: restore editable list views behaviors

Since the new views, the editable list view lost some behaviors. These
behaviors were indeed implemented in the form controller/renderer but
as the new editable list view does not use an inline form view, these
behaviors had to be implemented in the basic controller/renderer.
For this to work, the list editable renderer had to be changed as it
was doing work that should be done by the controller.

The problem is even more complex because the x2m fields are using a
list renderer but not a list controller. So moving code from the
renderer to the controller obviously broke the x2m fields. Right now,
the problem is solved by catching renderer events and forwarding new
ones to the form controller (which handles the x2m specifically).

The initial goal of this commit was to share the validation of records
on save. Indeed, the fields were marked as invalid in the form view but
not in the list view. Also, before the new views, the editable list
view had different behaviors if they were used for a x2m field or not.
As these behaviors are making sense, this commit tries to restore them.

Basically, what we want is:
- When a record is saved (form view save or list view line leaving),
the invalid fields are marked (in red), the names of the related fields
are notified to the user and the record is not left.
- When a record is discarded (form view discard or list view discard),
the user is asked to confirm if the record is dirty before making the
record readonly.
- When a record is discarded, if the record is a new one, then the
record should be abandoned (removed as if never existed). In the form
view this induces to go back in the history and in the list view, to
remove a row.
- For x2m fields, the notification of invalid fields is not triggered
but the user is instead asked if he wants to discard the changes made
to the row (indeed, this replaces the list "Discard" button, as non
existent for x2m lists).
- ...

Saving, discarding, marking the fields as invalid and other behaviors
are thus now shared behavior of basic views.

The management of the dirty flag has also been moved to the model
as it was handle by the controller for the form view but by the
renderer for the list view. Now this flag is directly managed in the
basic model (the model can have changes thanks to the `_changes`
property but not be dirty (this is the case for creations)).
This change however created a problem. The view manager is currently
keeping asking if there are changes to discard at each action which
might lead to leave a dirty record (appswitcher / url change / ...).
It however did not discard anything as leaving if the user is ok with
it will lead to an implicit discard. However, as the view manager might
ask for this discard multiple times by second, the controller was
marking the record as not dirty the first time but without discarding
the changes. This is more complex to do now, as the dirty state is
part of the model and that the renderer should match the model data.
To solve this problem, the view manager now actually discard changes
explicitely when asked to. Even though this had been optimized to not
cause any rerender in some cases where it is not needed, this could
cause some performance decreasing. However, this makes some cases more
logical (opening the app switcher on a dirty form view then going back
to the form view by hitting the "go back" button left the form view
untouched although the user asked to discard it). This solution will
be improved with the view manager refactoring.

This commit is also making use of the `commitChanges` system which had
been implemented for HTML fields. Indeed, these fields cannot know
about all of their changes, so when hitting the save button, we asked
those fields to commit their value. Using this system is a great way
to make the `isValid` method of x2m fields synchronous. Indeed, before
this commit, the method was sometimes asking the user if he wants to
discard an invalid line before save. That case can be handled by the
x2m `commitChanges` method: we consider that saving the lines of x2m is
an operation that has to be done before considering the save, so we ask
all the x2m fields to do so at that time. Also, the system was broken
since a recent commit: we indeed protected the changes - save order
with a mutex but unfortunately, the `commitChanges` method was part of
the save and the changes it triggered were not able to be considered
because of this mutex. This had not been detected by tests as there is
not current way to test html fields.

21 files changed, 742 insertions, 668 deletions

diff --git a/addons/web/static/src/js/chrome/view_manager.js b/addons/web/static/src/js/chrome/view_manager.js
index d96cb711f32..e15df8f0acc 100644
--- a/addons/web/static/src/js/chrome/view_manager.js
+++ b/addons/web/static/src/js/chrome/view_manager.js
@@ -183,7 +183,9 @@ var ViewManager = Widget.extend(ControlPanelMixin, {
             core.bus.on('clear_uncommitted_changes', self, function(chain_callbacks) {
                 chain_callbacks(function() {
                     if (self.active_view.controller) {
-                        return self.active_view.controller.canBeDiscarded();
+                        return self.active_view.controller.discardChanges(undefined, {
+                            readonlyIfRealDiscard: true,
+                        });
                     }
                 });
             });
diff --git a/addons/web/static/src/js/fields/abstract_field.js b/addons/web/static/src/js/fields/abstract_field.js
index 87b3bf3f512..757e917dd36 100644
--- a/addons/web/static/src/js/fields/abstract_field.js
+++ b/addons/web/static/src/js/fields/abstract_field.js
@@ -192,7 +192,7 @@ var AbstractField = Widget.extend({
      *
      * @abstract
      */
-    commitChanges: function () {
+    commitChanges: function () { // TODO maybe should be done for all fields (keyboard save)
     },
     /**
      * Returns the main field's DOM element (jQuery form) which can be focused
@@ -217,16 +217,10 @@ var AbstractField = Widget.extend({
      * value was changed by the user. This is checked before saving a record, by
      * the view.
      *
-     * Note: the return value may not be a boolean. It is necessary for some
-     * fields (x2many) to sometimes ask confirmation to the user if the current
-     * dirty invalid line can be discarded. This is an asynchronous operation,
-     * so we need the expressivity of something like a deferred.
-     *
-     * Note2: this is the responsability of the view to check that required
+     * Note: this is the responsability of the view to check that required
      * fields have a set value.
      *
-     * @returns {boolean|Deferred} true/false if the widget is valid, or a
-     *   deferred which resolves to true/false if the widget is valid or not
+     * @returns {boolean} true/false if the widget is valid
      */
     isValid: function () {
         return this._isValid;
diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js
index 6a7ea1869b7..2a3d5e01125 100644
--- a/addons/web/static/src/js/fields/relational_fields.js
+++ b/addons/web/static/src/js/fields/relational_fields.js
@@ -517,11 +517,13 @@ var KanbanFieldMany2One = AbstractField.extend({
 var FieldX2Many = AbstractField.extend({
     tagName: 'div',
     custom_events: _.extend({}, AbstractField.prototype.custom_events, {
+        add_record: '_onAddRecord',
+        edit_line: '_onEditLine',
         field_changed: '_onFieldChanged',
         kanban_record_delete: '_onDeleteRecord',
         list_record_delete: '_onDeleteRecord',
-        add_record: '_onAddRecord',
         open_record: '_onOpenRecord',
+        save_line: '_onSaveLine',
         toggle_column_order: '_onToggleColumnOrder',
     }),
 
@@ -563,31 +565,29 @@ var FieldX2Many = AbstractField.extend({
     //--------------------------------------------------------------------------
 
     /**
+     * A x2m field can only be saved if it finished the edition of all its rows.
+     * On parent view saving, we have to ask the x2m fields to commit their
+     * changes, that is confirming the save of the in-edition row or asking the
+     * user if he wants to discard it if necessary.
+     *
      * @override
+     * @returns {Deferred}
      */
-    isSet: function () {
-        return true;
+    commitChanges: function () {
+        var inEditionRecordID =
+            this.renderer
+            && this.renderer.viewType === "list"
+            && this.renderer.getEditableRecordID();
+        if (inEditionRecordID) {
+            return this._saveLine(inEditionRecordID);
+        }
+        return this._super.apply(this, arguments);
     },
     /**
-     * This is a tricky override.  For relational fields, we don't just need to
-     * know if the field is valid.  We also need to discard some invalid lines,
-     * if the user is ok with it (or automatically if the invalid line is not
-     * dirty). This is the reason why we need to change the signature of the
-     * AbstractField isValid method to optionally return a deferred.
-     *
      * @override
-     * @returns {boolean|Deferred} if there is a renderer (sub list or kanban
-     *   view), we may need to ask the user => it will return a deferred.
-     *   Otherwise, it returns a boolean
      */
-    isValid: function () {
-        if (!this.renderer) {
-            return this._super.apply(this, arguments);
-        }
-        var def = $.Deferred();
-        this.renderer.canBeSaved()
-            .then(def.resolve.bind(def, true), def.resolve.bind(def, false));
-        return def;
+    isSet: function () {
+        return true;
     },
 
     //--------------------------------------------------------------------------
@@ -687,6 +687,39 @@ var FieldX2Many = AbstractField.extend({
             this.$buttons.on('click', 'button.o-kanban-button-new', this._onAddRecord.bind(this));
         }
     },
+    /**
+     * Saves the line associated to the given recordID. If the line is valid,
+     * it only has to be switched to readonly mode as all the line changes have
+     * already been notified to the model so that they can be saved in db if the
+     * parent view is actually saved. If the line is not valid, the line is to
+     * be discarded if the user agrees (this behavior is not a list editable
+     * one but a x2m one as it is made to replace the "discard" button which
+     * exists for list editable views).
+     *
+     * @private
+     * @param {string} recordID
+     * @returns {Deferred} resolved if the line was properly saved or discarded.
+     *                     rejected if the line could not be saved and the user
+     *                     did not agree to discard.
+     */
+    _saveLine: function (recordID) {
+        var self = this;
+        var def = $.Deferred();
+        this.renderer.commitChanges(recordID).then(function () { // TODO wrong as no mutex protection
+            var fieldNames = self.renderer.canBeSaved(recordID);
+            if (fieldNames.length) {
+                self.trigger_up('discard_x2m_changes', {
+                    recordID: recordID,
+                    onSuccess: def.resolve.bind(def),
+                    onFailure: def.reject.bind(def),
+                });
+            } else {
+                self.renderer.setRowMode(recordID, 'readonly', true);
+                def.resolve();
+            }
+        });
+        return def;
+    },
 
     //--------------------------------------------------------------------------
     // Handlers
@@ -708,33 +741,44 @@ var FieldX2Many = AbstractField.extend({
      * by the parent controller.
      *
      * @private
-     * @param   {OdooEvent}
+     * @param {OdooEvent} ev
      */
-    _onDeleteRecord: function (event) {
-        event.stopPropagation();
+    _onDeleteRecord: function (ev) {
+        ev.stopPropagation();
         this._setValue({
             operation: 'REMOVE',
-            ids: [event.data.id],
+            ids: [ev.data.id],
         });
     },
     /**
+     * Called when the renderer asks to edit a line, in that case simply tells
+     * him back to toggle the mode of this row.
+     *
+     * @private
+     * @param {OdooEvent} ev
+     */
+    _onEditLine: function (ev) {
+        ev.stopPropagation();
+        this.renderer.setRowMode(ev.data.recordID, 'edit');
+    },
+    /**
      * Updates the given record with the changes.
      *
      * @private
-     * @param   {OdooEvent}
+     * @param {OdooEvent} ev
      */
-    _onFieldChanged: function (event) {
-        if (event.target === this) {
+    _onFieldChanged: function (ev) {
+        if (ev.target === this) {
             return;
         }
-        event.stopPropagation();
+        ev.stopPropagation();
         // changes occured in an editable list
-        var changes = event.data.changes;
+        var changes = ev.data.changes;
         if (Object.keys(changes).length) {
             this._setValue({
                 operation: 'UPDATE',
-                id: event.data.dataPointID,
-                data: changes
+                id: ev.data.dataPointID,
+                data: changes,
             });
         }
     },
@@ -748,14 +792,30 @@ var FieldX2Many = AbstractField.extend({
         // to implement
     },
     /**
+     * Called when the renderer ask to save a line (the user tries to leave it)
+     * -> Nothing is to "save" here, the model was already notified of the line
+     * changes; if the row could be saved, we make the row readonly. Otherwise,
+     * we trigger a new event for the view to tell it to discard the changes
+     * made to that row.
+     *
+     * @private
+     * @param {OdooEvent} ev
+     */
+    _onSaveLine: function (ev) {
+        ev.stopPropagation();
+        this._saveLine(ev.data.recordID)
+            .done(ev.data.onSuccess)
+            .fail(ev.data.onFailure);
+    },
+    /**
      * Adds field name information to the event, so that the view upstream is
      * aware of which widgets it has to redraw.
      *
      * @private
-     * @param   {OdooEvent}
+     * @param {OdooEvent} ev
      */
-    _onToggleColumnOrder: function (event) {
-        event.data.field = this.name;
+    _onToggleColumnOrder: function (ev) {
+        ev.data.field = this.name;
     },
 });
 
@@ -769,13 +829,13 @@ var FieldOne2Many = FieldX2Many.extend({
     /**
      * @override
      * @param {Object} record
-     * @param {OdooEvent} [event] an event that triggered the reset action
+     * @param {OdooEvent} [ev] an event that triggered the reset action
      * @returns {Deferred}
      */
-    reset: function (record, event) {
+    reset: function (record, ev) {
         var self = this;
-        if (event && event.target === this && self.view.arch.tag === 'tree' && this.editable) {
-            var command = event.data.changes[this.name];
+        if (ev && ev.target === this && ev.data.changes && self.view.arch.tag === 'tree' && this.editable) {
+            var command = ev.data.changes[this.name];
             if (command.operation === 'UPDATE') {
                 var state = record.data[this.name];
                 var fieldNames = state.getFieldNames();
@@ -784,8 +844,8 @@ var FieldOne2Many = FieldX2Many.extend({
             }
         }
         return this._super.apply(this, arguments).then(function () {
-            if (event && event.target === self && self.view.arch.tag === 'tree') {
-                if (event.data.changes[self.name].operation === 'CREATE') {
+            if (ev && ev.target === self && ev.data.changes && self.view.arch.tag === 'tree') {
+                if (ev.data.changes[self.name].operation === 'CREATE') {
                     var index = self.editable === 'top' ? 0 : self.value.data.length - 1;
                     var newID = self.value.data[index].id;
                     self.renderer.editRecord(newID);
@@ -833,13 +893,13 @@ var FieldOne2Many = FieldX2Many.extend({
      *
      * @override
      * @private
-     * @param {OdooEvent|MouseEvent} event this event comes either from the 'Add
+     * @param {OdooEvent|MouseEvent} ev this event comes either from the 'Add
      *   record' link in the list editable renderer, or from the 'Create' button
      *   in the kanban view
      */
-    _onAddRecord: function (event) {
+    _onAddRecord: function (ev) {
         // we don't want interference with the components upstream.
-        event.stopPropagation();
+        ev.stopPropagation();
 
         if (this.editable) {
             this._setValue({
@@ -863,14 +923,14 @@ var FieldOne2Many = FieldX2Many.extend({
      * form view.
      *
      * @private
-     * @param {OdooEvent} event
+     * @param {OdooEvent} ev
      */
-    _onOpenRecord: function (event) {
+    _onOpenRecord: function (ev) {
         // we don't want interference with the components upstream.
-        event.stopPropagation();
+        ev.stopPropagation();
 
         this._openFormDialog({
-            id: event.data.id,
+            id: ev.data.id,
             on_saved: this._setValue.bind(this, { operation: 'NOOP' }),
             readonly: this.mode === 'readonly',
         });
@@ -932,14 +992,14 @@ var FieldMany2Many = FieldX2Many.extend({
      * to the form view.
      *
      * @private
-     * @param {OdooEvent} event
+     * @param {OdooEvent} ev
      */
-    _onOpenRecord: function (event) {
-        _.extend(event.data, {
+    _onOpenRecord: function (ev) {
+        _.extend(ev.data, {
             context: this.record.getContext(this.recordParams),
             domain: this.record.getDomain(this.recordParams),
             fields_view: this.attrs.views && this.attrs.views.form,
-            on_saved: this.trigger_up.bind(this, 'reload', {db_id: event.data.id}),
+            on_saved: this.trigger_up.bind(this, 'reload', {db_id: ev.data.id}),
             readonly: this.mode === 'readonly',
             string: this.string,
         });
@@ -1297,9 +1357,9 @@ var FieldMany2ManyTags = AbstractField.extend({
      * @param {KeyboardEvent} event
      */
     _onKeyDown: function (event) {
-        if($(event.target).val() === "" && event.which === $.ui.keyCode.BACKSPACE) {
+        if ($(event.target).val() === "" && event.which === $.ui.keyCode.BACKSPACE) {
             var $badges = this.$('.badge');
-            if($badges.length) {
+            if ($badges.length) {
                 this._removeTag($badges.last().data('id'));
             }
         }
diff --git a/addons/web/static/src/js/views/abstract_controller.js b/addons/web/static/src/js/views/abstract_controller.js
index df44309636b..1dacce59fa6 100644
--- a/addons/web/static/src/js/views/abstract_controller.js
+++ b/addons/web/static/src/js/views/abstract_controller.js
@@ -67,19 +67,20 @@ var AbstractController = Widget.extend({
     //--------------------------------------------------------------------------
 
     /**
-     * Determines if we can discard the current state.  For example, when the
-     * user open the 'home' screen, the view manager will call this method on
-     * the active view to make sure it is ok to open the home screen (and
-     * potentially loose all current state).
+     * Discards the changes made on the record associated to the given ID, or
+     * all changes made by the current controller if no recordID is given. For
+     * example, when the user open the 'home' screen, the view manager will call
+     * this method on the active view to make sure it is ok to open the home
+     * screen (and lose all current state).
      *
      * Note that it returns a deferred, because the view could choose to ask the
      * user if he agrees to discard.
      *
-     * @override
-     * @returns {Deferred} If the deferred is resolved, we assume the changes
-     *   can be discarded.  If it is rejected, then we cannot discard.
+     * @param {string} [recordID]
+     *        if not given, we consider all the changes made by the controller
+     * @returns {Deferred} resolved if properly discarded, rejected otherwise
      */
-    canBeDiscarded: function () {
+    discardChanges: function (recordID) {
         return $.when();
     },
     /**
@@ -296,4 +297,3 @@ var AbstractController = Widget.extend({
 return AbstractController;
 
 });
-
diff --git a/addons/web/static/src/js/views/basic/basic_controller.js b/addons/web/static/src/js/views/basic/basic_controller.js
index 97e0ca0f4be..33f6727987e 100644
--- a/addons/web/static/src/js/views/basic/basic_controller.js
+++ b/addons/web/static/src/js/views/basic/basic_controller.js
@@ -16,7 +16,7 @@ var Pager = require('web.Pager');
 
 var _t = core._t;
 
-return AbstractController.extend(FieldManagerMixin, {
+var BasicController = AbstractController.extend(FieldManagerMixin, {
     custom_events: _.extend({}, AbstractController.prototype.custom_events, FieldManagerMixin.custom_events, {
         reload: '_onReload',
     }),
@@ -34,7 +34,7 @@ return AbstractController.extend(FieldManagerMixin, {
         this.hasButtons = params.hasButtons;
         FieldManagerMixin.init.call(this, this.model);
         this.handle = params.initialState.id;
-        this.isDirty = false;
+        this.mode = params.mode || 'readonly';
         this.mutex = new concurrency.Mutex();
     },
     /**
@@ -51,6 +51,82 @@ return AbstractController.extend(FieldManagerMixin, {
     //--------------------------------------------------------------------------
 
     /**
+     * Determines if we can discard the current changes. If the model is not
+     * dirty, that is not a problem. However, if it is dirty, we have to ask
+     * the user for confirmation.
+     *
+     * @override
+     * @param {string} [recordID] - default to main recordID
+     * @returns {Deferred<boolean>}
+     *          resolved if can be discarded, a boolean value is given to tells
+     *          if there is something to discard or not
+     *          rejected otherwise
+     */
+    canBeDiscarded: function (recordID) {
+        if (!this.model.isDirty(recordID || this.handle)) {
+            return $.when(false);
+        }
+
+        var message = _t("The record has been modified, your changes will be discarded. Are you sure you want to ?");
+        var def = $.Deferred();
+        var dialog = Dialog.confirm(this, message, {
+            title: _t("Warning"),
+            confirm_callback: def.resolve.bind(def, true),
+            cancel_callback: def.reject.bind(def),
+        });
+        dialog.on('closed', def, def.reject);
+        return def;
+    },
+    /**
+     * Ask the renderer if all associated field widget are in a valid state for
+     * saving (valid value and non-empty value for required fields). If this is
+     * not the case, this notifies the user with a warning containing the names
+     * of the invalid fields.
+     *
+     * Note: changing the style of invalid fields is the renderer's job.
+     *
+     * @param {string} [recordID] - default to main recordID
+     * @return {boolean}
+     */
+    canBeSaved: function (recordID) {
+        var fieldNames = this.renderer.canBeSaved(recordID || this.handle);
+        if (fieldNames.length) {
+            this._notifyInvalidFields(fieldNames);
+            return false;
+        }
+        return true;
+    },
+    /**
+     * Discards the changes made to the record whose ID is given, if necessary.
+     * Automatically leaves to default mode for the given record.
+     *
+     * @param {string} [recordID] - default to main recordID
+     * @param {Object} [options]
+     * @param {boolean} [options.readonlyIfRealDiscard=false]
+     *        After discarding record changes, the usual option is to make the
+     *        record readonly. However, the view manager calls this function
+     *        at inappropriate times in the current code and in that case, we
+     *        don't want to go back to readonly if there is nothing to discard
+     *        (e.g. when switching record in edit mode in form view, we expect
+     *        the new record to be in edit mode too, but the view manager calls
+     *        this function as the URL changes...) @todo get rid of this when
+     *        the view manager is improved.
+     * @returns {Deferred}
+     */
+    discardChanges: function (recordID, options) {
+        var self = this;
+        recordID = recordID || this.handle;
+        return this.canBeDiscarded(recordID).then(function (needDiscard) {
+            if (needDiscard) { // Just some optimization
+                self.model.discardChanges(recordID);
+            }
+            if (options && options.readonlyIfRealDiscard && !needDiscard) {
+                return;
+            }
+            self._setMode('readonly', recordID);
+        });
+    },
+    /**
      * @override
      */
     renderPager: function ($node, options) {
@@ -74,11 +150,32 @@ return AbstractController.extend(FieldManagerMixin, {
         this._updatePager();  // to force proper visibility
     },
     /**
+     * Saves the record whose ID is given if necessary (@see _saveRecord).
+     *
+     * @param {string} [recordID] - default to main recordID
+     * @param {Object} [options]
+     * @returns {Deferred}
+     */
+    saveRecord: function (recordID, options) {
+        // Some field widgets (e.g. 'html') can't detect (all) their changes, so
+        // we ask them to commit their current value before saving. This has to
+        // be done outside of the mutex protection of saving because
+        // commitChanges will trigger changes and these are also protected. So
+        // the actual saving has to be done after these changes. Also the
+        // commitChanges operation might not be synchronous for other reason
+        // (e.g. the x2m fields will ask the user if some discarding has to be
+        // made). This operation must also be mutex-protected as commitChanges
+        // function of x2m has to be aware of all final changes made to a row.
+        this.mutex.exec(this.renderer.commitChanges.bind(this.renderer, recordID || this.handle)); // TODO write a test for this
+        return this.mutex.exec(this._saveRecord.bind(this, recordID, options));
+    },
+    /**
      * @override
      * @returns {Deferred}
      */
     update: function (params, options) {
         var self = this;
+        this.mode = params.mode || this.mode;
         return this._super(params, options).then(function () {
             self._updateEnv();
             self._updatePager();
@@ -90,6 +187,24 @@ return AbstractController.extend(FieldManagerMixin, {
     //--------------------------------------------------------------------------
 
     /**
+     * Does the necessary action when trying to "abandon" a given record (e.g.
+     * when trying to make a new record readonly without having saved it). By
+     * default, if the abandoned record is the main view one, the only possible
+     * action is to leave the current view. Otherwise, it is a x2m line, ask the
+     * model to remove it.
+     *
+     * @private
+     * @param {string} [recordID] - default to main recordID
+     */
+    _abandonRecord: function (recordID) {
+        recordID = recordID || this.handle;
+        if (recordID === this.handle) {
+            this.trigger_up('switch_to_previous_view');
+        } else {
+            this.model.removeLine(recordID);
+        }
+    },
+    /**
      * We override applyChanges (from the field manager mixin) to protect it
      * with a mutex.
      *
@@ -184,6 +299,24 @@ return AbstractController.extend(FieldManagerMixin, {
         return state.count !== 0;
     },
     /**
+     * Helper function to display a warning that some fields have an invalid
+     * value. This is used when a save operation cannot be completed.
+     *
+     * @private
+     * @param {string[]} invalidFields - list of field names
+     */
+    _notifyInvalidFields: function (invalidFields) {
+        var record = this.model.get(this.handle, {raw: true});
+        var fields = record.fields;
+        var warnings = invalidFields.map(function (fieldName) {
+            var fieldStr = fields[fieldName].string;
+            return _.str.sprintf('<li>%s</li>', _.escape(fieldStr));
+        });
+        warnings.unshift('<ul>');
+        warnings.push('</ul>');
+        this.do_warn(_t("The following fields are invalid:"), warnings.join(''));
+    },
+    /**
      * Hook method, called when record(s) has been deleted.
      *
      * @see _deleteRecord
@@ -193,6 +326,67 @@ return AbstractController.extend(FieldManagerMixin, {
         this.update({});
     },
     /**
+     * Saves the record whose ID is given, if necessary. Automatically leaves
+     * edit mode for the given record, unless told otherwise.
+     *
+     * @param {string} [recordID] - default to main recordID
+     * @param {Object} [options]
+     * @param {boolean} [options.stayInEdit=false]
+     *        if true, leave the record in edit mode after save
+     * @param {boolean} [options.reload=true]
+     *        if true, reload the record after (real) save
+     * @param {boolean} [options.savePoint=false]
+     *        if true, the record will only be 'locally' saved: its changes
+     *        will move from the _changes key to the data key
+     * @returns {Deferred}
+     */
+    _saveRecord: function (recordID, options) {
+        recordID = recordID || this.handle;
+        options = _.defaults(options || {}, {
+            stayInEdit: false,
+            reload: true,
+            savePoint: false,
+        });
+
+        var def = $.Deferred();
+        // Check if the view is in a valid state for saving
+        // Note: it is the model's job to do nothing if there is nothing to save
+        if (this.canBeSaved(recordID)) {
+            def = this.model.save(recordID, { // Save then leave edit mode
+                reload: options.reload,
+                savePoint: options.savePoint,
+            });
+        } else {
+            def.reject(); // Cannot be saved, do nothing at all
+        }
+
+        if (options.stayInEdit) {
+            return def;
+        } else {
+            return def.then(this._setMode.bind(this, 'readonly', recordID));
+        }
+    },
+    /**
+     * Change the mode for the record associated to the given ID.
+     * If the given recordID is the view's main one, then the whole view mode is
+     * changed (@see BasicController.update).
+     *
+     * @private
+     * @param {string} mode - 'readonly' or 'edit'
+     * @param {string} [recordID]
+     */
+    _setMode: function (mode, recordID) {
+        recordID = recordID || this.handle;
+        // If trying to make a temporary record readonly, discard the record
+        if (mode === 'readonly' && this.model.isNew(recordID)) {
+            this._abandonRecord(recordID);
+            return;
+        }
+        if (recordID === this.handle) {
+            this.update({mode: mode}, {reload: false});
+        }
+    },
+    /**
      * Helper method, to get the current environment variables from the model
      * and notifies the component chain (by bubbling an event up)
      */
@@ -226,15 +420,17 @@ return AbstractController.extend(FieldManagerMixin, {
     //--------------------------------------------------------------------------
 
     /**
-     * We need to keep manually track of the isDirty property.  Note that it is
-     * not the same as the one from the basic model.  A new record from the
-     * basic model is considered dirty, but from the controller point of view,
-     * there was no user interaction and should not be dirty.
+     * Forces to save directly the changes if the controller is in readonly,
+     * because in that case the changes come from widgets that are editable even
+     * in readonly (e.g. Priority).
      *
      * @private
+     * @param {OdooEvent}
      */
-    _onFieldChanged: function () {
-        this.isDirty = true;
+    _onFieldChanged: function (ev) {
+        if (this.mode === 'readonly') {
+            ev.data.force_save = true;
+        }
         FieldManagerMixin._onFieldChanged.apply(this, arguments);
     },
     /**
@@ -261,7 +457,7 @@ return AbstractController.extend(FieldManagerMixin, {
             this.reload({fieldNames: data.fieldNames});
         }
     },
-
 });
 
+return BasicController;
 });
diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js
index ea8986f9da8..f3e69a6b453 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -219,7 +219,8 @@ var BasicModel = AbstractModel.extend({
     discardChanges: function (id, options) {
         options = options || {};
         var element = this.localData[id];
-        var rollback = options.rollback || this.isNew(id);
+        var isNew = this.isNew(id);
+        var rollback = options.rollback || isNew;
         this._visitChildren(element, function (elem) {
             if (rollback && elem._savePoint) {
                 if (elem._savePoint instanceof Array) {
@@ -227,8 +228,10 @@ var BasicModel = AbstractModel.extend({
                 } else {
                     elem._changes = _.extend({}, elem._savePoint);
                 }
+                elem._isDirty = !isNew;
             } else {
                 elem._changes = null;
+                elem._isDirty = false;
             }
         });
     },
@@ -383,26 +386,26 @@ var BasicModel = AbstractModel.extend({
         return element;
     },
     /**
-     * return true if a record is dirty. A record is considered dirty if it has
-     * some unsaved changes. A list is considered dirty if its _changes key is
-     * set to an array of its new datapoints (possibly empty)
+     * Returns true if a record is dirty. A record is considered dirty if it has
+     * some unsaved changes, marked by the _isDirty property on the record or
+     * one of its subrecords.
      *
-     * @param {string} id id for a local resource
+     * @param {string} id - the local resource id
      * @returns {boolean}
      */
     isDirty: function (id) {
         var isDirty = false;
-        var record = this.localData[id];
-        this._visitChildren(record, function (r) {
-            if (r.type === "record" ? !_.isEmpty(r._changes) : r._changes) {
+        this._visitChildren(this.localData[id], function (r) {
+            if (r._isDirty) {
                 isDirty = true;
             }
         });
         return isDirty;
     },
     /**
-     * Check if a record is new, meaning if it is in the process of being created
-     * and no actual record exists in db.
+     * Check if a localData is new, meaning if it is in the process of being
+     * created and no actual record exists in db. Note: if the localData is not
+     * of the "record" type, then it is always considered as not new.
      *
      * Note: A virtual id is a character string composed of an integer and has
      * a dash and other information.
@@ -413,7 +416,11 @@ var BasicModel = AbstractModel.extend({
      * @returns {boolean}
      */
     isNew: function (id) {
-        var res_id = this.localData[id].res_id;
+        var data = this.localData[id];
+        if (data.type !== "record") {
+            return false;
+        }
+        var res_id = data.res_id;
         if (typeof res_id === 'number') {
             return false;
         } else if (typeof res_id === 'string' && /^[0-9]+-/.test(res_id)) {
@@ -707,15 +714,20 @@ var BasicModel = AbstractModel.extend({
                             record.res_ids.push(id);
                             record.count++;
                         }
+
+                        // Update the data directly or reload them
+                        var def;
                         if (shouldReload) {
-                            // erase changes as they have been applied
-                            record._changes = {};
-                            return self._fetchRecord(record);
+                            def = self._fetchRecord(record);
                         } else {
                             _.extend(record.data, record._changes);
-                            record._changes = {};
-                            return false;
                         }
+
+                        // Erase changes as they have been applied
+                        record._changes = {};
+                        record._isDirty = false;
+
+                        return def;
                     });
             } else {
                 return $.when(record_id);
@@ -837,6 +849,7 @@ var BasicModel = AbstractModel.extend({
         var field;
         var defs = [];
         record._changes = record._changes || {};
+        record._isDirty = true;
 
         // apply changes to local data
         for (var fieldName in changes) {
@@ -874,6 +887,7 @@ var BasicModel = AbstractModel.extend({
                 _.each(fieldNames, function (name) {
                     if (record._changes && record._changes[name] === record.data[name]) {
                         delete record._changes[name];
+                        record._isDirty = !_.isEmpty(record._changes);
                     }
                 });
                 return self._fetchSpecialData(record).then(function (fieldNames2) {
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 dd0f59a4176..ed9726478a0 100644
--- a/addons/web/static/src/js/views/basic/basic_renderer.js
+++ b/addons/web/static/src/js/views/basic/basic_renderer.js
@@ -36,6 +36,41 @@ var BasicRenderer = AbstractRenderer.extend({
         this.mode = params.mode || 'readonly';
     },
     /**
+     * This method has two responsabilities: find every invalid fields in the
+     * current view, and making sure that they are displayed as invalid, by
+     * toggling the o_form_invalid css class. It has to be done both on the
+     * widget, and on the label, if any.
+     *
+     * @param {string} recordID
+     * @returns {string[]} the list of invalid field names
+     */
+    canBeSaved: function (recordID) {
+        var self = this;
+        var invalidFields = [];
+        _.each(this.allFieldWidgets[recordID], function (widget) {
+            var canBeSaved = self._canWidgetBeSaved(widget);
+            if (!canBeSaved) {
+                invalidFields.push(widget.name);
+            }
+            widget.$el.toggleClass('o_form_invalid', !canBeSaved);
+        });
+        return invalidFields;
+    },
+    /**
+     * Calls 'commitChanges' on all field widgets, so that they can notify the
+     * environment with their current value (useful for widgets that can't
+     * detect when their value changes, e.g. field 'html').
+     *
+     * @param {string} recordID
+     * @return {Deferred}
+     */
+    commitChanges: function (recordID) {
+        var defs = _.map(this.allFieldWidgets[recordID], function (widget) {
+            return widget.commitChanges();
+        });
+        return $.when.apply($, defs);
+    },
+    /**
      * Updates the internal state of the renderer to the new state. By default,
      * this also implements the recomputation of the modifiers and their
      * application to the DOM and the reset of the field widgets if needed.
@@ -222,14 +257,7 @@ var BasicRenderer = AbstractRenderer.extend({
      */
     _canWidgetBeSaved: function (widget) {
         var modifiers = this._getEvaluatedModifiers(widget.__node, widget.record);
-        var isSetOrNotRequired = (widget.isSet() || !modifiers.required);
-        var isValid = widget.isValid();
-        if (isValid instanceof $.Deferred) {
-            return isValid.then(function (isValid) {
-                return isValid && isSetOrNotRequired;
-            });
-        }
-        return isValid && isSetOrNotRequired;
+        return widget.isValid() && (widget.isSet() || !modifiers.required);
     },
     /**
      * Updates the modifiers evaluation associated to a given modifiers data and
diff --git a/addons/web/static/src/js/views/field_manager_mixin.js b/addons/web/static/src/js/views/field_manager_mixin.js
index 287bd800576..ceedbb2df6e 100644
--- a/addons/web/static/src/js/views/field_manager_mixin.js
+++ b/addons/web/static/src/js/views/field_manager_mixin.js
@@ -12,8 +12,6 @@ var BasicModel = require('web.BasicModel');
 
 var FieldManagerMixin = {
     custom_events: {
-        discard_changes: '_onDiscardChanges',
-        discard_line: '_onDiscardLine',
         field_changed: '_onFieldChanged',
         load: '_onLoad',
     },
@@ -88,33 +86,6 @@ var FieldManagerMixin = {
     //--------------------------------------------------------------------------
 
     /**
-     * Controlled elements might want to ask for a discard of the changes made
-     * to a whole record (see editable list view). In that case, this handler
-     * is called and call the appropriate @see BasicModel.discardChanges
-     * function. The result is then notified thanks to a call to
-     * @see _confirmChange.
-     *
-     * @private
-     * @param {OdooEvent} ev
-     */
-    _onDiscardChanges: function (ev) {
-        this.model.discardChanges(ev.data.id);
-        this._confirmChange(ev.data.id, [], ev); // TODO get real list of changes ?
-    },
-    /**
-     * In some case, we may need to remove an element from a list, without going
-     * through the notifyChanges machinery.  The motivation for this is when the
-     * user click on 'Add an item' in a field one2many with a required field,
-     * then clicks somewhere else.  The new line need to be discarded, but we
-     * don't want to trigger a real notifyChanges (no need for that, and also,
-     * we don't want to rerender the UI).
-     *
-     * @param {OdooEvent} event
-     */
-    _onDiscardLine: function (event) {
-        this.model.removeLine(event.data.id);
-    },
-    /**
      * This is the main job of the FMM: deciding what to do when a controlled
      * field changes.  Most of the time, it notifies the model that a change
      * just occurred, then confirm the change.
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 2cf90dcf3db..e3b129c7915 100644
--- a/addons/web/static/src/js/views/form/form_controller.js
+++ b/addons/web/static/src/js/views/form/form_controller.js
@@ -15,6 +15,7 @@ var FormController = BasicController.extend({
         open_one2many_record: '_onOpenOne2ManyRecord',
         bounce_edit: '_onBounceEdit',
         button_clicked: '_onButtonClicked',
+        discard_x2m_changes: '_onDiscardX2MChanges',
         open_record: '_onOpenRecord',
         toggle_column_order: '_onToggleColumnOrder',
     }),
@@ -29,7 +30,6 @@ var FormController = BasicController.extend({
         this.defaultButtons = params.defaultButtons;
         this.hasSidebar = params.hasSidebar;
         this.toolbar = params.toolbar;
-        this.mode = params.mode;
     },
 
     //--------------------------------------------------------------------------
@@ -45,45 +45,9 @@ var FormController = BasicController.extend({
     autofocus: function () {
     },
     /**
-     * Determines if we can discard the current change.  If the model is not
-     * dirty, that is not a problem.  However, if it is dirty, we have to ask
-     * the user for confirmation.
-     *
-     * @override
-     * @returns {Deferred} If the deferred is resolved, we assume the changes
-     *   can be discarded.  If it is rejected, then we cannot discard.
-     */
-    canBeDiscarded: function () {
-        var self = this;
-        if (!this.isDirty) {
-            return $.when();
-        }
-        var message = _t("The record has been modified, your changes will be discarded. Are you sure you want to leave this page ?");
-        var def = $.Deferred();
-        var options = {
-            title: _t("Warning"),
-            confirm_callback: function () {
-                this.on('closed', null, function () {
-                    self.isDirty = false;
-                    def.resolve();
-                });
-            },
-            cancel_callback: def.reject.bind(def)
-        };
-        var dialog = Dialog.confirm(this, message, options);
-        dialog.$modal.on('hidden.bs.modal', def.reject.bind(def));
-        return def;
-    },
-    /**
-     * @returns {Deferred}
-     */
-    canBeSaved: function () {
-        return this.renderer.canBeSaved()
-            .fail(this._notifyInvalidFields.bind(this));
-    },
-    /**
      * This method switches the form view in edit mode, with a new record.
      *
+     * @todo make record creation a basic controller feature
      * @returns {Deferred}
      */
     createRecord: function () {
@@ -100,7 +64,7 @@ var FormController = BasicController.extend({
         }).then(function (handle) {
             self.handle = handle;
             self._updateEnv();
-            self._toEditMode();
+            self._setMode('edit');
         });
     },
     /**
@@ -143,10 +107,10 @@ var FormController = BasicController.extend({
             this.$buttons.append($footer);
         } else {
             this.$buttons.append(qweb.render("FormView.buttons", {widget: this}));
-            this.$buttons.on('click', '.o_form_button_edit', this._toEditMode.bind(this));
-            this.$buttons.on('click', '.o_form_button_save', this.saveRecord.bind(this));
-            this.$buttons.on('click', '.o_form_button_cancel', this._onDiscardChange.bind(this));
-            this.$buttons.on('click', '.o_form_button_create', this.createRecord.bind(this));
+            this.$buttons.on('click', '.o_form_button_edit', this._onEdit.bind(this));
+            this.$buttons.on('click', '.o_form_button_create', this._onCreate.bind(this));
+            this.$buttons.on('click', '.o_form_button_save', this._onSave.bind(this));
+            this.$buttons.on('click', '.o_form_button_cancel', this._onDiscard.bind(this));
 
             this._updateButtons();
         }
@@ -199,27 +163,6 @@ var FormController = BasicController.extend({
             this._updateSidebar();
         }
     },
-    /**
-     * Save the current record (@see _saveRecord)
-     *
-     * @param {Object} options
-     * @returns {Deferred}
-     */
-    saveRecord: function (options) {
-        return this.mutex.exec(this._saveRecord.bind(this, options));
-    },
-    /**
-     * We need to check for all 'mode' change, because the information is not
-     * available from the model.
-     *
-     * @override
-     * @param {Object} params
-     * @returns {Deferred}
-     */
-    update: function (params) {
-        this.mode = params.mode || this.mode;
-        return this._super.apply(this, arguments);
-    },
 
     //--------------------------------------------------------------------------
     // Private
@@ -234,7 +177,6 @@ var FormController = BasicController.extend({
      * @param {string} id
      */
     _confirmSave: function (id) {
-        this.isDirty = false;
         if (id === this.handle) {
             return this.reload();
         } else {
@@ -250,24 +192,6 @@ var FormController = BasicController.extend({
         }
     },
     /**
-     * Helper function to display a warning that some field have an invalid
-     * value.  This is used when a save operation cannot be completed.
-     *
-     * @private
-     * @param {string[]} invalidFields list of field names
-     */
-    _notifyInvalidFields: function (invalidFields) {
-        var record = this.model.get(this.handle, {raw: true});
-        var fields = record.fields;
-        var warnings = invalidFields.map(function (field_name) {
-            var fieldStr = fields[field_name].string;
-            return _.str.sprintf('<li>%s</li>', _.escape(fieldStr));
-        });
-        warnings.unshift('<ul>');
-        warnings.push('</ul>');
-        this.do_warn(_t("The following fields are invalid:"), warnings.join(''));
-    },
-    /**
      * Hook method, called when record(s) has been deleted.
      *
      * @override
@@ -295,71 +219,6 @@ var FormController = BasicController.extend({
         this._super(state);
     },
     /**
-     * Save the record
-     *
-     * @param {Object} options
-     * @param {boolean} [options.stayInEdit=false] if true, don't switch to
-     *   readonly mode after saving the record
-     * @param {boolean} [options.reload=true] if true, reload the record after
-     *   saving
-     * @param {boolean} [options.savePoint=false] if true, the record will only
-     *   be 'locally' saved: its changes will move from the _changes key to the
-     *   data key
-     * @returns {Deferred}
-     */
-    _saveRecord: function (options) {
-        var self = this;
-        options = options || {};
-
-        // some field widgets (e.g. 'html') can't detect (all) their changes, so
-        // we ask them to commit their current value before saving
-        this.renderer.commitChanges();
-
-        var stayInEdit = 'stayInEdit' in options ? options.stayInEdit : false;
-        var shouldReload = 'reload' in options ? options.reload : true;
-        if (!this.model.isDirty(this.handle) && !this.model.isNew(this.handle)) {
-            if (!stayInEdit) {
-                this._toReadOnlyMode();
-            }
-            return $.Deferred().resolve();
-        } else {
-            return self.canBeSaved().then(function () {
-                return self.model.save(self.handle, {
-                    reload: shouldReload,
-                    savePoint: options.savePoint
-                })
-                .then(function () {
-                    if (!stayInEdit) {
-                        self._toReadOnlyMode();
-                    }
-                    self.isDirty = false;
-                });
-            });
-        }
-    },
-    /**
-     * Change the view mode to 'edit'
-     *
-     * @see _toReadOnlyMode
-     * @private
-     */
-    _toEditMode: function () {
-        this.$el.addClass('o_form_editable');
-        this.update({mode: "edit"}, {reload: false});
-    },
-    /**
-     * Change the view mode to 'readonly'
-     *
-     * @see _toEditMode
-     * @private
-     */
-    _toReadOnlyMode: function () {
-        if (this.mode !== 'readonly') {
-            this.$el.removeClass('o_form_editable');
-            this.update({mode: "readonly"}, {reload: false});
-        }
-    },
-    /**
      * Updates the controller's title according to the new state
      *
      * @override
@@ -434,7 +293,10 @@ var FormController = BasicController.extend({
             def = this._callButtonAction(attrs, event.data.record);
         } else {
             // save the record but don't switch to readonly mode
-            def = this.saveRecord({stayInEdit: true, reload: false}).then(function () {
+            def = this.saveRecord(this.handle, {
+                stayInEdit: true,
+                reload: false,
+            }).then(function () {
                 // we need to reget the record to make sure we have changes made
                 // by the basic model, such as the new res_id, if the record is
                 // new.
@@ -453,22 +315,47 @@ var FormController = BasicController.extend({
         }
     },
     /**
-     * If the user clicks on 'Discard', we have to check if the changes can be
-     * discarded, then actually do it.
+     * Called when the user wants to create a new record -> @see createRecord
+     *
+     * @private
+     */
+    _onCreate: function () {
+        this.createRecord();
+    },
+    /**
+     * Called when the user wants to discard the changes made to the current
+     * record -> @see discardChanges
+     *
+     * @private
+     */
+    _onDiscard: function () {
+        this.discardChanges();
+    },
+    /**
+     * Called when a x2m asks to discard the changes made to one of its row.
+     *
+     * @todo find a better way to handle this... this could also be used outside
+     * of form views
      *
      * @private
+     * @param {OdooEvent} ev
      */
-    _onDiscardChange: function () {
+    _onDiscardX2MChanges: function (ev) {
         var self = this;
-        this.canBeDiscarded().then(function () {
-            self.model.discardChanges(self.handle);
-            self.isDirty = false;
-            if (!self.model.isNew(self.handle)) {
-                self._toReadOnlyMode();
-            } else {
-                self.trigger_up('switch_to_previous_view');
-            }
-        });
+        ev.stopPropagation();
+        var recordID = ev.data.recordID;
+        this.discardChanges(recordID)
+            .done(function () {
+                if (self.model.isNew(recordID)) {
+                    self._abandonRecord(recordID);
+                }
+                // TODO this will tell the renderer to rerender the widget that
+                // asked for the discard but will unfortunately lose the click
+                // made on another row if any
+                self._confirmChange(self.handle, [ev.target.name], ev)
+                    .always(ev.data.onSuccess);
+            })
+            .fail(ev.data.onFailure);
     },
     /**
      * Called when the user clicks on 'Duplicate Record' in the sidebar
@@ -479,26 +366,18 @@ var FormController = BasicController.extend({
         var self = this;
         this.model.duplicateRecord(this.handle)
             .then(function (handle) {
-                self.isDirty = false;
                 self.handle = handle;
                 self._updateEnv();
-                self._toEditMode();
+                self._setMode('edit');
             });
     },
     /**
-     * This method comes from the field manager mixin. We force to save directly
-     * the changes if the form is in readonly, because in that case the changes
-     * come from widgets that are editable even in readonly (e.g. Priority).
+     * Called when the user wants to edit the current record -> @see _setMode
      *
-     * @override
      * @private
-     * @param {OdooEvent} event
      */
-    _onFieldChanged: function (event) {
-        if (this.mode === 'readonly') {
-            event.data.force_save = true;
-        }
-        this._super.apply(this, arguments);
+    _onEdit: function () {
+        this._setMode('edit');
     },
     /**
      * Opens a one2many record (potentially new) in a dialog. This handler is
@@ -555,6 +434,16 @@ var FormController = BasicController.extend({
         }).open();
     },
     /**
+     * Called when the user wants to save the current record -> @see saveRecord
+     *
+     * @private
+     * @param {MouseEvent} ev
+     */
+    _onSave: function (ev) {
+        ev.stopPropagation(); // Prevent x2m lines to be auto-saved
+        this.saveRecord();
+    },
+    /**
      * This method is called when someone tries to sort a column, most likely
      * in a x2many list view
      *
@@ -567,7 +456,6 @@ var FormController = BasicController.extend({
         var state = this.model.get(this.handle);
         this.renderer.confirmChange(state, state.id, [field]);
     },
-
 });
 
 return FormController;
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 1e40ce13dd6..75c2b987a8f 100644
--- a/addons/web/static/src/js/views/form/form_renderer.js
+++ b/addons/web/static/src/js/views/form/form_renderer.js
@@ -27,54 +27,30 @@ var FormRenderer = BasicRenderer.extend({
     //--------------------------------------------------------------------------
 
     /**
-     * This method has two responsabilities: find every invalid fields in the
-     * current form, and making sure that they are displayed as invalid, by
-     * toggling the o_form_invalid css class.  It has to be done both on the
-     * widget, and on the label, if there is a label.
+     * Extend the method so that labels also receive the 'o_form_invalid' class
+     * if necessary.
      *
-     * @returns {Deferred}
-     *   if it fails, it gives the list of invalid field names
+     * @override
+     * @see BasicRenderer.canBeSaved
+     * @param {string} recordID
+     * @returns {string[]}
      */
-    canBeSaved: function () {
+    canBeSaved: function (recordID) {
         var self = this;
-        var invalidFields = [];
-        var defs = [];
+        var fieldNames = this._super.apply(this, arguments);
 
-        function markWidget(widget, isValid) {
-            if (!isValid) {
-                invalidFields.push(widget.name);
-            }
-            widget.$el.toggleClass('o_form_invalid', !isValid);
-            var idForLabel = self.idsForLabels[widget.name];
-            var $label = idForLabel ? self.$('label[for=' + idForLabel + ']') : $();
-            $label.toggleClass('o_form_invalid', !isValid);
-        }
+        var $labels = this.$('label');
+        $labels.removeClass('o_form_invalid');
 
-        _.each(this.allFieldWidgets[this.state.id], function (widget) {
-            var isValid = self._canWidgetBeSaved(widget);
-            if (isValid instanceof $.Deferred) {
-                defs.push(isValid.then(function (isValid) {
-                    markWidget(widget, isValid);
-                }));
-            } else {
-                markWidget(widget, isValid);
+        _.each(fieldNames, function (fieldName) {
+            var idForLabel = self.idsForLabels[fieldName];
+            if (idForLabel) {
+                $labels
+                    .filter('[for=' + idForLabel + ']')
+                    .addClass('o_form_invalid');
             }
         });
-        return $.when.apply($, defs).then(function () {
-            if (invalidFields.length) {
-                return $.Deferred().reject(invalidFields);
-            }
-        });
-    },
-    /**
-     * Calls 'commitChanges' on all field widgets, so that they can notify the
-     * environment with their current value (useful for widgets that can't
-     * detect when their value changes, e.g. field 'html').
-     */
-    commitChanges: function () { // TODO add a test
-        _.each(this.allFieldWidgets, function (recordWidgets) {
-            _.invoke(recordWidgets, 'commitChanges');
-        });
+        return fieldNames;
     },
     /**
      * returns the active tab pages for each notebook
diff --git a/addons/web/static/src/js/views/kanban/kanban_controller.js b/addons/web/static/src/js/views/kanban/kanban_controller.js
index b3eb314b612..9bb728011e8 100644
--- a/addons/web/static/src/js/views/kanban/kanban_controller.js
+++ b/addons/web/static/src/js/views/kanban/kanban_controller.js
@@ -219,17 +219,6 @@ var KanbanController = BasicController.extend({
             });
     },
     /**
-     * This method comes from the field manager mixin.  Since there is no
-     * onchange for kanban, we force a save for every change
-     *
-     * @override
-     * @param {OdooEvent} event
-     */
-    _onFieldChanged: function (event) {
-        event.data.force_save = true;
-        this._super.apply(this, arguments);
-    },
-    /**
      * @param {OdooRevent} event
      */
     _onLoadMore: function (event) {
diff --git a/addons/web/static/src/js/views/kanban/kanban_renderer.js b/addons/web/static/src/js/views/kanban/kanban_renderer.js
index 5c976bba903..8152ce4987b 100644
--- a/addons/web/static/src/js/views/kanban/kanban_renderer.js
+++ b/addons/web/static/src/js/views/kanban/kanban_renderer.js
@@ -113,12 +113,6 @@ var KanbanRenderer = BasicRenderer.extend({
         this.widgets[0].addQuickCreate();
     },
     /**
-     * @returns {Deferred}
-     */
-    canBeSaved: function () {
-        return $.when();
-    },
-    /**
      * Removes a widget (record if ungrouped, column if grouped) from the view.
      *
      * @param {Widget} widget the instance of the widget to remove
diff --git a/addons/web/static/src/js/views/list/list_controller.js b/addons/web/static/src/js/views/list/list_controller.js
index 88887f45fc3..50f47a73fc0 100644
--- a/addons/web/static/src/js/views/list/list_controller.js
+++ b/addons/web/static/src/js/views/list/list_controller.js
@@ -20,7 +20,8 @@ var ListController = BasicController.extend({
     custom_events: _.extend({}, BasicController.prototype.custom_events, {
         add_record: '_onAddRecord',
         button_clicked: '_onButtonClicked',
-        change_mode: '_onChangeMode',
+        edit_line: '_onEditLine',
+        save_line: '_onSaveLine',
         selection_changed: '_onSelectionChanged',
         toggle_column_order: '_onToggleColumnOrder',
         toggle_group: '_onToggleGroup',
@@ -49,6 +50,24 @@ var ListController = BasicController.extend({
     //--------------------------------------------------------------------------
 
     /**
+     * To improve performance, list view must not be rerendered if it is asked
+     * to discard all its changes. Indeed, only the in-edition row needs to be
+     * discarded in that case.
+     *
+     * @override
+     * @param {string} [recordID] - default to main recordID
+     * @returns {Deferred}
+     */
+    discardChanges: function (recordID) {
+        if ((recordID || this.handle) === this.handle) {
+            recordID = this.renderer.getEditableRecordID();
+            if (recordID === null) {
+                return $.when();
+            }
+        }
+        return this._super(recordID);
+    },
+    /**
      * Calculate the active domain of the list view. This should be done only
      * if the header checkbox has been checked. This is done by evaluating the
      * search results, and then adding the dataset domain (i.e. action domain).
@@ -93,14 +112,17 @@ var ListController = BasicController.extend({
     /**
      * Display and bind all buttons in the control panel
      *
+     * Note: clicking on the "Save" button does nothing special. Indeed, all
+     * editable rows are saved once left and clicking on the "Save" button does
+     * induce the leaving of the current row.
+     *
      * @override
-     * @param {jQuery Node} $node
+     * @param {jQuery} $node
      */
     renderButtons: function ($node) {
         if (!this.noLeaf && this.hasButtons) {
             this.$buttons = $(qweb.render('ListView.buttons', {widget: this}));
             this.$buttons.on('click', '.o_list_button_add', this._onCreateRecord.bind(this));
-            this.$buttons.on('click', '.o_list_button_save', this._onSave.bind(this));
             this.$buttons.on('click', '.o_list_button_discard', this._onDiscard.bind(this));
             this.$buttons.appendTo($node);
         }
@@ -151,8 +173,25 @@ var ListController = BasicController.extend({
     //--------------------------------------------------------------------------
 
     /**
+     * @see BasicController._abandonRecord
+     * If the given abandoned record is not the main one, notifies the renderer
+     * to remove the appropriate subrecord (line).
+     *
+     * @override
+     * @private
+     * @param {string} [recordID] - default to the main recordID
+     */
+    _abandonRecord: function (recordID) {
+        this._super.apply(this, arguments);
+        if ((recordID || this.handle) !== this.handle) {
+            var state = this.model.get(this.handle);
+            this.renderer.removeLine(state, recordID);
+        }
+    },
+    /**
      * Add a record to the list
      *
+     * @todo make record creation a basic controller feature
      * @private
      */
     _addRecord: function () {
@@ -194,6 +233,21 @@ var ListController = BasicController.extend({
         return this.renderer.confirmSave(state, id);
     },
     /**
+     * Allows to change the mode of a single row.
+     *
+     * @override
+     * @private
+     * @param {string} mode
+     * @param {string} [recordID] - default to main recordID
+     */
+    _setMode: function (mode, recordID) {
+        this._super.apply(this, arguments);
+        if ((recordID || this.handle) !== this.handle) {
+            this.renderer.setRowMode(recordID, mode);
+            this._updateButtons(mode);
+        }
+    },
+    /**
      * Display the sidebar (the 'action' menu in the control panel) if we have
      * some selected records.
      */
@@ -249,17 +303,6 @@ var ListController = BasicController.extend({
         this._callButtonAction(event.data.attrs, event.data.record);
     },
     /**
-     * This event is triggered when a list renderer goes from mode = readonly to
-     * edit, and vice versa. In that case, we need to make sure that the buttons
-     * displayed in the control panel are correct.
-     *
-     * @private
-     * @param {OdooEvent} event
-     */
-    _onChangeMode: function (event) {
-        this._updateButtons(event.data.mode);
-    },
-    /**
      * When the user clicks on the 'create' button, two things can happen. We
      * can switch to the form view with no active res_id, so it is in 'create'
      * mode, or we can edit inline.
@@ -290,53 +333,41 @@ var ListController = BasicController.extend({
     /**
      * Handler called when the user clicked on the 'Discard' button.
      *
-     * @private
-     * @param {MouseEvent} event
+     * @param {Event} ev
      */
-    _onDiscard: function (event) {
-        event.stopPropagation();
-        this.model.discardChanges(this.handle);
-        this._updateButtons('readonly');
-        this.update(this.handle, {reload: false});
+    _onDiscard: function (ev) {
+        ev.stopPropagation(); // So that it is not considered as a row leaving
+        this.discardChanges();
     },
     /**
-     * Opens the Export Dialog
+     * Called when the user asks to edit a row -> Updates the controller buttons
      *
-     * @private
+     * @param {OdooEvent} ev
      */
-    _onExportData: function () {
-        var record = this.model.get(this.handle);
-        new DataExport(this, record).open();
+    _onEditLine: function (ev) {
+        ev.stopPropagation();
+        this._setMode('edit', ev.data.recordID);
     },
     /**
-     * This method comes from the field manager mixin.
-     * @todo: there is a very similar method in kanban and form controllers.
-     * This should be moved in basic controller, and shared between basic views.
+     * Opens the Export Dialog
      *
-     * @override
      * @private
-     * @param {OdooEvent} event
      */
-    _onFieldChanged: function (event) {
-        if (this.renderer.mode === 'readonly') {
-            event.data.force_save = true;
-        }
-        this._super.apply(this, arguments);
+    _onExportData: function () {
+        var record = this.model.get(this.handle);
+        new DataExport(this, record).open();
     },
     /**
-     * changes of the list editable are automatically saved when unselecting the
-     * row, which is done when clicking on 'Save' (anywhere outside the row
-     * actually), so this function should only switch back to readonly mode
+     * Called when the renderer displays an editable row and the user tries to
+     * leave it -> Saves the record associated to that line.
      *
-     * @private
-     * @param {MouseEvent} event
+     * @param {OdooEvent} ev
      */
-    _onSave: function (event) {
-        // we prevent the event propagation because we don't want this event to
-        // trigger a click on the main bus, which would be then caught by the
-        // list editable renderer and would unselect the newly created row
-        // event.stopPropagation();
-        this._updateButtons('readonly');
+    _onSaveLine: function (ev) {
+        var recordID = ev.data.recordID;
+        this.saveRecord(recordID)
+            .done(ev.data.onSuccess)
+            .fail(ev.data.onFailure);
     },
     /**
      * When the current selection changes (by clicking on the checkboxes on the
@@ -373,7 +404,7 @@ var ListController = BasicController.extend({
             this.pager.updateState({current_min: 1});
         }
         this.model.setSort(data.id, event.data.name);
-        this.update();
+        this.update({});
     },
     /**
      * In a grouped list view, each group can be clicked on to open/close them.
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 b24ec6a0c29..54c85e9c888 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
@@ -12,7 +12,6 @@ odoo.define('web.EditableListRenderer', function (require) {
  * view. It uses the same widgets, but the code is totally stand alone.
  */
 var core = require('web.core');
-var Dialog = require('web.Dialog');
 var ListRenderer = require('web.ListRenderer');
 
 var _t = core._t;
@@ -53,10 +52,6 @@ ListRenderer.include({
 
         this.currentRow = null;
         this.currentCol = null;
-
-        // the dirtyRows object is there to keep track of lines that have been
-        // modified by the user
-        this.dirtyRows = {};
     },
     /**
      * @override
@@ -75,37 +70,24 @@ ListRenderer.include({
     //--------------------------------------------------------------------------
 
     /**
-     * @returns {Deferred}
+     * If the given recordID is the list main one (or that no recordID is
+     * given), then the whole view can be saved if one of the two following
+     * conditions is true:
+     * - There is no line in edition (all lines are saved so they are all valid)
+     * - The line in edition can be saved
+     *
+     * @override
+     * @param {string} [recordID]
+     * @returns {string[]}
      */
-    canBeSaved: function () {
-        var self = this;
-        var invalidDirtyWidgets = [];
-        var invalidCleanWidgets = [];
-        _.each(this.allFieldWidgets, function (recordWidgets, recordID) {
-            _.each(recordWidgets, function (widget) {
-                // Note: the _canWidgetBeSaved method may return a deferred in
-                // general, but not in this case. Here, this method is called by
-                // a fieldone2many when it needs to make sure the state is
-                // valid. So, the widgets that we consider here are widgets that
-                // can be in a one2many, and we don't support a list inside a
-                // list (for now at least).
-                if (!self._canWidgetBeSaved(widget)) {
-                    if (self.dirtyRows[recordID]) {
-                        invalidDirtyWidgets.push(widget);
-                    } else {
-                        invalidCleanWidgets.push(widget);
-                    }
-                }
-            });
-        });
-        if (invalidDirtyWidgets.length) {
-            // TODO: ask for confirmation, open dialog, and depending on user
-            // input, resolve and reject deferred
-        }
-        if (invalidCleanWidgets.length) {
-            this._unselectRow();
+    canBeSaved: function (recordID) {
+        if ((recordID || this.state.id) === this.state.id) {
+            recordID = this.getEditableRecordID();
+            if (recordID === null) {
+                return [];
+            }
         }
-        return $.when();
+        return this._super(recordID);
     },
     /**
      * @see BasicRenderer.confirmChange
@@ -115,6 +97,8 @@ ListRenderer.include({
      * to have a visual feedback on which fields changed, and on which fields
      * have been saved.
      *
+     * @todo maybe should be a basic behavior
+     *
      * @override
      * @param {Object} state - a record resource (the new full state)
      * @param {string} id - the local id for the changed record
@@ -123,8 +107,6 @@ ListRenderer.include({
      * @returns {Deferred}
      */
     confirmChange: function (state, id, fields, e) {
-        this.dirtyRows[id] = true;
-
         return this._super.apply(this, arguments).then(function (resetWidgets) {
             _.each(resetWidgets, function (widget) {
                 var $td = widget.$el.parent();
@@ -144,10 +126,7 @@ ListRenderer.include({
      */
     confirmSave: function (state, savedRecordID) {
         this.state = state;
-        var index = _.findIndex(this.state.data, {id: savedRecordID});
-        for (var j = 0; j < this.columns.length; j++) {
-            this._setCellValue(index, j, false);
-        }
+        this.setRowMode(savedRecordID, 'readonly');
     },
     /**
      * Edit a given record in the list
@@ -155,10 +134,86 @@ ListRenderer.include({
      * @param {string} recordID
      */
     editRecord: function (recordID) {
-        var index = _.findIndex(this.state.data, function (record) {
-            return record.id === recordID;
-        });
-        this._selectCell(index, 0);
+        var rowIndex = _.findIndex(this.state.data, {id: recordID});
+        this._selectCell(rowIndex, 0);
+    },
+    /**
+     * Returns the recordID associated to the line which is currently in edition
+     * or null if there is no line in edition.
+     *
+     * @returns {string|null}
+     */
+    getEditableRecordID: function () {
+        if (this.currentRow !== null) {
+            return this.state.data[this.currentRow].id;
+        }
+        return null;
+    },
+    /**
+     * Removes the line associated to the given recordID (the index of the row
+     * is found thanks to the old state), then updates the state.
+     *
+     * @param {Object} state
+     * @param {string} recordID
+     */
+    removeLine: function (state, recordID) {
+        var rowIndex = _.findIndex(this.state.data, {id: recordID});
+        if (rowIndex === this.currentRow) {
+            this.currentRow = null;
+        }
+        var $row = this.$('tbody > tr:nth(' + rowIndex + ')');
+        $row.remove(); // FIXME row indexes are not updated... let's remove data('index') and check index dynamically ?
+
+        this.state = state;
+    },
+    /**
+     * Updates the already rendered row associated to the given recordID so that
+     * it fits the given mode.
+     *
+     * @param {string} recordID
+     * @param {string} mode
+     * @param {boolean} [keepDirtyFlag=false]
+     */
+    setRowMode: function (recordID, mode, keepDirtyFlag) {
+        var self = this;
+        var rowIndex = _.findIndex(this.state.data, {id: recordID});
+        if (rowIndex < 0) {
+            return;
+        }
+        var editMode = (mode === 'edit');
+        var record = this.state.data[rowIndex];
+
+        this.currentRow = editMode ? rowIndex : null;
+        var $row = this.$('tbody > tr:nth(' + rowIndex + ')');
+        $row.toggleClass('o_selected_row', editMode);
+
+        if (editMode) {
+            // Instantiate column widgets and destroy potential readonly ones
+            var oldWidgets = _.clone(this.allFieldWidgets[record.id]);
+            _.each(this.columns, function (node, colIndex) {
+                var $td = self._findTd($row, colIndex);
+                var $newTd = self._renderBodyCell(record, node, colIndex, {
+                    renderInvisible: true,
+                    renderWidgets: true,
+                });
+
+                // TODO this is ugly...
+                if ($td.hasClass('o_list_button')) {
+                    self._unregisterModifiersElement(node, record, $td.children());
+                }
+
+                // Force the width of each cell to its current value so that it
+                // won't change if its content changes, to prevent the view from
+                // flickering
+                $td.width($td.width());
+                $td.empty().append($newTd.contents());
+            });
+            _.each(oldWidgets, this._destroyFieldWidget.bind(this, record));
+        } else {
+            for (var colIndex = 0; colIndex < this.columns.length; colIndex++) {
+                this._setCellValue(rowIndex, colIndex, keepDirtyFlag || false);
+            }
+        }
     },
 
     //--------------------------------------------------------------------------
@@ -192,6 +247,15 @@ ListRenderer.include({
         return n;
     },
     /**
+     * @override
+     * @returns {Deferred}
+     */
+    _render: function () {
+        this.currentRow = null;
+        this.currentCol = null;
+        return this._super.apply(this, arguments);
+    },
+    /**
      * The renderer needs to support reordering lines.  This is only active in
      * edit mode. The hasHandle attribute is used when there is a sequence
      * widget.
@@ -296,20 +360,13 @@ ListRenderer.include({
             return;
         }
 
-        var record = this.state.data[rowIndex];
-
-        // If this is a different row, then we need to prepare for it by
-        // unselecting the current row and selecting the new one.
-        var def = $.when();
-        if (rowIndex !== this.currentRow) {
-            def = this._unselectRow(); // This will trigger the save
-            def.then(this._selectRow.bind(this, rowIndex));
-        }
-
-        // In all cases (new row or not), activate the proper column widget
+        // Select the row then activate the widget in the correct cell
         var self = this;
-        def.then(function () {
+        this._selectRow(rowIndex).then(function () {
+            var record = self.state.data[rowIndex];
             var fieldIndex = self._activateFieldWidget(record, colIndex - getNbButtonBefore(colIndex));
+
+            // If no widget to activate in the row, unselect the row
             if (fieldIndex < 0) {
                 return self._unselectRow();
             }
@@ -333,36 +390,20 @@ ListRenderer.include({
      * @param {integer} rowIndex
      */
     _selectRow: function (rowIndex) {
-        this.currentRow = rowIndex;
+        // Do nothing if already selected
+        if (rowIndex === this.currentRow) {
+            return $.when();
+        }
 
+        // To select a row, the currently selected one must be unselected first
         var self = this;
-        var record = this.state.data[rowIndex];
-
-        // Force the width of each cell to its current value so that it won't
-        // change if its content changes, to prevent the view from flickering
-        var $row = this.$('tbody > tr:nth(' + rowIndex + ')');
-        $row.addClass('o_selected_row');
-        $row.children().each(function () {
-            var $td = $(this);
-            $td.width($td.width());
-        });
-
-        // Instantiate column widgets and destroy potential readonly ones
-        var oldWidgets = _.clone(this.allFieldWidgets[record.id]);
-        _.each(this.columns, function (node, index) {
-            var $newTd = self._renderBodyCell(record, node, index, {
-                renderInvisible: true,
-                renderWidgets: true,
+        return this._unselectRow().then(function () {
+            // Notify the controller we want to make a record editable
+            var record = self.state.data[rowIndex];
+            self.trigger_up('edit_line', {
+                recordID: record.id,
             });
-            var $td = self._findTd($row, index);
-            if ($td.hasClass('o_list_button')) {
-                self._unregisterModifiersElement(node, record, $td.children()); // FIXME this is ugly...
-            }
-            $td.empty().append($newTd.contents());
         });
-        _.each(oldWidgets, this._destroyFieldWidget.bind(this, record));
-
-        this.trigger_up('change_mode', {mode: 'edit'});
     },
     /**
      * Set the value of a cell.  This method can be called when the value of a
@@ -375,6 +416,7 @@ ListRenderer.include({
     _setCellValue: function (rowIndex, colIndex, keepDirtyFlag) {
         var record = this.state.data[rowIndex];
         var node = this.columns[colIndex];
+
         var $oldTd = this._findTd(this.$('tbody > tr:nth(' + rowIndex + ')'), colIndex);
         var $newTd = this._renderBodyCell(record, node, colIndex, {mode: 'readonly'});
         if (keepDirtyFlag && $oldTd.hasClass('o_field_dirty')) {
@@ -382,17 +424,24 @@ ListRenderer.include({
         }
         this._unregisterModifiersElement(node, record, $oldTd);
         $oldTd.replaceWith($newTd);
+
+        // Destroy old cell field widget if any
+        // TODO this is very inefficient O(n^3) instead of O(n) on row update
+        // because this method is called for each cell (O(n)), each time we have
+        // to find the associated record widget (O(n)) and once found, the
+        // search is performed again by the _destroyFieldWidget function
+        if (node.tag === 'field') {
+            var recordWidgets = this.allFieldWidgets[record.id];
+            var w = _.findWhere(recordWidgets, {name: node.attrs.name}); //
+            if (w) {
+                this._destroyFieldWidget(record, w);
+            }
+        }
     },
     /**
      * This method is called whenever we click/move outside of a row that was
      * in edit mode. This is the moment we save all accumulated changes on that
-     * row, if needed.
-     *
-     * If the current row is valid, then it can be unselected. If it is not
-     * valid, we need to check if it is dirty. If it is not dirty, it can be
-     * unselected (and will be removed). If it is dirty, we need to ask the user
-     * for confirmation. If the user agrees, the row will be removed, otherwise
-     * nothing happens (it is still selected)
+     * row, if needed (@see BasicController.saveRecord).
      *
      * @returns {Deferred} The deferred resolves if the row was unselected (and
      *   possibly removed). If may be rejected, when the row is dirty and the
@@ -404,64 +453,14 @@ ListRenderer.include({
             return $.when();
         }
 
-        var self = this;
         var record = this.state.data[this.currentRow];
-        var rowWidgets = _.clone(this.allFieldWidgets[record.id]);
-        var isRowValid = _.every(rowWidgets, this._canWidgetBeSaved.bind(this));
-        var $row = this.$('tbody > tr.o_selected_row');
-
-        // If the row is not valid and is dirty, we have to ask the user
-        // confirmation before going further
-        var def = $.when();
-        if (!isRowValid && this.dirtyRows[record.id]) {
-            def = $.Deferred();
-            var message = _t("The line has been modified, your changes will be discarded. Are you sure you want to discard the changes ?");
-            var dialog = Dialog.confirm(this, message, {
-                title: _t("Warning"),
-                confirm_callback: def.resolve.bind(def),
-                cancel_callback: def.reject.bind(def),
-            });
-            dialog.$modal.on('hidden.bs.modal', def.reject.bind(def));
-        }
-
-        return def.then(function finalizeUnselect() {
-            var rowRemoved = false;
-            var keepDirtyFlag = true;
-            if (isRowValid) {
-                // Trigger the save (if the record isn't dirty, the datamodel
-                // won't save anything)
-                self.trigger_up('field_changed', {
-                    dataPointID: record.id,
-                    changes: {},
-                    force_save: true,
-                });
-            } else if (record.res_id) {
-                // Not a creation, tell the model to discard the changes
-                self.trigger_up('discard_changes', {id: record.id});
-                keepDirtyFlag = false;
-            } else {
-                // Creation, tell the model to discard the row and remove the
-                // associated row DOM element
-                self.trigger_up('discard_line', {id: record.id});
-                $row.remove(); // FIXME row indexes are not updated... let's remove data('index') and check index dynamically ?
-                rowRemoved = true;
-            }
-
-            // Update row if it was not removed
-            if (!rowRemoved) {
-                $row.removeClass('o_selected_row');
-                _.each(self.columns, function (col, colIndex) {
-                    self._setCellValue(self.currentRow, colIndex, keepDirtyFlag);
-                });
-            }
-
-            // Destroy the edition widgets
-            _.each(rowWidgets, self._destroyFieldWidget.bind(self, record));
-
-            delete self.dirtyRows[record.id];
-            self.currentRow = null;
-            self.trigger_up('change_mode', {mode: 'readonly'});
+        var def = $.Deferred();
+        this.trigger_up('save_line', {
+            recordID: record.id,
+            onSuccess: def.resolve.bind(def),
+            onFailure: def.reject.bind(def),
         });
+        return def;
     },
 
     //--------------------------------------------------------------------------
@@ -483,9 +482,10 @@ ListRenderer.include({
         event.stopPropagation();
 
         // but we do want to unselect current row
-        this._unselectRow();
-
-        this.trigger_up('add_record');
+        var self = this;
+        this._unselectRow().then(function () {
+            self.trigger_up('add_record'); // TODO write a test, the deferred was not considered
+        });
     },
     /**
      * When the user clicks on a cell, we simply select it.
@@ -640,7 +640,7 @@ ListRenderer.include({
             return;
         }
 
-        // there is currenctly no selected row
+        // there is currently no selected row
         if (this.currentRow === null) {
             return;
         }
@@ -652,10 +652,10 @@ ListRenderer.include({
 
         // ignore clicks in modals, except if the list is in a modal, and the
         // click is performed in that modal
-        var $listModal = this.$el.closest('.modal');
-        if ($listModal.length) {
-            var $click_modal = $(event.target).parents('.modal');
-            if ($click_modal.prop('id') === $listModal.prop('id')) {
+        var $clickModal = $(event.target).closest('.modal');
+        if ($clickModal.length) {
+            var $listModal = this.$el.closest('.modal');
+            if ($clickModal.prop('id') !== $listModal.prop('id')) {
                 return;
             }
         }
diff --git a/addons/web/static/src/js/views/list/list_view.js b/addons/web/static/src/js/views/list/list_view.js
index 1260a8bc69d..7b597c712bc 100644
--- a/addons/web/static/src/js/views/list/list_view.js
+++ b/addons/web/static/src/js/views/list/list_view.js
@@ -37,16 +37,18 @@ var ListView = BasicView.extend({
         this._super.apply(this, arguments);
 
         var arch = viewInfo.arch;
+        var mode = arch.attrs.editable && !params.readonly ? "edit" : "readonly";
+
         this.controllerParams.editable = arch.attrs.editable;
         this.controllerParams.hasSidebar = params.sidebar;
         this.controllerParams.activeActions.delete = true;
         this.controllerParams.noLeaf = !!this.loadParams.context.group_by_no_leaf;
+        this.controllerParams.mode = mode;
 
         this.rendererParams.arch = arch;
         this.rendererParams.hasSelectors =
                 'hasSelectors' in params ? params.hasSelectors : true;
-        this.rendererParams.mode =
-                arch.attrs.editable && !params.readonly ? "edit" : "readonly";
+        this.rendererParams.mode = mode;
 
         this.loadParams.limit = this.loadParams.limit || 80;
         this.loadParams.type = 'list';
diff --git a/addons/web/static/src/js/views/view_dialogs.js b/addons/web/static/src/js/views/view_dialogs.js
index 6f0fe8328c3..d0c1fa75fdf 100644
--- a/addons/web/static/src/js/views/view_dialogs.js
+++ b/addons/web/static/src/js/views/view_dialogs.js
@@ -212,7 +212,7 @@ var FormViewDialog = ViewDialog.extend({
             }
             def = this.options.on_save(this.form_view.model.get(this.form_view.handle));
         } else {
-            def = this.form_view.saveRecord({
+            def = this.form_view.saveRecord(this.form_view.handle, {
                 stayInEdit: true,
                 reload: false,
                 savePoint: this.shouldSaveLocally,
diff --git a/addons/web/static/tests/fields/basic_fields_tests.js b/addons/web/static/tests/fields/basic_fields_tests.js
index dbe91188242..fa5b1224728 100644
--- a/addons/web/static/tests/fields/basic_fields_tests.js
+++ b/addons/web/static/tests/fields/basic_fields_tests.js
@@ -1022,7 +1022,7 @@ QUnit.module('basic_fields', {
 
     QUnit.module('HandleWidget');
 
-    QUnit.test('handle widget', function (assert) {
+    QUnit.test('handle widget in x2m', function (assert) {
         assert.expect(6);
 
         this.data.partner.records[0].p = [2, 4];
diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js
index db981d78b88..a33a8f076af 100644
--- a/addons/web/static/tests/fields/relational_fields_tests.js
+++ b/addons/web/static/tests/fields/relational_fields_tests.js
@@ -258,7 +258,7 @@ QUnit.module('relational_fields', {
             event.data.callback({user_context: {}});
         });
 
-        form._toEditMode();
+        form.$buttons.find('.o_form_button_edit').click();
         var $dropdown = form.$('.o_form_field_many2one input').autocomplete('widget');
 
         form.$('.o_form_field_many2one input').click();
diff --git a/addons/web/static/tests/views/basic_model_tests.js b/addons/web/static/tests/views/basic_model_tests.js
index 6b87a90598c..5f4f94352df 100644
--- a/addons/web/static/tests/views/basic_model_tests.js
+++ b/addons/web/static/tests/views/basic_model_tests.js
@@ -1156,32 +1156,6 @@ QUnit.module('Views', {
         model.destroy();
     });
 
-    QUnit.test('get method works on unset x2many, after save without reload', function (assert) {
-        assert.expect(3);
-
-        var model = createModel({
-            Model: BasicModel,
-            data: this.data,
-        });
-
-        var params = {
-            fieldNames: ['product_ids'],
-            fields: this.data.partner.fields,
-            modelName: 'partner',
-            type: 'record',
-        };
-
-        model.load(params).then(function (resultID) {
-            assert.ok(model.isDirty(resultID), "datapoint should be dirty");
-            model.save(resultID, {reload: false});
-            var record = model.get(resultID, {raw: true});
-            assert.deepEqual(record.data.product_ids, [],
-                "should have correct value for product_ids field");
-            assert.notOk(model.isDirty(resultID), "datapoint should not be dirty");
-        });
-        model.destroy();
-    });
-
     QUnit.test('call makeRecord with a pre-fetched many2one field', function (assert) {
         assert.expect(3);
         var rpcCount = 0;
@@ -1346,7 +1320,7 @@ QUnit.module('Views', {
         var model = createModel({
             Model: BasicModel,
             data: this.data,
-            mockRPC: function(route, args) {
+            mockRPC: function (route, args) {
                 assert.step(args.method);
                 return this._super(route, args);
             },
diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js
index f59c484b3c9..2eb5f6e127b 100644
--- a/addons/web/static/tests/views/form_tests.js
+++ b/addons/web/static/tests/views/form_tests.js
@@ -620,17 +620,17 @@ QUnit.module('Views', {
 
         assert.strictEqual(form.mode, 'readonly', 'form view should be in readonly mode');
         assert.ok(form.$('.o_form_view').hasClass('o_form_readonly'),
-                    'form view should have .o_form_readonly');
+            'form view should be .o_form_readonly');
         assert.ok(form.$buttons.find('.o_form_buttons_view').is(':visible'),
             'readonly buttons should be visible');
         assert.ok(!form.$buttons.find('.o_form_buttons_edit').is(':visible'),
             'edit buttons should not be visible');
         form.$buttons.find('.o_form_button_edit').click();
         assert.strictEqual(form.mode, 'edit', 'form view should be in edit mode');
-        assert.ok(form.$el.hasClass('o_form_editable'),
-                    'form view should have .o_form_editable');
-        assert.ok(!form.$el.hasClass('o_form_readonly'),
-                    'form view should not have .o_form_readonly');
+        assert.ok(form.$('.o_form_view').hasClass('o_form_editable'),
+            'form view should be .o_form_editable');
+        assert.ok(!form.$('.o_form_view').hasClass('o_form_readonly'),
+            'form view should not be .o_form_readonly');
         assert.ok(!form.$buttons.find('.o_form_buttons_view').is(':visible'),
             'readonly buttons should not be visible');
         assert.ok(form.$buttons.find('.o_form_buttons_edit').is(':visible'),
@@ -1456,7 +1456,7 @@ QUnit.module('Views', {
 
         // click on discard
         form.$buttons.find('.o_form_button_cancel').click();
-        assert.ok(!$('.modal').length, 'no confirm modal should be displayed');
+        assert.ok(!$('.modal:visible').length, 'no confirm modal should be displayed');
         assert.strictEqual(form.$('.o_form_field').text(), 'yop', 'field in readonly should display yop');
 
         assert.strictEqual(nbWrite, 0, 'no write RPC should have been done');
@@ -1768,7 +1768,7 @@ QUnit.module('Views', {
 
         // click on the pager to switch to the next record and cancel the confirm request
         form.pager.$('.o_pager_next').click(); // click on next
-        assert.strictEqual($('.modal').length, 0, 'no confirm modal should be displayed');
+        assert.strictEqual($('.modal:visible').length, 0, 'no confirm modal should be displayed');
         assert.strictEqual(form.pager.$('.o_pager_value').text(), "2", 'pager value should be 2');
 
         assert.strictEqual(form.$('.o_priority .fa-star-o').length, 2,
@@ -1780,7 +1780,7 @@ QUnit.module('Views', {
             'priority widget should have been updated');
 
         form.pager.$('.o_pager_next').click(); // click on next
-        assert.strictEqual($('.modal').length, 0, 'no confirm modal should be displayed');
+        assert.strictEqual($('.modal:visible').length, 0, 'no confirm modal should be displayed');
         assert.strictEqual(form.pager.$('.o_pager_value').text(), "1", 'pager value should be 1');
 
         // switch to edit mode
@@ -1798,38 +1798,6 @@ QUnit.module('Views', {
         form.destroy();
     });
 
-    QUnit.test('handling dirty state: canBeDiscarded should be idempotent', function (assert) {
-        assert.expect(3);
-
-        var form = createView({
-            View: FormView,
-            model: 'partner',
-            data: this.data,
-            arch: '<form string="Partners">' +
-                    '<field name="foo"></field>' +
-                '</form>',
-            res_id: 1,
-        });
-
-        // switch to edit mode
-        form.$buttons.find('.o_form_button_edit').click();
-        assert.strictEqual(form.$('.o_form_input').val(), 'yop', 'input should contain yop');
-
-        // edit the foo field to make it dirty
-        form.$('.o_form_input').val('new value').trigger('input');
-
-        // discard changes once
-        form.canBeDiscarded();
-        assert.strictEqual($('.modal').length, 1, 'a confirm modal should be displayed');
-        $('.modal .modal-footer .btn-primary').click(); // click on confirm
-
-        // discard changes a second time
-        form.canBeDiscarded();
-        assert.strictEqual($('.modal').length, 0, 'no confirm modal should be displayed');
-
-        form.destroy();
-    });
-
     QUnit.test('restore local state when switching to another record', function (assert) {
         assert.expect(4);
 
@@ -2047,7 +2015,7 @@ QUnit.module('Views', {
         assert.strictEqual(form.pager.$('.o_pager_limit').text(), "3", 'pager limit should be 3');
         assert.strictEqual(form.$('span:contains(yop)').length, 1,
             'should have a field with foo value for record 1');
-        assert.ok(!$('.modal').length, 'no confirm modal should be displayed');
+        assert.ok(!$('.modal:visible').length, 'no confirm modal should be displayed');
 
         // open sidebar
         form.sidebar.$('button.o_dropdown_toggler_btn').click();
@@ -2347,7 +2315,8 @@ QUnit.module('Views', {
         assert.strictEqual(form.$('.oe_stat_button').length, 1,
             "button box should be displayed in edit on an existing record");
 
-        // create mode
+        // create mode (leave edition first!)
+        form.$buttons.find('.o_form_button_cancel').click();
         form.$buttons.find('.o_form_button_create').click();
         assert.strictEqual(form.$('.oe_stat_button').length, 0,
             "button box should not be displayed when creating a new record");
diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js
index 0167c5ab453..d3c46c3f9f7 100644
--- a/addons/web/static/tests/views/list_tests.js
+++ b/addons/web/static/tests/views/list_tests.js
@@ -1483,9 +1483,10 @@ QUnit.module('Views', {
         list.destroy();
     });
 
-    QUnit.test('discarding (or not) changes after row edition', function (assert) {
-        assert.expect(8);
+    QUnit.test('leaving unvalid rows in edition', function (assert) {
+        assert.expect(4);
 
+        var warnings = 0;
         var list = createView({
             View: ListView,
             model: 'foo',
@@ -1495,6 +1496,11 @@ QUnit.module('Views', {
                     '<field name="foo" required="1"/>' +
                     '<field name="bar"/>' +
                 '</tree>',
+            intercepts: {
+                warning: function (ev) {
+                    warnings++;
+                },
+            },
         });
 
         // Start first line edition
@@ -1509,33 +1515,13 @@ QUnit.module('Views', {
         $secondFooTd.click();
 
         assert.strictEqual($firstFooTd.parent('.o_selected_row').length, 1,
-            "first line should still be in edition as cannot be auto discarded");
-        assert.strictEqual(list.$('tbody tr.o_selected_row').length, 1,
-            "no other line should be in edition");
-
-        // Prevent discard of first line
-        $('.modal-footer:visible > .btn-default').click();
-
-        assert.strictEqual($firstFooTd.parent('.o_selected_row').length, 1,
-            "first line should still be in edition as not discarded");
-        assert.strictEqual(list.$('tbody tr.o_selected_row').length, 1,
-            "no other line should be in edition");
-
-        // Retry starting other line edition
-        $secondFooTd.click();
-
-        assert.strictEqual($firstFooTd.parent('.o_selected_row').length, 1,
-            "first line should still be in edition as cannot be auto discarded");
-        assert.strictEqual(list.$('tbody tr.o_selected_row').length, 1,
-            "no other line should be in edition");
-
-        // Discard first line
-        $('.modal-footer:visible > .btn-primary').click();
-
-        assert.strictEqual($secondFooTd.parent('.o_selected_row').length, 1,
-            "second line should now be in edition");
+            "first line should still be in edition as invalid");
         assert.strictEqual(list.$('tbody tr.o_selected_row').length, 1,
             "no other line should be in edition");
+        assert.strictEqual($firstFooTd.find('input.o_form_invalid').length, 1,
+            "the required field should be marked as invalid");
+        assert.strictEqual(warnings, 1,
+            "a warning should have been displayed");
 
         list.destroy();
     });