Bug 1102038 - The 'Change Search Settings' button and the open search items cannot be used via the keyboard. r=Mossop, a=lmandel
authorFlorian Quèze <florian@queze.net>
Thu, 05 Feb 2015 00:08:19 +0100
changeset 249778 8129c046754162de198217124ce02e9044912eec
parent 249777 da3e1d683106c33d689c30893fbc8e417f926f01
child 249779 8c0692b67d2d7611d4a249c8898730f8d284e052
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMossop, lmandel
bugs1102038
milestone37.0a2
Bug 1102038 - The 'Change Search Settings' button and the open search items cannot be used via the keyboard. r=Mossop, a=lmandel
browser/base/content/urlbarBindings.xml
browser/components/search/content/search.xml
browser/components/search/test/browser.ini
browser/components/search/test/browser_searchbar_keyboard_navigation.js
browser/components/search/test/opensearch.html
browser/themes/linux/searchbar.css
browser/themes/osx/searchbar.css
browser/themes/windows/searchbar.css
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -1236,24 +1236,34 @@
 
       <handler event="mousedown"><![CDATA[
         // Required to receive click events from the buttons on Linux.
         event.preventDefault();
       ]]></handler>
 
       <handler event="mouseover"><![CDATA[
         let target = event.originalTarget;
-        if (target.localName == "button" &&
-            target.classList.contains("searchbar-engine-one-off-item") &&
-            !target.classList.contains("dummy")) {
-          let list = document.getAnonymousElementByAttribute(this, "anonid",
-                                                             "search-panel-one-offs")
-          for (let button = list.firstChild; button; button = button.nextSibling)
-            button.removeAttribute("selected");
-        }
+        if (target.localName != "button")
+          return;
+
+        if ((target.classList.contains("searchbar-engine-one-off-item") &&
+             !target.classList.contains("dummy")) ||
+            target.classList.contains("addengine-item") ||
+            target.classList.contains("search-setting-button"))
+          document.getElementById("searchbar").textbox.selectedButton = target;
+      ]]></handler>
+
+      <handler event="mouseout"><![CDATA[
+        let target = event.originalTarget;
+        if (target.localName != "button")
+          return;
+
+        let textbox = document.getElementById("searchbar").textbox;
+        if (textbox.selectedButton == target)
+          textbox.selectedButton = null;
       ]]></handler>
 
       <handler event="click"><![CDATA[
         if (event.button == 2)
           return; // ignore right clicks.
 
         let button = event.originalTarget;
         let engine = button.engine || button.parentNode.engine;
--- a/browser/components/search/content/search.xml
+++ b/browser/components/search/content/search.xml
@@ -980,18 +980,24 @@
       <!-- override |onTextEntered| in autocomplete.xml -->
       <method name="onTextEntered">
         <parameter name="aEvent"/>
         <body><![CDATA[
           var evt = aEvent || this.mEnterEvent;
 
           let engine;
           let oneOff = this.selectedButton;
-          if (oneOff)
+          if (oneOff) {
+            if (!oneOff.engine) {
+              oneOff.doCommand();
+              this.mEnterEvent = null;
+              return;
+            }
             engine = oneOff.engine;
+          }
           if (this.mEnterEvent && this._selectionDetails &&
               this._selectionDetails.currentIndex != -1) {
             BrowserSearch.searchBar.telemetrySearchDetails = this._selectionDetails;
             this._selectionDetails = null;
           }
           document.getBindingParent(this).handleSearchCommand(evt, engine);
 
           this.mEnterEvent = null;
@@ -1010,40 +1016,72 @@
             this._selectedButton = val;
             return;
           }
 
           this._selectedButton = null;
         ]]></setter>
       </property>
 
+      <method name="getSelectableButtons">
+        <parameter name="aCycleEngines"/>
+        <body><![CDATA[
+          let buttons = [];
+          let oneOff = document.getAnonymousElementByAttribute(this.popup, "anonid",
+                                                               "search-panel-one-offs");
+          for (oneOff = oneOff.firstChild; oneOff; oneOff = oneOff.nextSibling) {
+            if (oneOff.classList.contains("dummy"))
+              break;
+            buttons.push(oneOff);
+          }
+
+          if (aCycleEngines)
+            return buttons;
+
+          let addEngine =
+            document.getAnonymousElementByAttribute(this.popup, "anonid", "add-engines");
+          for (addEngine = addEngine.firstChild; addEngine; addEngine = addEngine.nextSibling)
+            buttons.push(addEngine);
+
+          buttons.push(document.getAnonymousElementByAttribute(this.popup, "anonid",
+                                                               "search-settings"));
+          return buttons;
+        ]]></body>
+      </method>
+
       <method name="advanceSelection">
         <parameter name="aForward"/>
         <parameter name="aSkipSuggestions"/>
         <parameter name="aCycleEngines"/>
         <body><![CDATA[
           let popup = this.popup;
           let list = document.getAnonymousElementByAttribute(popup, "anonid",
                                                              "search-panel-one-offs");
           let selectedButton = this.selectedButton;
+          let buttons = this.getSelectableButtons(aCycleEngines);
 
           // If the last suggestion is selected, DOWN selects the first button.
           if (!aSkipSuggestions && aForward &&
               popup.selectedIndex + 1 == popup.view.rowCount) {
-            this.selectedButton = list.firstChild;
+            this.selectedButton = buttons[0];
             return false;
           }
 
           // If a one-off is selected and no suggestion is selected (or we skip them)
           if (selectedButton && (popup.selectedIndex == -1 || aSkipSuggestions)) {
             // cycle through one-off buttons.
+            let index = buttons.indexOf(selectedButton);
             if (aForward)
-              this.selectedButton = selectedButton.nextSibling;
+              ++index;
             else
-              this.selectedButton = selectedButton.previousSibling;
+              --index;
+            if (index >= 0 && index < buttons.length)
+              this.selectedButton = buttons[index];
+            else
+              this.selectedButton = null;
 
             if (this.selectedButton || aCycleEngines)
               return true;
 
             // Set the selectedIndex to something that will make
             // handleKeyNavigation (called by autocomplete.xml's onKeyPress
             // method) reset the text field value to what the user typed.
             // Doesn't work when aSkipSuggestions=true, see bug 1124747.
@@ -1052,27 +1090,24 @@
             else
               popup.selectedIndex = popup.view.rowCount;
             return false;
           }
 
           if (!selectedButton) {
             // If no selection, select the first button or ...
             if (aForward && aSkipSuggestions) {
-              this.selectedButton = list.firstChild;
+              this.selectedButton = buttons[0];
               return true;
             }
 
             if (!aForward && (aCycleEngines ||
                               (!aSkipSuggestions && popup.selectedIndex == -1))) {
               // the last button.
-              selectedButton = list.lastChild;
-              while (selectedButton.classList.contains("dummy"))
-                selectedButton = selectedButton.previousSibling;
-              this.selectedButton = selectedButton;
+              this.selectedButton = buttons[buttons.length - 1];
               return true;
             }
           }
 
           return false;
         ]]></body>
       </method>
 
--- a/browser/components/search/test/browser.ini
+++ b/browser/components/search/test/browser.ini
@@ -1,15 +1,16 @@
 [DEFAULT]
 skip-if = buildapp == 'mulet'
 support-files =
   426329.xml
   483086-1.xml
   483086-2.xml
   head.js
+  opensearch.html
   test.html
   testEngine.src
   testEngine.xml
   testEngine_mozsearch.xml
 
 [browser_405664.js]
 [browser_426329.js]
 skip-if = e10s # Bug ?????? - Test uses load event and checks event.target.
--- a/browser/components/search/test/browser_searchbar_keyboard_navigation.js
+++ b/browser/components/search/test/browser_searchbar_keyboard_navigation.js
@@ -16,16 +16,28 @@ function getOneOffs() {
     if (oneOff.classList.contains("dummy"))
       break;
     oneOffs.push(oneOff);
   }
 
   return oneOffs;
 }
 
