Bug 1334633 - Record the engine position in the list when a one-off button is clicked. f?gfritzsche draft
authorDrew Willcoxon <adw@mozilla.com>
Fri, 17 Mar 2017 19:15:41 -0700
changeset 500931 07846c78297c0f632225301ce527ae01172224c6
parent 496768 a8d497b09753c91783b68c5805c64f34a2f39629
child 549758 418dfb407229066e66a77fdc34b1d3551bb18570
push id49852
push userdwillcoxon@mozilla.com
push dateSat, 18 Mar 2017 02:16:07 +0000
bugs1334633
milestone55.0a1
Bug 1334633 - Record the engine position in the list when a one-off button is clicked. f?gfritzsche MozReview-Commit-ID: C0GAwPvNt16
browser/base/content/browser.js
browser/components/search/content/search.xml
browser/modules/BrowserUsageTelemetry.jsm
toolkit/components/telemetry/Histograms.json
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -3945,22 +3945,30 @@ const BrowserSearch = {
    *        (nsISearchEngine) The engine handling the search.
    * @param source
    *        (string) Where the search originated from. See BrowserUsageTelemetry for
    *        allowed values.
    * @param type
    *        (string) Indicates how the user selected the search item.
    * @param where
    *        (string) Where was the search link opened (e.g. new tab, current tab, ..).
+   * @param engineIndex
+   *        (integer) The index of the engine/button that was chosen in the list
+   *        of one-off buttons, or -1 if the search was performed without
+   *        choosing a one-off or the one-off is unknown.
    */
-  recordOneoffSearchInTelemetry(engine, source, type, where) {
+  recordOneoffSearchInTelemetry(engine, source, type, where, engineIndex = -1) {
     let id = this._getSearchEngineId(engine) + "." + source;
     BrowserUITelemetry.countOneoffSearchEvent(id, type, where);
     try {
-      const details = {type, isOneOff: true};
+      const details = {
+        type,
+        isOneOff: true,
+        oneOffEngineIndex: engineIndex,
+      };
       BrowserUsageTelemetry.recordSearch(engine, source, details);
     } catch (ex) {
       Cu.reportError(ex);
     }
   }
 };
 
 XPCOMUtils.defineConstant(this, "BrowserSearch", BrowserSearch);
--- a/browser/components/search/content/search.xml
+++ b/browser/components/search/content/search.xml
@@ -397,17 +397,17 @@
                 if (target.getAttribute("anonid") == "paste-and-search") {
                   source = "paste";
                 }
               }
               if (!aEngine) {
                 aEngine = this.currentEngine;
               }
               BrowserSearch.recordOneoffSearchInTelemetry(aEngine, source, type,
-                                                          aWhere);
+                                                          aWhere, -1);
             }
           }
 
           // This is a one-off search only if oneOffRecorded is true.
           this.doSearch(textValue, aWhere, aEngine, aParams, oneOffRecorded);
 
           if (aWhere == "tab" && aParams && aParams.inBackground)
             this.focus();
@@ -2121,18 +2121,24 @@
           if (this.telemetryOrigin) {
             source += "-" + this.telemetryOrigin;
           }
 
           let tabBackground = aOpenUILinkWhere == "tab" &&
                               aOpenUILinkParams &&
                               aOpenUILinkParams.inBackground;
           let where = tabBackground ? "tab-background" : aOpenUILinkWhere;
+
+          let buttons = this.getSelectableButtons(false);
+          let engineIndex = !engine ? -1 : buttons.findIndex(button => {
+            return button.engine == engine;
+          });
+
           BrowserSearch.recordOneoffSearchInTelemetry(engine, source, type,
-                                                      where);
+                                                      where, engineIndex);
           return true;
         ]]></body>
       </method>
 
       <!-- All this stuff is to make the add-engines menu button behave like an
            actual menu.  The add-engines menu button is shown when there are
            many engines offered by the current site. -->
       <field name="_addEngineMenuTimeoutMs">200</field>
--- a/browser/modules/BrowserUsageTelemetry.jsm
+++ b/browser/modules/BrowserUsageTelemetry.jsm
@@ -41,16 +41,20 @@ const KNOWN_SEARCH_SOURCES = [
   "newtab",
   "searchbar",
   "urlbar",
 ];
 
 const KNOWN_ONEOFF_SOURCES = [
   "oneoff-urlbar",
   "oneoff-searchbar",
+
+  //XXXadw shouldn't this include "oneoff-context-searchbar",
+  // "oneoff-context-urlbar"?
+
   "unknown", // Edge case: this is the searchbar (see bug 1195733 comment 7).
 ];
 
 /**
  * The buckets used for logging telemetry to the FX_URLBAR_SELECTED_RESULT_TYPE
  * histogram.
  */
 const URLBAR_SELECTED_RESULT_TYPES = {
@@ -348,16 +352,20 @@ let BrowserUsageTelemetry = {
    * @param {nsISearchEngine} engine
    *        The engine handling the search.
    * @param {String} source
    *        Where the search originated from. See KNOWN_SEARCH_SOURCES for allowed
    *        values.
    * @param {Object} [details] Options object.
    * @param {Boolean} [details.isOneOff=false]
    *        true if this event was generated by a one-off search.
+   * @param {Number} [details.oneOffEngineIndex=-1]
+   *        If the search was generated by a one-off search, the index of the
+   *        engine that was chosen in the list of one-off engines.  Otherwise,
+   *        or if the engine is unknown, -1.
    * @param {Boolean} [details.isSuggestion=false]
    *        true if this event was generated by a suggested search.
    * @param {Boolean} [details.isAlias=false]
    *        true if this event was generated by a search using an alias.
    * @param {Object} [details.type=null]
    *        The object describing the event that triggered the search.
    * @throws if source is not in the known sources list.
    */
@@ -384,30 +392,38 @@ let BrowserUsageTelemetry = {
       }
       Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").add(countId);
     }
 
     // Dispatch the search signal to other handlers.
     this._handleSearchAction(engine, source, details);
   },
 
