Bug 1194669 - Dismissing the urlbar search suggestions opt-in notification in one window should dismiss it in all windows. r=mak, a=sledru
authorDrew Willcoxon <adw@mozilla.com>
Wed, 26 Aug 2015 10:41:36 -0700
changeset 288952 bd7b9630b3d8fcc04766275a80d2f13cfcb957f1
parent 288951 80c9f107c3aedea0da43ef1c3769b644b83ec654
child 288953 56bdc151e79f27ada574302f004261888db99144
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, sledru
bugs1194669
milestone42.0a2
Bug 1194669 - Dismissing the urlbar search suggestions opt-in notification in one window should dismiss it in all windows. r=mak, a=sledru
browser/base/content/test/general/browser_urlbarSearchSuggestionsNotification.js
browser/base/content/urlbarBindings.xml
--- a/browser/base/content/test/general/browser_urlbarSearchSuggestionsNotification.js
+++ b/browser/base/content/test/general/browser_urlbarSearchSuggestionsNotification.js
@@ -13,37 +13,37 @@ add_task(function* prepare() {
   registerCleanupFunction(function () {
     Services.prefs.clearUserPref("browser.urlbar.unifiedcomplete");
     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
     // them.  clearUserPref() won't work because by default the pref is false.
-    Services.prefs.setBoolPref(CHOICE_PREF, true);
+    yield setUserMadeChoicePref(true);
 
     // Make sure the popup is closed for the next test.
     gURLBar.blur();
     Assert.ok(!gURLBar.popup.popupOpen, "popup should be closed");
   });
 });
 
 add_task(function* focus() {
   // Focusing the urlbar used to open the popup in order to show the
   // notification, but it doesn't anymore.  Make sure it does not.
   Services.prefs.setBoolPref(SUGGEST_ALL_PREF, true);
-  Services.prefs.setBoolPref(CHOICE_PREF, false);
+  yield setUserMadeChoicePref(false);
   gURLBar.blur();
   gURLBar.focus();
   Assert.ok(!gURLBar.popup.popupOpen, "popup should remain closed");
 });
 
 add_task(function* dismissWithoutResults() {
   Services.prefs.setBoolPref(SUGGEST_ALL_PREF, true);
-  Services.prefs.setBoolPref(CHOICE_PREF, false);
+  yield setUserMadeChoicePref(false);
   gURLBar.blur();
   gURLBar.focus();
   let popupPromise = promisePopupShown(gURLBar.popup);
   gURLBar.openPopup();
   yield popupPromise;
   Assert.ok(gURLBar.popup.popupOpen, "popup should be open");
   assertVisible(true);
   Assert.equal(gURLBar.popup._matchCount, 0, "popup should have no results");
@@ -59,17 +59,17 @@ add_task(function* dismissWithoutResults
   Assert.ok(!gURLBar.popup.popupOpen, "popup should remain closed");
   yield promiseAutocompleteResultPopup("foo");
   Assert.ok(gURLBar.popup.popupOpen, "popup should be open");
   assertVisible(false);
 });
 
 add_task(function* dismissWithResults() {
   Services.prefs.setBoolPref(SUGGEST_ALL_PREF, true);
-  Services.prefs.setBoolPref(CHOICE_PREF, false);
+  yield setUserMadeChoicePref(false);
   gURLBar.blur();
   gURLBar.focus();
   yield promiseAutocompleteResultPopup("foo");
   Assert.ok(gURLBar.popup.popupOpen, "popup should be open");
   assertVisible(true);
   Assert.ok(gURLBar.popup._matchCount > 0, "popup should have results");
   let disableButton = document.getAnonymousElementByAttribute(
     gURLBar.popup, "anonid", "search-suggestions-notification-disable"
@@ -83,95 +83,168 @@ add_task(function* dismissWithResults() 
   Assert.ok(!gURLBar.popup.popupOpen, "popup should remain closed");
   yield promiseAutocompleteResultPopup("foo");
   Assert.ok(gURLBar.popup.popupOpen, "popup should be open");
   assertVisible(false);
 });
 
 add_task(function* disable() {
   Services.prefs.setBoolPref(SUGGEST_ALL_PREF, true);
-  Services.prefs.setBoolPref(CHOICE_PREF, false);
+  yield setUserMadeChoicePref(false);
   gURLBar.blur();
   gURLBar.focus();
   yield promiseAutocompleteResultPopup("foo");
   Assert.ok(gURLBar.popup.popupOpen, "popup should be open");
   assertVisible(true);
   let disableButton = document.getAnonymousElementByAttribute(
     gURLBar.popup, "anonid", "search-suggestions-notification-disable"
   );
   let transitionPromise = promiseTransition();
   disableButton.click();
   yield transitionPromise;
   gURLBar.blur();
   yield promiseAutocompleteResultPopup("foo");
-  assertSuggestionsPresent(false);
+  Assert.ok(!suggestionsPresent());
 });
 
 add_task(function* enable() {
   Services.prefs.setBoolPref(SUGGEST_ALL_PREF, true);
   Services.prefs.setBoolPref(SUGGEST_URLBAR_PREF, false);
-  Services.prefs.setBoolPref(CHOICE_PREF, false);
+  yield setUserMadeChoicePref(false);
   gURLBar.blur();
   gURLBar.focus();
   yield promiseAutocompleteResultPopup("foo");
   assertVisible(true);
-  assertSuggestionsPresent(false);
+  Assert.ok(!suggestionsPresent());
   let enableButton = document.getAnonymousElementByAttribute(
     gURLBar.popup, "anonid", "search-suggestions-notification-enable"
   );
-  let searchPromise = promiseSearchComplete();
+  let searchPromise = promiseSuggestionsPresent();
   enableButton.click();
   yield searchPromise;
   // Clicking Yes should trigger a new search so that suggestions appear
   // immediately.
-  assertSuggestionsPresent(true);
+  Assert.ok(suggestionsPresent());
   gURLBar.blur();
   gURLBar.focus();
   // Suggestions should still be present in a new search of course.
   yield promiseAutocompleteResultPopup("bar");
-  assertSuggestionsPresent(true);
+  Assert.ok(suggestionsPresent());
 });
 
 add_task(function* privateWindow() {
   // Since suggestions are disabled in private windows, the notification should
   // not appear even when suggestions are otherwise enabled.
   let win = yield BrowserTestUtils.openNewBrowserWindow({ private: true });
   win.gURLBar.blur();
   win.gURLBar.focus();
   yield promiseAutocompleteResultPopup("foo", win);
   assertVisible(false, win);
   win.gURLBar.blur();
   yield BrowserTestUtils.closeWindow(win);
 });
 
-function assertSuggestionsPresent(expectedPresent) {
+add_task(function* multipleWindows() {
+  // Opening multiple windows, using their urlbars, and then dismissing the
+  // notification in one should dismiss the notification in all.
+  Services.prefs.setBoolPref(SUGGEST_ALL_PREF, true);
+  Services.prefs.setBoolPref(SUGGEST_URLBAR_PREF, false);
+  yield setUserMadeChoicePref(false);
+
+  gURLBar.focus();
+  yield promiseAutocompleteResultPopup("win1");
+  assertVisible(true);
+
+  let win2 = yield BrowserTestUtils.openNewBrowserWindow();
+  win2.gURLBar.focus();
+  yield promiseAutocompleteResultPopup("win2", win2);
+  assertVisible(true, win2);
+
+  let win3 = yield BrowserTestUtils.openNewBrowserWindow();
+  win3.gURLBar.focus();
+  yield promiseAutocompleteResultPopup("win3", win3);
+  assertVisible(true, win3);
+
+  let enableButton = win3.document.getAnonymousElementByAttribute(
+    win3.gURLBar.popup, "anonid", "search-suggestions-notification-enable"
+  );
+  let transitionPromise = promiseTransition(win3);
+  enableButton.click();
+  yield transitionPromise;
+  assertVisible(false, win3);
+
+  win2.gURLBar.focus();
+  yield promiseAutocompleteResultPopup("win2done", win2);
+  assertVisible(false, win2);
+
+  gURLBar.focus();
+  yield promiseAutocompleteResultPopup("win1done");
+  assertVisible(false);
+
+  yield BrowserTestUtils.closeWindow(win2);
+  yield BrowserTestUtils.closeWindow(win3);
+});
+
+/**
+ * Setting the choice pref triggers a pref observer in the urlbar, which hides
+ * the notification if it's present.  This function returns a promise that's
+ * resolved once the observer fires.
+ *
+ * @param userMadeChoice  A boolean, the pref's new value.
+ * @return A Promise that's resolved when the observer fires -- or, if the pref
+ *         is currently the given value, that's resolved immediately.
+ */
+function setUserMadeChoicePref(userMadeChoice) {
+  return new Promise(resolve => {
+    let currentUserMadeChoice = Services.prefs.getBoolPref(CHOICE_PREF);
+    if (currentUserMadeChoice != userMadeChoice) {
+      Services.prefs.addObserver(CHOICE_PREF, function obs(subj, topic, data) {
+        Services.prefs.removeObserver(CHOICE_PREF, obs);
+        resolve();
+      }, false);
+    }
+    Services.prefs.setBoolPref(CHOICE_PREF, userMadeChoice);
+    if (currentUserMadeChoice == userMadeChoice) {
+      resolve();
+    }
+  });
+}
+
+function suggestionsPresent() {
   let controller = gURLBar.popup.input.controller;
   let matchCount = controller.matchCount;
-  let actualPresent = false;
+  let present = false;
   for (let i = 0; i < matchCount; i++) {
     let url = controller.getValueAt(i);
     let [, type, paramStr] = url.match(/^moz-action:([^,]+),(.*)$/);
     let params = {};
     try {
       params = JSON.parse(paramStr);
     } catch (err) {}
-    let isSuggestion = type == "searchengine" && "searchSuggestion" in params;
-    actualPresent = actualPresent || isSuggestion;
+    if (type == "searchengine" && "searchSuggestion" in params) {
+      return true;
+    }
   }
-  Assert.equal(actualPresent, expectedPresent);
+  return false;
+}
+
+function promiseSuggestionsPresent() {
+  return new Promise(resolve => {
+    waitForCondition(suggestionsPresent, resolve);
+  });
 }
 
 function assertVisible(visible, win=window) {
   let style =
     win.getComputedStyle(win.gURLBar.popup.searchSuggestionsNotification);
   Assert.equal(style.visibility, visible ? "visible" : "collapse");
 }
 
-function promiseTransition() {
+function promiseTransition(win=window) {
   return new Promise(resolve => {
-    gURLBar.popup.addEventListener("transitionend", function onEnd() {
-      gURLBar.popup.removeEventListener("transitionend", onEnd, true);
+    win.gURLBar.popup.addEventListener("transitionend", function onEnd() {
+      win.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/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -660,16 +660,19 @@ file, You can obtain one at http://mozil
                 this.timeout = this._prefs.getIntPref(aData);
                 break;
               case "formatting.enabled":
                 this._formattingEnabled = this._prefs.getBoolPref(aData);
                 break;
               case "userMadeSearchSuggestionsChoice":
                 this._userMadeSearchSuggestionsChoice =
                   this._prefs.getBoolPref(aData);
+                this.popup.searchSuggestionsNotificationWasDismissed(
+                  this._prefs.getBoolPref("suggest.searches")
+                );
                 break;
               case "trimURLs":
                 this._mayTrimURLs = this._prefs.getBoolPref(aData);
                 break;
               case "unifiedcomplete":
                 let useUnifiedComplete = false;
                 try {
                   useUnifiedComplete = this._prefs.getBoolPref(aData);
@@ -1549,26 +1552,22 @@ file, You can obtain one at http://mozil
       <field name="footer" readonly="true">
         document.getAnonymousElementByAttribute(this, "anonid", "footer");
       </field>
 
       <method name="dismissSearchSuggestionsNotification">
         <parameter name="enableSuggestions"/>
         <body><![CDATA[
           Services.prefs.setBoolPref(
-            "browser.urlbar.userMadeSearchSuggestionsChoice", true
-          );
-          Services.prefs.setBoolPref(
             "browser.urlbar.suggest.searches", enableSuggestions
           );
-          this._hideSearchSuggestionsNotification(true);
-          if (enableSuggestions && this.input.textValue) {
-            // Start a new search so that suggestions appear immediately.
-            this.input.controller.startSearch(this.input.textValue);
-          }
+          Services.prefs.setBoolPref(
+            "browser.urlbar.userMadeSearchSuggestionsChoice", true
+          );
+          // The input's pref observer will now hide the notification.
         ]]></body>
       </method>
 
       <!-- Override this so that when UnifiedComplete is enabled, navigating
            between items results in an item always being selected. This is
            contrary to the old behaviour (UnifiedComplete disabled) where
            if you navigate beyond either end of the list, no item will be
            selected. -->
@@ -1683,57 +1682,73 @@ file, You can obtain one at http://mozil
           this.setAttribute("dontanimate", "true");
 
           this.classList.add("showSearchSuggestionsNotification");
           this._updateFooterVisibility();
           ]]>
         </body>
       </method>
 
-      <method name="_hideSearchSuggestionsNotification">
-        <parameter name="animate"/>
+      <method name="searchSuggestionsNotificationWasDismissed">
+        <parameter name="enableSuggestions"/>
         <body>
           <![CDATA[
-          if (animate) {
-            this._hideSearchSuggestionsNotificationWithAnimation();
+          if (!this.popupOpen) {
+            this._hideSearchSuggestionsNotification();
             return;
           }
+          this._hideSearchSuggestionsNotificationWithAnimation().then(() => {
+            if (enableSuggestions && this.input.textValue) {
+              // Start a new search so that suggestions appear immediately.
+              this.input.controller.startSearch(this.input.textValue);
+            }
+          });
+          ]]>
+        </body>
+      </method>
+
+      <method name="_hideSearchSuggestionsNotification">
+        <body>
+          <![CDATA[
           this.classList.remove("showSearchSuggestionsNotification");
           this.richlistbox.flex = 1;
           this.removeAttribute("dontanimate");
           if (this._matchCount) {
             // Update popup height.
             this._invalidate();
           } else {
             this.closePopup();
           }
           ]]>
         </body>
       </method>
 
       <method name="_hideSearchSuggestionsNotificationWithAnimation">
         <body>
           <![CDATA[
-          let notificationHeight = this.searchSuggestionsNotification
-                                       .getBoundingClientRect()
-                                       .height;
-          this.searchSuggestionsNotification.style.marginTop =
-            "-" + notificationHeight + "px";
-
-          let popupHeightPx =
-            (this.getBoundingClientRect().height - notificationHeight) + "px";
-          this.style.height = popupHeightPx;
-
-          let onTransitionEnd = () => {
-            this.removeEventListener("transitionend", onTransitionEnd, true);
-            this.searchSuggestionsNotification.style.marginTop = "0px";
-            this.style.removeProperty("height");
-            this._hideSearchSuggestionsNotification(false);
-          };
-          this.addEventListener("transitionend", onTransitionEnd, true);
+          return new Promise(resolve => {
+            let notificationHeight = this.searchSuggestionsNotification
+                                         .getBoundingClientRect()
+                                         .height;
+            this.searchSuggestionsNotification.style.marginTop =
+              "-" + notificationHeight + "px";
+
+            let popupHeightPx =
+              (this.getBoundingClientRect().height - notificationHeight) + "px";
+            this.style.height = popupHeightPx;
+
+            let onTransitionEnd = () => {
+              this.removeEventListener("transitionend", onTransitionEnd, true);
+              this.searchSuggestionsNotification.style.marginTop = "0px";
+              this.style.removeProperty("height");
+              this._hideSearchSuggestionsNotification();
+              resolve();
+            };
+            this.addEventListener("transitionend", onTransitionEnd, true);
+          });
           ]]>
         </body>
       </method>
 
       <method name="onPopupClick">
         <parameter name="aEvent"/>
         <body>
           <![CDATA[