Commit f5cc29e5 authored by Demetrios Papadopoulos's avatar Demetrios Papadopoulos Committed by Commit Bot

[Reland] Extensions: Restore focus after clearing keyboard shortcut.

Original CL at r806929 was reverted due to failures on Mac bots. Fixed
by moving the test in question from browser_tests to
interactive_ui_tests. It's a known issue that tests involving DOM
focus don't work on Mac unless they are interactive_ui_tests.

Fixed: 1065659
Change-Id: I8ed0d23a7318c5fa28596d0bd9d0b57f41db536c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2411945Reviewed-by: default avatarJohn Lee <johntlee@chromium.org>
Commit-Queue: dpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807863}
parent ea1ed3ea
...@@ -32,6 +32,6 @@ ...@@ -32,6 +32,6 @@
slot="suffix" class="icon-cancel no-overlap" slot="suffix" class="icon-cancel no-overlap"
invisible$="[[computeClearInvisible_(capturing_, shortcut)]]" invisible$="[[computeClearInvisible_(capturing_, shortcut)]]"
hidden$="[[computeClearHidden_(shortcut)]]" hidden$="[[computeClearHidden_(shortcut)]]"
on-click="onClearTap_"></cr-icon-button> on-click="onClearClick_"></cr-icon-button>
</cr-input> </cr-input>
</div> </div>
...@@ -96,8 +96,7 @@ Polymer({ ...@@ -96,8 +96,7 @@ Polymer({
} }
this.pendingShortcut_ = ''; this.pendingShortcut_ = '';
this.capturing_ = false; this.capturing_ = false;
const input = this.$.input; this.$.input.blur();
input.blur();
this.error_ = ShortcutError.NO_ERROR; this.error_ = ShortcutError.NO_ERROR;
this.delegate.setShortcutHandlingSuspended(false); this.delegate.setShortcutHandlingSuspended(false);
}, },
...@@ -261,11 +260,12 @@ Polymer({ ...@@ -261,11 +260,12 @@ Polymer({
}, },
/** @private */ /** @private */
onClearTap_() { onClearClick_() {
assert(this.shortcut); assert(this.shortcut);
this.pendingShortcut_ = ''; this.pendingShortcut_ = '';
this.commitPending_(); this.commitPending_();
this.endCapture_(); this.endCapture_();
this.$.input.focus();
}, },
}); });
...@@ -53,6 +53,7 @@ if (include_js_tests) { ...@@ -53,6 +53,7 @@ if (include_js_tests) {
"$root_gen_dir/chrome/test/data/webui/cr_elements/cr_toolbar_search_field_tests.m.js", "$root_gen_dir/chrome/test/data/webui/cr_elements/cr_toolbar_search_field_tests.m.js",
"$root_gen_dir/chrome/test/data/webui/cr_elements/iron_list_focus_test.m.js", "$root_gen_dir/chrome/test/data/webui/cr_elements/iron_list_focus_test.m.js",
"$root_gen_dir/chrome/test/data/webui/cr_focus_row_behavior_test.m.js", "$root_gen_dir/chrome/test/data/webui/cr_focus_row_behavior_test.m.js",
"$root_gen_dir/chrome/test/data/webui/fake_chrome_event.m.js",
"$root_gen_dir/chrome/test/data/webui/mock_controller.m.js", "$root_gen_dir/chrome/test/data/webui/mock_controller.m.js",
"$root_gen_dir/chrome/test/data/webui/settings/sync_test_util.m.js", "$root_gen_dir/chrome/test/data/webui/settings/sync_test_util.m.js",
"$root_gen_dir/chrome/test/data/webui/settings/test_sync_browser_proxy.m.js", "$root_gen_dir/chrome/test/data/webui/settings/test_sync_browser_proxy.m.js",
......
...@@ -581,23 +581,6 @@ TEST_F('CrExtensionsShortcutTest', 'ScopeChange', function() { ...@@ -581,23 +581,6 @@ TEST_F('CrExtensionsShortcutTest', 'ScopeChange', function() {
this.runMochaTest(extension_shortcut_tests.TestNames.ScopeChange); this.runMochaTest(extension_shortcut_tests.TestNames.ScopeChange);
}); });
// eslint-disable-next-line no-var
var CrExtensionsShortcutInputTest = class extends CrExtensionsBrowserTest {
/** @override */
get browsePreload() {
return 'chrome://extensions/test_loader.html?module=extensions/shortcut_input_test.js';
}
/** @override */
get suiteName() {
return extension_shortcut_input_tests.suiteName;
}
};
TEST_F('CrExtensionsShortcutInputTest', 'Basic', function() {
this.runMochaTest(extension_shortcut_input_tests.TestNames.Basic);
});
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
// Extension Pack Dialog Tests // Extension Pack Dialog Tests
......
...@@ -29,6 +29,16 @@ const CrExtensionsInteractiveUITest = class extends PolymerInteractiveUITest { ...@@ -29,6 +29,16 @@ const CrExtensionsInteractiveUITest = class extends PolymerInteractiveUITest {
'//chrome/test/data/webui/mocha_adapter.js', '//chrome/test/data/webui/mocha_adapter.js',
]; ];
} }
// The name of the mocha suite. Should be overridden by subclasses.
get suiteName() {
return null;
}
/** @param {string} testName The name of the test to run. */
runMochaTest(testName) {
runMochaTest(this.suiteName, testName);
}
}; };
...@@ -55,3 +65,21 @@ var CrExtensionsOptionsPageTest = class extends CrExtensionsInteractiveUITest { ...@@ -55,3 +65,21 @@ var CrExtensionsOptionsPageTest = class extends CrExtensionsInteractiveUITest {
TEST_F('CrExtensionsOptionsPageTest', 'DISABLED_All', function() { TEST_F('CrExtensionsOptionsPageTest', 'DISABLED_All', function() {
mocha.run(); mocha.run();
}); });
// eslint-disable-next-line no-var
var CrExtensionsShortcutInputTest =
class extends CrExtensionsInteractiveUITest {
/** @override */
get browsePreload() {
return 'chrome://extensions/test_loader.html?module=extensions/shortcut_input_test.js';
}
/** @override */
get suiteName() {
return extension_shortcut_input_tests.suiteName;
}
};
TEST_F('CrExtensionsShortcutInputTest', 'Basic', function() {
this.runMochaTest(extension_shortcut_input_tests.TestNames.Basic);
});
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
import 'chrome://extensions/extensions.js'; import 'chrome://extensions/extensions.js';
import {assert} from 'chrome://resources/js/assert.m.js'; import {assert} from 'chrome://resources/js/assert.m.js';
import {keyDownOn, keyUpOn, tap} from 'chrome://resources/polymer/v3_0/iron-test-helpers/mock-interactions.js'; import {keyDownOn, keyUpOn} from 'chrome://resources/polymer/v3_0/iron-test-helpers/mock-interactions.js';
import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {isChildVisible} from '../test_util.m.js'; import {isChildVisible} from '../test_util.m.js';
...@@ -45,8 +45,8 @@ suite(extension_shortcut_input_tests.suiteName, function() { ...@@ -45,8 +45,8 @@ suite(extension_shortcut_input_tests.suiteName, function() {
const isClearVisible = isChildVisible.bind(null, input, '#clear', false); const isClearVisible = isChildVisible.bind(null, input, '#clear', false);
expectFalse(isClearVisible()); expectFalse(isClearVisible());
// Click the input. Capture should start. // Focus the input. Capture should start.
tap(field); field.focus();
return input.delegate.whenCalled('setShortcutHandlingSuspended') return input.delegate.whenCalled('setShortcutHandlingSuspended')
.then((arg) => { .then((arg) => {
assertTrue(arg); assertTrue(arg);
...@@ -96,16 +96,18 @@ suite(extension_shortcut_input_tests.suiteName, function() { ...@@ -96,16 +96,18 @@ suite(extension_shortcut_input_tests.suiteName, function() {
expectTrue(isClearVisible()); expectTrue(isClearVisible());
// Test clearing the shortcut. // Test clearing the shortcut.
tap(input.$['clear']); input.$['clear'].click();
assertEquals(input.$.input, input.shadowRoot.activeElement);
return input.delegate.whenCalled('updateExtensionCommandKeybinding'); return input.delegate.whenCalled('updateExtensionCommandKeybinding');
}) })
.then((arg) => { .then((arg) => {
field.blur();
input.delegate.reset(); input.delegate.reset();
expectDeepEquals(['itemid', 'Command', ''], arg); expectDeepEquals(['itemid', 'Command', ''], arg);
assertEquals('', input.shortcut); assertEquals('', input.shortcut);
expectFalse(isClearVisible()); expectFalse(isClearVisible());
tap(field); field.focus();
return input.delegate.whenCalled('setShortcutHandlingSuspended'); return input.delegate.whenCalled('setShortcutHandlingSuspended');
}) })
.then((arg) => { .then((arg) => {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment