Skip to content

Commit

Permalink
Bug 1576509: When a CFR doorhanger is activated via the keyboard, foc…
Browse files Browse the repository at this point in the history
…us its first element. r=andreio

Previously, keyboard users had to press f6 several times to get to the doorhanger.
If a user is activating the doorhanger from the keyboard, it makes sense that they'd want to interact with it using the keyboard.

Differential Revision: https://phabricator.services.mozilla.com/D47719
  • Loading branch information
jcsteh committed Oct 4, 2019
1 parent 56cab5f commit 2a20e2c
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 1 deletion.
18 changes: 17 additions & 1 deletion browser/components/newtab/lib/CFRPageActions.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,14 @@ class PageAction {
// This is called when the popup closes as a result of interaction _outside_
// the popup, e.g. by hitting <esc>
_popupStateChange(state) {
if (["dismissed", "removed"].includes(state)) {
if (state === "shown") {
if (this._autoFocus) {
this.window.document.commandDispatcher.advanceFocusIntoSubtree(
this.currentNotification.owner.panel
);
this._autoFocus = false;
}
} else if (["dismissed", "removed"].includes(state)) {
this._collapse();
if (this.currentNotification) {
this.window.PopupNotifications.remove(this.currentNotification);
Expand Down Expand Up @@ -703,6 +710,15 @@ class PageAction {
})
);

// If the recommendation button is focused, it was probably activated via
// the keyboard. Therefore, focus the first element in the notification when
// it appears.
// We don't use the autofocus option provided by PopupNotifications.show
// because it doesn't focus the first element; i.e. the user still has to
// press tab once. That's not good enough, especially for screen reader
// users. Instead, we handle this ourselves in _popupStateChange.
this._autoFocus = this.window.document.activeElement === this.container;

// Actually show the notification
this.currentNotification = this.window.PopupNotifications.show(
browser,
Expand Down
44 changes: 44 additions & 0 deletions browser/components/newtab/test/browser/browser_asrouter_cfr.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ add_task(async function test_cfr_notification_show() {
"Should return true if addRecommendation checks were successful"
);

const oldFocus = document.activeElement;
const showPanel = BrowserTestUtils.waitForEvent(
PopupNotifications.panel,
"popupshown"
Expand All @@ -215,6 +216,11 @@ add_task(async function test_cfr_notification_show() {
.hidden === false,
"Panel should be visible"
);
Assert.equal(
document.activeElement,
oldFocus,
"Focus didn't move when panel was shown"
);

// Check there is a primary button and click it. It will trigger the callback.
Assert.ok(
Expand Down Expand Up @@ -758,3 +764,41 @@ add_task(async function test_providerNames() {
}
}
});

add_task(async function test_cfr_notification_keyboard() {
// addRecommendation checks that scheme starts with http and host matches
const browser = gBrowser.selectedBrowser;
await BrowserTestUtils.loadURI(browser, "http://example.com/");
await BrowserTestUtils.browserLoaded(browser, false, "http://example.com/");

const response = await trigger_cfr_panel(browser, "example.com");
Assert.ok(
response,
"Should return true if addRecommendation checks were successful"
);

// Open the panel with the keyboard.
// Toolbar buttons aren't always focusable; toolbar keyboard navigation
// makes them focusable on demand. Therefore, we must force focus.
const button = document.getElementById("contextual-feature-recommendation");
button.setAttribute("tabindex", "-1");
button.focus();
button.removeAttribute("tabindex");

let focused = BrowserTestUtils.waitForEvent(
PopupNotifications.panel,
"focus",
true
);
EventUtils.synthesizeKey(" ");
await focused;
Assert.ok(true, "Focus inside panel after button pressed");

let hidden = BrowserTestUtils.waitForEvent(
PopupNotifications.panel,
"popuphidden"
);
EventUtils.synthesizeKey("KEY_Escape");
await hidden;
Assert.ok(true, "Panel hidden after Escape pressed");
});

0 comments on commit 2a20e2c

Please sign in to comment.