Bug 1297976 - Fix accessibility for the unified one-off search buttons in the urlbar and searchbar. r=florian
authorDrew Willcoxon <adw@mozilla.com>
Tue, 13 Sep 2016 17:48:15 -0700
changeset 355190 24cefbae2be9111f84604e18fbacfd2548ac26ed
parent 355189 3a61bfe089b2b2cbef1b0cad0b664c973773a0fb
child 355191 8bce29dda76e4e5d09eb7d91ef4e335629d6861f
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersflorian
bugs1297976
milestone51.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 1297976 - Fix accessibility for the unified one-off search buttons in the urlbar and searchbar. r=florian MozReview-Commit-ID: 1rWsC0eGVkG
browser/components/search/content/search.xml
--- a/browser/components/search/content/search.xml
+++ b/browser/components/search/content/search.xml
@@ -1061,16 +1061,17 @@
       </xul:deck>
       <xul:description anonid="search-panel-one-offs"
                        role="group"
                        class="search-panel-one-offs"
                        xbl:inherits="compact">
         <xul:button anonid="search-settings-compact"
                     oncommand="showSettings();"
                     class="searchbar-engine-one-off-item search-setting-button-compact"
+                    aria-label="&changeSearchSettings.button;"
                     xbl:inherits="compact"/>
       </xul:description>
       <xul:vbox anonid="add-engines"/>
       <xul:button anonid="search-settings"
                   oncommand="showSettings();"
                   class="search-setting-button search-panel-header"
                   label="&changeSearchSettings.button;"
                   xbl:inherits="compact"/>
