[FIX] web: properly handle field navigation for all fields

Before this commit, only fields which were specializations of the
`InputField` were handling up/right/down/left navigation in editable
list view. As m2o field is not an `InputField` (yet?), it was not
possible to navigate from one of those for example.

Now, navigation events are handled by the `AbstractField` class. If a
given field has to handle navigation a different way, it is its
responsability to stop the associated navigation event. For example,
`InputField` instances are stopping the left/right event if the cursor
is not at the start/end of the <input/> element.

Also, before this commit, there were many events ("move_down",
"move_left", "move_next", ...). Now there is only one event for
navigation: "navigation_move". This event is given a "direction" data
("left", "next", ...). It also comes with a "target" data which is a
reference to the last `AbstractField` instance the navigation event
has crossed.

6 files changed, 188 insertions, 194 deletions

diff --git a/addons/web/static/src/js/fields/abstract_field.js b/addons/web/static/src/js/fields/abstract_field.js
index 5e972f647e6..7278e45dedb 100644
--- a/addons/web/static/src/js/fields/abstract_field.js
+++ b/addons/web/static/src/js/fields/abstract_field.js
@@ -37,6 +37,9 @@ var AbstractField = Widget.extend({
     events: {
         'keydown': '_onKeydown',
     },
+    custom_events: {
+        navigation_move: '_onNavigationMove',
+    },
     /**
      * If this flag is set to true, the field widget will be reset on every
      * change which is made in the view (if the view supports it). This is
@@ -374,21 +377,60 @@ var AbstractField = Widget.extend({
     //--------------------------------------------------------------------------
 
     /**
-     * might be controversial: intercept the tab key, to allow the editable list
-     * view to control where the focus is.
+     * Intercepts navigation keyboard events to prevent their default behavior
+     * and notifies the view so that it can handle it its own way.
+     *
+     * Note: the navigation keyboard events are stopped so that potential parent
+     * abstract field does not trigger the navigation_move event a second time.
+     * However, this might be controversial, we might wanna let the event
+     * continue its propagation and flag it to say that navigation has already
+     * been handled (TODO ?).
      *
      * @private
-     * @param {KeyEvent} event
+     * @param {KeyEvent} ev
      */
-    _onKeydown: function (event) {
-        if (event.which === $.ui.keyCode.TAB) {
-            // the event needs to be stopped, to prevent other field widgets
-            // to retrigger a move event
-            event.stopPropagation();
-            this.trigger_up(event.shiftKey ? 'move_previous' : 'move_next');
-            event.preventDefault();
+    _onKeydown: function (ev) {
+        switch (ev.which) {
+            case $.ui.keyCode.TAB:
+                ev.preventDefault();
+                ev.stopPropagation();
+                this.trigger_up('navigation_move', {
+                    direction: ev.shiftKey ? 'previous' : 'next',
+                });
+                break;
+            case $.ui.keyCode.ENTER:
+                ev.stopPropagation();
+                this.trigger_up('navigation_move', {direction: 'next_line'});
+                break;
+            case $.ui.keyCode.UP:
+                ev.stopPropagation();
+                this.trigger_up('navigation_move', {direction: 'up'});
+                break;
+            case $.ui.keyCode.RIGHT:
+                ev.stopPropagation();
+                this.trigger_up('navigation_move', {direction: 'right'});
+                break;
+            case $.ui.keyCode.DOWN:
+                ev.stopPropagation();
+                this.trigger_up('navigation_move', {direction: 'down'});
+                break;
+            case $.ui.keyCode.LEFT:
+                ev.stopPropagation();
+                this.trigger_up('navigation_move', {direction: 'left'});
+                break;
         }
     },
+    /**
+     * Updates the target data value with the current AbstractField instance.
+     * This allows to consider the parent field in case of nested fields. The
+     * field which triggered the event is still accessible through ev.target.
+     *
+     * @private
+     * @param {OdooEvent} ev
+     */
+    _onNavigationMove: function (ev) {
+        ev.data.target = this;
+    },
 });
 
 return AbstractField;
diff --git a/addons/web/static/src/js/fields/basic_fields.js b/addons/web/static/src/js/fields/basic_fields.js
index fc4af105865..9df9a49fac2 100644
--- a/addons/web/static/src/js/fields/basic_fields.js
+++ b/addons/web/static/src/js/fields/basic_fields.js
@@ -277,40 +277,20 @@ var InputField = DebouncedField.extend({
         this._doDebouncedAction();
     },
     /**
-     * Implement keyboard movements.  Mostly useful for its environment, such
-     * as a list view.
+     * Stops the left/right navigation move event if the cursor is not at the
+     * start/end of the input element.
      *
-     * @override
      * @private
-     * @param {any} event
+     * @param {OdooEvent} ev
      */
-    _onKeydown: function (event) {
+    _onNavigationMove: function (ev) {
+        this._super.apply(this, arguments);
         var input = this.$input[0];
-        var is_not_selecting;
-        switch (event.which) {
-            case $.ui.keyCode.DOWN:
-                this.trigger_up('move_down');
-                break;
-            case $.ui.keyCode.UP:
-                this.trigger_up('move_up');
-                break;
-            case $.ui.keyCode.LEFT:
-                is_not_selecting = input.selectionEnd === input.selectionStart;
-                if (is_not_selecting && input.selectionStart === 0) {
-                    this.trigger_up('move_left');
-                }
-                break;
-            case $.ui.keyCode.RIGHT:
-                is_not_selecting = input.selectionEnd === input.selectionStart;
-                if (is_not_selecting && input.selectionEnd === input.value.length) {
-                    this.trigger_up('move_right');
-                }
-                break;
-            case $.ui.keyCode.ENTER:
-                this.trigger_up('move_next_line');
-                break;
+        var selecting = (input.selectionEnd !== input.selectionStart);
+        if ((ev.data.direction === "left" && (selecting || input.selectionStart !== 0))
+         || (ev.data.direction === "right" && (selecting || input.selectionStart !== input.value.length))) {
+            ev.stopPropagation();
         }
-        this._super.apply(this, arguments);
     },
 });
 
@@ -660,28 +640,16 @@ var FieldBoolean = AbstractField.extend({
      *
      * @override
      * @private
-     * @param {KeyEvent} event
+     * @param {KeyEvent} ev
      */
-    _onKeydown: function (event) {
-        this._super.apply(this, arguments);
-        switch (event.which) {
-            case $.ui.keyCode.DOWN:
-                this.trigger_up('move_down');
-                break;
-            case $.ui.keyCode.UP:
-                this.trigger_up('move_up');
-                break;
-            case $.ui.keyCode.LEFT:
-                this.trigger_up('move_left');
-                break;
-            case $.ui.keyCode.RIGHT:
-                this.trigger_up('move_right');
-                break;
+    _onKeydown: function (ev) {
+        switch (ev.which) {
             case $.ui.keyCode.ENTER:
                 this.$input.prop('checked', !this.value);
                 this._setValue(!this.value);
-                break;
+                return;
         }
+        this._super.apply(this, arguments);
     },
 });
 
@@ -1904,10 +1872,10 @@ var FieldDomain = AbstractField.extend({
         "click .o_domain_show_selection_button": "_onShowSelectionButtonClick",
         "click .o_field_domain_dialog_button": "_onDialogEditButtonClick",
     }),
-    custom_events: {
+    custom_events: _.extend({}, AbstractField.prototype.custom_events, {
         "domain_changed": "_onDomainSelectorValueChange",
         "domain_selected": "_onDomainSelectorDialogValueChange",
-    },
+    }),
     /**
      * @constructor
      * @override init from AbstractField
diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js
index db2db031a15..135d21e5adf 100644
--- a/addons/web/static/src/js/fields/relational_fields.js
+++ b/addons/web/static/src/js/fields/relational_fields.js
@@ -14,6 +14,7 @@ odoo.define('web.relational_fields', function (require) {
  */
 
 var AbstractField = require('web.AbstractField');
+var basicFields = require('web.basic_fields');
 var concurrency = require('web.concurrency');
 var ControlPanel = require('web.ControlPanel');
 var dialogs = require('web.view_dialogs');
@@ -76,10 +77,10 @@ var M2ODialog = Dialog.extend({
 var FieldMany2One = AbstractField.extend({
     supportedFieldTypes: ['many2one'],
     template: 'FieldMany2One',
-    custom_events: {
+    custom_events: _.extend({}, AbstractField.prototype.custom_events, {
         'quick_create': '_onQuickCreate',
         'search_create_popup': '_onSearchCreatePopup',
-    },
+    }),
     events: _.extend({}, AbstractField.prototype.events, {
         'click input': '_onInputClick',
         'focusout input': '_onInputFocusout',
@@ -461,6 +462,19 @@ var FieldMany2One = AbstractField.extend({
         }
     },
     /**
+     * Stops the left/right navigation move event if the cursor is not at the
+     * start/end of the input element. Stops any navigation move event if the
+     * user is selecting text.
+     *
+     * TODO this is a duplicate of InputField._onNavigationMove, maybe this
+     * should be done in a mixin or, better, the m2o field should be an
+     * InputField (but this requires some refactoring).
+     *
+     * @private
+     * @param {OdooEvent} ev
+     */
+    _onNavigationMove: basicFields.InputField.prototype._onNavigationMove,
+    /**
      * @private
      * @param {OdooEvent} event
      */
@@ -1215,13 +1229,11 @@ var FieldMany2ManyTags = AbstractField.extend({
     tag_template: "FieldMany2ManyTag",
     className: "o_field_many2manytags",
     supportedFieldTypes: ['many2many'],
-    custom_events: {
+    custom_events: _.extend({}, AbstractField.prototype.custom_events, {
         field_changed: '_onFieldChanged',
-        move_next: '_onMoveNext',
-    },
+    }),
     events: _.extend({}, AbstractField.prototype.events, {
         'click .o_delete': '_onDeleteTag',
-        'keydown .o_field_many2one input': '_onKeyDown',
     }),
     fieldsToFetch: {
         color: {type: 'integer'},
@@ -1365,42 +1377,35 @@ var FieldMany2ManyTags = AbstractField.extend({
         this._removeTag($(event.target).parent().data('id'));
     },
     /**
+     * Controls the changes made in the internal m2o field.
+     *
      * @private
-     * @param {OdooEvent} event
+     * @param {OdooEvent} ev
      */
-    _onFieldChanged: function (event) {
-        if (event.target === this) {
+    _onFieldChanged: function (ev) {
+        if (ev.target !== this.many2one) {
             return;
         }
-        event.stopPropagation();
-        var new_value = event.data.changes[this.name];
-        if (new_value) {
-            this._addTag(new_value);
+        ev.stopPropagation();
+        var newValue = ev.data.changes[this.name];
+        if (newValue) {
+            this._addTag(newValue);
             this.many2one.reinitialize(false);
         }
     },
     /**
      * @private
-     * @param {KeyboardEvent} event
+     * @param {KeyboardEvent} ev
      */
-    _onKeyDown: function (event) {
-        if ($(event.target).val() === "" && event.which === $.ui.keyCode.BACKSPACE) {
+    _onKeydown: function (ev) {
+        if (ev.which === $.ui.keyCode.BACKSPACE && this.$('input').val() === "") {
             var $badges = this.$('.badge');
             if ($badges.length) {
                 this._removeTag($badges.last().data('id'));
+                return;
             }
         }
-    },
-    /**
-     * @private
-     * @param {OdooEvent} event
-     */
-    _onMoveNext: function (event) {
-        if (event.target === this) {
-            return;
-        }
-        // Intercept event triggered up by the many2one, to prevent triggering it twice
-        event.stopPropagation();
+        this._super.apply(this, arguments);
     },
     /**
      * @private
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 65f444ad7b3..5babc71ab6b 100644
--- a/addons/web/static/src/js/views/basic/basic_renderer.js
+++ b/addons/web/static/src/js/views/basic/basic_renderer.js
@@ -16,12 +16,7 @@ var qweb = core.qweb;
 
 var BasicRenderer = AbstractRenderer.extend({
     custom_events: {
-        /**
-         * All basic renderers should be able to handle widget navigation
-         * through the TAB key.
-         */
-        move_next: '_onMoveNext',
-        move_previous: '_onMovePrevious',
+        navigation_move: '_onNavigationMove',
     },
     /**
      * Basic renderers implements the concept of "mode", they can either be in
@@ -600,24 +595,15 @@ var BasicRenderer = AbstractRenderer.extend({
     //--------------------------------------------------------------------------
 
     /**
-     * When someone presses TAB in a widget, it is nice to be able to go to the
-     * next widget, especially since the TAB key event is preventDefaulted
+     * When someone presses the TAB/UP/DOWN/... key in a widget, it is nice to
+     * be able to navigate in the view (default browser behaviors are disabled
+     * by Odoo).
      *
      * @abstract
      * @private
      * @param {OdooEvent} ev
      */
-    _onMoveNext: function (ev) {},
-    /**
-     * When someone presses SHIFT+TAB in a widget, it is nice to be able to go
-     * back to the previous widget, especially since the TAB key event is
-     * preventDefaulted
-     *
-     * @abstract
-     * @private
-     * @param {OdooEvent} ev
-     */
-    _onMovePrevious: function (ev) {},
+    _onNavigationMove: function (ev) {},
 });
 
 return BasicRenderer;
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 f6bcbd49eeb..e9d84ce1f5f 100644
--- a/addons/web/static/src/js/views/form/form_renderer.js
+++ b/addons/web/static/src/js/views/form/form_renderer.js
@@ -740,20 +740,16 @@ var FormRenderer = BasicRenderer.extend({
      * @private
      * @param {OdooEvent} ev
      */
-    _onMoveNext: function (ev) {
+    _onNavigationMove: function (ev) {
         ev.stopPropagation();
-        var index = this.allFieldWidgets[this.state.id].indexOf(ev.target);
-        this._activateNextFieldWidget(this.state, index);
-    },
-    /**
-     * @override
-     * @private
-     * @param {OdooEvent} event
-     */
-    _onMovePrevious: function (ev) {
-        ev.stopPropagation();
-        var index = this.allFieldWidgets[this.state.id].indexOf(ev.target);
-        this._activatePreviousFieldWidget(this.state, index);
+        var index;
+        if (ev.data.direction === "next") {
+            index = this.allFieldWidgets[this.state.id].indexOf(ev.data.target);
+            this._activateNextFieldWidget(this.state, index);
+        } else if (ev.data.direction === "previous") {
+            index = this.allFieldWidgets[this.state.id].indexOf(ev.data.target);
+            this._activatePreviousFieldWidget(this.state, index);
+        }
     },
 });
 
diff --git a/addons/web/static/src/js/views/list/list_editable_renderer.js b/addons/web/static/src/js/views/list/list_editable_renderer.js
index 1b662fa46af..901e178eba6 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
@@ -18,11 +18,7 @@ var _t = core._t;
 
 ListRenderer.include({
     custom_events: _.extend({}, ListRenderer.prototype.custom_events, {
-        move_down: '_onMoveDown',
-        move_up: '_onMoveUp',
-        move_left: '_onMoveLeft',
-        move_right: '_onMoveRight',
-        move_next_line: '_onMoveNextLine',
+        navigation_move: '_onNavigationMove',
     }),
     events: _.extend({}, ListRenderer.prototype.events, {
         'click tbody td.o_data_cell': '_onCellClick',
@@ -214,7 +210,20 @@ ListRenderer.include({
         return n;
     },
     /**
-     * Move the cursor one the beginning of the next line, if possible.
+     * Move the cursor on the end of the previous line, if possible.
+     * If there is no previous line, then we create a new record.
+     *
+     * @private
+     */
+    _moveToPreviousLine: function () {
+        if (this.currentRow > 0) {
+            this._selectCell(this.currentRow - 1, this.columns.length - 1);
+        } else {
+            this._unselectRow().then(this.trigger_up.bind(this, 'add_record'));
+        }
+    },
+    /**
+     * Move the cursor on the beginning of the next line, if possible.
      * If there is no next line, then we create a new record.
      *
      * @private
@@ -223,9 +232,7 @@ ListRenderer.include({
         if (this.currentRow < this.state.data.length - 1) {
             this._selectCell(this.currentRow + 1, 0);
         } else {
-            this._unselectRow().then(
-                this.trigger_up.bind(this, 'add_record')
-            );
+            this._unselectRow().then(this.trigger_up.bind(this, 'add_record'));
         }
     },
     /**
@@ -495,78 +502,68 @@ ListRenderer.include({
         this._unselectRow();
     },
     /**
-     * Move the cursor on the cell just down the current cell.
-     */
-    _onMoveDown: function () {
-        if (this.currentRow < this.state.data.length - 1) {
-            this._selectCell(this.currentRow + 1, this.currentCol);
-        }
-    },
-    /**
-     * Move the cursor on the left, if possible
-     */
-    _onMoveLeft: function () {
-        if (this.currentCol > 0) {
-            this._selectCell(this.currentRow, this.currentCol - 1);
-        }
-    },
-    /**
-     * Move the cursor on the next available cell, so either the next available
-     * cell on the right, or the first on the next line.  This method is called
-     * when the user press the TAB key.
+     * Handles the keyboard navigation according to events triggered by field
+     * widgets.
+     * - up/down: move to the cell above/below if any, or the first activable
+     *          one on the row above/below if any on the right of this cell
+     *          above/below (if none on the right, wrap to the beginning of the
+     *          line).
+     * - left/right: move to the first activable cell on the left/right if any
+     *          (wrap to the end/beginning of the line if necessary).
+     * - previous: move to the first activable cell on the left if any, if not
+     *          move to the rightmost activable cell on the row above.
+     * - next: move to the first activable cell on the right if any, if not move
+     *          to the leftmost activable cell on the row below.
+     * - next_line: move to leftmost activable cell on the row below.
      *
-     * @param {OdooEvent} ev
-     */
-    _onMoveNext: function (ev) {
-        // we need to stop the event, to prevent interference with some other
-        // component up there, such as a form renderer.
-        ev.stopPropagation();
-        if (this.currentCol + 1 < this.columns.length) {
-            this._selectCell(this.currentRow, this.currentCol + 1, false)
-                .fail(this._moveToNextLine.bind(this));
-        } else {
-            this._moveToNextLine();
-        }
-    },
-    /**
-     * @private
-     * @param {OdooEvent} event
-     */
-    _onMoveNextLine: function (event) {
-        event.stopPropagation();
-        this._moveToNextLine();
-    },
-    /**
-     * Move the cursor on the previous available cell, so either the previous
-     * available cell on the left, or the last on the previous line. This method
-     * is called when the user press the TAB key while holding the shift key.
+     * Note: moving to a line below if on the last line or moving to a line
+     * above if on the first line automatically creates a new line.
      *
+     * @private
      * @param {OdooEvent} ev
      */
-    _onMovePrevious: function (ev) {
-        // we need to stop the event, to prevent interference with some other
-        // component up there, such as a form renderer.
-        ev.stopPropagation();
-        if (this.currentCol > 0) {
-            this._selectCell(this.currentRow, this.currentCol - 1);
-        } else if (this.currentRow > 0) {
-            this._selectCell(this.currentRow - 1, this.columns.length - 1);
-        }
-    },
-    /**
-     * Move the cursor one cell right, if possible
-     */
-    _onMoveRight: function () {
-        if (this.currentCol + 1 < this.columns.length) {
-            this._selectCell(this.currentRow, this.currentCol + 1);
-        }
-    },
-    /**
-     * Move the cursor one line up
-     */
-    _onMoveUp: function () {
-        if (this.currentRow > 0) {
-            this._selectCell(this.currentRow - 1, this.currentCol);
+    _onNavigationMove: function (ev) {
+        ev.stopPropagation(); // stop the event, the action is done by this renderer
+        switch (ev.data.direction) {
+            case 'up':
+                if (this.currentRow > 0) {
+                    this._selectCell(this.currentRow - 1, this.currentCol);
+                }
+                break;
+            case 'right':
+                if (this.currentCol + 1 < this.columns.length) {
+                    this._selectCell(this.currentRow, this.currentCol + 1);
+                }
+                break;
+            case 'down':
+                if (this.currentRow < this.state.data.length - 1) {
+                    this._selectCell(this.currentRow + 1, this.currentCol);
+                }
+                break;
+            case 'left':
+                if (this.currentCol > 0) {
+                    this._selectCell(this.currentRow, this.currentCol - 1);
+                }
+                break;
+            case 'previous':
+                if (this.currentCol > 0) {
+                    this._selectCell(this.currentRow, this.currentCol - 1, false)
+                        .fail(this._moveToPreviousLine.bind(this));
+                } else {
+                    this._moveToPreviousLine();
+                }
+                break;
+            case 'next':
+                if (this.currentCol + 1 < this.columns.length) {
+                    this._selectCell(this.currentRow, this.currentCol + 1, false)
+                        .fail(this._moveToNextLine.bind(this));
+                } else {
+                    this._moveToNextLine();
+                }
+                break;
+            case 'next_line':
+                this._moveToNextLine();
+                break;
         }
     },
     /**