Bug 1118135: Clicking the magnifying glass while the suggestions are open should close the popup and not re-open it. r=felipe

The popup gets closed when native events trigger rollup. This is before the any mousedown event reaches the DOM so it is difficult to detect the case where it was a click on the magnifying glass icon that closed the popup. Here a field is added to the popup that is set to true when rollup is triggered and false on the next tick of the event loop which will be after the mousedown event reaches the DOM. This allows us to detect the case and ignore the click on the magnifying glass. --HG-- extra : rebase_source : 521d024b33011d0f501a5cd6a1317363f585ffed
2015-01-06 11:05:32 -08:00 · 2015-01-06 11:05:32 -08:00 · 263931f8de
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@ -1011,6 +1011,12 @@
                  label="&changeSearchSettings.button;"/>
    </content>
    <implementation>
+      <!-- Popup rollup is triggered by native events before the mousedown event
+           reaches the DOM. The will be set to true by the popuphiding event and
+           false after the mousedown event has been triggered to detect what
+           caused rollup. -->
+      <field name="_isHiding">false</field>
+
      <method name="updateHeader">
        <body><![CDATA[
          let currentEngine = Services.search.currentEngine;
@ -1264,6 +1270,13 @@
                                    installCallback);
        }
      ]]></handler>
+
+      <handler event="popuphiding"><![CDATA[
+        this._isHiding = true;
+        setTimeout(() => {
+          this._isHiding = false;
+        }, 0);
+      ]]></handler>
    </handlers>
  </binding>

--- a/browser/components/search/content/search.xml
+++ b/browser/components/search/content/search.xml
@ -678,6 +678,7 @@
      ]]></destructor>

      <field name="_ignoreFocus">false</field>
+      <field name="_clickClosedPopup">false</field>

      <method name="rebuildPopup">
        <body><![CDATA[
@ -749,6 +750,13 @@
        this.openSuggestionsPanel();
      ]]></handler>

+      <handler event="mousedown" phase="capturing">
+      <![CDATA[
+        if (event.originalTarget.getAttribute("anonid") == "searchbar-search-button") {
+          this._clickClosedPopup = this._textbox.popup._isHiding;
+        }
+      ]]></handler>
+
      <handler event="click" button="0">
      <![CDATA[
        // Ignore clicks on the search go button.
@ -756,10 +764,16 @@
          return;
        }

+        let isIconClick = event.originalTarget.getAttribute("anonid") == "searchbar-search-button";
+
+        // Ignore clicks on the icon if they were made to close the popup
+        if (isIconClick && this._clickClosedPopup) {
+          return;
+        }
+
        // Open the suggestions whenever clicking on the search icon or if there
        // is text in the textbox.
-        if (event.originalTarget.getAttribute("anonid") == "searchbar-search-button" ||
-            this._textbox.value) {
+        if (isIconClick || this._textbox.value) {
          this.openSuggestionsPanel(true);
        }
      ]]></handler>
