diff options
author | dbeam <dbeam@chromium.org> | 2015-06-04 12:49:10 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-06-04 19:49:32 +0000 |
commit | 9a5d453e3874add8cf533d20100767369b76d2f8 (patch) | |
tree | b33f506ab13253a4a7dc086ebd27f6349a000e5a | |
parent | bfc4a0dc0750b6d2cefc48921e3701b8a00aa5d8 (diff) | |
download | chromium_src-9a5d453e3874add8cf533d20100767369b76d2f8.zip chromium_src-9a5d453e3874add8cf533d20100767369b76d2f8.tar.gz chromium_src-9a5d453e3874add8cf533d20100767369b76d2f8.tar.bz2 |
Fix some lingering WebUI focus issues.
1) make (x) dialog buttons update whether focus outline is visible
2) don't restore focus when keyboard triggered dialogs are closed by mouse
3) make chrome://extensions overlays act more sanely
4) remove odd code that required activeElement != body to set initial focus
R=dmazzoni@chromium.org
BUG=472031,481502
Review URL: https://codereview.chromium.org/1162293008
Cr-Commit-Position: refs/heads/master@{#332887}
8 files changed, 56 insertions, 23 deletions
diff --git a/chrome/browser/resources/extensions/extension_commands_overlay.js b/chrome/browser/resources/extensions/extension_commands_overlay.js index ab56eb4..9b51f34 100644 --- a/chrome/browser/resources/extensions/extension_commands_overlay.js +++ b/chrome/browser/resources/extensions/extension_commands_overlay.js @@ -34,8 +34,9 @@ cr.define('extensions', function() { this.extensionCommandList_ = new ExtensionCommandList( /**@type {HTMLDivElement} */($('extension-command-list'))); - $('extension-commands-dismiss').addEventListener('click', - this.handleDismiss_.bind(this)); + $('extension-commands-dismiss').addEventListener('click', function() { + cr.dispatchSimpleEvent(overlay, 'cancelOverlay'); + }); // The ExtensionList will update us with its data, so we don't need to // handle that here. diff --git a/chrome/browser/resources/extensions/extension_error_overlay.js b/chrome/browser/resources/extensions/extension_error_overlay.js index 84e32a9..1f7975e 100644 --- a/chrome/browser/resources/extensions/extension_error_overlay.js +++ b/chrome/browser/resources/extensions/extension_error_overlay.js @@ -292,8 +292,10 @@ cr.define('extensions', function() { cr.ui.overlay.globalInitialization(); overlay.addEventListener('cancelOverlay', this.handleDismiss_.bind(this)); - $('extension-error-overlay-dismiss').addEventListener( - 'click', this.handleDismiss_.bind(this)); + $('extension-error-overlay-dismiss').addEventListener('click', + function() { + cr.dispatchSimpleEvent(overlay, 'cancelOverlay'); + }); /** * The element of the full overlay. diff --git a/chrome/browser/resources/extensions/extensions.js b/chrome/browser/resources/extensions/extensions.js index 3dc9635..d50ceb3 100644 --- a/chrome/browser/resources/extensions/extensions.js +++ b/chrome/browser/resources/extensions/extensions.js @@ -386,9 +386,19 @@ cr.define('extensions', function() { } if (node) { - var lastFocused = document.activeElement; + var lastFocused; + + var focusOutlineManager = cr.ui.FocusOutlineManager.forDocument(document); + if (focusOutlineManager.visible) + lastFocused = document.activeElement; + $('overlay').addEventListener('cancelOverlay', function f() { - lastFocused.focus(); + console.log('cancelOverlay'); + console.log('lastFocused', lastFocused); + console.log('focusOutlineManager.visible', focusOutlineManager.visible); + if (lastFocused && focusOutlineManager.visible) + lastFocused.focus(); + $('overlay').removeEventListener('cancelOverlay', f); }); node.classList.add('showing'); diff --git a/chrome/browser/resources/extensions/pack_extension_overlay.js b/chrome/browser/resources/extensions/pack_extension_overlay.js index 03e6180..05d23d1 100644 --- a/chrome/browser/resources/extensions/pack_extension_overlay.js +++ b/chrome/browser/resources/extensions/pack_extension_overlay.js @@ -23,8 +23,9 @@ cr.define('extensions', function() { cr.ui.overlay.globalInitialization(); overlay.addEventListener('cancelOverlay', this.handleDismiss_.bind(this)); - $('pack-extension-dismiss').addEventListener('click', - this.handleDismiss_.bind(this)); + $('pack-extension-dismiss').addEventListener('click', function() { + cr.dispatchSimpleEvent(overlay, 'cancelOverlay'); + }); $('pack-extension-commit').addEventListener('click', this.handleCommit_.bind(this)); $('browse-extension-dir').addEventListener('click', diff --git a/ui/webui/resources/js/cr/ui/compiled_resources.gyp b/ui/webui/resources/js/cr/ui/compiled_resources.gyp index c8da03c..06c91f9 100644 --- a/ui/webui/resources/js/cr/ui/compiled_resources.gyp +++ b/ui/webui/resources/js/cr/ui/compiled_resources.gyp @@ -13,5 +13,18 @@ }, 'includes': ['../../../../../../third_party/closure_compiler/compile_js.gypi'], }, + { + 'target_name': 'overlay', + 'variables': { + 'depends': [ + '../../cr.js', + '../../util.js', + ], + 'externs': [ + '../../../../../../third_party/closure_compiler/externs/chrome_send_externs.js', + ], + }, + 'includes': ['../../../../../../third_party/closure_compiler/compile_js.gypi'], + }, ], } diff --git a/ui/webui/resources/js/cr/ui/focus_outline_manager.js b/ui/webui/resources/js/cr/ui/focus_outline_manager.js index df8dadc..5a8ed8e 100644 --- a/ui/webui/resources/js/cr/ui/focus_outline_manager.js +++ b/ui/webui/resources/js/cr/ui/focus_outline_manager.js @@ -41,19 +41,19 @@ cr.define('cr.ui', function() { doc.addEventListener('focus', function(event) { // Update visibility only when focus is actually changed. - self.updateVisiblity_(); + self.updateVisibility(); }, true); doc.addEventListener('focusout', function(event) { window.setTimeout(function() { if (!doc.hasFocus()) { self.focusByKeyboard_ = true; - self.updateVisiblity_(); + self.updateVisibility(); } }, 0); }); - this.updateVisiblity_(); + this.updateVisibility(); } FocusOutlineManager.prototype = { @@ -64,8 +64,7 @@ cr.define('cr.ui', function() { */ focusByKeyboard_: true, - /** @private */ - updateVisiblity_: function() { + updateVisibility: function() { this.visible = this.focusByKeyboard_; }, diff --git a/ui/webui/resources/js/cr/ui/overlay.js b/ui/webui/resources/js/cr/ui/overlay.js index 892df02..56f339f 100644 --- a/ui/webui/resources/js/cr/ui/overlay.js +++ b/ui/webui/resources/js/cr/ui/overlay.js @@ -97,6 +97,8 @@ cr.define('cr.ui.overlay', function() { var closeButtons = overlay.querySelectorAll('.page > .close-button'); for (var i = 0; i < closeButtons.length; i++) { closeButtons[i].addEventListener('click', function(e) { + if (cr.ui.FocusOutlineManager) + cr.ui.FocusOutlineManager.forDocument(document).updateVisibility(); cr.dispatchSimpleEvent(overlay, 'cancelOverlay'); }); } diff --git a/ui/webui/resources/js/cr/ui/page_manager/page_manager.js b/ui/webui/resources/js/cr/ui/page_manager/page_manager.js index 578adbd..8fafc82 100644 --- a/ui/webui/resources/js/cr/ui/page_manager/page_manager.js +++ b/ui/webui/resources/js/cr/ui/page_manager/page_manager.js @@ -476,9 +476,11 @@ cr.define('cr.ui.pageManager', function() { if (!overlay || !overlay.canShowPage()) return false; + var focusOutlineManager = cr.ui.FocusOutlineManager.forDocument(document); + // Save the currently focused element in the page for restoration later. var currentPage = this.getTopmostVisiblePage(); - if (currentPage) + if (currentPage && focusOutlineManager.visible) currentPage.lastFocusedElement = document.activeElement; if ((!rootPage || !rootPage.sticky) && @@ -495,14 +497,11 @@ cr.define('cr.ui.pageManager', function() { overlay.didChangeHash(); } - // Change focus to the overlay if any other control was focused by - // keyboard before. Otherwise, no one should have focus. - if (document.activeElement != document.body) { - if (cr.ui.FocusOutlineManager.forDocument(document).visible) - overlay.focus(); - if (!overlay.pageDiv.contains(document.activeElement)) - document.activeElement.blur(); - } + if (focusOutlineManager.visible) + overlay.focus(); + + if (!overlay.pageDiv.contains(document.activeElement)) + document.activeElement.blur(); if ($('search-field') && $('search-field').value == '') { var section = overlay.associatedSection; @@ -627,8 +626,14 @@ cr.define('cr.ui.pageManager', function() { */ restoreLastFocusedElement_: function() { var currentPage = this.getTopmostVisiblePage(); - if (currentPage.lastFocusedElement) + + if (!currentPage.lastFocusedElement) + return; + + if (cr.ui.FocusOutlineManager.forDocument(document).visible) currentPage.lastFocusedElement.focus(); + + currentPage.lastFocusedElement = null; }, /** |