Bug 1706334 - Improving the accessible text on Add Engine items. r=adw,Jamie,fluent-reviewers,flod

Differential Revision: https://phabricator.services.mozilla.com/D113146

browser/components/search/SearchOneOffs.jsm

@@ -85,8 +85,6 @@ class SearchOneOffs {
 
     this.contextMenuPopup = this.querySelector(".search-one-offs-context-menu");
 
-    this._bundle = null;
-
     /**
      * When a context menu is opened on a one-off button, this is set to the
      * engine of that button for use with the context menu actions.
@@ -377,14 +375,6 @@ class SearchOneOffs {
     return this.getAttribute("compact") == "true";
   }
 
-  get bundle() {
-    if (!this._bundle) {
-      const kBundleURI = "chrome://browser/locale/search.properties";
-      this._bundle = Services.strings.createBundle(kBundleURI);
-    }
-    return this._bundle;
-  }
-
   async getEngineInfo() {
     if (this._engineInfo) {
       return this._engineInfo;
@@ -601,10 +591,7 @@ class SearchOneOffs {
       button.setAttribute("badged", "true");
       button.setAttribute("type", "menu");
       button.setAttribute("wantdropmarker", "true");
-      button.setAttribute(
+      button.setAttribute("data-l10n-id", "search-one-offs-add-engine-menu");
-        "label",
-        this.bundle.GetStringFromName("cmd_addFoundEngineMenu")
-      );
       button.setAttribute("crop", "end");
       button.setAttribute("pack", "start");
 
@@ -654,14 +641,15 @@ class SearchOneOffs {
         this.telemetryOrigin +
         "-add-engine-" +
         this._fixUpEngineNameForID(engine.title);
-      let label = this.bundle.formatStringFromName("cmd_addFoundEngine", [
+      button.setAttribute("data-l10n-id", "search-one-offs-add-engine");
-        engine.title,
+      button.setAttribute(
-      ]);
+        "data-l10n-args",
-      button.setAttribute("label", label);
+        JSON.stringify({ engineName: engine.title })
+      );
       button.setAttribute("crop", "end");
       button.setAttribute("tooltiptext", engine.title + "\n" + engine.uri);
       button.setAttribute("uri", engine.uri);
-      button.setAttribute("title", engine.title);
+      button.setAttribute("engine-name", engine.title);
       if (engine.icon) {
         button.setAttribute("image", engine.icon);
       }

browser/components/search/content/contentSearchUI.js

@@ -993,7 +993,7 @@ this.ContentSearchUIController = (function() {
         );
         button.appendChild(img);
         button.style.width = buttonWidth + "px";
-        button.setAttribute("title", engine.name);
+        button.setAttribute("engine-name", engine.name);
 
         button.engineName = engine.name;
         button.addEventListener("click", this);

browser/components/search/test/browser/browser_tooManyEnginesOffered.js

@@ -5,7 +5,6 @@
 // itself.
 
 const searchPopup = document.getElementById("PopupSearchAutoComplete");
-const oneOffsContainer = searchPopup.searchOneOffsContainer;
 
 add_task(async function test_setup() {
   await gCUITestUtils.addSearchBar();
@@ -27,6 +26,11 @@ add_task(async function test() {
   let promise = promiseEvent(searchPopup, "popupshown");
   info("Opening search panel");
   searchbar.focus();
+  // In TV we may try opening too early, when the searchbar is not ready yet.
+  await TestUtils.waitForCondition(
+    () => BrowserSearch.searchBar.textbox.controller.input,
+    "Wait for the searchbar controller to connect"
+  );
   EventUtils.synthesizeKey("KEY_ArrowDown");
   await promise;
 
@@ -35,6 +39,8 @@ add_task(async function test() {
   Assert.equal(items.length, 1, "A single button");
   let menuButton = items[0];
   Assert.equal(menuButton.type, "menu", "A menu button");
+  await document.l10n.translateElements([menuButton]);
+  Assert.equal(menuButton.label, "Add search engine");
 
   // Mouse over the menu button to open it.
   let buttonPopup = menuButton.menupopup;
@@ -49,7 +55,7 @@ add_task(async function test() {
   for (let i = 0; i < buttonPopup.children.length; i++) {
     let item = buttonPopup.children[i];
     Assert.equal(
-      item.getAttribute("title"),
+      item.getAttribute("engine-name"),
       "engine" + (i + 1),
       "Expected engine title"
     );
@@ -91,7 +97,7 @@ add_task(async function test() {
 function getOpenSearchItems() {
   let os = [];
 
-  let addEngineList = oneOffsContainer.querySelector(".search-add-engines");
+  let addEngineList = searchPopup.oneOffButtons.addEngines;
   for (
     let item = addEngineList.firstElementChild;
     item;

browser/components/urlbar/UrlbarInput.jsm

@@ -3575,10 +3575,6 @@ class AddSearchEngineHelper {
   constructor(input) {
     this.input = input;
     this.shortcutButtons = input.view.oneOffSearchButtons;
-
-    XPCOMUtils.defineLazyGetter(this, "_bundle", () =>
-      Services.strings.createBundle("chrome://browser/locale/search.properties")
-    );
   }
 
   /**
@@ -3605,9 +3601,6 @@ class AddSearchEngineHelper {
       this.shortcutButtons.updateWebEngines(
         engines.slice(0, this.maxInlineEngines).map(e => ({
           name: e.title,
-          tooltip: this._bundle.formatStringFromName("cmd_addFoundEngine", [
-            e.title,
-          ]),
           uri: e.uri,
           get icon() {
             // The icon is actually the browser favicon, that may not be in
@@ -3635,9 +3628,10 @@ class AddSearchEngineHelper {
     elt.setAttribute("anonid", `add-engine-${index}`);
     elt.classList.add("menuitem-iconic");
     elt.classList.add("context-menu-add-engine");
+    elt.setAttribute("data-l10n-id", "search-one-offs-add-engine");
     elt.setAttribute(
-      "label",
+      "data-l10n-args",
-      this._bundle.formatStringFromName("cmd_addFoundEngine", [engine.title])
+      JSON.stringify({ engineName: engine.title })
     );
     elt.setAttribute("uri", engine.uri);
     if (engine.icon) {
@@ -3654,10 +3648,7 @@ class AddSearchEngineHelper {
     elt.setAttribute("anonid", "add-engine-menu");
     elt.classList.add("menu-iconic");
     elt.classList.add("context-menu-add-engine");
-    elt.setAttribute(
+    elt.setAttribute("data-l10n-id", "search-one-offs-add-engine-menu");
-      "label",
-      this._bundle.GetStringFromName("cmd_addFoundEngineMenu")
-    );
     if (engine.icon) {
       elt.setAttribute("image", engine.icon);
     }

browser/components/urlbar/UrlbarSearchOneOffs.jsm

@@ -348,9 +348,13 @@ class UrlbarSearchOneOffs extends SearchOneOffs {
         if (engine.icon) {
           button.setAttribute("image", engine.icon);
         }
-        button.setAttribute("title", engine.name);
+        button.setAttribute("data-l10n-id", "search-one-offs-add-engine");
+        button.setAttribute(
+          "data-l10n-args",
+          JSON.stringify({ engineName: engine.name })
+        );
+        button.setAttribute("engine-name", engine.name);
         button.setAttribute("uri", engine.uri);
-        button.setAttribute("tooltiptext", engine.tooltip);
         this.buttons.appendChild(button);
       }
     }

browser/components/urlbar/tests/UrlbarTestUtils.jsm

@@ -407,6 +407,8 @@ var UrlbarTestUtils = {
       win
     );
     await openPromise;
+    // On Mac sometimes the menuitems are not ready.
+    await new Promise(win.requestAnimationFrame);
     try {
       await task(cxmenu);
     } finally {

browser/components/urlbar/tests/browser-proton/browser_add_search_engine.js

@@ -40,6 +40,7 @@ add_task(async function context_one() {
 
       elt = popup.parentNode.getMenuItem("add-engine-0");
       Assert.ok(BrowserTestUtils.is_visible(elt));
+      await document.l10n.translateElements([elt]);
       Assert.ok(elt.label.includes("add_search_engine_0"));
       Assert.ok(elt.hasAttribute("image"));
       Assert.equal(
@@ -66,7 +67,7 @@ add_task(async function context_one() {
     let engine = await Services.search.getEngineByName("add_search_engine_0");
     await Services.search.removeEngine(engine);
 
-    await UrlbarTestUtils.withContextMenu(window, popup => {
+    await UrlbarTestUtils.withContextMenu(window, async popup => {
       info("The separator and the add engine item should be present again.");
       let elt = popup.parentNode.getMenuItem("add-engine-separator");
       Assert.ok(BrowserTestUtils.is_visible(elt));
@@ -76,6 +77,7 @@ add_task(async function context_one() {
 
       elt = popup.parentNode.getMenuItem("add-engine-0");
       Assert.ok(BrowserTestUtils.is_visible(elt));
+      await document.l10n.translateElements([elt]);
       Assert.ok(elt.label.includes("add_search_engine_0"));
     });
   });
@@ -97,6 +99,7 @@ add_task(async function context_invalid() {
 
       let elt = popup.parentNode.getMenuItem("add-engine-0");
       Assert.ok(BrowserTestUtils.is_visible(elt));
+      await document.l10n.translateElements([elt]);
       Assert.ok(elt.label.includes("add_search_engine_404"));
       Assert.equal(
         elt.getAttribute("uri"),
@@ -138,6 +141,7 @@ add_task(async function context_same_name() {
 
       elt = popup.parentNode.getMenuItem("add-engine-0");
       Assert.ok(BrowserTestUtils.is_visible(elt));
+      await document.l10n.translateElements([elt]);
       Assert.ok(elt.label.includes("add_search_engine_0"));
     });
   });
@@ -156,9 +160,11 @@ add_task(async function context_two() {
 
       elt = popup.parentNode.getMenuItem("add-engine-0");
       Assert.ok(BrowserTestUtils.is_visible(elt));
+      await document.l10n.translateElements([elt]);
       Assert.ok(elt.label.includes("add_search_engine_0"));
       elt = popup.parentNode.getMenuItem("add-engine-1");
       Assert.ok(BrowserTestUtils.is_visible(elt));
+      await document.l10n.translateElements([elt]);
       Assert.ok(elt.label.includes("add_search_engine_1"));
     });
   });
@@ -216,6 +222,7 @@ add_task(async function context_many() {
         let elt = popup.parentNode.getMenuItem(`add-engine-${i}`);
         Assert.equal(elt.parentNode, popup);
         Assert.ok(BrowserTestUtils.is_visible(elt));
+        await document.l10n.translateElements([elt]);
         Assert.ok(elt.label.includes(`add_search_engine_${i + 1}`));
       }
     });
@@ -273,6 +280,7 @@ add_task(async function context_after_customize() {
 
       elt = popup.parentNode.getMenuItem("add-engine-0");
       Assert.ok(BrowserTestUtils.is_visible(elt));
+      await document.l10n.translateElements([elt]);
       Assert.ok(elt.label.includes("add_search_engine_0"));
     });
 
@@ -296,6 +304,7 @@ add_task(async function context_after_customize() {
 
       elt = popup.parentNode.getMenuItem("add-engine-0");
       Assert.ok(BrowserTestUtils.is_visible(elt));
+      await document.l10n.translateElements([elt]);
       Assert.ok(elt.label.includes("add_search_engine_0"));
     });
   });

browser/components/urlbar/tests/browser-proton/browser_shortcuts_add_search_engine.js

@@ -93,10 +93,13 @@ async function do_test_shortcuts(activateTask) {
     for (let button of addEngineButtons) {
       Assert.ok(BrowserTestUtils.is_visible(button));
       Assert.ok(button.hasAttribute("image"));
+      await document.l10n.translateElements([button]);
       Assert.ok(
         button.getAttribute("tooltiptext").includes("add_search_engine_")
       );
-      Assert.ok(button.getAttribute("title").startsWith("add_search_engine_"));
+      Assert.ok(
+        button.getAttribute("engine-name").startsWith("add_search_engine_")
+      );
       Assert.ok(
         button.classList.contains("searchbar-engine-one-off-add-engine")
       );
@@ -125,7 +128,7 @@ async function do_test_shortcuts(activateTask) {
       "Check there's one button to add engines"
     );
     Assert.equal(
-      addEngineButtons[0].getAttribute("title"),
+      addEngineButtons[0].getAttribute("engine-name"),
       "add_search_engine_1"
     );
     let installedEngineButton = addEngineButtons[0].previousElementSibling;

browser/locales/en-US/browser/browser.ftl

@@ -247,6 +247,19 @@ search-one-offs-context-set-as-default-private =
 search-one-offs-engine-with-alias =
     .tooltiptext = { $engineName } ({ $alias })
 
+# Shown when adding new engines from the address bar shortcut buttons or context
+# menu, or from the search bar shortcut buttons.
+# Variables:
+#  $engineName (String): The name of the engine.
+search-one-offs-add-engine =
+    .label = Add “{ $engineName }”
+    .tooltiptext = Add search engine “{ $engineName }”
+    .aria-label = Add search engine “{ $engineName }”
+# When more than 5 engines are offered by a web page, they are grouped in a
+# submenu using this as its label.
+search-one-offs-add-engine-menu =
+    .label = Add search engine
+
 ## Local search mode one-off buttons
 ## Variables:
 ##  $restrict (String): The restriction token corresponding to the search mode.

browser/locales/en-US/chrome/browser/search.properties

@@ -16,16 +16,6 @@ cmd_pasteAndSearch=Paste & Search
 cmd_clearHistory=Clear Search History
 cmd_clearHistory_accesskey=H
 
-# LOCALIZATION NOTE (cmd_addFoundEngine): %S is replaced by the name of
-# a search engine offered by a web page. Each engine is displayed as a
-# menuitem at the bottom of the search panel.
-cmd_addFoundEngine=Add “%S”
-# LOCALIZATION NOTE (cmd_addFoundEngineMenu): When more than 5 engines
-# are offered by a web page, instead of listing all of them in the
-# search panel using the cmd_addFoundEngine string, they will be
-# grouped in a submenu using cmd_addFoundEngineMenu as a label.
-cmd_addFoundEngineMenu=Add search engine
-
 searchAddFoundEngine2=Add Search Engine
 
 # LOCALIZATION NOTE (searchForSomethingWith2):

python/l10n/fluent_migrations/bug_1706334_add_search_engine.py

@@ -0,0 +1,22 @@
+# coding=utf8
+
+# Any copyright is dedicated to the Public Domain.
+# http://creativecommons.org/publicdomain/zero/1.0/
+
+from __future__ import absolute_import
+from fluent.migrate.helpers import transforms_from
+
+
+def migrate(ctx):
+    """Bug 1706334 - Improving accessible text on Add Engine items, part {index}."""
+    ctx.add_transforms(
+        "browser/browser/browser.ftl",
+        "browser/browser/browser.ftl",
+        transforms_from(
+            """
+search-one-offs-add-engine-menu =
+    .label = { COPY(from_path, "cmd_addFoundEngineMenu") }
+""",
+            from_path="browser/chrome/browser/search.properties",
+        ),
+    )