From 2c777546a790340af5ec8e2dc5a3311370ff4bf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Wed, 3 Mar 2021 14:53:54 +0000 Subject: [PATCH] [FIX] web: prevent traceback when pressing TAB in color_picker widget The color_picker widget, that can be seen for example in the track form view for event tracks, had an issue: if the user clicked on it, then pressed TAB, a traceback was displayed. The problem comes from the fact that the color picker widget inherits from FieldInput, but is not a fieldinput, so many expectations made by the FieldInput code do not hold, such as the code run when handling navigation (by TAB and such keypress). Because of that, the code in _onNavigationMove crashed, because it expected an input. Since this is a bug fix, I simply disabled the navigation in that case, so no crash happens. Sadly, this widget has still a big issue: it clearly does not work as most users would expect: pressing TAB or arrows should update the selection. But this would be a more complicated refactoring, for a bug which is clearly not critical, therefore this commit implements the simple and safe solution. Also, we disable the focus outline to minimize the wrong expectation. Seeing them kind of implied that one could update the selection with the keyboard. Note that the widget color_picker was moved from another addon to web/, without any tests nor documentation. OPW 2467369 closes odoo/odoo#67195 X-original-commit: 580456b6ab36d16195eae9ffd776ff245ecd936e Signed-off-by: Aaron Bohy (aab) --- .../web/static/src/js/fields/basic_fields.js | 3 ++ addons/web/static/src/scss/color_picker.scss | 3 ++ .../static/tests/fields/basic_fields_tests.js | 32 +++++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/addons/web/static/src/js/fields/basic_fields.js b/addons/web/static/src/js/fields/basic_fields.js index f5dda59d27e..c370bf6aede 100644 --- a/addons/web/static/src/js/fields/basic_fields.js +++ b/addons/web/static/src/js/fields/basic_fields.js @@ -3770,6 +3770,9 @@ var FieldColorPicker = FieldInteger.extend({ } }, + _onNavigationMove() { + // disable navigation from FieldInput, to prevent a crash + } }); return { diff --git a/addons/web/static/src/scss/color_picker.scss b/addons/web/static/src/scss/color_picker.scss index 304563f7d90..18eb7aea841 100644 --- a/addons/web/static/src/scss/color_picker.scss +++ b/addons/web/static/src/scss/color_picker.scss @@ -15,6 +15,9 @@ > li { border: 2px solid white; box-shadow: 0 0 0 1px gray('300'); + > a:focus { + outline: none; + } } } } diff --git a/addons/web/static/tests/fields/basic_fields_tests.js b/addons/web/static/tests/fields/basic_fields_tests.js index c7024e67830..7d44b6e3af1 100644 --- a/addons/web/static/tests/fields/basic_fields_tests.js +++ b/addons/web/static/tests/fields/basic_fields_tests.js @@ -7824,6 +7824,38 @@ QUnit.module('basic_fields', { form.destroy(); }); + QUnit.module('FieldColorPicker'); + + QUnit.test('FieldColorPicker: can navigate away with TAB', async function (assert) { + assert.expect(1); + + const form = await createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: ` +
+ + + `, + res_id: 1, + viewOptions: { + mode: 'edit', + }, + }); + + form.$el.find('a.oe_kanban_color_1')[0].focus(); + + form.$el.find('a.oe_kanban_color_1').trigger($.Event('keydown', { + which: $.ui.keyCode.TAB, + keyCode: $.ui.keyCode.TAB, + })); + assert.strictEqual(document.activeElement, form.$el.find('input[name="foo"]')[0], + "foo field should be focused"); + form.destroy(); + }); + + QUnit.module('FieldBadge'); QUnit.test('FieldBadge component on a char field in list view', async function (assert) {