Bug 1467385 - Use PanelMultiView APIs for keyboard navigation in the identity popup. r=Paolo
PanelMultiView has its own concept of "selected", so we should use this rather than move focus
using advanceFocusIntoSubTree.
MozReview-Commit-ID: DW2JqqggLl1
--- a/browser/base/content/browser-siteIdentity.js
+++ b/browser/base/content/browser-siteIdentity.js
@@ -41,22 +41,16 @@ var gIdentityHandler = {
_sslStatus: null,
/**
* Bitmask provided by nsIWebProgressListener.onSecurityChange.
*/
_state: 0,
/**
- * This flag gets set if the identity popup was opened by a keypress,
- * to be able to focus it on the popupshown event.
- */
- _popupTriggeredByKeyboard: false,
-
- /**
* RegExp used to decide if an about url should be shown as being part of
* the browser UI.
*/
_secureInternalUIWhitelist: /^(?:accounts|addons|cache|config|crashes|customizing|downloads|healthreport|license|permissions|preferences|rights|searchreset|sessionrestore|support|welcomeback)(?:[?#]|$)/i,
get _isBroken() {
return this._state & Ci.nsIWebProgressListener.STATE_IS_BROKEN;
},
@@ -826,17 +820,24 @@ var gIdentityHandler = {
// being focused (and therefore, interacted with) by the user. However, we
// want to allow opening the identity popup from the device control menu,
// which calls click() on the identity button, so we don't return early.
if (!this._sharingState &&
gURLBar.getAttribute("pageproxystate") != "valid") {
return;
}
- this._popupTriggeredByKeyboard = event.type == "keypress";
+ // Move focus to the next available element in the identity popup.
+ // This is required by role=alertdialog and fixes an issue where
+ // an already open panel would steal focus from the identity popup.
+ if (event.type == "keypress") {
+ let panelView = PanelView.forNode(this._identityPopupMainView);
+ this._identityPopupMainView.addEventListener("ViewShown", () => panelView.focusFirstNavigableElement(),
+ {once: true});
+ }
// Make sure that the display:none style we set in xul is removed now that
// the popup is actually needed
this._identityPopup.hidden = false;
// Remove the reload hint that we show after a user has cleared a permission.
this._permissionReloadHint.setAttribute("hidden", "true");
@@ -848,23 +849,16 @@ var gIdentityHandler = {
// Now open the popup, anchored off the primary chrome element
PanelMultiView.openPopup(this._identityPopup, this._identityIcon,
"bottomcenter topleft").catch(Cu.reportError);
},
onPopupShown(event) {
if (event.target == this._identityPopup) {
- if (this._popupTriggeredByKeyboard) {
- // Move focus to the next available element in the identity popup.
- // This is required by role=alertdialog and fixes an issue where
- // an already open panel would steal focus from the identity popup.
- document.commandDispatcher.advanceFocusIntoSubtree(this._identityPopup);
- }
-
window.addEventListener("focus", this, true);
}
},
onPopupHidden(event) {
if (event.target == this._identityPopup) {
window.removeEventListener("focus", this, true);
this._identityBox.removeAttribute("open");
--- a/browser/components/controlcenter/content/panel.inc.xul
+++ b/browser/components/controlcenter/content/panel.inc.xul
@@ -42,17 +42,17 @@
when-mixedcontent="passive-loaded">&identity.passiveLoaded;</description>
<description when-mixedcontent="active-loaded">&identity.activeLoaded;</description>
<description class="identity-popup-warning-yellow"
when-ciphers="weak">&identity.weakEncryption;</description>
<description when-loginforms="insecure">&identity.insecureLoginForms2;</description>
</vbox>
</vbox>
<button id="identity-popup-security-expander"
- class="identity-popup-expander"
+ class="identity-popup-expander subviewkeynav"
when-connection="not-secure secure secure-ev secure-cert-user-overridden"
oncommand="gIdentityHandler.showSecuritySubView();"/>
</hbox>
<!-- Tracking Protection Section -->
<hbox id="tracking-protection-container"
class="identity-popup-section"
when-connection="not-secure secure secure-ev secure-cert-user-overridden extension">
@@ -79,32 +79,32 @@
<description id="tracking-loaded-exception"
crop="end">&trackingProtection.detectedException;</description>
<description id="tracking-not-detected-exception"
crop="end">&trackingProtection.notDetectedException;</description>
<description id="tracking-reload-required"
crop="end">&trackingProtection.reloadRequired2;</description>
<button id="tracking-action-reload"
- class="tracking-protection-button"
+ class="tracking-protection-button subviewkeynav"
label="&trackingProtection.reload2.label;"
accesskey="&trackingProtection.reload2.accesskey;"
oncommand="TrackingProtection.hideIdentityPopupAndReload();" />
<button id="tracking-action-unblock"
- class="tracking-protection-button"
+ class="tracking-protection-button subviewkeynav"
label="&trackingProtection.unblock3.label;"
accesskey="&trackingProtection.unblock3.accesskey;"
oncommand="TrackingProtection.disableForCurrentPage();" />
<button id="tracking-action-unblock-private"
- class="tracking-protection-button"
+ class="tracking-protection-button subviewkeynav"
label="&trackingProtection.unblockPrivate3.label;"
accesskey="&trackingProtection.unblockPrivate3.accesskey;"
oncommand="TrackingProtection.disableForCurrentPage();" />
<button id="tracking-action-block"
- class="tracking-protection-button"
+ class="tracking-protection-button subviewkeynav"
label="&trackingProtection.block4.label;"
accesskey="&trackingProtection.block4.accesskey;"
oncommand="TrackingProtection.enableForCurrentPage();" />
</vbox>
</hbox>
<!-- Permissions Section -->
<hbox class="identity-popup-section"
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -1358,46 +1358,55 @@ var PanelView = class extends Associated
}
/**
* Retrieves the button elements that can be used for navigation using the
* keyboard, that is all enabled buttons including the back button if visible.
*
* @return {Array}
*/
- getNavigableElements() {
+ _getNavigableElements() {
let buttons = Array.from(this.node.querySelectorAll(
".subviewbutton:not([disabled]), .subviewkeynav:not([disabled])"));
let dwu = this._dwu;
return buttons.filter(button => {
let bounds = dwu.getBoundsWithoutFlushing(button);
return bounds.width > 0 && bounds.height > 0;
});
}
/**
* Element that is currently selected with the keyboard, or null if no element
* is selected. Since the reference is held weakly, it can become null or
* undefined at any time.
*
* The element is usually, but not necessarily, in the "buttons" property
- * which in turn is initialized from the getNavigableElements list.
+ * which in turn is initialized from the _getNavigableElements list.
*/
get selectedElement() {
return this._selectedElement && this._selectedElement.get();
}
set selectedElement(value) {
if (!value) {
delete this._selectedElement;
} else {
this._selectedElement = Cu.getWeakReference(value);
}
}
/**
+ * Focuses and moves keyboard selection to the first navigable element.
+ * This is a no-op if there are no navigable elements.
+ */
+ focusFirstNavigableElement() {
+ this.selectedElement = this._getNavigableElements()[0];
+ this.focusSelectedElement();
+ }
+
+ /**
* Based on going up or down, select the previous or next focusable button.
*
* @param {Boolean} isDown whether we're going down (true) or up (false).
*
* @return {DOMNode} the button we selected.
*/
moveSelection(isDown) {
let buttons = this.buttons;
@@ -1467,17 +1476,17 @@ var PanelView = class extends Associated
*/
keyNavigation(event, dir) {
if (!this.active) {
return;
}
let buttons = this.buttons;
if (!buttons || !buttons.length) {
- buttons = this.buttons = this.getNavigableElements();
+ buttons = this.buttons = this._getNavigableElements();
// Set the 'tabindex' attribute on the buttons to make sure they're focussable.
for (let button of buttons) {
if (!button.classList.contains("subviewbutton-back") &&
!button.hasAttribute("tabindex")) {
button.setAttribute("tabindex", 0);
}
}
}
--- a/browser/components/customizableui/test/browser_panel_keyboard_navigation.js
+++ b/browser/components/customizableui/test/browser_panel_keyboard_navigation.js
@@ -5,17 +5,17 @@
*/
const {PanelView} = ChromeUtils.import("resource:///modules/PanelMultiView.jsm", {});
const kHelpButtonId = "appMenu-help-button";
add_task(async function testUpDownKeys() {
await gCUITestUtils.openMainMenu();
- let buttons = PanelView.forNode(PanelUI.mainView).getNavigableElements();
+ let buttons = PanelView.forNode(PanelUI.mainView)._getNavigableElements();
for (let button of buttons) {
if (button.disabled)
continue;
EventUtils.synthesizeKey("KEY_ArrowDown");
Assert.equal(document.commandDispatcher.focusedElement, button,
"The correct button should be focused after navigating downward");
}
@@ -34,34 +34,34 @@ add_task(async function testUpDownKeys()
}
await gCUITestUtils.hideMainMenu();
});
add_task(async function testEnterKeyBehaviors() {
await gCUITestUtils.openMainMenu();
- let buttons = PanelView.forNode(PanelUI.mainView).getNavigableElements();
+ let buttons = PanelView.forNode(PanelUI.mainView)._getNavigableElements();
// Navigate to the 'Help' button, which points to a subview.
EventUtils.synthesizeKey("KEY_ArrowUp");
let focusedElement = document.commandDispatcher.focusedElement;
Assert.equal(focusedElement, buttons[buttons.length - 1],
"The last button should be focused after navigating upward");
let promise = BrowserTestUtils.waitForEvent(PanelUI.helpView, "ViewShown");
// Make sure the Help button is in focus.
while (!focusedElement || !focusedElement.id || focusedElement.id != kHelpButtonId) {
EventUtils.synthesizeKey("KEY_ArrowUp");
focusedElement = document.commandDispatcher.focusedElement;
}
EventUtils.synthesizeKey("KEY_Enter");
await promise;
- let helpButtons = PanelView.forNode(PanelUI.helpView).getNavigableElements();
+ let helpButtons = PanelView.forNode(PanelUI.helpView)._getNavigableElements();
Assert.ok(helpButtons[0].classList.contains("subviewbutton-back"),
"First button in help view should be a back button");
// For posterity, check navigating the subview using up/ down arrow keys as well.
for (let i = helpButtons.length - 1; i >= 0; --i) {
let button = helpButtons[i];
if (button.disabled)
continue;
@@ -127,17 +127,17 @@ add_task(async function testLeftRightKey
"Help button should be focused again now that we're back in the main view");
await gCUITestUtils.hideMainMenu();
});
add_task(async function testTabKey() {
await gCUITestUtils.openMainMenu();
- let buttons = PanelView.forNode(PanelUI.mainView).getNavigableElements();
+ let buttons = PanelView.forNode(PanelUI.mainView)._getNavigableElements();
for (let button of buttons) {
if (button.disabled)
continue;
EventUtils.synthesizeKey("KEY_Tab");
Assert.equal(document.commandDispatcher.focusedElement, button,
"The correct button should be focused after tabbing");
}
@@ -160,17 +160,17 @@ add_task(async function testTabKey() {
"Pressing shift + tab should cycle around and select the last button again");
await gCUITestUtils.hideMainMenu();
});
add_task(async function testInterleavedTabAndArrowKeys() {
await gCUITestUtils.openMainMenu();
- let buttons = PanelView.forNode(PanelUI.mainView).getNavigableElements();
+ let buttons = PanelView.forNode(PanelUI.mainView)._getNavigableElements();
let tab = false;
for (let button of buttons) {
if (button.disabled)
continue;
if (tab) {
EventUtils.synthesizeKey("KEY_Tab");
} else {
@@ -183,17 +183,17 @@ add_task(async function testInterleavedT
"The last button should be focused after a mix of Tab and ArrowDown");
await gCUITestUtils.hideMainMenu();
});
add_task(async function testSpaceDownAfterTabNavigation() {
await gCUITestUtils.openMainMenu();
- let buttons = PanelView.forNode(PanelUI.mainView).getNavigableElements();
+ let buttons = PanelView.forNode(PanelUI.mainView)._getNavigableElements();
let button;
for (button of buttons) {
if (button.disabled)
continue;
EventUtils.synthesizeKey("KEY_Tab");
if (button.id == kHelpButtonId) {
break;