[FIX] web: various fixes in list view navigation

With this commit, we try to improve the way navigation is done in the
list view.  In particular, we want to make sure that pressing TAB on the
last editable cell move the cursor to the next line (and create a new
line if necessary).

Also, we add a few tests for keyboard navigation (it was almost not
tested before this commit, and while working on this commit, it was
broken at some point).

2 files changed, 39 insertions, 20 deletions

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 ed9726478a0..e150b0b2dd0 100644
--- a/addons/web/static/src/js/views/basic/basic_renderer.js
+++ b/addons/web/static/src/js/views/basic/basic_renderer.js
@@ -147,32 +147,45 @@ var BasicRenderer = AbstractRenderer.extend({
     },
     /**
      * Activates the widget at the given index for the given record if possible
-     * or the "next" possible one.
+     * or the "next" possible one. Usually, a widget can be activated if it is
+     * in edit mode, and if it is visible.
      *
      * @private
      * @param {Object} record
      * @param {integer} currentIndex
-     * @param {integer} [inc=1] - the increment to use when searching for the
-     *                          "next" possible one
+     * @param {Object} [options]
+     * @param {integer} [options.inc=1] - the increment to use when searching for the
+     *   "next" possible one
+     * @param {boolean} [options.wrap=true] if true, when we arrive at the end of the
+     *   list of widget, we wrap around and try to activate widgets starting at
+     *   the beginning. Otherwise, we just stop trying and return -1
      * @returns {integer} the index of the widget that was activated or -1 if
-     *                    none was possible to activate
+     *   none was possible to activate
      */
-    _activateFieldWidget: function (record, currentIndex, inc) {
-        inc = inc === undefined ? 1 : inc;
+    _activateFieldWidget: function (record, currentIndex, options) {
+        options = options || {};
+        _.defaults(options, {inc: 1, wrap: true});
 
-        var activated;
         var recordWidgets = this.allFieldWidgets[record.id] || [];
         for (var i = 0 ; i < recordWidgets.length ; i++) {
-            activated = recordWidgets[currentIndex].activate();
+            var activated = recordWidgets[currentIndex].activate();
             if (activated) {
                 return currentIndex;
             }
 
-            currentIndex += inc;
+            currentIndex += options.inc;
             if (currentIndex >= recordWidgets.length) {
-                currentIndex -= recordWidgets.length;
+                if (options.wrap) {
+                    currentIndex -= recordWidgets.length;
+                } else {
+                    return -1;
+                }
             } else if (currentIndex < 0) {
-                currentIndex += recordWidgets.length;
+                if (options.wrap) {
+                    currentIndex += recordWidgets.length;
+                } else {
+                    return -1;
+                }
             }
         }
         return -1;
@@ -188,7 +201,7 @@ var BasicRenderer = AbstractRenderer.extend({
      */
     _activateNextFieldWidget: function (record, currentIndex) {
         currentIndex = (currentIndex + 1) % (this.allFieldWidgets[record.id] || []).length;
-        return this._activateFieldWidget(record, currentIndex, +1);
+        return this._activateFieldWidget(record, currentIndex, {inc: 1});
     },
     /**
      * This is a wrapper of the {@see _activateFieldWidget} function to select
@@ -201,7 +214,7 @@ var BasicRenderer = AbstractRenderer.extend({
      */
     _activatePreviousFieldWidget: function (record, currentIndex) {
         currentIndex = currentIndex ? (currentIndex - 1) : ((this.allFieldWidgets[record.id] || []).length - 1);
-        return this._activateFieldWidget(record, currentIndex, -1);
+        return this._activateFieldWidget(record, currentIndex, {inc:-1});
     },
     /**
      * Does the necessary DOM updates to match the given modifiers data. The
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 12ef168c131..3d34b80f846 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
@@ -335,22 +335,27 @@ ListRenderer.include({
      *
      * @param {integer} rowIndex
      * @param {integer} colIndex
+     * @param {boolean} [wrap=true] if true and no widget could be selected from
+     *   the colIndex to the last column, then we wrap around and try to select
+     *   a widget starting from the beginning
+     * @return {Deferred} fails if no cell could be selected
      */
-    _selectCell: function (rowIndex, colIndex) {
+    _selectCell: function (rowIndex, colIndex, wrap) {
         // Do nothing if the user tries to select current cell
         if (rowIndex === this.currentRow && colIndex === this.currentCol) {
-            return;
+            return $.when();
         }
+        wrap = wrap === undefined ? true : wrap;
 
         // Select the row then activate the widget in the correct cell
         var self = this;
-        this._selectRow(rowIndex).then(function () {
+        return this._selectRow(rowIndex).then(function () {
             var record = self.state.data[rowIndex];
-            var fieldIndex = self._activateFieldWidget(record, colIndex - getNbButtonBefore(colIndex));
+            var correctedIndex = colIndex - getNbButtonBefore(colIndex);
+            var fieldIndex = self._activateFieldWidget(record, correctedIndex, {inc: 1, wrap: wrap});
 
-            // If no widget to activate in the row, unselect the row
             if (fieldIndex < 0) {
-                return self._unselectRow();
+                return $.Deferred().reject();
             }
 
             self.currentCol = fieldIndex + getNbButtonBefore(fieldIndex);
@@ -543,7 +548,8 @@ ListRenderer.include({
         // component up there, such as a form renderer.
         ev.stopPropagation();
         if (this.currentCol + 1 < this.columns.length) {
-            this._selectCell(this.currentRow, this.currentCol + 1);
+            this._selectCell(this.currentRow, this.currentCol + 1, false)
+                .fail(this._moveToNextLine.bind(this));
         } else {
             this._moveToNextLine();
         }