Bug 1306639 - Searching in locationbar by typing something and pressing enter is not accounted in telemetry. r=adw, a=ritu
☠☠ backed out by 0cf671e08f5c ☠ ☠
authorMarco Bonardo <mbonardo@mozilla.com>
Tue, 18 Oct 2016 14:47:31 +0200
changeset 350747 fa0189889818d142c9e7161f72419c9a6c501adf
parent 350746 274a1f17a29aeba2f75f7bf53a931a946c6ab05f
child 350748 69c68bce430d468037bc82223acb9ea4155dbc7d
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw, ritu
bugs1306639
milestone50.0
Bug 1306639 - Searching in locationbar by typing something and pressing enter is not accounted in telemetry. r=adw, a=ritu MozReview-Commit-ID: 513LnXPrg8z
browser/base/content/test/urlbar/browser_urlbarSearchTelemetry.js
browser/base/content/urlbarBindings.xml
--- a/browser/base/content/test/urlbar/browser_urlbarSearchTelemetry.js
+++ b/browser/base/content/test/urlbar/browser_urlbarSearchTelemetry.js
@@ -6,56 +6,89 @@ const SUGGEST_URLBAR_PREF = "browser.url
 const TEST_ENGINE_BASENAME = "searchSuggestionEngine.xml";
 
 // Must run first.
 add_task(function* prepare() {
   Services.prefs.setBoolPref(SUGGEST_URLBAR_PREF, true);
   let engine = yield promiseNewSearchEngine(TEST_ENGINE_BASENAME);
   let oldCurrentEngine = Services.search.currentEngine;
   Services.search.currentEngine = engine;
+
   registerCleanupFunction(function* () {
     Services.prefs.clearUserPref(SUGGEST_URLBAR_PREF);
     Services.search.currentEngine = oldCurrentEngine;
 
     // Clicking urlbar results causes visits to their associated pages, so clear
     // that history now.
     yield PlacesTestUtils.clearHistory();
 
     // Make sure the popup is closed for the next test.
     gURLBar.blur();
     Assert.ok(!gURLBar.popup.popupOpen, "popup should be closed");
   });
 });
 
-add_task(function* heuristicResult() {
+add_task(function* heuristicResultMouse() {
   yield compareCounts(function* () {
-    gBrowser.selectedTab = gBrowser.addTab();
+    let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser);
+    gURLBar.focus();
+    yield promiseAutocompleteResultPopup("heuristicResult");
+    let action = getActionAtIndex(0);
+    Assert.ok(!!action, "there should be an action at index 0");
+    Assert.equal(action.type, "searchengine", "type should be searchengine");
+    let loadPromise = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+    gURLBar.popup.richlistbox.getItemAtIndex(0).click();
+    yield loadPromise;
+    yield BrowserTestUtils.removeTab(tab);
+  });
+});
+
+add_task(function* heuristicResultKeyboard() {
+  yield compareCounts(function* () {
+    let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser);
+    gURLBar.focus();
     yield promiseAutocompleteResultPopup("heuristicResult");
     let action = getActionAtIndex(0);
     Assert.ok(!!action, "there should be an action at index 0");
     Assert.equal(action.type, "searchengine", "type should be searchengine");
-    let item = gURLBar.popup.richlistbox.getItemAtIndex(0);
-    let loadPromise = promiseTabLoaded(gBrowser.selectedTab);
-    item.click();
+    let loadPromise = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+    EventUtils.sendKey("return");
     yield loadPromise;
-    gBrowser.removeTab(gBrowser.selectedTab);
+    yield BrowserTestUtils.removeTab(tab);
   });
 });
 
