Backed out changeset e7822e311e0c (bug 1790020) for causing mochitests failures in browser/components/urlbar/tests/browser-tips/browser_searchTips_interaction.js. CLOSED TREE

browser/components/urlbar/UrlbarController.sys.mjs

@@ -844,7 +844,7 @@ class TelemetryEvent {
    *        for "blur".
    * @param {string} [details.selType] type of the selected element, undefined
    *        for "blur". One of "unknown", "autofill", "visiturl", "bookmark",
-   *        "help", "history", "keyword", "searchengine", "searchsuggestion",
+   *        "history", "keyword", "searchengine", "searchsuggestion",
    *        "switchtab", "remotetab", "extension", "oneoff", "dismiss".
    * @param {string} [details.provider] The name of the provider for the selected
    *        result.
@@ -946,8 +946,6 @@ class TelemetryEvent {
       action = "blur";
     } else if (MouseEvent.isInstance(event)) {
       action = event.target.id == "urlbar-go-button" ? "go_button" : "click";
-    } else if (event.type == "command") {
-      action = "click";
     } else {
       action = "enter";
     }
@@ -982,7 +980,7 @@ class TelemetryEvent {
     );
 
     if (details.selType === "dismiss") {
-      // The conventional telemetry doesn't support "dismiss" event.
+      // The conventional telemetry dones't support "dismiss" event.
       return;
     }
 
@@ -1308,36 +1306,29 @@ class TelemetryEvent {
   }
 
   /**
-   * Extracts a telemetry type from a result and the element being interacted
-   * with for event telemetry.
+   * Extracts a telemetry type from an element for event telemetry.
    *
-   * @param {object} result The element to analyze.
    * @param {Element} element The element to analyze.
    * @returns {string} a string type for the telemetry event.
    */
-  typeFromElement(result, element) {
+  typeFromElement(element) {
     if (!element) {
       return "none";
     }
-    if (result?.providerName != "UrlbarProviderTopSites") {
+    let row = element.closest(".urlbarView-row");
+    if (row.result && row.result.providerName != "UrlbarProviderTopSites") {
       // Element handlers go here.
-      if (
-        element.classList.contains("urlbarView-button-help") ||
-        element.dataset.command == "help"
-      ) {
-        return result?.type == lazy.UrlbarUtils.RESULT_TYPE.TIP
+      if (element.classList.contains("urlbarView-button-help")) {
+        return row.result.type == lazy.UrlbarUtils.RESULT_TYPE.TIP
           ? "tiphelp"
           : "help";
       }
-      if (
-        element.classList.contains("urlbarView-button-block") ||
-        element.dataset.command == "dismiss"
-      ) {
+      if (element.classList.contains("urlbarView-button-block")) {
         return "block";
       }
     }
     // Now handle the result.
-    return lazy.UrlbarUtils.telemetryTypeFromResult(result);
+    return lazy.UrlbarUtils.telemetryTypeFromResult(row.result);
   }
 
   /**

browser/components/urlbar/UrlbarInput.sys.mjs

@@ -619,10 +619,7 @@ export class UrlbarInput {
     }
 
     let url;
-    let selType = this.controller.engagementEvent.typeFromElement(
-      result,
-      element
-    );
+    let selType = this.controller.engagementEvent.typeFromElement(element);
     let typedValue = this.value;
     if (oneOffParams?.engine) {
       selType = "oneoff";
@@ -840,10 +837,7 @@ export class UrlbarInput {
       return;
     }
 
-    if (
-      element?.classList.contains("urlbarView-button-block") ||
-      element?.dataset.command == "dismiss"
-    ) {
+    if (element?.classList.contains("urlbarView-button-block")) {
       this.controller.handleDeleteEntry(event, result);
       return;
     }
@@ -1047,8 +1041,7 @@ export class UrlbarInput {
       }
       case lazy.UrlbarUtils.RESULT_TYPE.TIP: {
         let scalarName =
-          element.classList.contains("urlbarView-button-help") ||
-          element.dataset.command == "help"
+          element.dataset.name == "help"
             ? `${result.payload.type}-help`
             : `${result.payload.type}-picked`;
         Services.telemetry.keyedScalarAdd("urlbar.tips", scalarName, 1);
@@ -1095,10 +1088,7 @@ export class UrlbarInput {
             selIndex,
             searchString: this._lastSearchString,
             searchMode,
-            selType: this.controller.engagementEvent.typeFromElement(
-              result,
-              element
-            ),
+            selType: this.controller.engagementEvent.typeFromElement(element),
             provider: result.providerName,
             element,
             startEventInfo,
@@ -1156,7 +1146,7 @@ export class UrlbarInput {
     this.controller.engagementEvent.record(event, {
       searchString: this._lastSearchString,
       selIndex,
-      selType: this.controller.engagementEvent.typeFromElement(result, element),
+      selType: this.controller.engagementEvent.typeFromElement(element),
       provider: result.providerName,
       searchSource: this.getSearchSource(event),
     });
@@ -3004,13 +2994,12 @@ export class UrlbarInput {
 
   _on_command(event) {
     // Something is executing a command, likely causing a focus change. This
-    // should not be recorded as an abandonment. If the user is selecting a
-    // result menu item or entering search mode from a one-off, then they are
-    // in the same engagement and we should not discard.
+    // should not be recorded as an abandonment. If the user is entering search
+    // mode from a one-off, then they are in the same engagement and we should
+    // not discard.
     if (
-      !event.target.classList.contains("urlbarView-result-menuitem") &&
-      (!event.target.classList.contains("searchbar-engine-one-off-item") ||
-        this.searchMode?.entry != "oneoff")
+      !event.target.classList.contains("searchbar-engine-one-off-item") ||
+      this.searchMode?.entry != "oneoff"
     ) {
       this.controller.engagementEvent.discard();
     }

browser/components/urlbar/UrlbarView.sys.mjs

@@ -40,8 +40,8 @@ const ZERO_PREFIX_SCALAR_ENGAGEMENT = "urlbar.zeroprefix.engagement";
 const ZERO_PREFIX_SCALAR_EXPOSURE = "urlbar.zeroprefix.exposure";
 
 const RESULT_MENU_COMMANDS = {
-  DISMISS: "dismiss",
-  HELP: "help",
+  BLOCK: "block",
+  LEARN_MORE: "learn-more",
 };
 
 const getBoundsWithoutFlushing = element =>
@@ -258,9 +258,7 @@ export class UrlbarView {
    *   The result of the element's row.
    */
   getResultFromElement(element) {
-    return element?.classList.contains("urlbarView-result-menuitem")
-      ? this.#resultMenuResult
-      : this.#getRowFromElement(element)?.result;
+    return this.#getRowFromElement(element)?.result;
   }
 
   /**
@@ -2661,20 +2659,20 @@ export class UrlbarView {
       result.source == lazy.UrlbarUtils.RESULT_SOURCE.HISTORY &&
       !result.autofill
     ) {
-      commands.set(RESULT_MENU_COMMANDS.DISMISS, {
+      commands.set(RESULT_MENU_COMMANDS.BLOCK, {
         l10n: { id: "urlbar-result-menu-remove-from-history" },
       });
-      commands.set(RESULT_MENU_COMMANDS.HELP, {
+      commands.set(RESULT_MENU_COMMANDS.LEARN_MORE, {
         l10n: { id: "urlbar-result-menu-learn-more" },
       });
     }
     if (result.payload.isBlockable) {
-      commands.set(RESULT_MENU_COMMANDS.DISMISS, {
+      commands.set(RESULT_MENU_COMMANDS.BLOCK, {
         l10n: result.payload.blockL10n,
       });
     }
     if (result.payload.helpUrl) {
-      commands.set(RESULT_MENU_COMMANDS.HELP, {
+      commands.set(RESULT_MENU_COMMANDS.LEARN_MORE, {
         l10n: result.payload.helpL10n,
       });
     }
@@ -2693,7 +2691,6 @@ export class UrlbarView {
         "menuitem"
       );
       menuitem.dataset.command = command;
-      menuitem.classList.add("urlbarView-result-menuitem");
       this.#setElementL10n(menuitem, data.l10n);
       this.resultMenu.appendChild(menuitem);
     }
@@ -2971,14 +2968,18 @@ export class UrlbarView {
       this.#resultMenuResult = null;
       let menuitem = event.target;
       switch (menuitem.dataset.command) {
-        case RESULT_MENU_COMMANDS.HELP:
-          menuitem.dataset.url =
+        case RESULT_MENU_COMMANDS.BLOCK:
+          this.controller.handleDeleteEntry(null, result);
+          break;
+        case RESULT_MENU_COMMANDS.LEARN_MORE:
+          this.window.openTrustedLinkIn(
             result.payload.helpUrl ||
-            Services.urlFormatter.formatURLPref("app.support.baseURL") +
-              "awesome-bar-result-menu";
+              Services.urlFormatter.formatURLPref("app.support.baseURL") +
+                "awesome-bar-result-menu",
+            "tab"
+          );
           break;
       }
-      this.input.pickResult(result, event, menuitem);
     }
   }
 

browser/components/urlbar/tests/browser-tips/browser_interventions.js

@@ -273,17 +273,17 @@ add_task(async function pickHelpButton() {
     );
 
     if (UrlbarPrefs.get("resultMenu")) {
-      let loadPromise = BrowserTestUtils.browserLoaded(
-        gBrowser.selectedBrowser,
-        false,
+      let tabOpenPromise = BrowserTestUtils.waitForNewTab(
+        gBrowser,
         "http://example.com/"
       );
       await UrlbarTestUtils.openResultMenuAndPressAccesskey(window, "h", {
         openByMouse: true,
         resultIndex: 1,
       });
-      info("Waiting for help URL to load in the current tab");
-      await loadPromise;
+      info("Waiting for help URL to load in a new tab");
+      await tabOpenPromise;
+      gBrowser.removeCurrentTab();
     } else {
       let helpButton = element._buttons.get("help");
       Assert.ok(helpButton, "Help button exists");
@@ -297,6 +297,13 @@ add_task(async function pickHelpButton() {
       await BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser);
     }
 
+    if (UrlbarPrefs.get("resultMenu")) {
+      todo(
+        false,
+        "help telemetry for the result menu to be implemented in bug 1790020"
+      );
+      return;
+    }
     const scalars = TelemetryTestUtils.getProcessScalars("parent", true, true);
     TelemetryTestUtils.assertKeyedScalar(
       scalars,

browser/components/urlbar/tests/browser-tips/browser_picks.js

@@ -28,7 +28,7 @@ add_task(async function enter_mainButton_noURL() {
   await doTest({ click: false });
 });
 
-add_task(async function enter_help() {
+add_task(async function enter_helpButton() {
   await doTest({ click: false, helpUrl: HELP_URL });
 });
 
@@ -40,7 +40,7 @@ add_task(async function mouse_mainButton_noURL() {
   await doTest({ click: true });
 });
 
-add_task(async function mouse_help() {
+add_task(async function mouse_helpButton() {
   await doTest({ click: true, helpUrl: HELP_URL });
 });
 
@@ -105,6 +105,14 @@ add_task(async function mouse_insideTipButNotOnButtons() {
  *   to pick the main button instead.
  */
 async function doTest({ click, buttonUrl = undefined, helpUrl = undefined }) {
+  if (UrlbarPrefs.get("resultMenu") && helpUrl) {
+    todo(
+      false,
+      "help telemetry for the result menu to be implemented in bug 1790020"
+    );
+    return;
+  }
+
   // Open a new tab for the test if we expect to load a URL.
   let tab;
   if (buttonUrl || helpUrl) {
@@ -136,9 +144,8 @@ async function doTest({ click, buttonUrl = undefined, helpUrl = undefined }) {
   });
   let row = await UrlbarTestUtils.waitForAutocompleteResultAt(window, 0);
   let mainButton = row._buttons.get("0");
-  let target = helpUrl
-    ? row._buttons.get(UrlbarPrefs.get("resultMenu") ? "menu" : "help")
-    : mainButton;
+  let helpButton = row._buttons.get("help");
+  let target = helpUrl ? helpButton : mainButton;
 
   // If we're picking the tip with the keyboard, TAB to select the proper
   // target.
@@ -156,12 +163,7 @@ async function doTest({ click, buttonUrl = undefined, helpUrl = undefined }) {
   await Promise.all([
     pickedPromise || BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser),
     UrlbarTestUtils.promisePopupClose(window, () => {
-      if (helpUrl && UrlbarPrefs.get("resultMenu")) {
-        UrlbarTestUtils.openResultMenuAndPressAccesskey(window, "h", {
-          openByMouse: click,
-          resultIndex: 0,
-        });
-      } else if (click) {
+      if (click) {
         EventUtils.synthesizeMouseAtCenter(target, {});
       } else {
         EventUtils.synthesizeKey("KEY_Enter");
@@ -182,10 +184,7 @@ async function doTest({ click, buttonUrl = undefined, helpUrl = undefined }) {
       {
         category: "urlbar",
         method: "engagement",
-        object:
-          click || (helpUrl && UrlbarPrefs.get("resultMenu"))
-            ? "click"
-            : "enter",
+        object: click ? "click" : "enter",
         value: "typed",
       },
     ],
@@ -213,11 +212,7 @@ function makeTipResult({ buttonUrl, helpUrl }) {
         },
       ],
       helpUrl,
-      helpL10n: {
-        id: UrlbarPrefs.get("resultMenu")
-          ? "urlbar-result-menu-tip-get-help"
-          : "urlbar-tip-help-icon",
-      },
+      helpL10n: { id: "urlbar-search-tips-confirm" },
     }
   );
 }

browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry_engagement.js

@@ -257,25 +257,28 @@ const tests = [
   },
 
   async function(win) {
+    if (UrlbarPrefs.get("resultMenu")) {
+      todo(
+        false,
+        "telemetry for the result menu to be implemented in bug 1790020"
+      );
+      return null;
+    }
     let tipProvider = registerTipProvider();
-    info("Selecting a tip's help option.");
+    info("Selecting a tip's help button, enter.");
     let promise = BrowserTestUtils.browserLoaded(win.gBrowser.selectedBrowser);
     win.gURLBar.search("x");
     await UrlbarTestUtils.promiseSearchComplete(win);
     EventUtils.synthesizeKey("KEY_ArrowDown", {}, win);
     EventUtils.synthesizeKey("KEY_ArrowDown", {}, win);
-    if (UrlbarPrefs.get("resultMenu")) {
-      await UrlbarTestUtils.openResultMenuAndPressAccesskey(win, "h");
-    } else {
-      EventUtils.synthesizeKey("KEY_Tab", {}, win);
-      EventUtils.synthesizeKey("VK_RETURN", {}, win);
-    }
+    EventUtils.synthesizeKey("KEY_Tab", {}, win);
+    EventUtils.synthesizeKey("VK_RETURN", {}, win);
     await promise;
     unregisterTipProvider(tipProvider);
     return {
       category: "urlbar",
       method: "engagement",
-      object: UrlbarPrefs.get("resultMenu") ? "click" : "enter",
+      object: "enter",
       value: "typed",
       extra: {
         elapsed: val => parseInt(val) > 0,
@@ -1276,11 +1279,6 @@ let tipMatches = [
     UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL,
     {
       helpUrl: "http://example.com/",
-      helpL10n: {
-        id: UrlbarPrefs.get("resultMenu")
-          ? "urlbar-result-menu-tip-get-help"
-          : "urlbar-tip-help-icon",
-      },
       type: "test",
       titleL10n: { id: "urlbar-search-tips-confirm" },
       buttons: [

browser/components/urlbar/tests/engagementTelemetry/browser/browser_glean_telemetry_engagement_type.js

@@ -87,7 +87,14 @@ add_task(async function engagement_type_dismiss() {
         () => originalResultCount != UrlbarTestUtils.getResultCount(window)
       );
 
-      assertEngagementTelemetry([{ engagement_type: "dismiss" }]);
+      if (UrlbarPrefs.get("resultMenu")) {
+        todo(
+          false,
+          "telemetry for the result menu to be implemented in bug 1790020"
+        );
+      } else {
+        assertEngagementTelemetry([{ engagement_type: "dismiss" }]);
+      }
     });
 
     await doTest(async browser => {
@@ -129,7 +136,14 @@ add_task(async function engagement_type_help() {
       const tab = await onTabOpened;
       BrowserTestUtils.removeTab(tab);
 
-      assertEngagementTelemetry([{ engagement_type: "help" }]);
+      if (UrlbarPrefs.get("resultMenu")) {
+        todo(
+          false,
+          "telemetry for the result menu to be implemented in bug 1790020"
+        );
+      } else {
+        assertEngagementTelemetry([{ engagement_type: "help" }]);
+      }
     });
 
     await SpecialPowers.popPrefEnv();