-  _recordSearch(engine, source, action = null) {
+  _recordSearch(engine, source, action = null, oneOffEngineIndex = -1) {
     let scalarKey = action ? "search_" + action : "search";
     Services.telemetry.keyedScalarAdd("browser.engagement.navigation." + source,
                                       scalarKey, 1);
     Services.telemetry.recordEvent("navigation", "search", source, action,
                                    { engine: getSearchEngineId(engine) });
+    if (oneOffEngineIndex >= 0) {
+      oneOffEngineIndex = Math.min(oneOffEngineIndex, 32);
+      let histogram =
+        Services.telemetry.getHistogramById("FX_SEARCH_ONE_OFF_INDEX");
+      histogram.add(oneOffEngineIndex);
+    }
   },
 
   _handleSearchAction(engine, source, details) {
     switch (source) {
       case "urlbar":
       case "oneoff-urlbar":
       case "searchbar":
       case "oneoff-searchbar":
+      //XXXadw shouldn't this include "oneoff-context-searchbar",
+      // "oneoff-context-urlbar"?
       case "unknown": // Edge case: this is the searchbar (see bug 1195733 comment 7).
         this._handleSearchAndUrlbar(engine, source, details);
         break;
       case "abouthome":
         this._recordSearch(engine, "about_home", "enter");
         break;
       case "newtab":
         this._recordSearch(engine, "about_newtab", "enter");
@@ -416,16 +432,18 @@ let BrowserUsageTelemetry = {
         this._recordSearch(engine, "contextmenu");
         break;
     }
   },
 
   /**
    * This function handles the "urlbar", "urlbar-oneoff", "searchbar" and
    * "searchbar-oneoff" sources.
+   //XXXadw shouldn't this include "oneoff-context-searchbar",
+   // "oneoff-context-urlbar"?
    */
   _handleSearchAndUrlbar(engine, source, details) {
     // We want "urlbar" and "urlbar-oneoff" (and similar cases) to go in the same
     // scalar, but in a different key.
 
     // When using one-offs in the searchbar we get an "unknown" source. See bug
     // 1195733 comment 7 for the context. Fix-up the label here.
     const sourceName =
@@ -439,17 +457,19 @@ let BrowserUsageTelemetry = {
       // Moreover, we skip the "unknown" source that comes from the searchbar
       // when performing searches from the default search engine. See bug 1195733
       // comment 7 for context.
       if (["urlbar", "searchbar", "unknown"].includes(source)) {
         return;
       }
 
       // If that's a legit one-off search signal, record it using the relative key.
-      this._recordSearch(engine, sourceName, "oneoff");
+      let engineIndex = typeof(details.oneOffEngineIndex) == "number" ?
+                        details.oneOffEngineIndex : -1;
+      this._recordSearch(engine, sourceName, "oneoff", engineIndex);
       return;
     }
 
     // The search was not a one-off. It was a search with the default search engine.
     if (details.isSuggestion) {
       // It came from a suggested search, so count it as such.
       this._recordSearch(engine, sourceName, "suggestion");
       return;
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -5481,16 +5481,24 @@
     "bug_numbers": [1324167],
     "alert_emails": ["jaws@mozilla.com"],
     "expires_in_version": "56",
     "kind": "categorical",
     "labels": ["unknown", "general", "search", "content", "applications", "privacy", "security", "sync", "advancedGeneral", "advancedDataChoices", "advancedNetwork", "advancedUpdates", "advancedCerts"],
     "releaseChannelCollection": "opt-out",
     "description": "Count how often each preference category is opened."
   },
+  "FX_SEARCH_ONE_OFF_INDEX": {
+    "bug_numbers": [1334633],
+    "alert_emails": ["adw@mozilla.com"],
+    "expires_in_version": "58",
+    "kind": "enumerated",
+    "n_values": 32,
+    "description": "Index of the selected engine in one-off searches."
+  },
   "INPUT_EVENT_RESPONSE_MS": {
     "alert_emails": ["perf-telemetry-alerts@mozilla.com"],
     "bug_numbers": [1235908],
     "expires_in_version": "never",
     "kind": "exponential",
     "high": 10000,
     "n_buckets": 50,
     "description": "Time (ms) from the Input event being created to the end of it being handled"