-add_task(function* searchSuggestion() {
+add_task(function* searchSuggestionMouse() {
   yield compareCounts(function* () {
-    gBrowser.selectedTab = gBrowser.addTab();
+    let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser);
+    gURLBar.focus();
     yield promiseAutocompleteResultPopup("searchSuggestion");
     let idx = getFirstSuggestionIndex();
     Assert.ok(idx >= 0, "there should be a first suggestion");
-    let item = gURLBar.popup.richlistbox.getItemAtIndex(idx);
-    let loadPromise = promiseTabLoaded(gBrowser.selectedTab);
-    item.click();
+    let loadPromise = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+    gURLBar.popup.richlistbox.getItemAtIndex(idx).click();
     yield loadPromise;
-    gBrowser.removeTab(gBrowser.selectedTab);
+    yield BrowserTestUtils.removeTab(tab);
+  });
+});
+
+add_task(function* searchSuggestionKeyboard() {
+  yield compareCounts(function* () {
+    let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser);
+    gURLBar.focus();
+    yield promiseAutocompleteResultPopup("searchSuggestion");
+    let idx = getFirstSuggestionIndex();
+    Assert.ok(idx >= 0, "there should be a first suggestion");
+    let loadPromise = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+    while (idx--) {
+      EventUtils.sendKey("down");
+    }
+    EventUtils.sendKey("return");
+    yield loadPromise;
+    yield BrowserTestUtils.removeTab(tab);
   });
 });
 
 /**
  * This does three things: gets current telemetry/FHR counts, calls
  * clickCallback, gets telemetry/FHR counts again to compare them to the old
  * counts.
  *
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -516,16 +516,31 @@ file, You can obtain one at http://mozil
               Services.search.getEngineByName(engineOrEngineName) :
               engineOrEngineName;
           BrowserSearch.recordSearchInTelemetry(engine, "urlbar");
           let submission = engine.getSubmission(query, null, "keyword");
           return [submission.uri.spec, submission.postData];
         ]]></body>
       </method>
 
+      <!-- this is a temporary workaround for bug 1306639 -->
+      <method name="_maybeRecordSearchTelemetry">
+        <body><![CDATA[
+          if (this.popup.selectedIndex == 0 &&
+              this.popup._isFirstResultHeuristic &&
+              !this._parseActionUrl(this.value)) {
+            let action = this._parseActionUrl(this.controller.getValueAt(0));
+            if (action && action.type == "searchengine") {
+              let engine = Services.search.getEngineByName(action.params.engineName);
+              BrowserSearch.recordSearchInTelemetry(engine, "urlbar");
+            }
+          }
+        ]]></body>
+      </method>
+
       <method name="_canonizeURL">
         <parameter name="aTriggeringEvent"/>
         <parameter name="aUrl"/>
         <body><![CDATA[
           let url = aUrl;
 
           // Only add the suffix when the URL bar value isn't already "URL-like",
           // and only if we get a keyboard event, to match user expectations.
@@ -979,16 +994,17 @@ file, You can obtain one at http://mozil
           // Note this is also used to detect if we should perform a delayed
           // handleEnter, in such a case it won't have been cleared.
           this.handleEnterInstance = {
             searchString: this.mController.searchString,
             event: event
           };
 
           if (this.popup.selectedIndex != 0 || this.gotResultForCurrentQuery) {
+            this._maybeRecordSearchTelemetry();
             let rv = this.mController.handleEnter(false, event);
             this.handleEnterInstance = null;
             return rv;
           }
 
           return true;
         ]]></body>
       </method>
@@ -1616,16 +1632,17 @@ file, You can obtain one at http://mozil
             if (this.input.handleEnterInstance) {
               let instance = this.input.handleEnterInstance;
               this.input.handleEnterInstance = null;
               // Don't handle this immediately or we could cause a recursive
               // loop where the controller sets popupOpen and re-enters here.
               setTimeout(() => {
                 // Safety check: handle only if the search string didn't change.
                 if (this.input.mController.searchString == instance.searchString) {
+                  this.input._maybeRecordSearchTelemetry();
                   this.input.mController.handleEnter(false, instance.event);
                 }
               }, 0);
             }
           ]]>
         </body>
       </method>