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

3 files changed, 100 insertions, 85 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