From 415beb83a9b07bd669d18836c6cdd47873f235db Mon Sep 17 00:00:00 2001 From: Stephanie Cunnane Date: Tue, 23 May 2023 22:04:08 +0000 Subject: [PATCH] Bug 1816738 - Add opened_in_new_tab tracking to SERP impression event. r=jteow Differential Revision: https://phabricator.services.mozilla.com/D177531 --- .../search/SearchSERPTelemetry.sys.mjs | 35 ++- browser/components/search/metrics.yaml | 4 +- .../search/test/browser/browser.ini | 4 + ...ser_search_telemetry_engagement_content.js | 4 +- .../browser_search_telemetry_sources.js | 12 +- ...ser_search_telemetry_sources_in_content.js | 256 ++++++++++++++++++ .../components/search/test/browser/head.js | 2 +- 7 files changed, 303 insertions(+), 14 deletions(-) create mode 100644 browser/components/search/test/browser/browser_search_telemetry_sources_in_content.js diff --git a/browser/components/search/SearchSERPTelemetry.sys.mjs b/browser/components/search/SearchSERPTelemetry.sys.mjs index e9ac215e2127..ab6c192e0cf2 100644 --- a/browser/components/search/SearchSERPTelemetry.sys.mjs +++ b/browser/components/search/SearchSERPTelemetry.sys.mjs @@ -54,11 +54,12 @@ export var SearchSERPTelemetryUtils = { SHOPPING_TAB: "shopping_tab", }, ABANDONMENTS: { + NAVIGATION: "navigation", TAB_CLOSE: "tab_close", WINDOW_CLOSE: "window_close", - NAVIGATION: "navigation", }, INCONTENT_SOURCES: { + OPENED_IN_NEW_TAB: "opened_in_new_tab", SEARCHBOX: "follow_on_from_refine_on_incontent_search", }, }; @@ -120,7 +121,9 @@ class TelemetryHandler { * @param {browser} browser * The browser object associated with the page that should be a SERP. * @param {string} type - * The component type that started the load. + * The source that started the load. One of + * SearchSERPTelemetryUtils.COMPONENTS.INCONTENT_SEARCHBOX or + * SearchSERPTelemetryUtils.INCONTENT_SOURCES.OPENED_IN_NEW_TAB. */ setBrowserContentSource(browser, type) { switch (type) { @@ -130,6 +133,12 @@ class TelemetryHandler { SearchSERPTelemetryUtils.INCONTENT_SOURCES.SEARCHBOX ); break; + case SearchSERPTelemetryUtils.INCONTENT_SOURCES.OPENED_IN_NEW_TAB: + this.#browserContentSourceMap.set( + browser, + SearchSERPTelemetryUtils.INCONTENT_SOURCES.OPENED_IN_NEW_TAB + ); + break; } } @@ -960,6 +969,7 @@ class ContentHandler { // tracked SERP. let browser = wrappedChannel.browserElement; let telemetryState; + let isFromNewtab = false; if (item.browserTelemetryStateMap.has(browser)) { // Current browser is tracked. telemetryState = item.browserTelemetryStateMap.get(browser); @@ -969,6 +979,9 @@ class ContentHandler { let tabBrowser = browser.getTabBrowser(); let tab = tabBrowser.getTabForBrowser(browser).openerTab; telemetryState = item.browserTelemetryStateMap.get(tab.linkedBrowser); + if (telemetryState) { + isFromNewtab = true; + } } // Step 2: If we have telemetryState, the browser object must be @@ -979,11 +992,16 @@ class ContentHandler { // engagement because the event was logged elsewhere. // - If the ad impression hasn't been recorded yet, we have no way of // knowing precisely what kind of component was selected. + let isSerp = false; if ( telemetryState && telemetryState.adImpressionsReported && !telemetryState.searchBoxSubmitted ) { + if (info.searchPageRegexp?.test(originURL)) { + isSerp = true; + } + // Determine the "type" of the link. let type = telemetryState.hrefToComponentMap?.get(URL); // The SERP provider may have modified the url with different query @@ -1010,10 +1028,8 @@ class ContentHandler { } // The SERP may have moved onto another page that matches a SERP page // e.g. Related Search - if (!type) { - type = info.searchPageRegexp?.test(URL) - ? SearchSERPTelemetryUtils.COMPONENTS.NON_ADS_LINK - : ""; + if (!type && isSerp) { + type = SearchSERPTelemetryUtils.COMPONENTS.NON_ADS_LINK; } // There might be other types of pages on a SERP that don't fall // neatly into expected non-ad expressions or SERPs, such as Image @@ -1024,6 +1040,13 @@ class ContentHandler { : ""; } + if (isSerp && isFromNewtab) { + SearchSERPTelemetry.setBrowserContentSource( + browser, + SearchSERPTelemetryUtils.INCONTENT_SOURCES.OPENED_IN_NEW_TAB + ); + } + // Step 3: If we have a type, record an engagement. // Exceptions: // - Related searches on some SERPs can be encoded with a URL that diff --git a/browser/components/search/metrics.yaml b/browser/components/search/metrics.yaml index 50e7a4df2cef..997835b2ea67 100644 --- a/browser/components/search/metrics.yaml +++ b/browser/components/search/metrics.yaml @@ -140,6 +140,7 @@ serp: - https://bugzilla.mozilla.org/show_bug.cgi?id=1813162 - https://bugzilla.mozilla.org/show_bug.cgi?id=1824543 - https://bugzilla.mozilla.org/show_bug.cgi?id=1816736 + - https://bugzilla.mozilla.org/show_bug.cgi?id=1816738 data_reviews: - https://bugzilla.mozilla.org/show_bug.cgi?id=1813162 - https://bugzilla.mozilla.org/show_bug.cgi?id=1824543 @@ -173,7 +174,8 @@ serp: Possible values are: `urlbar`, `urlbar_handoff`, `urlbar_searchmode`, `urlbar_persisted`, `searchbar`, `contextmenu`, `webextension`, `system`, `reload`, - `tabhistory`, `unknown`, `follow_on_from_refine_on_incontent_search`. + `tabhistory`, `follow_on_from_refine_on_incontent_search`, + `opened_in_new_tab`, `unknown`. This will be `unknown` if we cannot determine the source. type: string shopping_tab_displayed: diff --git a/browser/components/search/test/browser/browser.ini b/browser/components/search/test/browser/browser.ini index ed702f4d90d0..baca463c6b3e 100644 --- a/browser/components/search/test/browser/browser.ini +++ b/browser/components/search/test/browser/browser.ini @@ -123,6 +123,10 @@ support-files = searchTelemetryAd_dataAttributes.html searchTelemetryAd_dataAttributes_href.html searchTelemetryAd_dataAttributes_none.html +[browser_search_telemetry_sources_in_content.js] +tags = search-telemetry +support-files = + searchTelemetryAd_searchbox_with_content.html [browser_search_telemetry_sources_navigation.js] tags = search-telemetry support-files = diff --git a/browser/components/search/test/browser/browser_search_telemetry_engagement_content.js b/browser/components/search/test/browser/browser_search_telemetry_engagement_content.js index 6912b36c39d8..6ae74aa0366e 100644 --- a/browser/components/search/test/browser/browser_search_telemetry_engagement_content.js +++ b/browser/components/search/test/browser/browser_search_telemetry_engagement_content.js @@ -304,7 +304,7 @@ add_task(async function test_click_related_search_in_new_tab() { provider: "example", tagged: "false", partner_code: "", - source: "unknown", + source: "opened_in_new_tab", is_shopping_page: "false", shopping_tab_displayed: "true", }, @@ -366,7 +366,7 @@ add_task(async function test_click_redirect_search_in_newtab() { provider: "example", tagged: "false", partner_code: "", - source: "unknown", + source: "opened_in_new_tab", is_shopping_page: "false", shopping_tab_displayed: "true", }, diff --git a/browser/components/search/test/browser/browser_search_telemetry_sources.js b/browser/components/search/test/browser/browser_search_telemetry_sources.js index 7cabe40445fc..b7838a5d90d3 100644 --- a/browser/components/search/test/browser/browser_search_telemetry_sources.js +++ b/browser/components/search/test/browser/browser_search_telemetry_sources.js @@ -3,6 +3,10 @@ /* * Main tests for SearchSERPTelemetry - general engine visiting and link clicking. + * + * NOTE: As this test file is already fairly long-running, adding to this file + * will likely cause timeout errors with test-verify jobs on Treeherder. + * Therefore, please do not add further tasks to this file. */ "use strict"; @@ -20,7 +24,7 @@ const TEST_PROVIDER_INFO = [ { telemetryId: "example", searchPageRegexp: - /^https:\/\/example.com\/browser\/browser\/components\/search\/test\/browser\/searchTelemetry(?:Ad)?.html/, + /^https:\/\/example.org\/browser\/browser\/components\/search\/test\/browser\/searchTelemetry(?:Ad)?/, queryParamName: "s", codeParamName: "abc", taggedCodes: ["ff"], @@ -37,7 +41,7 @@ const TEST_PROVIDER_INFO = [ function getPageUrl(useAdPage = false) { let page = useAdPage ? "searchTelemetryAd.html" : "searchTelemetry.html"; - return `https://example.com/browser/browser/components/search/test/browser/${page}`; + return `https://example.org/browser/browser/components/search/test/browser/${page}`; } /** @@ -96,7 +100,7 @@ add_setup(async function () { search_url: getPageUrl(true), search_url_get_params: "s={searchTerms}&abc=ff", suggest_url: - "https://example.com/browser/browser/components/search/test/browser/searchSuggestionEngine.sjs", + "https://example.org/browser/browser/components/search/test/browser/searchSuggestionEngine.sjs", suggest_url_get_params: "query={searchTerms}", }, { setAsDefault: true } @@ -109,7 +113,7 @@ add_setup(async function () { Services.prefs.clearUserPref("browser.search.log"); SearchSERPTelemetry.overrideSearchTelemetryForTests(); Services.telemetry.canRecordExtended = oldCanRecord; - Services.telemetry.clearScalars(); + resetTelemetry(); }); }); diff --git a/browser/components/search/test/browser/browser_search_telemetry_sources_in_content.js b/browser/components/search/test/browser/browser_search_telemetry_sources_in_content.js new file mode 100644 index 000000000000..d4ff79bb8823 --- /dev/null +++ b/browser/components/search/test/browser/browser_search_telemetry_sources_in_content.js @@ -0,0 +1,256 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +/* + * SearchSERPTelemetry tests related to in-content sources. + */ + +"use strict"; + +const { SearchSERPTelemetry, SearchSERPTelemetryUtils } = + ChromeUtils.importESModule("resource:///modules/SearchSERPTelemetry.sys.mjs"); + +const TEST_PROVIDER_INFO = [ + { + telemetryId: "example", + searchPageRegexp: + /^https:\/\/example.org\/browser\/browser\/components\/search\/test\/browser\/searchTelemetry(?:Ad)?/, + queryParamName: "s", + codeParamName: "abc", + taggedCodes: ["ff"], + followOnParamNames: ["a"], + extraAdServersRegexps: [/^https:\/\/example\.com\/ad2?/], + components: [ + { + type: SearchSERPTelemetryUtils.COMPONENTS.AD_LINK, + default: true, + }, + ], + }, +]; + +function getSERPUrl(page, organic = false) { + let url = + getRootDirectory(gTestPath).replace( + "chrome://mochitests/content", + "https://example.org" + ) + page; + return `${url}?s=test${organic ? "" : "&abc=ff"}`; +} + +// sharedData messages are only passed to the child on idle. Therefore +// we wait for a few idles to try and ensure the messages have been able +// to be passed across and handled. +async function waitForIdle() { + for (let i = 0; i < 10; i++) { + await new Promise(resolve => Services.tm.idleDispatchToMainThread(resolve)); + } +} + +add_setup(async function () { + SearchSERPTelemetry.overrideSearchTelemetryForTests(TEST_PROVIDER_INFO); + await waitForIdle(); + await SpecialPowers.pushPrefEnv({ + set: [["browser.search.serpEventTelemetry.enabled", true]], + }); + // Enable local telemetry recording for the duration of the tests. + let oldCanRecord = Services.telemetry.canRecordExtended; + Services.telemetry.canRecordExtended = true; + Services.prefs.setBoolPref("browser.search.log", true); + + registerCleanupFunction(async () => { + Services.prefs.clearUserPref("browser.search.log"); + SearchSERPTelemetry.overrideSearchTelemetryForTests(); + Services.telemetry.canRecordExtended = oldCanRecord; + resetTelemetry(); + }); +}); + +add_task(async function test_source_opened_in_new_tab_via_middle_click() { + resetTelemetry(); + let url = getSERPUrl("searchTelemetryAd_searchbox_with_content.html"); + let tab1 = await BrowserTestUtils.openNewForegroundTab(gBrowser, url); + await waitForPageWithAdImpressions(); + + let targetUrl = + getRootDirectory(gTestPath).replace( + "chrome://mochitests/content", + "https://example.org" + ) + "searchTelemetryAd_searchbox_with_content.html?s=test+one+two+three"; + + let tabPromise = BrowserTestUtils.waitForNewTab(gBrowser, targetUrl, true); + await BrowserTestUtils.synthesizeMouseAtCenter( + "a#related-in-page", + { button: 1 }, + tab1.linkedBrowser + ); + let tab2 = await tabPromise; + + await TestUtils.waitForCondition(() => { + return Glean.serp.impression?.testGetValue()?.length == 2; + }, "Should have two impressions."); + + assertImpressionEvents([ + { + impression: { + provider: "example", + tagged: "true", + partner_code: "ff", + source: "unknown", + is_shopping_page: "false", + shopping_tab_displayed: "false", + }, + engagements: [ + { + action: SearchSERPTelemetryUtils.ACTIONS.CLICKED, + target: SearchSERPTelemetryUtils.COMPONENTS.NON_ADS_LINK, + }, + ], + }, + { + impression: { + provider: "example", + tagged: "false", + partner_code: "", + source: "opened_in_new_tab", + is_shopping_page: "false", + shopping_tab_displayed: "false", + }, + }, + ]); + + BrowserTestUtils.removeTab(tab1); + BrowserTestUtils.removeTab(tab2); +}); + +add_task(async function test_source_opened_in_new_tab_via_target_blank() { + resetTelemetry(); + let url = getSERPUrl("searchTelemetryAd_searchbox_with_content.html"); + let tab1 = await BrowserTestUtils.openNewForegroundTab(gBrowser, url); + await waitForPageWithAdImpressions(); + + let targetUrl = + getRootDirectory(gTestPath).replace( + "chrome://mochitests/content", + "https://example.org" + ) + "searchTelemetryAd_searchbox_with_content.html?s=test+one+two+three"; + + let tabPromise = BrowserTestUtils.waitForNewTab(gBrowser, targetUrl, true); + // Note: the anchor element with id "related-new-tab" has a target=_blank + // attribute. + await BrowserTestUtils.synthesizeMouseAtCenter( + "a#related-new-tab", + {}, + tab1.linkedBrowser + ); + let tab2 = await tabPromise; + + await TestUtils.waitForCondition(() => { + return Glean.serp.impression?.testGetValue()?.length == 2; + }, "Should have two impressions."); + + assertImpressionEvents([ + { + impression: { + provider: "example", + tagged: "true", + partner_code: "ff", + source: "unknown", + is_shopping_page: "false", + shopping_tab_displayed: "false", + }, + engagements: [ + { + action: SearchSERPTelemetryUtils.ACTIONS.CLICKED, + target: SearchSERPTelemetryUtils.COMPONENTS.NON_ADS_LINK, + }, + ], + }, + { + impression: { + provider: "example", + tagged: "false", + partner_code: "", + source: "opened_in_new_tab", + is_shopping_page: "false", + shopping_tab_displayed: "false", + }, + }, + ]); + + BrowserTestUtils.removeTab(tab1); + BrowserTestUtils.removeTab(tab2); +}); + +add_task(async function test_source_opened_in_new_tab_via_context_menu() { + resetTelemetry(); + let url = getSERPUrl("searchTelemetryAd_searchbox_with_content.html"); + let tab1 = await BrowserTestUtils.openNewForegroundTab(gBrowser, url); + await waitForPageWithAdImpressions(); + + let targetUrl = + getRootDirectory(gTestPath).replace( + "chrome://mochitests/content", + "https://example.org" + ) + "searchTelemetryAd_searchbox_with_content.html?s=test+one+two+three"; + + let tabPromise = BrowserTestUtils.waitForNewTab(gBrowser, targetUrl, true); + + let contextMenu = document.getElementById("contentAreaContextMenu"); + let popupShownPromise = BrowserTestUtils.waitForEvent( + contextMenu, + "popupshown" + ); + await BrowserTestUtils.synthesizeMouseAtCenter( + "a#related-in-page", + { + button: 2, + type: "contextmenu", + }, + tab1.linkedBrowser + ); + await popupShownPromise; + + let openLinkInNewTabMenuItem = contextMenu.querySelector( + "#context-openlinkintab" + ); + contextMenu.activateItem(openLinkInNewTabMenuItem); + + let tab2 = await tabPromise; + + await TestUtils.waitForCondition(() => { + return Glean.serp.impression?.testGetValue()?.length == 2; + }, "Should have two impressions."); + + assertImpressionEvents([ + { + impression: { + provider: "example", + tagged: "true", + partner_code: "ff", + source: "unknown", + is_shopping_page: "false", + shopping_tab_displayed: "false", + }, + engagements: [ + { + action: SearchSERPTelemetryUtils.ACTIONS.CLICKED, + target: SearchSERPTelemetryUtils.COMPONENTS.NON_ADS_LINK, + }, + ], + }, + { + impression: { + provider: "example", + tagged: "false", + partner_code: "", + source: "opened_in_new_tab", + is_shopping_page: "false", + shopping_tab_displayed: "false", + }, + }, + ]); + + BrowserTestUtils.removeTab(tab1); + BrowserTestUtils.removeTab(tab2); +}); diff --git a/browser/components/search/test/browser/head.js b/browser/components/search/test/browser/head.js index aa062b5fcf82..c0b6b27fa687 100644 --- a/browser/components/search/test/browser/head.js +++ b/browser/components/search/test/browser/head.js @@ -364,7 +364,7 @@ async function promiseAdImpressionReceived(num) { return TestUtils.waitForCondition(() => { let adImpressions = Glean.serp.adImpression.testGetValue() ?? []; return adImpressions.length == num; - }, `Should have received an ${num} ad impressions.`); + }, `Should have received ${num} ad impressions.`); } return TestUtils.waitForCondition(() => { let adImpressions = Glean.serp.adImpression.testGetValue() ?? [];