Bug 1356758 - Intermittent timeouts in browser_UsageTelemetry_urlbar.js. r=adw
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 04 Oct 2017 17:39:59 +0200
changeset 384934 d115a21148097e669cd2603284b1e062de95f054
parent 384933 4279ffa1e0da62b01cc292486fced4dedd6df64d
child 384935 25db67fda0158f2565059b3b9ea40de08051f45d
push id95880
push userarchaeopteryx@coole-files.de
push dateSat, 07 Oct 2017 08:58:44 +0000
treeherdermozilla-inbound@156942799371 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1356758
milestone58.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 1356758 - Intermittent timeouts in browser_UsageTelemetry_urlbar.js. r=adw MozReview-Commit-ID: 3pCG35lHis2
browser/base/content/test/urlbar/browser_urlbarOneOffs_searchSuggestions.js
browser/base/content/urlbarBindings.xml
browser/modules/test/browser/browser.ini
browser/modules/test/browser/browser_UsageTelemetry_urlbar.js
toolkit/content/widgets/autocomplete.xml
--- a/browser/base/content/test/urlbar/browser_urlbarOneOffs_searchSuggestions.js
+++ b/browser/base/content/test/urlbar/browser_urlbarOneOffs_searchSuggestions.js
@@ -77,16 +77,43 @@ add_task(async function oneOffClickAfter
                                    `http://mochi.test:8888/?terms=foobar`);
   EventUtils.synthesizeMouseAtCenter(oneOffs[0], {});
   await resultsPromise;
 
   await PlacesTestUtils.clearHistory();
   await BrowserTestUtils.removeTab(tab);
 });
 
+add_task(async function overridden_engine_not_reused() {
+  info("An overridden search suggestion item should not be reused by a search with another engine");
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
+    let typedValue = "foo";
+    await promiseAutocompleteResultPopup(typedValue, window, true);
+    await BrowserTestUtils.waitForCondition(suggestionsPresent,
+                                            "waiting for suggestions");
+    // Down to select the first search suggestion.
+    EventUtils.synthesizeKey("VK_DOWN", {})
+    assertState(1, -1, "foofoo");
+    // ALT+Down to select the second search engine.
+    EventUtils.synthesizeKey("VK_DOWN", { altKey: true });
+    EventUtils.synthesizeKey("VK_DOWN", { altKey: true });
+    assertState(1, 1, "foofoo");
+
+    let label = gURLBar.popup.richlistbox.children[gURLBar.popup.richlistbox.selectedIndex].label;
+    // Run again the query, check the label has been replaced.
+    await promiseAutocompleteResultPopup(typedValue, window, true);
+    await BrowserTestUtils.waitForCondition(suggestionsPresent,
+                                            "waiting for suggestions");
+    assertState(0, -1, "foo");
+    let newLabel = gURLBar.popup.richlistbox.children[1].label;
+    Assert.notEqual(newLabel, label, "The label should have been updated");
+
+    await BrowserTestUtils.removeTab(tab);
+});
+
 function assertState(result, oneOff, textValue = undefined) {
   Assert.equal(gURLBar.popup.selectedIndex, result,
                "Expected result should be selected");
   Assert.equal(gURLBar.popup.oneOffSearchButtons.selectedButtonIndex, oneOff,
                "Expected one-off should be selected");
   if (textValue !== undefined) {
     Assert.equal(gURLBar.textValue, textValue, "Expected textValue");
   }
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -2262,16 +2262,23 @@ file, You can obtain one at http://mozil
           // 1. if a search starts we set selectedIndex to 0 here, and it will
           //    be updated by onResultsAdded. Since selectedIndex is 0,
           //    handleEnter will delay the action if a result didn't arrive yet.
           // 2. if a search doesn't start (for example if autocomplete is
           //    disabled), this won't be called, and the selectedIndex will be
           //    the default -1 value. Then handleEnter will know it should not
           //    delay the action, cause a result wont't ever arrive.
           this.input.controller.setInitiallySelectedIndex(0);
+
+          // Since we are starting a new search, reset the currently selected
+          // one-off button, to cover those cases where the oneoff buttons
+          // binding won't receive an actual DOM event. For example, a search
+          // could be started without an actual input event, and the popup may
+          // not have been closed from the previous search.
+          this.oneOffSearchButtons.selectedButton = null;
         ]]></body>
       </method>
 
       <field name="_addonIframe">null</field>
       <field name="_addonIframeOwner">null</field>
       <field name="_addonIframeOverriddenFunctionsByName">{}</field>
 
       <!-- These methods must be overridden and properly handled by the API
--- a/browser/modules/test/browser/browser.ini
+++ b/browser/modules/test/browser/browser.ini
@@ -32,17 +32,16 @@ skip-if = !e10s
 skip-if = os != "win"
 [browser_UnsubmittedCrashHandler.js]
 run-if = crashreporter
 [browser_urlBar_zoom.js]
 [browser_UsageTelemetry.js]
 [browser_UsageTelemetry_domains.js]
 [browser_UsageTelemetry_private_and_restore.js]
 [browser_UsageTelemetry_urlbar.js]
-skip-if = (os == 'linux' && bits == 32 && debug) # bug 1356758
 support-files =
   usageTelemetrySearchSuggestions.sjs
   usageTelemetrySearchSuggestions.xml
 [browser_UsageTelemetry_searchbar.js]
 support-files =
   usageTelemetrySearchSuggestions.sjs
   usageTelemetrySearchSuggestions.xml
 [browser_UsageTelemetry_content.js]
