diff --git a/browser/components/urlbar/UrlbarController.sys.mjs b/browser/components/urlbar/UrlbarController.sys.mjs index ed3af63f7edd..bb182dc8ecff 100644 --- a/browser/components/urlbar/UrlbarController.sys.mjs +++ b/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); } /** diff --git a/browser/components/urlbar/UrlbarInput.sys.mjs b/browser/components/urlbar/UrlbarInput.sys.mjs index ab40268252a3..389b3c73df84 100644 --- a/browser/components/urlbar/UrlbarInput.sys.mjs +++ b/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(); } diff --git a/browser/components/urlbar/UrlbarView.sys.mjs b/browser/components/urlbar/UrlbarView.sys.mjs index 9a670a3401d7..e75985efeec1 100644 --- a/browser/components/urlbar/UrlbarView.sys.mjs +++ b/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); } } diff --git a/browser/components/urlbar/tests/browser-tips/browser_interventions.js b/browser/components/urlbar/tests/browser-tips/browser_interventions.js index 054c33caf7f2..2db559f0eabd 100644 --- a/browser/components/urlbar/tests/browser-tips/browser_interventions.js +++ b/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, diff --git a/browser/components/urlbar/tests/browser-tips/browser_picks.js b/browser/components/urlbar/tests/browser-tips/browser_picks.js index aacbfac009cf..873246fda731 100644 --- a/browser/components/urlbar/tests/browser-tips/browser_picks.js +++ b/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" }, } ); } diff --git a/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry_engagement.js b/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry_engagement.js index 6c990a555a18..b11fc1294a98 100644 --- a/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry_engagement.js +++ b/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: [ diff --git a/browser/components/urlbar/tests/engagementTelemetry/browser/browser_glean_telemetry_engagement_type.js b/browser/components/urlbar/tests/engagementTelemetry/browser/browser_glean_telemetry_engagement_type.js index fa542dabc58a..e7657b9a4d80 100644 --- a/browser/components/urlbar/tests/engagementTelemetry/browser/browser_glean_telemetry_engagement_type.js +++ b/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();