Bug 1635239 - When an search engine is overriding the application provided, use a different telemetry id. r=daleharvey
☠☠ backed out by e7641c1071ea ☠ ☠
authorMark Banner <standard8@mozilla.com>
Sun, 24 May 2020 21:45:59 +0000
changeset 531817 97ecda13df1858b93690d09e5cf3f8d82831c03c
parent 531816 c9f80397bbecbaa3b026d20fe8fc58c9e44fbed2
child 531818 30a6807bb0fe41c6f254e6de323f25394dae14da
push id37446
push userabutkovits@mozilla.com
push dateMon, 25 May 2020 09:34:40 +0000
treeherdermozilla-central@9155018d4978 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdaleharvey
bugs1635239
milestone78.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 1635239 - When an search engine is overriding the application provided, use a different telemetry id. r=daleharvey Depends on D76473 Differential Revision: https://phabricator.services.mozilla.com/D76480
toolkit/components/search/SearchEngine.jsm
toolkit/components/search/tests/xpcshell/test_override_allowlist.js
--- a/toolkit/components/search/SearchEngine.jsm
+++ b/toolkit/components/search/SearchEngine.jsm
@@ -1914,17 +1914,22 @@ SearchEngine.prototype = {
    * - telemetryId: The telemetry id from the configuration.
    * - identifier: The built-in identifier of app-provided engines.
    * - other-<name>: The engine name prefixed by `other-` for non-app-provided
    *                 engines.
    *
    * @returns {string}
    */
   get telemetryId() {
-    return this._telemetryId || this.identifier || `other-${this.name}`;
+    let telemetryId =
+      this._telemetryId || this.identifier || `other-${this.name}`;
+    if (this.getAttr("overriddenBy")) {
+      return telemetryId + "-addon";
+    }
+    return telemetryId;
   },
 
   /**
    * Return the built-in identifier of app-provided engines.
    *
    * @returns {string|null}
    *   Returns a valid if this is a built-in engine, null otherwise.
    */
--- a/toolkit/components/search/tests/xpcshell/test_override_allowlist.js
+++ b/toolkit/components/search/tests/xpcshell/test_override_allowlist.js
@@ -274,16 +274,22 @@ for (const test of tests) {
 
     let engine = await Services.search.getEngineByName(kOverriddenEngineName);
     Assert.equal(
       !!engine.wrappedJSObject.getAttr("overriddenBy"),
       test.expected.overridesEngine,
       "Should have correctly overridden or not."
     );
 
+    Assert.equal(
+      engine.telemetryId,
+      "simple" + (test.expected.overridesEngine ? "-addon" : ""),
+      "Should set the correct telemetry Id"
+    );
+
     if (test.expected.overridesEngine) {
       let submission = engine.getSubmission("{searchTerms}");
       Assert.equal(
         decodeURI(submission.uri.spec),
         test.expected.searchUrl,
         "Should have set the correct url on an overriden engine"
       );
 
@@ -303,12 +309,33 @@ for (const test of tests) {
         let data = sis.read(submission.postData.available());
         Assert.equal(
           decodeURIComponent(data),
           test.expected.postData,
           "Should have overridden the postData"
         );
       }
 
+      // As we're not testing the WebExtension manager as well,
+      // set this engine as default so we can check the telemetry data.
+      let oldDefaultEngine = Services.search.defaultEngine;
+      Services.search.defaultEngine = engine;
+
+      let engineInfo = await Services.search.getDefaultEngineInfo();
+      Assert.deepEqual(
+        engineInfo,
+        {
+          defaultSearchEngine: "simple-addon",
+          defaultSearchEngineData: {
+            loadPath: "[other]addEngineWithDetails:simple@search.mozilla.org",
+            name: "Simple Engine",
+            origin: "default",
+            submissionURL: test.expected.searchUrl.replace("{searchTerms}", ""),
+          },
+        },
+        "Should return the extended identifier and alternate submission url to telemetry"
+      );
+      Services.search.defaultEngine = oldDefaultEngine;
+
       engine.wrappedJSObject.removeExtensionOverride();
     }
   });
 }