From f0d5de33b0c89b3dde92ebadcbdcaeca57d45ef4 Mon Sep 17 00:00:00 2001 From: James Teh Date: Mon, 21 Oct 2019 07:13:14 +0000 Subject: [PATCH] Bug 1587809: Give the CFR address bar button a more descriptive tooltip/a11y label. Announce its appearance to screen reader users. r=andreio,fluent-reviewers,flod 1. Previously, the label and tooltip for all recommendations was just "Recommendation", even though the icon was different for extension and feature recommendations. Because users might not be able to see the icon and/or determine its meaning, it is important that this is communicated in the tooltip and a11y label. 2. Screen reader users won't know this has appeared, even though this attracts some attention visually. Therefore, provide a specific announcement for screen reader users when the recommendation appears. Differential Revision: https://phabricator.services.mozilla.com/D47718 --HG-- extra : moz-landing-system : lando --- .../newtab/lib/CFRMessageProvider.jsm | 24 ++-- .../components/newtab/lib/CFRPageActions.jsm | 16 ++- .../newtab/lib/PanelTestProvider.jsm | 2 +- .../test/browser/browser_asrouter_cfr.js | 111 ++++++++++-------- .../test/unit/asrouter/ASRouter.test.js | 1 + .../test/unit/asrouter/CFRPageActions.test.js | 18 +-- .../newtab/test/unit/asrouter/constants.js | 5 +- .../locales/en-US/browser/newtab/asrouter.ftl | 10 +- 8 files changed, 114 insertions(+), 73 deletions(-) diff --git a/browser/components/newtab/lib/CFRMessageProvider.jsm b/browser/components/newtab/lib/CFRMessageProvider.jsm index 2ffcae3badd8..fb568a025155 100644 --- a/browser/components/newtab/lib/CFRMessageProvider.jsm +++ b/browser/components/newtab/lib/CFRMessageProvider.jsm @@ -129,7 +129,9 @@ const CFR_MESSAGES = [ layout: "addon_recommendation", category: "cfrAddons", bucket_id: "CFR_M1", - notification_text: { string_id: "cfr-doorhanger-extension-notification" }, + notification_text: { + string_id: "cfr-doorhanger-extension-notification2", + }, heading_text: { string_id: "cfr-doorhanger-extension-heading" }, info_icon: { label: { string_id: "cfr-doorhanger-extension-sumo-link" }, @@ -198,7 +200,9 @@ const CFR_MESSAGES = [ layout: "addon_recommendation", category: "cfrAddons", bucket_id: "CFR_M1", - notification_text: { string_id: "cfr-doorhanger-extension-notification" }, + notification_text: { + string_id: "cfr-doorhanger-extension-notification2", + }, heading_text: { string_id: "cfr-doorhanger-extension-heading" }, info_icon: { label: { string_id: "cfr-doorhanger-extension-sumo-link" }, @@ -268,7 +272,9 @@ const CFR_MESSAGES = [ layout: "addon_recommendation", category: "cfrAddons", bucket_id: "CFR_M1", - notification_text: { string_id: "cfr-doorhanger-extension-notification" }, + notification_text: { + string_id: "cfr-doorhanger-extension-notification2", + }, heading_text: { string_id: "cfr-doorhanger-extension-heading" }, info_icon: { label: { string_id: "cfr-doorhanger-extension-sumo-link" }, @@ -339,7 +345,9 @@ const CFR_MESSAGES = [ layout: "addon_recommendation", category: "cfrAddons", bucket_id: "CFR_M1", - notification_text: { string_id: "cfr-doorhanger-extension-notification" }, + notification_text: { + string_id: "cfr-doorhanger-extension-notification2", + }, heading_text: { string_id: "cfr-doorhanger-extension-heading" }, info_icon: { label: { string_id: "cfr-doorhanger-extension-sumo-link" }, @@ -413,7 +421,9 @@ const CFR_MESSAGES = [ layout: "addon_recommendation", category: "cfrAddons", bucket_id: "CFR_M1", - notification_text: { string_id: "cfr-doorhanger-extension-notification" }, + notification_text: { + string_id: "cfr-doorhanger-extension-notification2", + }, heading_text: { string_id: "cfr-doorhanger-extension-heading" }, info_icon: { label: { string_id: "cfr-doorhanger-extension-sumo-link" }, @@ -483,7 +493,7 @@ const CFR_MESSAGES = [ layout: "message_and_animation", category: "cfrFeatures", bucket_id: "CFR_PIN_TAB", - notification_text: { string_id: "cfr-doorhanger-extension-notification" }, + notification_text: { string_id: "cfr-doorhanger-feature-notification" }, heading_text: { string_id: "cfr-doorhanger-pintab-heading" }, info_icon: { label: { string_id: "cfr-doorhanger-extension-sumo-link" }, @@ -597,7 +607,7 @@ const CFR_MESSAGES = [ sumo_path: "extensionrecommendations", }, notification_text: { - string_id: "cfr-doorhanger-extension-notification", + string_id: "cfr-doorhanger-feature-notification", }, category: "cfrFeatures", }, diff --git a/browser/components/newtab/lib/CFRPageActions.jsm b/browser/components/newtab/lib/CFRPageActions.jsm index 2d7490e076d1..77efe7348c8c 100644 --- a/browser/components/newtab/lib/CFRPageActions.jsm +++ b/browser/components/newtab/lib/CFRPageActions.jsm @@ -182,13 +182,18 @@ class PageAction { async showAddressBarNotifier(recommendation, shouldExpand = false) { this.container.hidden = false; - this.label.value = await this.getStrings( + let notificationText = await this.getStrings( recommendation.content.notification_text ); - + this.label.value = notificationText; this.button.setAttribute( "tooltiptext", - await this.getStrings(recommendation.content.notification_text) + notificationText.attributes.tooltiptext + ); + // For a11y, we want the more descriptive text. + this.container.setAttribute( + "aria-label", + notificationText.attributes.tooltiptext ); this.button.setAttribute( "data-cfr-icon", @@ -216,6 +221,11 @@ class PageAction { this.addImpression(recommendation); } + + this.window.A11yUtils.announce({ + raw: notificationText.attributes["a11y-announcement"], + source: this.container, + }); } hideAddressBarNotifier() { diff --git a/browser/components/newtab/lib/PanelTestProvider.jsm b/browser/components/newtab/lib/PanelTestProvider.jsm index 5d4b758d33f7..7e352dda5257 100644 --- a/browser/components/newtab/lib/PanelTestProvider.jsm +++ b/browser/components/newtab/lib/PanelTestProvider.jsm @@ -143,7 +143,7 @@ const MESSAGES = () => [ content: { layout: "icon_and_message", category: "cfrFeatures", - notification_text: { string_id: "cfr-doorhanger-extension-notification" }, + notification_text: { string_id: "cfr-doorhanger-feature-notification" }, heading_text: { string_id: "cfr-doorhanger-sync-bookmarks-header" }, info_icon: { label: { string_id: "cfr-doorhanger-extension-sumo-link" }, diff --git a/browser/components/newtab/test/browser/browser_asrouter_cfr.js b/browser/components/newtab/test/browser/browser_asrouter_cfr.js index 2414ae4e74a5..d6c23d59d370 100644 --- a/browser/components/newtab/test/browser/browser_asrouter_cfr.js +++ b/browser/components/newtab/test/browser/browser_asrouter_cfr.js @@ -15,65 +15,72 @@ const createDummyRecommendation = ({ layout, skip_address_bar_notifier, template, -}) => ({ - template, - content: { - layout: layout || "addon_recommendation", - category, - anchor_id: "page-action-buttons", - skip_address_bar_notifier, - notification_text: "Mochitest", - heading_text: heading_text || "Mochitest", - info_icon: { - label: { attributes: { tooltiptext: "Why am I seeing this" } }, - sumo_path: "extensionrecommendations", - }, - icon: "foo", - icon_dark_theme: "bar", - learn_more: "extensionrecommendations", - addon: { - id: "addon-id", - title: "Addon name", - icon: "foo", - author: "Author name", - amo_url: "https://example.com", - }, - descriptionDetails: { steps: [] }, - text: "Mochitest", - buttons: { - primary: { - label: { - value: "OK", - attributes: { accesskey: "O" }, - }, - action: { - type: action.type, - data: {}, - }, +}) => { + let recommendation = { + template, + content: { + layout: layout || "addon_recommendation", + category, + anchor_id: "page-action-buttons", + skip_address_bar_notifier, + heading_text: heading_text || "Mochitest", + info_icon: { + label: { attributes: { tooltiptext: "Why am I seeing this" } }, + sumo_path: "extensionrecommendations", }, - secondary: [ - { + icon: "foo", + icon_dark_theme: "bar", + learn_more: "extensionrecommendations", + addon: { + id: "addon-id", + title: "Addon name", + icon: "foo", + author: "Author name", + amo_url: "https://example.com", + }, + descriptionDetails: { steps: [] }, + text: "Mochitest", + buttons: { + primary: { label: { - value: "Cancel", - attributes: { accesskey: "C" }, + value: "OK", + attributes: { accesskey: "O" }, + }, + action: { + type: action.type, + data: {}, }, }, - { - label: { - value: "Cancel 1", - attributes: { accesskey: "A" }, + secondary: [ + { + label: { + value: "Cancel", + attributes: { accesskey: "C" }, + }, }, - }, - { - label: { - value: "Cancel 2", - attributes: { accesskey: "B" }, + { + label: { + value: "Cancel 1", + attributes: { accesskey: "A" }, + }, }, - }, - ], + { + label: { + value: "Cancel 2", + attributes: { accesskey: "B" }, + }, + }, + ], + }, }, - }, -}); + }; + recommendation.content.notification_text = new String("Mochitest"); // eslint-disable-line + recommendation.content.notification_text.attributes = { + tooltiptext: "Mochitest tooltip", + "a11y-announcement": "Mochitest announcement", + }; + return recommendation; +}; function checkCFRFeaturesElements(notification) { Assert.ok(notification.hidden === false, "Panel should be visible"); diff --git a/browser/components/newtab/test/unit/asrouter/ASRouter.test.js b/browser/components/newtab/test/unit/asrouter/ASRouter.test.js index 4b71d451f69c..0c57f4cd5c02 100644 --- a/browser/components/newtab/test/unit/asrouter/ASRouter.test.js +++ b/browser/components/newtab/test/unit/asrouter/ASRouter.test.js @@ -2494,6 +2494,7 @@ describe("ASRouter", () => { document: { getElementById }, promiseDocumentFlushed: sandbox.stub().resolves([{ width: 0 }]), setTimeout: sandbox.stub(), + A11yUtils: { announce: sandbox.stub() }, }; const firstMessage = { ...FAKE_RECOMMENDATION, id: "first_message" }; const secondMessage = { ...FAKE_RECOMMENDATION, id: "second_message" }; diff --git a/browser/components/newtab/test/unit/asrouter/CFRPageActions.test.js b/browser/components/newtab/test/unit/asrouter/CFRPageActions.test.js index 63bd526c4beb..48be1f6e3d54 100644 --- a/browser/components/newtab/test/unit/asrouter/CFRPageActions.test.js +++ b/browser/components/newtab/test/unit/asrouter/CFRPageActions.test.js @@ -12,6 +12,7 @@ describe("CFRPageActions", () => { let globals; let containerElem; let elements; + let announceStub; const elementIDs = [ "urlbar", @@ -41,6 +42,8 @@ describe("CFRPageActions", () => { sandbox = sinon.createSandbox(); clock = sandbox.useFakeTimers(); + announceStub = sandbox.stub(); + const A11yUtils = { announce: announceStub }; fakeRecommendation = { ...FAKE_RECOMMENDATION }; fakeHost = "mozilla.org"; fakeBrowser = { @@ -64,6 +67,7 @@ describe("CFRPageActions", () => { }, PrivateBrowsingUtils: { isWindowPrivate: sandbox.stub().returns(false) }, gBrowser: { selectedBrowser: fakeBrowser }, + A11yUtils, }); document.createXULElement = document.createElement; @@ -94,22 +98,19 @@ describe("CFRPageActions", () => { describe("PageAction", () => { let pageAction; - let getStringsStub; beforeEach(() => { pageAction = new PageAction(window, dispatchStub); - getStringsStub = sandbox.stub(pageAction, "getStrings").resolves(""); }); describe("#showAddressBarNotifier", () => { it("should un-hideAddressBarNotifier the element and set the right label value", async () => { - const FAKE_NOTIFICATION_TEXT = "FAKE_NOTIFICATION_TEXT"; - getStringsStub - .withArgs(fakeRecommendation.content.notification_text) - .resolves(FAKE_NOTIFICATION_TEXT); await pageAction.showAddressBarNotifier(fakeRecommendation); assert.isFalse(pageAction.container.hidden); - assert.equal(pageAction.label.value, FAKE_NOTIFICATION_TEXT); + assert.equal( + pageAction.label.value, + fakeRecommendation.content.notification_text + ); }); it("should wait for the document layout to flush", async () => { sandbox.spy(pageAction.label, "getClientRects"); @@ -356,7 +357,6 @@ describe("CFRPageActions", () => { ]; beforeEach(() => { - getStringsStub.restore(); formatMessagesStub = sandbox .stub() .withArgs({ id: "hello_world" }) @@ -433,6 +433,7 @@ describe("CFRPageActions", () => { describe("#_showPopupOnClick", () => { let translateElementsStub; let setAttributesStub; + let getStringsStub; beforeEach(async () => { CFRPageActions.PageActionMap.set(fakeBrowser.ownerGlobal, pageAction); await CFRPageActions.addRecommendation( @@ -441,6 +442,7 @@ describe("CFRPageActions", () => { fakeRecommendation, dispatchStub ); + getStringsStub = sandbox.stub(pageAction, "getStrings").resolves(""); getStringsStub .callsFake(async a => a) // eslint-disable-line max-nested-callbacks .withArgs({ string_id: "primary_button_id" }) diff --git a/browser/components/newtab/test/unit/asrouter/constants.js b/browser/components/newtab/test/unit/asrouter/constants.js index bf644f6322ab..0bca2fa7e723 100644 --- a/browser/components/newtab/test/unit/asrouter/constants.js +++ b/browser/components/newtab/test/unit/asrouter/constants.js @@ -81,13 +81,16 @@ export const FAKE_REMOTE_SETTINGS_PROVIDER = { enabled: true, }; +const notificationText = new String("Fake notification text"); // eslint-disable-line +notificationText.attributes = { tooltiptext: "Fake tooltip text" }; + export const FAKE_RECOMMENDATION = { id: "fake_id", template: "cfr_doorhanger", content: { category: "cfrDummy", bucket_id: "fake_bucket_id", - notification_text: "Fake Notification Text", + notification_text: notificationText, info_icon: { label: "Fake Info Icon Label", sumo_path: "a_help_path_fragment", diff --git a/browser/locales/en-US/browser/newtab/asrouter.ftl b/browser/locales/en-US/browser/newtab/asrouter.ftl index f46c887d23ec..356754902347 100644 --- a/browser/locales/en-US/browser/newtab/asrouter.ftl +++ b/browser/locales/en-US/browser/newtab/asrouter.ftl @@ -35,7 +35,15 @@ cfr-doorhanger-extension-author = by { $name } # This is a notification displayed in the address bar. # When clicked it opens a panel with a message for the user. -cfr-doorhanger-extension-notification = Recommendation +cfr-doorhanger-extension-notification2 = Recommendation + .tooltiptext = Extension recommendation + .a11y-announcement = Extension recommendation available + +# This is a notification displayed in the address bar. +# When clicked it opens a panel with a message for the user. +cfr-doorhanger-feature-notification = Recommendation + .tooltiptext = Feature recommendation + .a11y-announcement = Feature recommendation available ## Add-on statistics ## These strings are used to display the total number of