diff --git a/browser/components/search/SearchTelemetry.jsm b/browser/components/search/SearchTelemetry.jsm index b33456fb85d6..deb76de6fc41 100644 --- a/browser/components/search/SearchTelemetry.jsm +++ b/browser/components/search/SearchTelemetry.jsm @@ -199,16 +199,19 @@ class TelemetryHandler { this._reportSerpPage(info, url); - let item = this._browserInfoByURL.get(url); - if (item) { - item.browsers.add(browser); - item.count++; - } else { - this._browserInfoByURL.set(url, { - browsers: new WeakSet([browser]), - info, - count: 1, - }); + // If we have a code, then we also track this for potential ad clicks. + if (info.code) { + let item = this._browserInfoByURL.get(url); + if (item) { + item.browsers.add(browser); + item.count++; + } else { + this._browserInfoByURL.set(url, { + browsers: new WeakSet([browser]), + info, + count: 1, + }); + } } } @@ -696,13 +699,13 @@ class ContentHandler { } let originURL = channel.originURI && channel.originURI.spec; - let item = this._findBrowserItemForURL(originURL); - if (!originURL || !item) { + let info = this._findBrowserItemForURL(originURL); + if (!originURL || !info) { return; } let URL = channel.finalURL; - let info = this._getProviderInfoForURL(URL, true); + info = this._getProviderInfoForURL(URL, true); if (!info) { return; } @@ -710,7 +713,7 @@ class ContentHandler { try { Services.telemetry.keyedScalarAdd( SEARCH_AD_CLICKS_SCALAR, - `${info.telemetryId}:${item.info.code ? "sap" : "organic"}`, + info.telemetryId, 1 ); channel._adClickRecorded = true; @@ -745,12 +748,12 @@ class ContentHandler { return; } - logConsole.debug("Counting ads in page for", item.info.provider, info.url); Services.telemetry.keyedScalarAdd( SEARCH_WITH_ADS_SCALAR, - `${item.info.provider}:${item.info.code ? "sap" : "organic"}`, + item.info.provider, 1 ); + logConsole.debug("Counting ads in page for", item.info.provider, info.url); } } diff --git a/browser/components/search/docs/index.rst b/browser/components/search/docs/index.rst deleted file mode 100644 index 42625189dbf6..000000000000 --- a/browser/components/search/docs/index.rst +++ /dev/null @@ -1,21 +0,0 @@ -Search -====== - -This document describes the implementation of parts of Firefox's search interfaces. - -The search area covers: - - * Search bar on the toolbar - * In-content search - * One-off search buttons on both the search and address bars - -Search Engine handling is taken care of with the `toolkit Search Service`_. - -Most of the search code lives in `browser/components/search`_. - -.. toctree:: - - telemetry - -.. _toolkit Search Service: /toolkit/search/index.html -.. _browser/components/search: https://searchfox.org/mozilla-central/source/browser/components/search diff --git a/browser/components/search/docs/telemetry.rst b/browser/components/search/docs/telemetry.rst deleted file mode 100644 index cbbb8c93a2e3..000000000000 --- a/browser/components/search/docs/telemetry.rst +++ /dev/null @@ -1,42 +0,0 @@ -Telemetry -========= - -This section describes existing telemetry probes measuring interaction with -search engines. - -Note: Some search related probes are also documented on the `address bar telemetry`_ -page. - -.. toctree:: - :caption: Table of Contents - - telemetry - -Search probes relevant to front-end searches --------------------------------------------- - -Definitions: - - * ``organic`` is a search that a user performs by visiting a search engine - directly. - * ``sap`` (search access point) is a search that a user performs by visiting - via one of Firefox's access points, using the associated partner codes. - * ``sap-follow-on`` is a SAP search where the user has first accessed the site - via a SAP, and then performed an additional search. - * ``SERP`` refers to a search engine result page. - -SEARCH_COUNTS - This histogram records search counts for visits to SERP in-content pages. It - also stores other items - see `address bar telemetry`_. For in-content - searches, the format is - ``.in-content:[sap|sap-follow-on|organic]:[code|none]``. - -browser.search.with_ads - This keyed scalar records counts of SERP pages with adverts displayed. - The key format is ``:``. - -browser.search.ad_clicks - Records clicks of adverts on SERP pages. The key format is - ``:``. - -.. _address bar telemetry: /browser/urlbar/telemetry.html diff --git a/browser/components/search/moz.build b/browser/components/search/moz.build index 9a229d0bb78d..efb73b82fe90 100644 --- a/browser/components/search/moz.build +++ b/browser/components/search/moz.build @@ -21,7 +21,5 @@ XPCSHELL_TESTS_MANIFESTS += ["test/unit/xpcshell.ini"] JAR_MANIFESTS += ["jar.mn"] -SPHINX_TREES["/browser/search"] = "docs" - with Files("**"): BUG_COMPONENT = ("Firefox", "Search") diff --git a/browser/components/search/schema/search-telemetry-schema.json b/browser/components/search/schema/search-telemetry-schema.json index 0851e7867b8c..f6b5673c8928 100644 --- a/browser/components/search/schema/search-telemetry-schema.json +++ b/browser/components/search/schema/search-telemetry-schema.json @@ -47,7 +47,7 @@ }, "followOnCookies": { "type": "array", - "title": "Follow-on Cookies", + "title": "Follow-on Cookes", "description": "An array of cookie details that are used to identify follow-on searches.", "items": { "type": "object", diff --git a/browser/components/search/test/browser/browser_searchTelemetry.js b/browser/components/search/test/browser/browser_searchTelemetry.js index 4885a29d62ee..43cd1ece4c94 100644 --- a/browser/components/search/test/browser/browser_searchTelemetry.js +++ b/browser/components/search/test/browser/browser_searchTelemetry.js @@ -37,8 +37,8 @@ function getPageUrl(useExample = false, useAdPage = false) { return `http://${server}/browser/browser/components/search/test/browser/${page}`; } -function getSERPUrl(page, organic = false) { - return `${page}?s=test${organic ? "" : "&abc=ff"}`; +function getSERPUrl(page) { + return page + "?s=test&abc=ff"; } function getSERPFollowOnUrl(page) { @@ -202,26 +202,7 @@ add_task(async function test_track_ad() { await assertTelemetry( { "example.in-content:sap:ff": 1 }, { - "browser.search.with_ads": { "example:sap": 1 }, - } - ); - - BrowserTestUtils.removeTab(tab); -}); - -add_task(async function test_track_ad_organic() { - Services.telemetry.clearScalars(); - searchCounts.clear(); - - let tab = await BrowserTestUtils.openNewForegroundTab( - gBrowser, - getSERPUrl(getPageUrl(false, true), true) - ); - - await assertTelemetry( - { "example.in-content:organic:none": 1 }, - { - "browser.search.with_ads": { "example:organic": 1 }, + "browser.search.with_ads": { example: 1 }, } ); @@ -245,7 +226,7 @@ add_task(async function test_track_ad_new_window() { await assertTelemetry( { "example.in-content:sap:ff": 1 }, { - "browser.search.with_ads": { "example:sap": 1 }, + "browser.search.with_ads": { example: 1 }, } ); @@ -275,7 +256,7 @@ add_task(async function test_track_ad_pages_without_ads() { await assertTelemetry( { "example.in-content:sap:ff": 2 }, { - "browser.search.with_ads": { "example:sap": 1 }, + "browser.search.with_ads": { example: 1 }, } ); @@ -284,25 +265,20 @@ add_task(async function test_track_ad_pages_without_ads() { } }); -async function track_ad_click(testOrganic) { +add_task(async function test_track_ad_click() { // Note: the above tests have already checked a page with no ad-urls. searchCounts.clear(); Services.telemetry.clearScalars(); - let expectedScalarKey = `example:${testOrganic ? "organic" : "sap"}`; - let expectedHistogramKey = `example.in-content:${ - testOrganic ? "organic:none" : "sap:ff" - }`; - let tab = await BrowserTestUtils.openNewForegroundTab( gBrowser, - getSERPUrl(getPageUrl(false, true), testOrganic) + getSERPUrl(getPageUrl(false, true)) ); await assertTelemetry( - { [expectedHistogramKey]: 1 }, + { "example.in-content:sap:ff": 1 }, { - "browser.search.with_ads": { [expectedScalarKey]: 1 }, + "browser.search.with_ads": { example: 1 }, } ); @@ -315,10 +291,10 @@ async function track_ad_click(testOrganic) { await new Promise(resolve => setTimeout(resolve, ADLINK_CHECK_TIMEOUT_MS)); await assertTelemetry( - { [expectedHistogramKey]: 1 }, + { "example.in-content:sap:ff": 1 }, { - "browser.search.with_ads": { [expectedScalarKey]: 1 }, - "browser.search.ad_clicks": { [expectedScalarKey]: 1 }, + "browser.search.with_ads": { example: 1 }, + "browser.search.ad_clicks": { example: 1 }, } ); @@ -331,10 +307,10 @@ async function track_ad_click(testOrganic) { // We've gone back, so we register an extra display & if it is with ads or not. await assertTelemetry( - { [expectedHistogramKey]: 2 }, + { "example.in-content:sap:ff": 2 }, { - "browser.search.with_ads": { [expectedScalarKey]: 2 }, - "browser.search.ad_clicks": { [expectedScalarKey]: 1 }, + "browser.search.with_ads": { example: 2 }, + "browser.search.ad_clicks": { example: 1 }, } ); @@ -347,22 +323,14 @@ async function track_ad_click(testOrganic) { await new Promise(resolve => setTimeout(resolve, ADLINK_CHECK_TIMEOUT_MS)); await assertTelemetry( - { [expectedHistogramKey]: 2 }, + { "example.in-content:sap:ff": 2 }, { - "browser.search.with_ads": { [expectedScalarKey]: 2 }, - "browser.search.ad_clicks": { [expectedScalarKey]: 2 }, + "browser.search.with_ads": { example: 2 }, + "browser.search.ad_clicks": { example: 2 }, } ); BrowserTestUtils.removeTab(tab); -} - -add_task(async function test_track_ad_click() { - await track_ad_click(false); -}); - -add_task(async function test_track_ad_click_organic() { - await track_ad_click(true); }); add_task(async function test_track_ad_click_with_location_change_other_tab() { @@ -374,7 +342,7 @@ add_task(async function test_track_ad_click_with_location_change_other_tab() { await assertTelemetry( { "example.in-content:sap:ff": 1 }, { - "browser.search.with_ads": { "example:sap": 1 }, + "browser.search.with_ads": { example: 1 }, } ); @@ -394,8 +362,8 @@ add_task(async function test_track_ad_click_with_location_change_other_tab() { await assertTelemetry( { "example.in-content:sap:ff": 1 }, { - "browser.search.with_ads": { "example:sap": 1 }, - "browser.search.ad_clicks": { "example:sap": 1 }, + "browser.search.with_ads": { example: 1 }, + "browser.search.ad_clicks": { example: 1 }, } ); diff --git a/browser/docs/index.rst b/browser/docs/index.rst index 25b0b43c12a8..968945adf219 100644 --- a/browser/docs/index.rst +++ b/browser/docs/index.rst @@ -15,9 +15,9 @@ This is the nascent documentation of the Firefox front-end code. installer/windows/installer/index /toolkit/mozapps/defaultagent/default-browser-agent/index components/newtab/content-src/asrouter/docs/index - search/index base/sslerrorreport/index base/tabbrowser/index touchbar/index components/uitour/docs/index components/payments/docs/index + diff --git a/toolkit/components/telemetry/Scalars.yaml b/toolkit/components/telemetry/Scalars.yaml index ae402d8eacd9..71b3b70a4eba 100644 --- a/toolkit/components/telemetry/Scalars.yaml +++ b/toolkit/components/telemetry/Scalars.yaml @@ -4749,15 +4749,14 @@ browser.search: bug_numbers: - 1495548 - 1505411 - - 1664849 description: > - Records counts of SERP pages with adverts displayed. The key format is ‘:’. + Records counts of SERP pages with adverts displayed. The key format is ‘’. expires: never keyed: true kind: uint notification_emails: - fx-search@mozilla.com - - teon@mozilla.com + - adw@mozilla.com release_channel_collection: opt-out products: - 'firefox' @@ -4769,15 +4768,14 @@ browser.search: bug_numbers: - 1495548 - 1505411 - - 1664849 description: > - Records clicks of adverts on SERP pages. The key format is ‘:’. + Records clicks of adverts on SERP pages. The key format is ‘’. expires: never keyed: true kind: uint notification_emails: - fx-search@mozilla.com - - teon@mozilla.com + - adw@mozilla.com release_channel_collection: opt-out products: - 'firefox'