@@ -1372,17 +1373,18 @@
           // could effectively break the urlbar popup by offering a ton of
           // engines.  We should probably make a smaller version of the buttons
           // for compact one-offs.
           if (!this.compact) {
             for (let engine of gBrowser.selectedBrowser.engines || []) {
               let button = document.createElementNS(kXULNS, "button");
               let label = this.bundle.formatStringFromName("cmd_addFoundEngine",
                                                            [engine.title], 1);
-              button.id = "searchbar-add-engine-" + engine.title.replace(/ /g, '-');
+              button.id = this.telemetryOrigin + "-add-engine-" +
+                          engine.title.replace(/ /g, '-');
               button.setAttribute("class", "addengine-item");
               button.setAttribute("label", label);
               button.setAttribute("pack", "start");
 
               button.setAttribute("crop", "end");
               button.setAttribute("tooltiptext", engine.uri);
               button.setAttribute("uri", engine.uri);
               if (engine.icon) {
@@ -1437,19 +1439,22 @@
           // of the suggestion <tree> to be hidden.
           let oneOffCount = engines.length;
           if (this.compact)
             ++oneOffCount;
           let rowCount = Math.ceil(oneOffCount / enginesPerRow);
           let height = rowCount * 33; // 32px per row, 1px border.
           list.setAttribute("height", height + "px");
 
-          // Ensure we can refer to the settings button by ID:
+          // Ensure we can refer to the settings buttons by ID:
           let settingsEl = document.getAnonymousElementByAttribute(this, "anonid", "search-settings");
-          settingsEl.id = this.id + "-anon-search-settings";
+          settingsEl.id = this.telemetryOrigin + "-anon-search-settings";
+          let compactSettingsEl = document.getAnonymousElementByAttribute(this, "anonid", "search-settings-compact");
+          compactSettingsEl.id = this.telemetryOrigin +
+                                 "-anon-search-settings-compact";
 
           let dummyItems = enginesPerRow - (oneOffCount % enginesPerRow || enginesPerRow);
           for (let i = 0; i < engines.length; ++i) {
             let engine = engines[i];
             let button = document.createElementNS(kXULNS, "button");
             button.id = this._buttonIDForEngine(engine);
             let uri = "chrome://browser/skin/search-engine-placeholder.png";
             if (engine.iconURI) {
@@ -1500,17 +1505,17 @@
             }
           }
         ]]></body>
       </method>
 
       <method name="_buttonIDForEngine">
         <parameter name="engine"/>
         <body><![CDATA[
-          return "searchbar-engine-one-off-item-" +
+          return this.telemetryOrigin + "-engine-one-off-item-" +
                  engine.name.replace(/ /g, '-');
         ]]></body>
       </method>
 
       <method name="_buttonForEngine">
         <parameter name="engine"/>
         <body><![CDATA[
           return document.getElementById(this._buttonIDForEngine(engine));
@@ -1537,21 +1542,25 @@
                 document.getAnonymousElementByAttribute(this, "anonid",
                                                         "searchbar-oneoffheader-engine");
               header.selectedIndex = 2;
               headerEngineText.value = val.engine.name;
             }
             else {
               header.selectedIndex = this.query ? 1 : 0;
             }
-            this.setAttribute("aria-activedescendant", val.id);
+            if (this.textbox) {
+              this.textbox.setAttribute("aria-activedescendant", val.id);
+            }
           } else {
             val = null;
             header.selectedIndex = this.query ? 1 : 0;
-            this.removeAttribute("aria-activedescendant");
+            if (this.textbox) {
+              this.textbox.removeAttribute("aria-activedescendant");
+            }
           }
 
           if (aUpdateLogicallySelectedButton) {
             this._selectedButton = val;
             if (val && !val.engine) {
               // If the button doesn't have an engine, then clear the popup's
               // selection to indicate that pressing Return while the button is
               // selected will do the button's command, not search.
@@ -1767,20 +1776,22 @@
           else if (event.keyCode == Ci.nsIDOMKeyEvent.DOM_VK_UP) {
             if (numListItems > 0) {
               if (this.popup.selectedIndex > 0) {
                 // The autocomplete controller should handle this case.
               } else if (this.popup.selectedIndex == 0) {
                 if (!allowEmptySelection) {
                   // Wrap around the selection to the last one-off.
                   this.selectedButton = null;
-                  stopEvent = this.advanceSelection(false, true, true);
-                  if (stopEvent) {
-                    this.popup.selectedIndex = -1;
-                  }
+                  this.popup.selectedIndex = -1;
+                  // Call advanceSelection after setting selectedIndex so that
+                  // screen readers see the newly selected one-off. Both trigger
+                  // accessibility events.
+                  this.advanceSelection(false, true, true);
+                  stopEvent = true;
                 }
               } else {
                 let firstButtonSelected =
                   this.selectedButton &&
                   this.selectedButton == this.getSelectableButtons(true)[0];
                 if (firstButtonSelected) {
                   this.selectedButton = null;
                 } else {
@@ -1794,26 +1805,27 @@
 
           else if (event.keyCode == Ci.nsIDOMKeyEvent.DOM_VK_DOWN) {
             if (numListItems > 0) {
               if (this.popup.selectedIndex >= 0 &&
                   this.popup.selectedIndex < numListItems - 1) {
                 // The autocomplete controller should handle this case.
               } else if (this.popup.selectedIndex == numListItems - 1) {
                 this.selectedButton = null;
-                stopEvent = this.advanceSelection(true, true, true);
-                if (stopEvent) {
-                  stopEvent = !allowEmptySelection;
-                  if (this.textbox && typeof(textboxUserValue) == "string") {
-                    this.textbox.value = textboxUserValue;
-                  }
-                  if (!allowEmptySelection) {
-                    this.popup.selectedIndex = -1;
-                  }
+                if (!allowEmptySelection) {
+                  this.popup.selectedIndex = -1;
+                  stopEvent = true;
                 }
+                if (this.textbox && typeof(textboxUserValue) == "string") {
+                  this.textbox.value = textboxUserValue;
+                }
+                // Call advanceSelection after setting selectedIndex so that
+                // screen readers see the newly selected one-off. Both trigger
+                // accessibility events.
+                this.advanceSelection(true, true, true);
               } else {
                 let buttons = this.getSelectableButtons(true);
                 let lastButtonSelected =
                   this.selectedButton &&
                   this.selectedButton == buttons[buttons.length - 1];
                 if (lastButtonSelected) {
                   this.selectedButton = null;
                   stopEvent = allowEmptySelection;