diff --git a/browser/base/content/browser-sets.inc b/browser/base/content/browser-sets.inc index 528e7fd15308..b76984a0167e 100644 --- a/browser/base/content/browser-sets.inc +++ b/browser/base/content/browser-sets.inc @@ -81,7 +81,7 @@ - + diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index b5fb28bc2b49..3f31b91678cf 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -2864,11 +2864,11 @@ function focusAndSelectUrlBar() { gURLBar.select(); } -function openLocation() { +function openLocation(event) { if (window.location.href == AppConstants.BROWSER_CHROME_URL) { focusAndSelectUrlBar(); if (gURLBar.openViewOnFocus && !gURLBar.view.isOpen) { - gURLBar.startQuery(); + gURLBar.startQuery({ event }); } return; } diff --git a/browser/components/urlbar/UrlbarController.jsm b/browser/components/urlbar/UrlbarController.jsm index 0a4a0ffbe0ca..6fa8d0dbf990 100644 --- a/browser/components/urlbar/UrlbarController.jsm +++ b/browser/components/urlbar/UrlbarController.jsm @@ -349,8 +349,10 @@ class UrlbarController { } if (executeAction) { this.userSelectionBehavior = "arrow"; - this.engagementEvent.start(event); - this.input.startQuery({ searchString: this.input.value }); + this.input.startQuery({ + searchString: this.input.value, + event, + }); } } event.preventDefault(); @@ -608,16 +610,19 @@ class TelemetryEvent { } /** - * Start measuring the elapsed time from an input event. + * Start measuring the elapsed time from a user-generated event. * After this has been invoked, any subsequent calls to start() are ignored, * until either record() or discard() are invoked. Thus, it is safe to keep - * invoking this on every input event. - * @param {event} event A DOM input event. + * invoking this on every input event as the user is typing, for example. + * @param {event} event A DOM event. + * @param {string} [searchString] Pass a search string related to the event if + * you have one. The event by itself sometimes isn't enough to + * determine the telemetry details we should record. * @note This should never throw, or it may break the urlbar. */ - start(event) { - // Start is invoked at any input, but we only count the first one. - // Once an engagement or abandoment happens, we clear the _startEventInfo. + start(event, searchString = null) { + // start is invoked on a user-generated event, but we only count the first + // one. Once an engagement or abandoment happens, we clear _startEventInfo. if (!this._category || this._startEventInfo) { return; } @@ -625,23 +630,35 @@ class TelemetryEvent { Cu.reportError("Must always provide an event"); return; } - if (!["input", "drop", "mousedown", "keydown"].includes(event.type)) { + const validEvents = ["command", "drop", "input", "keydown", "mousedown"]; + if (!validEvents.includes(event.type)) { Cu.reportError("Can't start recording from event type: " + event.type); return; } - // "typed" is used when the user types something, while "pasted" and - // "dropped" are used when the text is inserted at once, by a paste or drop - // operation. "topsites" is a bit special, it is used when the user opens - // the empty search dropdown, that is supposed to show top sites. That - // happens by clicking on the urlbar dropmarker, or pressing DOWN with an - // empty input field. Even if the user later types something, we still - // report "topsites", with a positive numChars. + // Possible interaction types: + // + // typed: + // The user typed something and the view opened. We also use this when + // the user has opened the view without typing anything (by clicking the + // dropdown arrow, for example) after having left the pageproxystate + // invalid. In both cases, the view reflects what the user typed. + // pasted: + // The user pasted text. + // dropped: + // The user dropped text. + // topsites: + // The user opened the view with an empty search string (for example, + // after deleting all text, or by clicking the dropdown arrow when the + // pageproxystate is valid). The view shows the user's top sites. + let interactionType = "topsites"; if (event.type == "input") { interactionType = UrlbarUtils.isPasteEvent(event) ? "pasted" : "typed"; } else if (event.type == "drop") { interactionType = "dropped"; + } else if (searchString) { + interactionType = "typed"; } this._startEventInfo = { @@ -744,9 +761,9 @@ class TelemetryEvent { } /** - * Resets the currently tracked input event, that was registered via start(), - * so it won't be recorded. - * If there's no tracked input event, this is a no-op. + * Resets the currently tracked user-generated event that was registered via + * start(), so it won't be recorded. If there's no tracked event, this is a + * no-op. */ discard() { this._startEventInfo = null; diff --git a/browser/components/urlbar/UrlbarInput.jsm b/browser/components/urlbar/UrlbarInput.jsm index d020bd477cc5..1da6c1f0f087 100644 --- a/browser/components/urlbar/UrlbarInput.jsm +++ b/browser/components/urlbar/UrlbarInput.jsm @@ -703,12 +703,27 @@ class UrlbarInput { * to false so that state is maintained during a single interaction. The * intended use for this parameter is that it should be set to false when * this method is called due to input events. + * @param {event} [options.event] + * The user-generated event that triggered the query, if any. If given, we + * will record engagement event telemetry for the query. */ startQuery({ allowAutofill = true, searchString = null, resetSearchState = true, + event = null, } = {}) { + if (!searchString) { + searchString = + this.getAttribute("pageproxystate") == "valid" ? "" : this.value; + } else if (!this.value.startsWith(searchString)) { + throw new Error("The current value doesn't start with the search string"); + } + + if (event) { + this.controller.engagementEvent.start(event, searchString); + } + if (this._suppressStartQuery) { return; } @@ -717,13 +732,6 @@ class UrlbarInput { this._resetSearchState(); } - if (!searchString) { - searchString = - this.getAttribute("pageproxystate") == "valid" ? "" : this.value; - } else if (!this.value.startsWith(searchString)) { - throw new Error("The current value doesn't start with the search string"); - } - this._lastSearchString = searchString; this._textValueOnLastSearch = this.value; @@ -1585,9 +1593,9 @@ class UrlbarInput { this.editor.selectAll(); event.preventDefault(); } else if (this.openViewOnFocusForCurrentTab && !this.view.isOpen) { - this.controller.engagementEvent.start(event); this.startQuery({ allowAutofill: false, + event, }); } return; @@ -1598,9 +1606,9 @@ class UrlbarInput { this.view.close(); } else { this.focus(); - this.controller.engagementEvent.start(event); this.startQuery({ allowAutofill: false, + event, }); this._maybeSelectAll(); } @@ -1652,8 +1660,6 @@ class UrlbarInput { return; } - this.controller.engagementEvent.start(event); - // Autofill only when text is inserted (i.e., event.data is not empty) and // it's not due to pasting. let allowAutofill = @@ -1665,6 +1671,7 @@ class UrlbarInput { searchString: value, allowAutofill, resetSearchState: false, + event, }); } diff --git a/browser/components/urlbar/tests/UrlbarTestUtils.jsm b/browser/components/urlbar/tests/UrlbarTestUtils.jsm index 55c62bc9d95e..9d47f8062192 100644 --- a/browser/components/urlbar/tests/UrlbarTestUtils.jsm +++ b/browser/components/urlbar/tests/UrlbarTestUtils.jsm @@ -246,7 +246,7 @@ var UrlbarTestUtils = { }, /** - * Waits for the popup to be hidden. + * Waits for the popup to be shown. * @param {object} win The window containing the urlbar * @param {function} openFn Function to be used to open the popup. * @returns {Promise} resolved once the popup is closed diff --git a/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry.js b/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry.js index 5459d3b4db0c..ed3e44457873 100644 --- a/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry.js +++ b/browser/components/urlbar/tests/browser/browser_urlbar_event_telemetry.js @@ -414,6 +414,127 @@ const tests = [ }; }, + async function() { + info( + "With pageproxystate=valid, open the panel with openViewOnFocus, select with DOWN, Enter." + ); + gURLBar.value = ""; + let promise = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); + Services.prefs.setBoolPref("browser.urlbar.openViewOnFocus", true); + await UrlbarTestUtils.promisePopupOpen(window, () => { + window.document.getElementById("Browser:OpenLocation").doCommand(); + }); + Services.prefs.clearUserPref("browser.urlbar.openViewOnFocus"); + await UrlbarTestUtils.promiseSearchComplete(window); + while (gURLBar.untrimmedValue != "http://mochi.test:8888/") { + EventUtils.synthesizeKey("KEY_ArrowDown"); + } + EventUtils.synthesizeKey("VK_RETURN"); + await promise; + return { + category: "urlbar", + method: "engagement", + object: "enter", + value: "topsites", + extra: { + elapsed: val => parseInt(val) > 0, + numChars: "0", + selType: "history", + selIndex: val => parseInt(val) >= 0, + }, + }; + }, + + async function() { + info( + "With pageproxystate=valid, open the panel with openViewOnFocus, click on entry." + ); + gURLBar.value = ""; + let promise = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); + Services.prefs.setBoolPref("browser.urlbar.openViewOnFocus", true); + await UrlbarTestUtils.promisePopupOpen(window, () => { + window.document.getElementById("Browser:OpenLocation").doCommand(); + }); + Services.prefs.clearUserPref("browser.urlbar.openViewOnFocus"); + while (gURLBar.untrimmedValue != "http://mochi.test:8888/") { + EventUtils.synthesizeKey("KEY_ArrowDown"); + } + let element = UrlbarTestUtils.getSelectedElement(window); + EventUtils.synthesizeMouseAtCenter(element, {}); + await promise; + return { + category: "urlbar", + method: "engagement", + object: "click", + value: "topsites", + extra: { + elapsed: val => parseInt(val) > 0, + numChars: "0", + selType: "history", + selIndex: "0", + }, + }; + }, + + async function() { + info( + "With pageproxystate=invalid, open the panel with openViewOnFocus, Enter." + ); + gURLBar.value = "mochi.test"; + gURLBar.setAttribute("pageproxystate", "invalid"); + let promise = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); + Services.prefs.setBoolPref("browser.urlbar.openViewOnFocus", true); + await UrlbarTestUtils.promisePopupOpen(window, () => { + window.document.getElementById("Browser:OpenLocation").doCommand(); + }); + Services.prefs.clearUserPref("browser.urlbar.openViewOnFocus"); + await UrlbarTestUtils.promiseSearchComplete(window); + EventUtils.synthesizeKey("VK_RETURN"); + await promise; + return { + category: "urlbar", + method: "engagement", + object: "enter", + value: "typed", + extra: { + elapsed: val => parseInt(val) > 0, + numChars: "10", + selType: "autofill", + selIndex: "0", + }, + }; + }, + + async function() { + info( + "With pageproxystate=invalid, open the panel with openViewOnFocus, click on entry." + ); + gURLBar.value = "mochi.test"; + gURLBar.setAttribute("pageproxystate", "invalid"); + let promise = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser); + Services.prefs.setBoolPref("browser.urlbar.openViewOnFocus", true); + await UrlbarTestUtils.promisePopupOpen(window, () => { + window.document.getElementById("Browser:OpenLocation").doCommand(); + }); + Services.prefs.clearUserPref("browser.urlbar.openViewOnFocus"); + await UrlbarTestUtils.promiseSearchComplete(window); + let element = UrlbarTestUtils.getSelectedElement(window); + EventUtils.synthesizeMouseAtCenter(element, {}); + await promise; + return { + category: "urlbar", + method: "engagement", + object: "click", + value: "typed", + extra: { + elapsed: val => parseInt(val) > 0, + numChars: "10", + selType: "autofill", + selIndex: "0", + }, + }; + }, + /* * Abandonment tests. */ @@ -480,6 +601,53 @@ const tests = [ }; }, + async function() { + info( + "With pageproxystate=valid, open the panel with openViewOnFocus, don't type, blur it." + ); + gURLBar.value = ""; + Services.prefs.setBoolPref("browser.urlbar.openViewOnFocus", true); + await UrlbarTestUtils.promisePopupOpen(window, () => { + window.document.getElementById("Browser:OpenLocation").doCommand(); + }); + Services.prefs.clearUserPref("browser.urlbar.openViewOnFocus"); + gURLBar.blur(); + return { + category: "urlbar", + method: "abandonment", + object: "blur", + value: "topsites", + extra: { + elapsed: val => parseInt(val) > 0, + numChars: "0", + }, + }; + }, + + async function() { + info( + "With pageproxystate=invalid, open the panel with openViewOnFocus, don't type, blur it." + ); + gURLBar.value = "mochi.test"; + gURLBar.setAttribute("pageproxystate", "invalid"); + Services.prefs.setBoolPref("browser.urlbar.openViewOnFocus", true); + await UrlbarTestUtils.promisePopupOpen(window, () => { + window.document.getElementById("Browser:OpenLocation").doCommand(); + }); + Services.prefs.clearUserPref("browser.urlbar.openViewOnFocus"); + gURLBar.blur(); + return { + category: "urlbar", + method: "abandonment", + object: "blur", + value: "typed", + extra: { + elapsed: val => parseInt(val) > 0, + numChars: "10", + }, + }; + }, + /* * No event tests. */ @@ -570,7 +738,9 @@ add_task(async function test() { // This is not necessary after each loop, because assertEvents does it. Services.telemetry.clearEvents(); - for (let testFn of tests) { + for (let i = 0; i < tests.length; i++) { + info(`Running test at index ${i}`); + let testFn = tests[i]; let expectedEvents = [await testFn()].filter(e => !!e); // Always blur to ensure it's not accounted as an additional abandonment. gURLBar.blur();