Bug 1190366 - Fix accessibility labels for results in the urlbar. r=mak
authorDrew Willcoxon <adw@mozilla.com>
Sat, 08 Aug 2015 15:16:08 +0200
changeset 288599 8cc0bb696585f5d8c2babf8dd029acf87c59d343
parent 288598 61e20d581a287fe0673feb058aa0b119248a5f59
child 288600 9dccd6cceff0ae5963ece393805b5c9f67312c4d
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1190366
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1190366 - Fix accessibility labels for results in the urlbar. r=mak
browser/base/content/test/general/browser_autocomplete_a11y_label.js
browser/base/content/test/general/browser_urlbarSearchSuggestionsNotification.js
browser/base/content/test/general/head.js
browser/base/content/urlbarBindings.xml
toolkit/content/widgets/autocomplete.xml
--- a/browser/base/content/test/general/browser_autocomplete_a11y_label.js
+++ b/browser/base/content/test/general/browser_autocomplete_a11y_label.js
@@ -1,26 +1,68 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
-add_task(function*() {
+const UNIFIEDCOMPLETE_PREF = "browser.urlbar.unifiedcomplete";
+const SUGGEST_ALL_PREF = "browser.search.suggest.enabled";
+const SUGGEST_URLBAR_PREF = "browser.urlbar.suggest.searches";
+const TEST_ENGINE_BASENAME = "searchSuggestionEngine.xml";
+
+add_task(function* prepare() {
   // This test is only relevant if UnifiedComplete is enabled.
-  let ucpref = Services.prefs.getBoolPref("browser.urlbar.unifiedcomplete");
-  Services.prefs.setBoolPref("browser.urlbar.unifiedcomplete", true);
+  Services.prefs.setBoolPref(UNIFIEDCOMPLETE_PREF, true);
   registerCleanupFunction(() => {
-    Services.prefs.setBoolPref("browser.urlbar.unifiedcomplete", ucpref);
+    Services.prefs.clearUserPref(UNIFIEDCOMPLETE_PREF);
   });
+});
 
+add_task(function* switchToTab() {
   let tab = gBrowser.addTab("about:about");
   yield promiseTabLoaded(tab);
 
   let actionURL = makeActionURI("switchtab", {url: "about:about"}).spec;
   yield promiseAutocompleteResultPopup("% about");
 
   ok(gURLBar.popup.richlistbox.children.length > 1, "Should get at least 2 results");
   let result = gURLBar.popup.richlistbox.children[1];
   is(result.getAttribute("type"), "action switchtab", "Expect right type attribute");
-  is(result.label, "about:about " + actionURL + " Tab", "Result a11y label should be as expected");
+  is(result.label, "about:about about:about Tab", "Result a11y label should be: <title> <url> Tab");
 
   gURLBar.popup.hidePopup();
   yield promisePopupHidden(gURLBar.popup);
   gBrowser.removeTab(tab);
 });
+
+add_task(function* searchSuggestions() {
+  let engine = yield promiseNewSearchEngine(TEST_ENGINE_BASENAME);
+  let oldCurrentEngine = Services.search.currentEngine;
+  Services.search.currentEngine = engine;
+  Services.prefs.setBoolPref(SUGGEST_ALL_PREF, true);
+  Services.prefs.setBoolPref(SUGGEST_URLBAR_PREF, true);
+  registerCleanupFunction(function () {
+    Services.search.currentEngine = oldCurrentEngine;
+    Services.prefs.clearUserPref(SUGGEST_ALL_PREF);
+    Services.prefs.clearUserPref(SUGGEST_URLBAR_PREF);
+  });
+
+  yield promiseAutocompleteResultPopup("foo");
+  // Don't assume that the search doesn't match history or bookmarks left around
+  // by earlier tests.
+  Assert.ok(gURLBar.popup.richlistbox.children.length >= 3,
+            "Should get at least heuristic result + two search suggestions");
+  // The first expected search is the search term itself since the heuristic
+  // result will come before the search suggestions.
+  let expectedSearches = [
+    "foo",
+    "foofoo",
+    "foobar",
+  ];
+  for (let child of gURLBar.popup.richlistbox.children) {
+    if (child.getAttribute("type").split(/\s+/).indexOf("searchengine") >= 0) {
+      Assert.ok(expectedSearches.length > 0);
+      let suggestion = expectedSearches.shift();
+      Assert.equal(child.label, suggestion + " browser_searchSuggestionEngine searchSuggestionEngine.xml Search",
+                   "Result label should be: <search term> <engine name> Search");
+    }
+  }
+  Assert.ok(expectedSearches.length == 0);
+  gURLBar.closePopup();
+});
--- a/browser/base/content/test/general/browser_urlbarSearchSuggestionsNotification.js
+++ b/browser/base/content/test/general/browser_urlbarSearchSuggestionsNotification.js
@@ -1,16 +1,16 @@
 const SUGGEST_ALL_PREF = "browser.search.suggest.enabled";
 const SUGGEST_URLBAR_PREF = "browser.urlbar.suggest.searches";
 const CHOICE_PREF = "browser.urlbar.userMadeSearchSuggestionsChoice";
 const TEST_ENGINE_BASENAME = "searchSuggestionEngine.xml";
 
 // Must run first.
 add_task(function* prepare() {
-  let engine = yield promiseNewEngine(TEST_ENGINE_BASENAME);
+  let engine = yield promiseNewSearchEngine(TEST_ENGINE_BASENAME);
   let oldCurrentEngine = Services.search.currentEngine;
   Services.search.currentEngine = engine;
   registerCleanupFunction(function () {
     Services.search.currentEngine = oldCurrentEngine;
     Services.prefs.clearUserPref(SUGGEST_ALL_PREF);
     Services.prefs.clearUserPref(SUGGEST_URLBAR_PREF);
 
     // Disable the notification for future tests so it doesn't interfere with
@@ -145,35 +145,16 @@ function assertSuggestionsPresent(expect
 }
 
 function assertVisible(visible) {
   let style =
     window.getComputedStyle(gURLBar.popup.searchSuggestionsNotification);
   Assert.equal(style.visibility, visible ? "visible" : "collapse");
 }
 
-function promiseNewEngine(basename) {
-  return new Promise((resolve, reject) => {
-    info("Waiting for engine to be added: " + basename);
-    let url = getRootDirectory(gTestPath) + basename;
-    Services.search.addEngine(url, Ci.nsISearchEngine.TYPE_MOZSEARCH, "",
-                              false, {
-      onSuccess: function (engine) {
-        info("Search engine added: " + basename);
-        registerCleanupFunction(() => Services.search.removeEngine(engine));
-        resolve(engine);
-      },
-      onError: function (errCode) {
-        Assert.ok(false, "addEngine failed with error code " + errCode);
-        reject();
-      },
-    });
-  });
-}
-
 function promiseTransition() {
   return new Promise(resolve => {
     gURLBar.popup.addEventListener("transitionend", function onEnd() {
       gURLBar.popup.removeEventListener("transitionend", onEnd, true);
       // The urlbar needs to handle the transitionend first, but that happens
       // naturally since promises are resolved at the end of the current tick.
       resolve();
     }, true);
--- a/browser/base/content/test/general/head.js
+++ b/browser/base/content/test/general/head.js
@@ -937,8 +937,27 @@ function promiseTopicObserved(aTopic)
   return new Promise((resolve) => {
     Services.obs.addObserver(
       function PTO_observe(aSubject, aTopic, aData) {
         Services.obs.removeObserver(PTO_observe, aTopic);
         resolve({subject: aSubject, data: aData});
       }, aTopic, false);
   });
 }
+
+function promiseNewSearchEngine(basename) {
+  return new Promise((resolve, reject) => {
+    info("Waiting for engine to be added: " + basename);
+    let url = getRootDirectory(gTestPath) + basename;
+    Services.search.addEngine(url, Ci.nsISearchEngine.TYPE_MOZSEARCH, "",
+                              false, {
+      onSuccess: function (engine) {
+        info("Search engine added: " + basename);
+        registerCleanupFunction(() => Services.search.removeEngine(engine));
+        resolve(engine);
+      },
+      onError: function (errCode) {
+        Assert.ok(false, "addEngine failed with error code " + errCode);
+        reject();
+      },
+    });
+  });
+}
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -1788,30 +1788,49 @@ file, You can obtain one at http://mozil
             // respect the usual clicking subtleties
             openUILink(url, aEvent, options);
           }
         ]]>
         </body>
       </method>
 
       <method name="createResultLabel">
-        <parameter name="aTitle"/>
-        <parameter name="aUrl"/>
-        <parameter name="aTypes"/>
+        <parameter name="item"/>
+        <parameter name="proposedLabel"/>
         <body>
           <![CDATA[
-            let label = aTitle + " " + aUrl;
-            let type = aTypes.find(v => v != "action" && v != "heuristic");
+            let parts = [proposedLabel];
+
+            let action = this.mInput._parseActionUrl(item.getAttribute("url"));
+            if (action) {
+              switch (action.type) {
+              case "searchengine":
+                parts = [
+                  action.params.searchSuggestion || action.params.searchQuery,
+                  action.params.engineName,
+                ];
+                break;
+              case "switchtab":
+                parts = [
+                  item.getAttribute("title"),
+                  action.params.url,
+                ];
+                break;
+              }
+            }
+
+            let types = item.getAttribute("type").split(/\s+/);
+            let type = types.find(type => type != "action" && type != "heuristic");
             try {
               // Some types intentionally do not map to strings, which is not
               // an error.
-              label += " " + this._bundle.GetStringFromName(type + "ResultLabel");
+              parts.push(this._bundle.GetStringFromName(type + "ResultLabel"));
             } catch (e) {}
 
-            return label;
+            return parts.filter(str => str).join(" ");
           ]]>
         </body>
       </method>
 
       <method name="onResultsAdded">
         <body>
           <![CDATA[
             if (!Services.prefs.getBoolPref("browser.urlbar.unifiedcomplete"))
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -1381,28 +1381,30 @@ extends="chrome://global/content/binding
       </constructor>
 
       <property name="label" readonly="true">
         <getter>
           <![CDATA[
             // This property is a string that is read aloud by screen readers,
             // so it must not contain anything that should not be user-facing.
 
-            var title = this.getAttribute("title");
-            var url = this.getAttribute("url");
-            var panel = this.parentNode.parentNode;
+            let parts = [
+              this.getAttribute("title"),
+              this.getAttribute("url"),
+            ];
+            let label = parts.filter(str => str).join(" ")
 
             // allow consumers that have extended popups to override
             // the label values for the richlistitems
+            let panel = this.parentNode.parentNode;
             if (panel.createResultLabel) {
-              let types = this.getAttribute("type").split(/\s+/);
-              return panel.createResultLabel(title, url, types);
+              return panel.createResultLabel(this, label);
             }
 
-            return title + " " + url;
+            return label;
           ]]>
         </getter>
       </property>
 
       <property name="_stringBundle">
         <getter><![CDATA[
           if (!this.__stringBundle) {
             this.__stringBundle = Services.strings.createBundle("chrome://global/locale/autocomplete.properties");