--- a/browser/modules/test/browser/browser_UsageTelemetry_urlbar.js
+++ b/browser/modules/test/browser/browser_UsageTelemetry_urlbar.js
@@ -26,17 +26,25 @@ function checkHistogramResults(resultInd
   }
 }
 
 let searchInAwesomebar = async function(inputText, win = window) {
   await new Promise(r => waitForFocus(r, win));
   // Write the search query in the urlbar.
   win.gURLBar.focus();
   win.gURLBar.value = inputText;
+
+  // This is not strictly necessary, but some things, like clearing oneoff
+  // buttons status, depend on actual input events that the user would normally
+  // generate.
+  let event = win.document.createEvent("Events");
+  event.initEvent("input", true, true);
+  win.gURLBar.dispatchEvent(event);
   win.gURLBar.controller.startSearch(inputText);
+
   // Wait for the popup to show.
   await BrowserTestUtils.waitForEvent(win.gURLBar.popup, "popupshown");
   // And then for the search to complete.
   await BrowserTestUtils.waitForCondition(() => win.gURLBar.controller.searchStatus >=
                                                 Ci.nsIAutoCompleteController.STATUS_COMPLETE_NO_MATCH);
 };
 
 /**
@@ -44,23 +52,26 @@ let searchInAwesomebar = async function(
  *
  * @param {String} entryName
  *        The name of the elemet to click on.
  */
 function clickURLBarSuggestion(entryName) {
   // The entry in the suggestion list should follow the format:
   // "<search term> <engine name> Search"
   const expectedSuggestionName = entryName + " " + SUGGESTION_ENGINE_NAME + " Search";
-  for (let child of gURLBar.popup.richlistbox.children) {
-    if (child.label === expectedSuggestionName) {
-      // This entry is the search suggestion we're looking for.
-      child.click();
-      return;
+  return BrowserTestUtils.waitForCondition(() => {
+    for (let child of gURLBar.popup.richlistbox.children) {
+      if (child.label === expectedSuggestionName) {
+        // This entry is the search suggestion we're looking for.
+        child.click();
+        return true;
+      }
     }
-  }
+    return false;
+  }, "Waiting for the expected suggestion to appear");
 }
 
 add_task(async function setup() {
   // Create a new search engine.
   Services.search.addEngineWithDetails("MozSearch", "", "mozalias", "", "GET",
                                        "http://example.com/?q={searchTerms}");
 
   // Make it the default search engine.
@@ -374,17 +385,17 @@ add_task(async function test_suggestion_
   Services.search.currentEngine = suggestionEngine;
 
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:blank");
 
   info("Type a query. Suggestions should be generated by the test engine.");
   let p = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
   await searchInAwesomebar("query");
   info("Clicking the urlbar suggestion.");
-  clickURLBarSuggestion("queryfoo");
+  await clickURLBarSuggestion("queryfoo");
   await p;
 
   // Check if the scalars contain the expected values.
   const scalars = getParentProcessScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true, false);
   checkKeyedScalar(scalars, SCALAR_URLBAR, "search_suggestion", 1);
   Assert.equal(Object.keys(scalars[SCALAR_URLBAR]).length, 1,
                "This search must only increment one entry in the scalar.");
 
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -1336,17 +1336,18 @@ extends="chrome://global/content/binding
             let style = controller.getStyleAt(this._currentIndex);
             let image = controller.getImageAt(this._currentIndex);
             // trim the leading/trailing whitespace
             let trimmedSearchString = controller.searchString.replace(/^\s+/, "").replace(/\s+$/, "");
 
             if (itemExists) {
               item = this.richlistbox.childNodes[this._currentIndex];
 
-              originalValue = item.getAttribute("ac-value");
+              // Url may be a modified version of value, see _adjustACItem().
+              originalValue = item.getAttribute("url") || item.getAttribute("ac-value");
               originalText = item.getAttribute("ac-text");
               originalType = item.getAttribute("originaltype");
 
               // All of types are reusable except for autofill-profile,
               // which has different structure of <content> and overrides
               // _adjustAcItem().
               reusable = originalType === style ||
                          (style !== "autofill-profile" && originalType !== "autofill-profile" &&
@@ -1379,17 +1380,16 @@ extends="chrome://global/content/binding
               if (reused) {
                 this._currentIndex++;
                 continue;
               }
             } else {
               if (typeof item._cleanup == "function") {
                 item._cleanup();
               }
-
               item.setAttribute("originaltype", style);
             }
 
             if (itemExists) {
               // Adjust only when the result's type is reusable for existing
               // item's. Otherwise, we might insensibly call old _adjustAcItem()
               // as new binding has not been attached yet.
               // We don't need to worry about switching to new binding, since
@@ -2109,28 +2109,26 @@ extends="chrome://global/content/binding
           ]]>
         </body>
       </method>
 
 
       <method name="_adjustAcItem">
         <body>
           <![CDATA[
-          this.setAttribute("url", this.getAttribute("ac-value"));
+          let originalUrl = this.getAttribute("ac-value");
+          let title = this.getAttribute("ac-comment");
+          this.setAttribute("url", originalUrl);
           this.setAttribute("image", this.getAttribute("ac-image"));
-          this.setAttribute("title", this.getAttribute("ac-comment"));
+          this.setAttribute("title", title);
           this.setAttribute("text", this.getAttribute("ac-text"));
 
           let popup = this.parentNode.parentNode;
-
-          let title = this.getAttribute("title");
           let titleLooksLikeUrl = false;
-
           let displayUrl;
-          let originalUrl = this.getAttribute("url");
           let emphasiseUrl = true;
 
           let type = this.getAttribute("originaltype");
           let types = new Set(type.split(/\s+/));
           let initialTypes = new Set(types);
           // Remove types that should ultimately not be in the `type` string.
           types.delete("action");
           types.delete("autofill");