--- a/browser/components/search/test/browser_searchbar_openpopup.js
+++ b/browser/components/search/test/browser_searchbar_openpopup.js
@ -7,6 +7,23 @@ const goButton = document.getAnonymousElementByAttribute(searchbar, "anonid", "s
 const textbox = searchbar._textbox;
 const searchPopup = document.getElementById("PopupSearchAutoComplete");

+const isWindows = Services.appinfo.OS == "WINNT";
+const mouseDown = isWindows ? 2 : 1;
+const mouseUp = isWindows ? 4 : 2;
+const utils = window.QueryInterface(Ci.nsIInterfaceRequestor)
+                    .getInterface(Ci.nsIDOMWindowUtils);
+const scale = utils.screenPixelsPerCSSPixel;
+
+function synthesizeNativeMouseClick(aElement) {
+  let rect = aElement.getBoundingClientRect();
+  let win = aElement.ownerDocument.defaultView;
+  let x = win.mozInnerScreenX + (rect.left + rect.right) / 2;
+  let y = win.mozInnerScreenY + (rect.top + rect.bottom) / 2;
+
+  utils.sendNativeMouseEvent(x * scale, y * scale, mouseDown, 0, null);
+  utils.sendNativeMouseEvent(x * scale, y * scale, mouseUp, 0, null);
+}
+
 function promiseNewEngine(basename) {
  return new Promise((resolve, reject) => {
    info("Waiting for engine to be added: " + basename);
@ -77,6 +94,7 @@ add_no_popup_task(function* open_icon_context() {
 });

 // With no text in the search box left clicking the icon should open the popup.
+// Clicking the icon again should hide the popup and not show it again.
 add_task(function* open_empty() {
  gURLBar.focus();

@ -85,11 +103,27 @@ add_task(function* open_empty() {
  EventUtils.synthesizeMouseAtCenter(searchIcon, {});
  yield promise;
  is(searchPopup.getAttribute("showonlysettings"), "true", "Should only show the settings");
+  is(textbox.mController.searchString, "", "Should be an empty search string");
+
+  // By giving the textbox some text any next attempt to open the search popup
+  // from the click handler will try to search for this text.
+  textbox.value = "foo";

  promise = promiseEvent(searchPopup, "popuphidden");
+  let clickPromise = promiseEvent(searchIcon, "click");
+
  info("Hiding popup");
-  searchPopup.hidePopup();
+  synthesizeNativeMouseClick(searchIcon);
  yield promise;
+
+  is(textbox.mController.searchString, "", "Should not have started to search for the new text");
+
+  // Cancel the search if it started.
+  if (textbox.mController.searchString != "") {
+    textbox.mController.stopSearch();
+  }
+
+  textbox.value = "";
 });

 // With no text in the search box left clicking it should not open the popup.
@ -223,29 +257,6 @@ add_no_popup_task(function* tab_doesnt_open_popup() {
  textbox.value = "";
 });

-// Clicks outside the search popup should close the popup but not consume the click.
-add_task(function* dont_consume_clicks() {
-  gURLBar.focus();
-  textbox.value = "foo";
-
-  let promise = promiseEvent(searchPopup, "popupshown");
-  EventUtils.synthesizeMouseAtCenter(textbox, {});
-  yield promise;
-  isnot(searchPopup.getAttribute("showonlysettings"), "true", "Should show the full popup");
-
-  is(Services.focus.focusedElement, textbox.inputField, "Should have focused the search bar");
-  is(textbox.selectionStart, 0, "Should have selected all of the text");
-  is(textbox.selectionEnd, 3, "Should have selected all of the text");
-
-  promise = promiseEvent(searchPopup, "popuphidden");
-  EventUtils.synthesizeMouseAtCenter(gURLBar, {});
-  yield promise;
-
-  is(Services.focus.focusedElement, gURLBar.inputField, "Should have focused the URL bar");
-
-  textbox.value = "";
-});
-
 // Switching back to the window when the search box has focus from mouse should not open the popup.
 add_task(function* refocus_window_doesnt_open_popup_mouse() {
  gURLBar.focus();
@ -335,3 +346,26 @@ add_no_popup_task(function* search_go_doesnt_open_popup() {
  textbox.value = "";
  gBrowser.removeCurrentTab();
 });
+
+// Clicks outside the search popup should close the popup but not consume the click.
+add_task(function* dont_consume_clicks() {
+  gURLBar.focus();
+  textbox.value = "foo";
+
+  let promise = promiseEvent(searchPopup, "popupshown");
+  EventUtils.synthesizeMouseAtCenter(textbox, {});
+  yield promise;
+  isnot(searchPopup.getAttribute("showonlysettings"), "true", "Should show the full popup");
+
+  is(Services.focus.focusedElement, textbox.inputField, "Should have focused the search bar");
+  is(textbox.selectionStart, 0, "Should have selected all of the text");
+  is(textbox.selectionEnd, 3, "Should have selected all of the text");
+
+  promise = promiseEvent(searchPopup, "popuphidden");
+  synthesizeNativeMouseClick(gURLBar);
+  yield promise;
+
+  is(Services.focus.focusedElement, gURLBar.inputField, "Should have focused the URL bar");
+
+  textbox.value = "";
+});