From c5f8f35747b4ca9633c664d6be74614c5ce22fcd Mon Sep 17 00:00:00 2001 From: Martin Geubelle Date: Mon, 19 Jun 2017 15:17:05 +0200 Subject: [PATCH] [FIX] web: properly handle tab navigation in readonly The tab navigation was only working in edit mode. Entering a readonly form view with input fields was causing a traceback. As the default browser behaviour has been overriden with custom navigation, we need to specify which widget can be focused (e.g.: url, many2one, etc.). --- .../web/static/src/js/fields/basic_fields.js | 51 +++++++++++++++++-- .../static/src/js/fields/relational_fields.js | 2 +- .../static/src/js/views/form/form_renderer.js | 1 + addons/web/static/tests/views/form_tests.js | 41 +++++++++++++++ 4 files changed, 89 insertions(+), 6 deletions(-) diff --git a/addons/web/static/src/js/fields/basic_fields.js b/addons/web/static/src/js/fields/basic_fields.js index ef1832e00d6..79654cc3095 100644 --- a/addons/web/static/src/js/fields/basic_fields.js +++ b/addons/web/static/src/js/fields/basic_fields.js @@ -285,11 +285,15 @@ var InputField = DebouncedField.extend({ */ _onNavigationMove: function (ev) { this._super.apply(this, arguments); - var input = this.$input[0]; - 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(); + + // the following code only makes sense in edit mode, with an input + if (this.mode === 'edit') { + var input = this.$input[0]; + 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(); + } } }, }); @@ -814,6 +818,19 @@ var FieldEmail = InputField.extend({ this.tagName = this.mode === 'readonly' ? 'a' : 'input'; }, + //-------------------------------------------------------------------------- + // Public + //-------------------------------------------------------------------------- + + /** + * Returns the associated link. + * + * @override + */ + getFocusableElement: function () { + return this.mode === 'readonly' && this.$el || $(); + }, + //-------------------------------------------------------------------------- // Private //-------------------------------------------------------------------------- @@ -849,6 +866,17 @@ var FieldPhone = FieldEmail.extend({ this.tagName = 'span'; } }, + /** + * Returns the associated link only if there is one. + * + * @override + */ + getFocusableElement: function () { + if (this._canCall()) { + return this._super.apply(this, arguments); + } + return $(); + }, //-------------------------------------------------------------------------- // Private @@ -902,6 +930,19 @@ var UrlWidget = InputField.extend({ this.tagName = this.mode === 'readonly' ? 'a' : 'input'; }, + //-------------------------------------------------------------------------- + // Public + //-------------------------------------------------------------------------- + + /** + * Returns the associated link. + * + * @override + */ + getFocusableElement: function () { + return this.mode === 'readonly' && this.$el || $(); + }, + //-------------------------------------------------------------------------- // Private //-------------------------------------------------------------------------- diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js index 95bef29f88e..35addc4a091 100644 --- a/addons/web/static/src/js/fields/relational_fields.js +++ b/addons/web/static/src/js/fields/relational_fields.js @@ -124,7 +124,7 @@ var FieldMany2One = AbstractField.extend({ * @returns {jQuery} */ getFocusableElement: function () { - return this.$input || $(); + return this.mode === 'edit' && this.$input || this.$el; }, /** * TODO 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 08f28438368..25b72e4fb5a 100644 --- a/addons/web/static/src/js/views/form/form_renderer.js +++ b/addons/web/static/src/js/views/form/form_renderer.js @@ -828,6 +828,7 @@ var FormRenderer = BasicRenderer.extend({ */ _onNavigationMove: function (ev) { ev.stopPropagation(); + var index; if (ev.data.direction === "next") { index = this.allFieldWidgets[this.state.id].indexOf(ev.data.target); diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js index 411b4ffa3d9..c898618e315 100644 --- a/addons/web/static/tests/views/form_tests.js +++ b/addons/web/static/tests/views/form_tests.js @@ -3278,6 +3278,47 @@ QUnit.module('Views', { form.destroy(); }); + QUnit.test('navigation with tab key in readonly form view', function (assert) { + assert.expect(3); + + this.data.partner.records[1].product_id = 37; + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
', + res_id: 2, + }); + + // focus first field, trigger tab + form.$('[name="trululu"]').focus(); + form.$('[name="trululu"]').trigger($.Event('keydown', {which: $.ui.keyCode.TAB})); + assert.strictEqual(form.$('[name="product_id"]')[0], document.activeElement, + "product_id should be focused"); + form.$('[name="product_id"]').trigger($.Event('keydown', {which: $.ui.keyCode.TAB})); + assert.strictEqual(form.$('[name="display_name"]')[0], document.activeElement, + "display_name should be focused (emails are focusable but phone aren't)"); + + // simulate shift+tab on active element + $(document.activeElement).trigger($.Event('keydown', {which: $.ui.keyCode.TAB, shiftKey: true})); + $(document.activeElement).trigger($.Event('keydown', {which: $.ui.keyCode.TAB, shiftKey: true})); + assert.strictEqual(document.activeElement, form.$('[name="trululu"]')[0], + "first many2one should be focused"); + + form.destroy(); + }); + QUnit.test('skip invisible fields when navigating with TAB', function (assert) { assert.expect(1);