Commit 109279fe authored by Tim Sergeant's avatar Tim Sergeant Committed by Commit Bot

MD Bookmarks: Disable keyboard shortcuts when a dialog is open

Even though our dialogs are modal, it was still possible to execute
keyboard commands on the background content by clicking on the dialog
background, which refocuses the document body and allowed commands
to execute.

Bug: 742972
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I01f9cb3060fd6a91a6fa5a832578f3f245712e0c
Reviewed-on: https://chromium-review.googlesource.com/575252Reviewed-by: default avatarcalamity <calamity@chromium.org>
Commit-Queue: Tim Sergeant <tsergeant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488116}
parent 1e4ec58f
...@@ -648,8 +648,10 @@ cr.define('bookmarks', function() { ...@@ -648,8 +648,10 @@ cr.define('bookmarks', function() {
*/ */
onKeydown_: function(e) { onKeydown_: function(e) {
var selection = this.getState().selection.items; var selection = this.getState().selection.items;
if (e.target == document.body) if (e.target == document.body &&
!bookmarks.DialogFocusManager.getInstance().hasOpenDialog()) {
this.handleKeyEvent(e, selection); this.handleKeyEvent(e, selection);
}
}, },
/** /**
......
...@@ -44,6 +44,13 @@ cr.define('bookmarks', function() { ...@@ -44,6 +44,13 @@ cr.define('bookmarks', function() {
showFn(); showFn();
}, },
/**
* @return {boolean} True if the document currently has an open dialog.
*/
hasOpenDialog: function() {
return this.dialogs_.size > 0;
},
/** @private */ /** @private */
updatePreviousFocus_: function() { updatePreviousFocus_: function() {
this.previousFocusElement_ = this.getFocusedElement_(); this.previousFocusElement_ = this.getFocusedElement_();
...@@ -75,7 +82,7 @@ cr.define('bookmarks', function() { ...@@ -75,7 +82,7 @@ cr.define('bookmarks', function() {
assert(this.dialogs_.delete(dialog)); assert(this.dialogs_.delete(dialog));
// Focus the originally focused element if there are no more dialogs. // Focus the originally focused element if there are no more dialogs.
if (!this.dialogs_.size) if (!this.hasOpenDialog())
this.previousFocusElement_.focus(); this.previousFocusElement_.focus();
dialog.removeEventListener('close', closeListener); dialog.removeEventListener('close', closeListener);
......
...@@ -58,6 +58,7 @@ suite('<bookmarks-command-manager>', function() { ...@@ -58,6 +58,7 @@ suite('<bookmarks-command-manager>', function() {
replaceBody(commandManager); replaceBody(commandManager);
document.body.appendChild( document.body.appendChild(
document.createElement('bookmarks-toast-manager')); document.createElement('bookmarks-toast-manager'));
bookmarks.DialogFocusManager.instance_ = null;
}); });
test('Copy URL is only active for single URL items', function() { test('Copy URL is only active for single URL items', function() {
...@@ -164,7 +165,7 @@ suite('<bookmarks-command-manager>', function() { ...@@ -164,7 +165,7 @@ suite('<bookmarks-command-manager>', function() {
commandManager.assertLastCommand('redo'); commandManager.assertLastCommand('redo');
}); });
test.only('Show In Folder is only available during search', function() { test('Show In Folder is only available during search', function() {
store.data.selection.items = new Set(['12']); store.data.selection.items = new Set(['12']);
store.notifyObservers(); store.notifyObservers();
...@@ -344,6 +345,21 @@ suite('<bookmarks-command-manager>', function() { ...@@ -344,6 +345,21 @@ suite('<bookmarks-command-manager>', function() {
MockInteractions.tap(commandItem[Command.EDIT]); MockInteractions.tap(commandItem[Command.EDIT]);
commandManager.assertLastCommand(null); commandManager.assertLastCommand(null);
}); });
test('keyboard shortcuts are disabled while a dialog is open', function() {
assertFalse(bookmarks.DialogFocusManager.getInstance().hasOpenDialog());
items = new Set(['12']);
store.data.selection.items = items;
store.notifyObservers();
var editKey = cr.isMac ? 'Enter' : 'F2';
MockInteractions.pressAndReleaseKeyOn(document.body, '', '', editKey);
commandManager.assertLastCommand(Command.EDIT);
assertTrue(bookmarks.DialogFocusManager.getInstance().hasOpenDialog());
MockInteractions.pressAndReleaseKeyOn(document.body, '', '', 'Delete');
commandManager.assertLastCommand(null);
});
}); });
suite('<bookmarks-item> CommandManager integration', function() { suite('<bookmarks-item> CommandManager integration', function() {
......
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