From 263f81f65da2c647c6fd901e6b7f88cee658e822 Mon Sep 17 00:00:00 2001 From: Vincent Schippefilt Date: Thu, 12 Apr 2018 16:49:59 +0200 Subject: [PATCH] [FIX] web: allow default behavior for unmanaged keyboard navigation Reproduce the issue: - open the manual reconciliation (it is a client action) - add a reconciliation (using the + sign) - expand it (using the > sign) - put the focus on the first field and hit TAB --> you would expect that the focus moves to the second field --> what happens is: the focus stays on the first field Before this fix, when in a client action (or any place that contains fields) that contains fields, the keyboard navigation did not occur correctly because the default behavior of the TAB key was prevented. After this fix, if the navigation_move event was not stopped, we allow the default behavior of the TAB key. --- addons/web/static/src/js/core/mixins.js | 1 + .../static/src/js/fields/abstract_field.js | 8 +++--- .../tests/chrome/action_manager_tests.js | 27 ++++++++++++++----- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/addons/web/static/src/js/core/mixins.js b/addons/web/static/src/js/core/mixins.js index fd8e8f93dbe..e29c97ea086 100644 --- a/addons/web/static/src/js/core/mixins.js +++ b/addons/web/static/src/js/core/mixins.js @@ -333,6 +333,7 @@ var EventDispatcherMixin = _.extend({}, ParentedMixin, { var event = new OdooEvent(this, name, info); //console.info('event: ', name, info); this._trigger_up(event); + return event; }, _trigger_up: function (event) { var parent; diff --git a/addons/web/static/src/js/fields/abstract_field.js b/addons/web/static/src/js/fields/abstract_field.js index b3694afb652..735e12a3861 100644 --- a/addons/web/static/src/js/fields/abstract_field.js +++ b/addons/web/static/src/js/fields/abstract_field.js @@ -487,11 +487,13 @@ var AbstractField = Widget.extend({ _onKeydown: function (ev) { switch (ev.which) { case $.ui.keyCode.TAB: - ev.preventDefault(); - ev.stopPropagation(); - this.trigger_up('navigation_move', { + var event = this.trigger_up('navigation_move', { direction: ev.shiftKey ? 'previous' : 'next', }); + if (event.is_stopped()) { + ev.preventDefault(); + ev.stopPropagation(); + } break; case $.ui.keyCode.ENTER: ev.stopPropagation(); diff --git a/addons/web/static/tests/chrome/action_manager_tests.js b/addons/web/static/tests/chrome/action_manager_tests.js index b640add63c1..c7117e7a4b7 100644 --- a/addons/web/static/tests/chrome/action_manager_tests.js +++ b/addons/web/static/tests/chrome/action_manager_tests.js @@ -3114,18 +3114,19 @@ QUnit.module('ActionManager', { delete core.action_registry.ClientAction; }); - QUnit.test('fields in abstract action do not crash on navigation_moves', function (assert) { + QUnit.test('fields in abstract action does not crash on navigation_moves', function (assert) { assert.expect(1); var self = this; - // create a client action with 1 input field + // create a client action with 2 input field var inputWidget; - var ClientAction = AbstractAction.extend(StandaloneFieldManagerMixin,{ - init: function() { + var secondInputWidget; + var ClientAction = AbstractAction.extend(StandaloneFieldManagerMixin, { + init: function () { this._super.apply(this, arguments); StandaloneFieldManagerMixin.init.call(this); }, - start: function(){ + start: function () { var _self = this; return this.model.makeRecord('partner', [{ @@ -3135,7 +3136,13 @@ QUnit.module('ActionManager', { var record = _self.model.get(recordID); inputWidget = new BasicFields.InputField(_self, 'display_name', record, {mode: 'edit',}); _self._registerWidget(recordID, 'display_name', inputWidget); + + secondInputWidget = new BasicFields.InputField(_self, 'display_name', record, {mode: 'edit',}); + secondInputWidget.attrs = {className:"secondField"}; + _self._registerWidget(recordID, 'display_name', secondInputWidget); + inputWidget.appendTo(_self.$el); + secondInputWidget.appendTo(_self.$el); }); } }); @@ -3146,9 +3153,15 @@ QUnit.module('ActionManager', { data: this.data, }); actionManager.doAction('ClientAction'); - inputWidget.$el.trigger($.Event('keydown', {which: $.ui.keyCode.TAB})); + inputWidget.$el[0].focus(); + var event = $.Event('keydown', { + which: $.ui.keyCode.TAB, + keyCode: $.ui.keyCode.TAB, + }); + $(inputWidget.$el[0]).trigger(event); - assert.ok(true); // no error so it's good + assert.notOk(event.isDefaultPrevented(), + "the keyboard event default should not be prevented"); // no crash is good actionManager.destroy(); delete core.action_registry.ClientAction; });