commit aca7ec66df41325d519b1fcc8d9355fd77d695ae
parent e0bf6829df84c54895a3766ded736435a08e6583
Author: Mark Banner <standard8@mozilla.com>
Date: Mon, 13 Oct 2025 13:14:27 +0000
Bug 1992354 - Enable TypeScript checks on UrlbarController. r=urlbar-reviewers,mak
The main part of this is reworking TelemetryEvent to provide details of all of the options that
are passed to events.
The logging and reporting to Glean is duplicated, as this allows TypeScript to properly detect issues
with the passed data. This is also why we need the 'toString()' calls. Glean events have extra params
recorded as strings, which is normally hidden because of the XPCOM interfaces.
Differential Revision: https://phabricator.services.mozilla.com/D267374
Diffstat:
4 files changed, 252 insertions(+), 150 deletions(-)
diff --git a/browser/components/urlbar/UrlbarController.sys.mjs b/browser/components/urlbar/UrlbarController.sys.mjs
@@ -6,6 +6,7 @@ import { AppConstants } from "resource://gre/modules/AppConstants.sys.mjs";
/**
* @import {ProvidersManager} from "moz-src:///browser/components/urlbar/UrlbarProvidersManager.sys.mjs"
+ * @import {UrlbarView} from "moz-src:///browser/components/urlbar/UrlbarView.sys.mjs"
*/
const lazy = {};
@@ -65,7 +66,7 @@ export class UrlbarController {
* Optional fake providers manager to override the built-in providers manager.
* Intended for use in unit tests only.
*/
- constructor(options = {}) {
+ constructor(options) {
if (!options.input) {
throw new Error("Missing options: input");
}
@@ -649,6 +650,7 @@ export class UrlbarController {
result,
selType: "dismiss",
searchString: queryContext.searchString,
+ searchSource: this.input.getSearchSource(event),
});
return true;
@@ -720,6 +722,7 @@ export class UrlbarController {
#focusOnUnifiedSearchButton() {
this.input.setUnifiedSearchButtonAvailability(true);
+ /** @type {HTMLElement} */
const switcher = this.input.querySelector(".searchmode-switcher");
// Set tabindex to be focusable.
switcher.setAttribute("tabindex", "-1");
@@ -731,11 +734,14 @@ export class UrlbarController {
this.input.addEventListener("blur", this.input);
switcher.addEventListener(
"blur",
+ /** @type {(e: FocusEvent) => void} */
e => {
switcher.removeAttribute("tabindex");
+
+ let relatedTarget = /** @type {HTMLElement} */ (e.relatedTarget);
if (
this.input.hasAttribute("focused") &&
- !e.relatedTarget?.closest("#urlbar")
+ !relatedTarget?.closest("#urlbar")
) {
// If the focus is not back to urlbar, fire blur event explicitly to
// clear the urlbar. Because the input field has been losing an
@@ -811,7 +817,7 @@ class TelemetryEvent {
}
// start is invoked on a user-generated event, but we only count the first
- // one. Once an engagement or abandoment happens, we clear _startEventInfo.
+ // one. Once an engagement or abandonment happens, we clear _startEventInfo.
return;
}
@@ -843,6 +849,43 @@ class TelemetryEvent {
}
/**
+ * @typedef {object} ActionDetails
+ * An object describing action details that are recorded in an event.
+ * @property {HTMLElement} [element]
+ * The picked view element.
+ * @property {UrlbarResult} [result]
+ * The engaged result. This should be set to the result related to the
+ * picked element.
+ * @property {boolean} [isSessionOngoing]
+ * Set to true if the search session is still ongoing.
+ * @property {object} [searchMode]
+ * The searchMode object to record.
+ * @property {string} searchSource
+ * The source of the search event.
+ * @property {string} [searchString]
+ * The user's search string. Note that this string is not sent with telemetry
+ * data. It is only used locally to discern other data, such as the number
+ * of characters and words in the string.
+ * @property {string} [selType]
+ * The Type of the selected element, undefined for "blur".
+ * One of "unknown", "autofill", "visiturl", "bookmark", "help", "history",
+ * "keyword", "searchengine", "searchsuggestion", "switchtab", "remotetab",
+ * "extension", "oneoff", "dismiss".
+ */
+
+ /**
+ * @typedef {object} AdditionalActionDetails
+ * @property {string} provider
+ * The name of the `UrlbarProvider` that provided the selected result.
+ * @property {number} selIndex
+ * The index of the selected result.
+ */
+
+ /**
+ * @typedef {ActionDetails & AdditionalActionDetails} InternalActionDetails
+ */
+
+ /**
* Record an engagement telemetry event.
* When the user picks a result from a search through the mouse or keyboard,
* an engagement event is recorded. If instead the user abandons a search, by
@@ -853,22 +896,10 @@ class TelemetryEvent {
* session remains ongoing when certain commands are picked (like dismissal)
* and results that enter search mode are picked.
*
- * @param {event} [event]
- * A DOM event.
- * Note: event can be null, that usually happens for paste&go or drop&go.
- * If there's no _startEventInfo this is a no-op.
- * @param {object} [details] An object describing action details.
- * @param {string} [details.searchString] The user's search string. Note that
- * this string is not sent with telemetry data. It is only used
- * locally to discern other data, such as the number of characters and
- * words in the string.
- * @param {string} [details.selType] type of the selected element, undefined
- * for "blur". One of "unknown", "autofill", "visiturl", "bookmark",
- * "help", "history", "keyword", "searchengine", "searchsuggestion",
- * "switchtab", "remotetab", "extension", "oneoff", "dismiss".
- * @param {UrlbarResult} [details.result] The engaged result. This should be
- * set to the result related to the picked element.
- * @param {HTMLElement} [details.element] The picked view element.
+ * @param {?event} event
+ * A DOM event. Note: event can be null, that usually happens for paste&go
+ * or drop&go. If there's no _startEventInfo this is a no-op.
+ * @param {ActionDetails} details
*/
record(event, details) {
// Prevent re-entering `record()`. This can happen because
@@ -903,6 +934,12 @@ class TelemetryEvent {
}
}
+ /**
+ * Internal record method, see the record function.
+ *
+ * @param {Event} event
+ * @param {ActionDetails} details
+ */
#internalRecord(event, details) {
const startEventInfo = this._startEventInfo;
@@ -927,6 +964,7 @@ class TelemetryEvent {
startEventInfo.interactionType
);
+ /** @type {"abandonment" | "engagement"} */
let method =
action == "blur" || action == "tab_switch" ? "abandonment" : "engagement";
@@ -955,25 +993,27 @@ class TelemetryEvent {
details.searchString
);
- details.provider = details.result?.providerName;
- details.selIndex = details.result?.rowIndex ?? -1;
+ let internalDetails = {
+ ...details,
+ provider: details.result?.providerName,
+ selIndex: details.result?.rowIndex ?? -1,
+ };
let { queryContext } = this._controller._lastQueryContextWrapper || {};
- this._recordSearchEngagementTelemetry(method, startEventInfo, {
+ this.#recordSearchEngagementTelemetry(method, startEventInfo, {
action,
numChars,
numWords,
searchWords,
- provider: details.provider,
- searchSource: details.searchSource,
- searchMode: details.searchMode,
- selectedElement: details.element,
- selIndex: details.selIndex,
- selType: details.selType,
+ provider: internalDetails.provider,
+ searchSource: internalDetails.searchSource,
+ searchMode: internalDetails.searchMode,
+ selIndex: internalDetails.selIndex,
+ selType: internalDetails.selType,
});
- if (!details.isSessionOngoing) {
+ if (!internalDetails.isSessionOngoing) {
this.#recordExposures(queryContext);
}
@@ -985,14 +1025,14 @@ class TelemetryEvent {
(method == "engagement" || method == "abandonment") &&
visibleResults.some(r => r.providerName == "UrlbarProviderQuickSuggest")
) {
- this.startTrackingDisableSuggest(event, details);
+ this.startTrackingDisableSuggest(event, internalDetails);
}
try {
this._controller.manager.notifyEngagementChange(
method,
queryContext,
- details,
+ internalDetails,
this._controller
);
} catch (error) {
@@ -1002,7 +1042,38 @@ class TelemetryEvent {
}
}
- _recordSearchEngagementTelemetry(
+ /**
+ * Records the relevant telemetry information for the given parameters.
+ *
+ * @param {"abandonment" | "engagement" | "disable" | "bounce"} method
+ * @param {{interactionType: any, searchString: string }} startEventInfo
+ * @param {object} details
+ * @param {string} details.action
+ * The type of action that caused this event. This may be recorded in the
+ * engagement_type field of the event.
+ * @param {number} details.numWords
+ * The length of words used for the search.
+ * @param {number} details.numChars
+ * The length of string used for the search. It includes whitespaces.
+ * @param {string} details.provider
+ * The name of the `UrlbarProvider` that provided the selected result.
+ * @param {string[]} details.searchWords
+ * The search words entered, used to determine if the search has been refined.
+ * @param {string} details.searchSource
+ * The source of the search event.
+ * @param {object} details.searchMode
+ * The searchMode object to record.
+ * @param {number} details.selIndex
+ * The index of the selected result.
+ * @param {string} details.selType
+ * The Type of the selected element, undefined for "blur".
+ * One of "unknown", "autofill", "visiturl", "bookmark", "help", "history",
+ * "keyword", "searchengine", "searchsuggestion", "switchtab", "remotetab",
+ * "extension", "oneoff", "dismiss".
+ * @param {number} [details.viewTime]
+ * The length of the view time in milliseconds.
+ */
+ #recordSearchEngagementTelemetry(
method,
startEventInfo,
{
@@ -1060,116 +1131,131 @@ class TelemetryEvent {
let available_semantic_sources = this.#getAvailableSemanticSources().join();
const search_engine_default_id = Services.search.defaultEngine.telemetryId;
- let eventInfo;
- if (method === "engagement") {
- let selected_result = lazy.UrlbarUtils.searchEngagementTelemetryType(
- currentResults[selIndex],
- selType
- );
-
- if (selType == "action") {
- let actionKey = lazy.UrlbarUtils.searchEngagementTelemetryAction(
+ switch (method) {
+ case "engagement": {
+ let selected_result = lazy.UrlbarUtils.searchEngagementTelemetryType(
currentResults[selIndex],
- selIndex
+ selType
);
- selected_result = `action_${actionKey}`;
- }
- if (selected_result === "input_field" && !this._controller.view?.isOpen) {
- numResults = 0;
- groups = "";
- results = "";
- }
+ if (selType == "action") {
+ let actionKey = lazy.UrlbarUtils.searchEngagementTelemetryAction(
+ currentResults[selIndex],
+ selIndex
+ );
+ selected_result = `action_${actionKey}`;
+ }
- eventInfo = {
- sap,
- interaction,
- search_mode,
- n_chars: numChars,
- n_words: numWords,
- n_results: numResults,
- selected_position: selIndex + 1,
- selected_result,
- provider,
- engagement_type:
- selType === "help" || selType === "dismiss" ? selType : action,
- search_engine_default_id,
- groups,
- results,
- actions,
- available_semantic_sources,
- };
- } else if (method === "abandonment") {
- eventInfo = {
- abandonment_type: action,
- sap,
- interaction,
- search_mode,
- n_chars: numChars,
- n_words: numWords,
- n_results: numResults,
- search_engine_default_id,
- groups,
- results,
- actions,
- available_semantic_sources,
- };
- } else if (method == "disable") {
- const previousEvent =
- action == "blur" || action == "tab_switch"
- ? "abandonment"
- : "engagement";
- let selected_result = "none";
- if (previousEvent == "engagement") {
- selected_result = lazy.UrlbarUtils.searchEngagementTelemetryType(
+ if (
+ selected_result === "input_field" &&
+ !this._controller.view?.isOpen
+ ) {
+ numResults = 0;
+ groups = "";
+ results = "";
+ }
+
+ let eventInfo = {
+ sap,
+ interaction,
+ search_mode,
+ n_chars: numChars.toString(),
+ n_words: numWords.toString(),
+ n_results: numResults,
+ selected_position: (selIndex + 1).toString(),
+ selected_result,
+ provider,
+ engagement_type:
+ selType === "help" || selType === "dismiss" ? selType : action,
+ search_engine_default_id,
+ groups,
+ results,
+ actions,
+ available_semantic_sources,
+ };
+ lazy.logger.info(`abandonment event:`, eventInfo);
+ Glean.urlbar.engagement.record(eventInfo);
+ break;
+ }
+ case "abandonment": {
+ let eventInfo = {
+ abandonment_type: action,
+ sap,
+ interaction,
+ search_mode,
+ n_chars: numChars.toString(),
+ n_words: numWords.toString(),
+ n_results: numResults,
+ search_engine_default_id,
+ groups,
+ results,
+ actions,
+ available_semantic_sources,
+ };
+ lazy.logger.info(`engagement event:`, eventInfo);
+ Glean.urlbar.abandonment.record(eventInfo);
+ break;
+ }
+ case "disable": {
+ const previousEvent =
+ action == "blur" || action == "tab_switch"
+ ? "abandonment"
+ : "engagement";
+ let selected_result = "none";
+ if (previousEvent == "engagement") {
+ selected_result = lazy.UrlbarUtils.searchEngagementTelemetryType(
+ currentResults[selIndex],
+ selType
+ );
+ }
+ let eventInfo = {
+ sap,
+ interaction,
+ search_mode,
+ search_engine_default_id,
+ n_chars: numChars.toString(),
+ n_words: numWords.toString(),
+ n_results: numResults,
+ selected_result,
+ results,
+ feature: "suggest",
+ };
+ lazy.logger.info(`disable event:`, eventInfo);
+ Glean.urlbar.disable.record(eventInfo);
+ break;
+ }
+ case "bounce": {
+ let selected_result = lazy.UrlbarUtils.searchEngagementTelemetryType(
currentResults[selIndex],
selType
);
+ let eventInfo = {
+ sap,
+ interaction,
+ search_mode,
+ search_engine_default_id,
+ n_chars: numChars.toString(),
+ n_words: numWords.toString(),
+ n_results: numResults,
+ selected_result,
+ selected_position: (selIndex + 1).toString(),
+ provider,
+ engagement_type:
+ selType === "help" || selType === "dismiss" ? selType : action,
+ results,
+ view_time: viewTime.toString(),
+ threshold: lazy.UrlbarPrefs.get(
+ "events.bounce.maxSecondsFromLastSearch"
+ ),
+ };
+ lazy.logger.info(`bounce event:`, eventInfo);
+ Glean.urlbar.bounce.record(eventInfo);
+ break;
+ }
+ default: {
+ console.error(`Unknown telemetry event method: ${method}`);
}
- eventInfo = {
- sap,
- interaction,
- search_mode,
- search_engine_default_id,
- n_chars: numChars,
- n_words: numWords,
- n_results: numResults,
- selected_result,
- results,
- feature: "suggest",
- };
- } else if (method === "bounce") {
- let selected_result = lazy.UrlbarUtils.searchEngagementTelemetryType(
- currentResults[selIndex],
- selType
- );
- eventInfo = {
- sap,
- interaction,
- search_mode,
- search_engine_default_id,
- n_chars: numChars,
- n_words: numWords,
- n_results: numResults,
- selected_result,
- selected_position: selIndex + 1,
- provider,
- engagement_type:
- selType === "help" || selType === "dismiss" ? selType : action,
- results,
- view_time: viewTime,
- threshold: lazy.UrlbarPrefs.get(
- "events.bounce.maxSecondsFromLastSearch"
- ),
- };
- } else {
- console.error(`Unknown telemetry event method: ${method}`);
- return;
}
-
- lazy.logger.info(`${method} event:`, eventInfo);
-
- Glean.urlbar[method].record(eventInfo);
}
/**
@@ -1224,7 +1310,11 @@ class TelemetryEvent {
// Record the `keyword_exposure` event if there's a keyword.
if (keyword) {
- let data = { keyword, terminal, result: resultType };
+ let data = {
+ keyword,
+ terminal: terminal.toString(),
+ result: resultType,
+ };
lazy.logger.debug("Recording keyword_exposure event", data);
Glean.urlbar.keywordExposure.record(data);
keywordExposureRecorded = true;
@@ -1413,7 +1503,9 @@ class TelemetryEvent {
return "dismiss";
}
if (MouseEvent.isInstance(event)) {
- return event.target.classList.contains("urlbar-go-button")
+ return /** @type {HTMLElement} */ (event.target).classList.contains(
+ "urlbar-go-button"
+ )
? "go_button"
: "click";
}
@@ -1498,7 +1590,7 @@ class TelemetryEvent {
* with for event telemetry.
*
* @param {object} result The element to analyze.
- * @param {Element} element The element to analyze.
+ * @param {HTMLElement} element The element to analyze.
* @returns {string} a string type for the telemetry event.
*/
typeFromElement(result, element) {
@@ -1603,8 +1695,10 @@ class TelemetryEvent {
* Start tracking a potential disable suggest event after user has seen a
* suggest result.
*
- * @param {event} event A DOM event.
- * @param {object} details An object describing interaction details.
+ * @param {event} event
+ * A DOM event.
+ * @param {InternalActionDetails} details
+ * An object describing interaction details.
*/
startTrackingDisableSuggest(event, details) {
this._lastSearchDetailsForDisableSuggestTracking = {
@@ -1656,7 +1750,6 @@ class TelemetryEvent {
details,
startEventInfo.interactionType
);
- let method = "disable";
let { numChars, numWords, searchWords } = this._parseSearchString(
details.searchString
@@ -1665,7 +1758,7 @@ class TelemetryEvent {
details.provider = details.result?.providerName;
details.selIndex = details.result?.rowIndex ?? -1;
- this._recordSearchEngagementTelemetry(method, startEventInfo, {
+ this.#recordSearchEngagementTelemetry("disable", startEventInfo, {
action,
numChars,
numWords,
@@ -1673,7 +1766,6 @@ class TelemetryEvent {
provider: details.provider,
searchSource: details.searchSource,
searchMode: details.searchMode,
- selectedElement: details.element,
selIndex: details.selIndex,
selType: details.selType,
});
@@ -1689,9 +1781,12 @@ class TelemetryEvent {
* Start tracking a potential bounce event after the user has engaged
* with a URL bar result.
*
- * @param {Browser} browser The browser object.
- * @param {event} event A DOM event.
- * @param {object} details An object describing interaction details.
+ * @param {object} browser
+ * The browser object.
+ * @param {event} event
+ * A DOM event.
+ * @param {ActionDetails} details
+ * An object describing interaction details.
*/
async startTrackingBounceEvent(browser, event, details) {
let state = this._controller.input.getBrowserState(browser);
@@ -1717,7 +1812,8 @@ class TelemetryEvent {
* browser chrome (this includes clicking on history or bookmark entries,
* and engaging with the URL bar).
*
- * @param {Browser} browser The browser object.
+ * @param {object} browser
+ * The browser object.
*/
async handleBounceEventTrigger(browser) {
let state = this._controller.input.getBrowserState(browser);
@@ -1759,7 +1855,8 @@ class TelemetryEvent {
/**
* Record a bounce event
*
- * @param {Browser} browser The browser object.
+ * @param {object} browser
+ * The browser object.
* @param {number} viewTime
* The time spent on a tab after a URL bar engagement before
* navigating away via browser chrome or closing the tab.
@@ -1792,7 +1889,6 @@ class TelemetryEvent {
details,
startEventInfo.interactionType
);
- let method = "bounce";
let { numChars, numWords, searchWords } = this._parseSearchString(
details.searchString
@@ -1801,7 +1897,7 @@ class TelemetryEvent {
details.provider = details.result?.providerName;
details.selIndex = details.result?.rowIndex ?? -1;
- this._recordSearchEngagementTelemetry(method, startEventInfo, {
+ this.#recordSearchEngagementTelemetry("bounce", startEventInfo, {
action,
numChars,
numWords,
diff --git a/browser/components/urlbar/UrlbarResult.sys.mjs b/browser/components/urlbar/UrlbarResult.sys.mjs
@@ -145,6 +145,13 @@ export class UrlbarResult {
this.#testForceNewContent = testForceNewContent;
}
+ /**
+ * @type {number}
+ * The index of the row where this result is in the suggestions. This is
+ * updated by UrlbarView when new result sets are displayed.
+ */
+ rowIndex = undefined;
+
get type() {
return this.#type;
}
diff --git a/browser/components/urlbar/tests/unit/test_UrlbarController_unit.js b/browser/components/urlbar/tests/unit/test_UrlbarController_unit.js
@@ -58,7 +58,7 @@ add_setup(function () {
add_task(function test_constructor_throws() {
Assert.throws(
() => new UrlbarController(),
- /Missing options: input/,
+ /options is undefined/,
"Should throw if the input was not supplied"
);
Assert.throws(
diff --git a/browser/components/urlbar/tsconfig.json b/browser/components/urlbar/tsconfig.json
@@ -1,7 +1,6 @@
{
"include": ["**/*.sys.mjs", "types/*.ts"],
"exclude": [
- "UrlbarController.sys.mjs",
"UrlbarEventBufferer.sys.mjs",
"UrlbarInput.sys.mjs",
"UrlbarProviderGlobalActions.sys.mjs",