+function getOpenSearchItems() {
+  let os = [];
+
+  let addEngineList =
+    document.getAnonymousElementByAttribute(searchPopup, "anonid",
+                                            "add-engines");
+  for (let item = addEngineList.firstChild; item; item = item.nextSibling)
+    os.push(item);
+
+  return os;
+}
+
 add_task(function* init() {
   yield promiseNewEngine("testEngine.xml");
 
   // First cleanup the form history in case other tests left things there.
   yield new Promise((resolve, reject) => {
     info("cleanup the search history");
     searchbar.FormHistory.update({op: "remove", fieldname: "searchbar-history"},
                                  {handleCompletion: resolve,
@@ -97,21 +109,29 @@ add_task(function* test_arrows() {
 
   // now cycle through the one-off items, the first one is already selected.
   for (let i = 0; i < oneOffs.length; ++i) {
     is(textbox.selectedButton, oneOffs[i],
        "the one-off button #" + (i + 1) + " should be selected");
     EventUtils.synthesizeKey("VK_DOWN", {});
   }
 
+  is(textbox.selectedButton.getAttribute("anonid"), "search-settings",
+     "the settings item should be selected");
+  EventUtils.synthesizeKey("VK_DOWN", {});
+
   // We should now be back to the initial situation.
   is(searchPopup.selectedIndex, -1, "no suggestion should be selected");
   ok(!textbox.selectedButton, "no one-off button should be selected");
 
   info("now test the up arrow key");
+  EventUtils.synthesizeKey("VK_UP", {});
+  is(textbox.selectedButton.getAttribute("anonid"), "search-settings",
+     "the settings item should be selected");
+
   // cycle through the one-off items, the first one is already selected.
   for (let i = oneOffs.length; i; --i) {
     EventUtils.synthesizeKey("VK_UP", {});
     is(textbox.selectedButton, oneOffs[i - 1],
        "the one-off button #" + i + " should be selected");
   }
 
   // Another press on up should clear the one-off selection and select the
@@ -144,16 +164,20 @@ add_task(function* test_tab() {
   for (let i = 0; i < oneOffs.length; ++i) {
     EventUtils.synthesizeKey("VK_TAB", {});
     is(textbox.selectedButton, oneOffs[i],
        "the one-off button #" + (i + 1) + " should be selected");
   }
   is(searchPopup.selectedIndex, -1, "no suggestion should be selected");
   is(textbox.value, kUserValue, "the textfield value should be unmodified");
 
+  // One more <tab> selects the settings button.
+  EventUtils.synthesizeKey("VK_TAB", {});
+  is(textbox.selectedButton.getAttribute("anonid"), "search-settings",
+     "the settings item should be selected");
 
   // Pressing tab again should close the panel...
   let promise = promiseEvent(searchPopup, "popuphidden");
   EventUtils.synthesizeKey("VK_TAB", {});
   yield promise;
 
   // ... and move the focus out of the searchbox.
   isnot(Services.focus.focusedElement, textbox.inputField,
@@ -165,17 +189,22 @@ add_task(function* test_shift_tab() {
   let promise = promiseEvent(searchPopup, "popupshown");
   info("Opening search panel");
   searchbar.focus();
   yield promise;
 
   let oneOffs = getOneOffs();
   ok(!textbox.selectedButton, "no one-off button should be selected");
 
-  // Press up once to select the last one-off button.
+  // Press up once to select the last button.
+  EventUtils.synthesizeKey("VK_UP", {});
+  is(textbox.selectedButton.getAttribute("anonid"), "search-settings",
+     "the settings item should be selected");
+
+  // Press up again to select the last one-off button.
   EventUtils.synthesizeKey("VK_UP", {});
 
   // Pressing shift+tab should cycle through the one-off items.
   for (let i = oneOffs.length - 1; i >= 0; --i) {
     is(textbox.selectedButton, oneOffs[i],
        "the one-off button #" + (i + 1) + " should be selected");
     if (i)
       EventUtils.synthesizeKey("VK_TAB", {shiftKey: true});
@@ -267,16 +296,19 @@ add_task(function* test_alt_up() {
 
   // another one and the last one-off should be selected.
   EventUtils.synthesizeKey("VK_UP", {altKey: true});
   is(textbox.selectedButton, oneOffs[oneOffs.length - 1],
      "the last one-off button should be selected");
 
   // Cleanup for the next test.
   EventUtils.synthesizeKey("VK_DOWN", {});
+  is(textbox.selectedButton.getAttribute("anonid"), "search-settings",
+     "the settings item should be selected");
+  EventUtils.synthesizeKey("VK_DOWN", {});
   ok(!textbox.selectedButton, "no one-off should be selected anymore");
 });
 
 add_task(function* test_tab_and_arrows() {
   // Check the initial state is as expected.
   ok(!textbox.selectedButton, "no one-off button should be selected");
   is(searchPopup.selectedIndex, -1, "no suggestion should be selected");
   is(textbox.value, kUserValue, "the textfield value should be unmodified");
@@ -325,8 +357,75 @@ add_task(function* test_tab_and_arrows()
      "the second one-off button should be selected");
   is(searchPopup.selectedIndex, -1, "there should still be no selected suggestion");
 
   // Finally close the panel.
   let promise = promiseEvent(searchPopup, "popuphidden");
   searchPopup.hidePopup();
   yield promise;
 });
+
+add_task(function* test_open_search() {
+  let tab = gBrowser.addTab();
+  gBrowser.selectedTab = tab;
+
+  let deferred = Promise.defer();
+  let browser = gBrowser.selectedBrowser;
+  browser.addEventListener("load", function onload() {
+    browser.removeEventListener("load", onload, true);
+    deferred.resolve();
+  }, true);
+
+  let rootDir = getRootDirectory(gTestPath);
+  content.location = rootDir + "opensearch.html";
+
+  yield deferred.promise;
+
+  let promise = promiseEvent(searchPopup, "popupshown");
+  info("Opening search panel");
+  searchbar.focus();
+  yield promise;
+
+  let engines = getOpenSearchItems();
+  is(engines.length, 2, "the opensearch.html page exposes 2 engines")
+
+  // Check that there's initially no selection.
+  is(searchPopup.selectedIndex, -1, "no suggestion should be selected");
+  ok(!textbox.selectedButton, "no button should be selected");
+
+  // Pressing up once selects the setting button...
+  EventUtils.synthesizeKey("VK_UP", {});
+  is(textbox.selectedButton.getAttribute("anonid"), "search-settings",
+     "the settings item should be selected");
+
+  // ...and then pressing up selects open search engines.
+  for (let i = engines.length; i; --i) {
+    EventUtils.synthesizeKey("VK_UP", {});
+    let selectedButton = textbox.selectedButton;
+    is(selectedButton, engines[i - 1],
+       "the engine #" + i + " should be selected");
+    ok(selectedButton.classList.contains("addengine-item"),
+       "the button is themed as an engine item");
+  }
+
+  // Pressing up again should select the last one-off button.
+  EventUtils.synthesizeKey("VK_UP", {});
+  is(textbox.selectedButton, getOneOffs().pop(),
+     "the last one-off button should be selected");
+
+  info("now check that the down key navigates open search items as expected");
+  for (let i = 0; i < engines.length; ++i) {
+    EventUtils.synthesizeKey("VK_DOWN", {});
+    is(textbox.selectedButton, engines[i],
+       "the engine #" + (i + 1) + " should be selected");
+  }
+
+  // Pressing down on the last engine item selects the settings button.
+  EventUtils.synthesizeKey("VK_DOWN", {});
+  is(textbox.selectedButton.getAttribute("anonid"), "search-settings",
+     "the settings item should be selected");
+
+  promise = promiseEvent(searchPopup, "popuphidden");
+  searchPopup.hidePopup();
+  yield promise;
+
+  gBrowser.removeCurrentTab();
+});
new file mode 100644
--- /dev/null
+++ b/browser/components/search/test/opensearch.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="UTF-8">
+<link rel="search" type="application/opensearchdescription+xml" title="engine1" href="http://mochi.test:8888/browser/browser/components/search/test/testEngine.xml">
+<link rel="search" type="application/opensearchdescription+xml" title="engine2" href="http://mochi.test:8888/browser/browser/components/search/test/testEngine_mozsearch.xml">
+</head>
+<body></body>
+</html>
--- a/browser/themes/linux/searchbar.css
+++ b/browser/themes/linux/searchbar.css
@@ -159,17 +159,16 @@ searchbar[oneoffui] .search-go-button:-m
   box-sizing: padding-box;
   border-bottom: 1px solid #ccc;
 }
 
 .searchbar-engine-one-off-item.last-of-row {
   background-image: none;
 }
 
-.searchbar-engine-one-off-item:hover:not(.dummy),
 .searchbar-engine-one-off-item[selected] {
   background-color: Highlight;
   background-image: none;
 }
 
 .searchbar-engine-one-off-item > .button-box {
   border: none;
   padding: 0;
@@ -197,17 +196,17 @@ searchbar[oneoffui] .search-go-button:-m
 .addengine-item > .button-box {
   -moz-box-pack: start;
 }
 
 .addengine-item:first-of-type {
   border-top: 1px solid #ccc;
 }
 
-.addengine-item:hover {
+.addengine-item[selected] {
   background-color: Highlight;
   color: HighlightText;
 }
 
 .addengine-icon {
   width: 16px;
 }
 
@@ -261,12 +260,12 @@ searchbar[oneoffui] .searchbar-engine-im
 .search-setting-button {
   -moz-appearance: none;
   border: none;
   border-top: 1px solid #ccc;
   margin: 0;
   min-height: 32px;
 }
 
-.search-setting-button:hover {
+.search-setting-button[selected] {
   background-color: #d3d3d3;
   border-top-color: #bdbebe;
 }
--- a/browser/themes/osx/searchbar.css
+++ b/browser/themes/osx/searchbar.css
@@ -182,17 +182,16 @@ searchbar[oneoffui] .search-go-button:-m
   box-sizing: padding-box;
   border-bottom: 1px solid #ccc;
 }
 
 .searchbar-engine-one-off-item.last-of-row {
   background-image: none;
 }
 
-.searchbar-engine-one-off-item:hover:not(.dummy),
 .searchbar-engine-one-off-item[selected] {
   background-color: Highlight;
   background-image: none;
 }
 
 .searchbar-engine-one-off-item > .button-box > .button-text {
   display: none;
 }
@@ -214,17 +213,17 @@ searchbar[oneoffui] .search-go-button:-m
 .addengine-item > .button-box {
   -moz-box-pack: start;
 }
 
 .addengine-item:first-of-type {
   border-top: 1px solid #ccc;
 }
 
-.addengine-item:hover {
+.addengine-item[selected] {
   background-color: Highlight;
   color: HighlightText;
 }
 
 .addengine-icon {
   width: 16px;
 }
 
@@ -287,12 +286,12 @@ searchbar[oneoffui] .searchbar-engine-bu
   border-radius: 0 0 4px 4px;
   min-height: 32px;
 }
 
 .search-setting-button[showonlysettings] {
   border-radius: 4px;
 }
 
-.search-setting-button:hover {
+.search-setting-button[selected] {
   background-color: #d3d3d3;
   border-top-color: #bdbebe;
 }
--- a/browser/themes/windows/searchbar.css
+++ b/browser/themes/windows/searchbar.css
@@ -173,17 +173,16 @@ searchbar[oneoffui] .search-go-button:-m
   box-sizing: padding-box;
   border-bottom: 1px solid #ccc;
 }
 
 .searchbar-engine-one-off-item.last-of-row {
   background-image: none;
 }
 
-.searchbar-engine-one-off-item:hover:not(.dummy),
 .searchbar-engine-one-off-item[selected] {
   background-color: Highlight;
   background-image: none;
 }
 
 .searchbar-engine-one-off-item > .button-box {
   border: none;
   padding: 0 0;
@@ -209,17 +208,17 @@ searchbar[oneoffui] .search-go-button:-m
 .addengine-item > .button-box {
   -moz-box-pack: start;
 }
 
 .addengine-item:first-of-type {
   border-top: 1px solid #ccc;
 }
 
-.addengine-item:hover {
+.addengine-item[selected] {
   background-color: Highlight;
   color: HighlightText;
 }
 
 .addengine-icon {
   width: 16px;
 }
 
@@ -274,12 +273,12 @@ searchbar[oneoffui] .searchbar-engine-im
   -moz-appearance: none;
   border-bottom: none;
   border-left: none;
   border-right: none;
   -moz-border-top-colors: none;
   min-height: 32px;
 }
 
-.search-setting-button:hover {
+.search-setting-button[selected] {
   background-color: #d3d3d3;
   border-top-color: #bdbebe;
 }