From f2e07044ae5c054937a79bbae00d27367e93b90c Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Fri, 11 Apr 2025 13:52:22 +0000 Subject: [PATCH] Bug 1938424 - metrics event when tabs added to tab group r=dao,jswinarton,tabbrowser-reviewers,extension-reviewers,robwu When using the tab context menu or drag-dropping tabs to put them into a group, record a metric for the number of tabs added to the group. Data Science asked us to launch with this metric disabled so that they could control it using server knobs. They expect a high volume of events, so they expect to only enable this metric for some proportion of users. I converted the existing `TabMove` event derived from `UIEvent` being fired when tabs change their tab index in the tab strip. `UIEvent` doesn't allow for attaching additional context/detail to the event. `TabMove` is now a `CustomEvent` that provides more context about the moved tab and it fires in more cases -- it's possible for the tab index not to change despite the tab having "moved" into/out of a tab group. This approach would not capture tab movements that occur across multiple frames/event loop iterations. Differential Revision: https://phabricator.services.mozilla.com/D244616 --- browser/base/content/main-popupset.js | 18 +- .../components/extensions/parent/ext-tabs.js | 8 +- .../sessionstore/SessionStore.sys.mjs | 10 +- .../components/tabbrowser/GroupsList.sys.mjs | 4 +- .../tabbrowser/TabGroupMetrics.sys.mjs | 32 --- .../components/tabbrowser/TabMetrics.sys.mjs | 62 +++++ .../components/tabbrowser/TabsList.sys.mjs | 3 + .../tabbrowser/content/tabbrowser.js | 217 +++++++++++++----- .../tabbrowser/content/tabgroup-menu.js | 14 +- .../components/tabbrowser/content/tabgroup.js | 8 +- browser/components/tabbrowser/content/tabs.js | 25 +- browser/components/tabbrowser/metrics.yaml | 27 +++ browser/components/tabbrowser/moz.build | 2 +- .../tabs/browser_tab_groups_telemetry.js | 96 +++++--- .../tabbrowser/test/browser/tabs/head.js | 33 +++ .../urlbar/ActionsProviderTabGroups.sys.mjs | 5 +- browser/modules/BrowserUsageTelemetry.sys.mjs | 82 ++++++- 17 files changed, 488 insertions(+), 158 deletions(-) delete mode 100644 browser/components/tabbrowser/TabGroupMetrics.sys.mjs create mode 100644 browser/components/tabbrowser/TabMetrics.sys.mjs diff --git a/browser/base/content/main-popupset.js b/browser/base/content/main-popupset.js index 5a84a31222cb..9c0234acda57 100644 --- a/browser/base/content/main-popupset.js +++ b/browser/base/content/main-popupset.js @@ -10,8 +10,7 @@ document.addEventListener( () => { const lazy = {}; ChromeUtils.defineESModuleGetters(lazy, { - TabGroupMetrics: - "moz-src:///browser/components/tabbrowser/TabGroupMetrics.sys.mjs", + TabMetrics: "moz-src:///browser/components/tabbrowser/TabMetrics.sys.mjs", }); let mainPopupSet = document.getElementById("mainPopupSet"); // eslint-disable-next-line complexity @@ -135,11 +134,12 @@ document.addEventListener( let tabGroup = gBrowser.getTabGroupById(tabGroupId); // Tabs need to be removed by their owning `Tabbrowser` or else // there are errors. - tabGroup.ownerGlobal.gBrowser.removeTabGroup(tabGroup, { - isUserTriggered: true, - telemetrySource: - lazy.TabGroupMetrics.METRIC_SOURCE.TAB_OVERFLOW_MENU, - }); + tabGroup.ownerGlobal.gBrowser.removeTabGroup( + tabGroup, + lazy.TabMetrics.userTriggeredContext( + lazy.TabMetrics.METRIC_SOURCE.TAB_OVERFLOW_MENU + ) + ); } break; @@ -148,7 +148,7 @@ document.addEventListener( { let { tabGroupId } = event.target.parentElement.triggerNode.dataset; SessionStore.openSavedTabGroup(tabGroupId, window, { - source: lazy.TabGroupMetrics.METRIC_SOURCE.RECENT_TABS, + source: lazy.TabMetrics.METRIC_SOURCE.RECENT_TABS, }); } break; @@ -157,7 +157,7 @@ document.addEventListener( // TODO Bug 1940112: "Open Group in New Window" should directly restore saved tab groups into a new window let { tabGroupId } = event.target.parentElement.triggerNode.dataset; let tabGroup = SessionStore.openSavedTabGroup(tabGroupId, window, { - source: lazy.TabGroupMetrics.METRIC_SOURCE.RECENT_TABS, + source: lazy.TabMetrics.METRIC_SOURCE.RECENT_TABS, }); gBrowser.replaceGroupWithWindow(tabGroup); } diff --git a/browser/components/extensions/parent/ext-tabs.js b/browser/components/extensions/parent/ext-tabs.js index 6c97302f5fa7..379c4d5b3bd1 100644 --- a/browser/components/extensions/parent/ext-tabs.js +++ b/browser/components/extensions/parent/ext-tabs.js @@ -260,13 +260,17 @@ this.tabs = class extends ExtensionAPIPersistent { }), onMoved({ fire }) { let { tabManager } = this.extension; + /** + * @param {CustomEvent} event + */ let moveListener = event => { let nativeTab = event.originalTarget; + let { previousTabState, currentTabState } = event.detail; if (tabManager.canAccessTab(nativeTab)) { fire.async(tabTracker.getId(nativeTab), { windowId: windowTracker.getId(nativeTab.ownerGlobal), - fromIndex: event.detail, - toIndex: nativeTab._tPos, + fromIndex: previousTabState.tabIndex, + toIndex: currentTabState.tabIndex, }); } }; diff --git a/browser/components/sessionstore/SessionStore.sys.mjs b/browser/components/sessionstore/SessionStore.sys.mjs index 364752466af3..89916d76d4ee 100644 --- a/browser/components/sessionstore/SessionStore.sys.mjs +++ b/browser/components/sessionstore/SessionStore.sys.mjs @@ -158,7 +158,7 @@ const kLastIndex = Number.MAX_SAFE_INTEGER - 1; import { PrivateBrowsingUtils } from "resource://gre/modules/PrivateBrowsingUtils.sys.mjs"; -import { TabGroupMetrics } from "moz-src:///browser/components/tabbrowser/TabGroupMetrics.sys.mjs"; +import { TabMetrics } from "moz-src:///browser/components/tabbrowser/TabMetrics.sys.mjs"; import { TelemetryTimestamps } from "resource://gre/modules/TelemetryTimestamps.sys.mjs"; import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs"; import { AppConstants } from "resource://gre/modules/AppConstants.sys.mjs"; @@ -899,16 +899,16 @@ export var SessionStore = { openSavedTabGroup( tabGroupId, targetWindow, - { source = TabGroupMetrics.METRIC_SOURCE.UNKNOWN } = {} + { source = TabMetrics.METRIC_SOURCE.UNKNOWN } = {} ) { let isVerticalMode = targetWindow.gBrowser.tabContainer.verticalMode; Glean.tabgroup.reopen.record({ id: tabGroupId, source, layout: isVerticalMode - ? TabGroupMetrics.METRIC_TABS_LAYOUT.VERTICAL - : TabGroupMetrics.METRIC_TABS_LAYOUT.HORIZONTAL, - type: TabGroupMetrics.METRIC_REOPEN_TYPE.SAVED, + ? TabMetrics.METRIC_TABS_LAYOUT.VERTICAL + : TabMetrics.METRIC_TABS_LAYOUT.HORIZONTAL, + type: TabMetrics.METRIC_REOPEN_TYPE.SAVED, }); return SessionStoreInternal.openSavedTabGroup(tabGroupId, targetWindow); diff --git a/browser/components/tabbrowser/GroupsList.sys.mjs b/browser/components/tabbrowser/GroupsList.sys.mjs index 0c4cd8b81489..1ca5019ccd69 100644 --- a/browser/components/tabbrowser/GroupsList.sys.mjs +++ b/browser/components/tabbrowser/GroupsList.sys.mjs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import { PrivateBrowsingUtils } from "resource://gre/modules/PrivateBrowsingUtils.sys.mjs"; -import { TabGroupMetrics } from "moz-src:///browser/components/tabbrowser/TabGroupMetrics.sys.mjs"; +import { TabMetrics } from "moz-src:///browser/components/tabbrowser/TabMetrics.sys.mjs"; const MAX_INITIAL_ITEMS = 5; @@ -79,7 +79,7 @@ export class GroupsPanel { case "allTabsGroupView_restoreGroup": this.win.SessionStore.openSavedTabGroup(tabGroupId, this.win, { - source: TabGroupMetrics.METRIC_SOURCE.TAB_OVERFLOW_MENU, + source: TabMetrics.METRIC_SOURCE.TAB_OVERFLOW_MENU, }); break; } diff --git a/browser/components/tabbrowser/TabGroupMetrics.sys.mjs b/browser/components/tabbrowser/TabGroupMetrics.sys.mjs deleted file mode 100644 index 0c41749ef402..000000000000 --- a/browser/components/tabbrowser/TabGroupMetrics.sys.mjs +++ /dev/null @@ -1,32 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -/** - * A common list of systems, surfaces, controls, etc. from which user - * interactions with tab groups could originate. These "source" values - * should be sent as extra data with tab group-related metrics events. - */ -const METRIC_SOURCE = Object.freeze({ - TAB_OVERFLOW_MENU: "tab_overflow", - TAB_GROUP_MENU: "tab_group", - SUGGEST: "suggest", - RECENT_TABS: "recent", - UNKNOWN: "unknown", -}); - -const METRIC_TABS_LAYOUT = Object.freeze({ - HORIZONTAL: "horizontal", - VERTICAL: "vertical", -}); - -const METRIC_REOPEN_TYPE = Object.freeze({ - SAVED: "saved", - DELETED: "deleted", -}); - -export const TabGroupMetrics = { - METRIC_SOURCE, - METRIC_TABS_LAYOUT, - METRIC_REOPEN_TYPE, -}; diff --git a/browser/components/tabbrowser/TabMetrics.sys.mjs b/browser/components/tabbrowser/TabMetrics.sys.mjs new file mode 100644 index 000000000000..80b3ec1d3a25 --- /dev/null +++ b/browser/components/tabbrowser/TabMetrics.sys.mjs @@ -0,0 +1,62 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/** + * A common list of systems, surfaces, controls, etc. from which user + * interactions with tabs could originate. These "source" values + * should be sent as extra data with tab-related metrics events. + */ +const METRIC_SOURCE = Object.freeze({ + TAB_OVERFLOW_MENU: "tab_overflow", + TAB_GROUP_MENU: "tab_group", + TAB_MENU: "tab_menu", + DRAG_AND_DROP: "drag", + SUGGEST: "suggest", + RECENT_TABS: "recent", + UNKNOWN: "unknown", +}); + +const METRIC_TABS_LAYOUT = Object.freeze({ + HORIZONTAL: "horizontal", + VERTICAL: "vertical", +}); + +const METRIC_REOPEN_TYPE = Object.freeze({ + SAVED: "saved", + DELETED: "deleted", +}); + +/** + * @typedef {object} TabMetricsContext + * @property {boolean} [isUserTriggered=false] + * Should be true if there was an explicit action/request from the user + * (as opposed to some action being taken internally or for technical + * bookkeeping reasons alone). This causes telemetry events to fire. + * @property {string} [telemetrySource="unknown"] + * The system, surface, or control the user used to take this action. + * @see TabMetrics.METRIC_SOURCE for possible values. + * Defaults to "unknown". + */ + +/** + * Creates a `TabMetricsContext` object for a user event originating from + * the specified source. + * + * @param {string} telemetrySource + * @see TabMetrics.METRIC_SOURCE + * @returns {TabMetricsContext} + */ +function userTriggeredContext(telemetrySource) { + return { + isUserTriggered: true, + telemetrySource, + }; +} + +export const TabMetrics = { + METRIC_SOURCE, + METRIC_TABS_LAYOUT, + METRIC_REOPEN_TYPE, + userTriggeredContext, +}; diff --git a/browser/components/tabbrowser/TabsList.sys.mjs b/browser/components/tabbrowser/TabsList.sys.mjs index bf26eff438d7..1cd1c7c9a704 100644 --- a/browser/components/tabbrowser/TabsList.sys.mjs +++ b/browser/components/tabbrowser/TabsList.sys.mjs @@ -235,6 +235,9 @@ class TabsListBase { } } + /** + * @param {MozTabbrowserTab} tab + */ _moveTab(tab) { let item = this.tabToElement.get(tab); if (item) { diff --git a/browser/components/tabbrowser/content/tabbrowser.js b/browser/components/tabbrowser/content/tabbrowser.js index a17cf026b2e7..e489099a4fc1 100644 --- a/browser/components/tabbrowser/content/tabbrowser.js +++ b/browser/components/tabbrowser/content/tabbrowser.js @@ -110,8 +110,8 @@ PictureInPicture: "resource://gre/modules/PictureInPicture.sys.mjs", SmartTabGroupingManager: "moz-src:///browser/components/tabbrowser/SmartTabGrouping.sys.mjs", - TabGroupMetrics: - "moz-src:///browser/components/tabbrowser/TabGroupMetrics.sys.mjs", + TabMetrics: + "moz-src:///browser/components/tabbrowser/TabMetrics.sys.mjs", TabStateFlusher: "resource:///modules/sessionstore/TabStateFlusher.sys.mjs", UrlbarProviderOpenTabs: @@ -2958,7 +2958,7 @@ * Causes the group create UI to be displayed and telemetry events to be fired. * @param {string} [options.telemetryUserCreateSource] * The means by which the tab group was created. - * @see TabGroupMetrics.METRIC_SOURCE for possible values. + * @see TabMetrics.METRIC_SOURCE for possible values. * Defaults to "unknown". */ addTabGroup( @@ -3035,14 +3035,14 @@ * switches windows). This causes telemetry events to fire. * @param {string} [options.telemetrySource="unknown"] * The means by which the tab group was removed. - * @see TabGroupMetrics.METRIC_SOURCE for possible values. + * @see TabMetrics.METRIC_SOURCE for possible values. * Defaults to "unknown". */ async removeTabGroup( group, options = { isUserTriggered: false, - telemetrySource: this.TabGroupMetrics.METRIC_SOURCE.UNKNOWN, + telemetrySource: this.TabMetrics.METRIC_SOURCE.UNKNOWN, } ) { if (this.tabGroupMenu.panel.state != "closed") { @@ -3943,7 +3943,7 @@ // Place tab at the end of the contextual tab group because one of: // 1) no `itemAfter` so `tab` should be the last tab in the tab strip // 2) `itemAfter` is in a different tab group - this.moveTabToGroup(tab, tabGroup); + tabGroup.appendChild(tab); } } else if ( (this.isTab(itemAfter) && itemAfter.group?.tabs[0] == itemAfter) || @@ -5925,8 +5925,26 @@ * any possibility of entering a tab group. For example, setting `true` * ensures that a pinned tab will not accidentally be placed inside of * a tab group, since pinned tabs are presently not allowed in tab groups. + * @property {boolean} [options.isUserTriggered=false] + * Should be true if there was an explicit action/request from the user + * (as opposed to some action being taken internally or for technical + * bookkeeping reasons alone) to move the tab. This causes telemetry + * events to fire. + * @property {string} [options.telemetrySource="unknown"] + * The system, surface, or control the user used to move the tab. + * @see TabMetrics.METRIC_SOURCE for possible values. + * Defaults to "unknown". */ - moveTabTo(aTab, { elementIndex, tabIndex, forceUngrouped = false } = {}) { + moveTabTo( + aTab, + { + elementIndex, + tabIndex, + forceUngrouped = false, + isUserTriggered = false, + telemetrySource = this.TabMetrics.METRIC_SOURCE.UNKNOWN, + } = {} + ) { if (typeof elementIndex == "number") { tabIndex = this.#elementIndexToTabIndex(elementIndex); } @@ -5954,49 +5972,57 @@ aTab = aTab.group; } - this.#handleTabMove(aTab, () => { - let neighbor = this.tabs[tabIndex]; - if (forceUngrouped && neighbor.group) { - neighbor = neighbor.group; - } - if (neighbor && tabIndex > aTab._tPos) { - neighbor.after(aTab); - } else { - this.tabContainer.insertBefore(aTab, neighbor); - } - }); + this.#handleTabMove( + aTab, + () => { + let neighbor = this.tabs[tabIndex]; + if (forceUngrouped && neighbor.group) { + neighbor = neighbor.group; + } + if (neighbor && tabIndex > aTab._tPos) { + neighbor.after(aTab); + } else { + this.tabContainer.insertBefore(aTab, neighbor); + } + }, + { isUserTriggered, telemetrySource } + ); } /** * @param {MozTabbrowserTab|MozTabbrowserTabGroup} tab * @param {MozTabbrowserTab|MozTabbrowserTabGroup} targetElement + * @param {TabMetricsContext} [metricsContext] */ - moveTabBefore(tab, targetElement) { - this.#moveTabNextTo(tab, targetElement, true); + moveTabBefore(tab, targetElement, metricsContext) { + this.#moveTabNextTo(tab, targetElement, true, metricsContext); } /** * @param {MozTabbrowserTab|MozTabbrowserTabGroup[]} tabs * @param {MozTabbrowserTab|MozTabbrowserTabGroup} targetElement + * @param {TabMetricsContext} [metricsContext] */ - moveTabsBefore(tabs, targetElement) { - this.#moveTabsNextTo(tabs, targetElement, true); + moveTabsBefore(tabs, targetElement, metricsContext) { + this.#moveTabsNextTo(tabs, targetElement, true, metricsContext); } /** * @param {MozTabbrowserTab|MozTabbrowserTabGroup} tab * @param {MozTabbrowserTab|MozTabbrowserTabGroup} targetElement + * @param {TabMetricsContext} [metricsContext] */ - moveTabAfter(tab, targetElement) { - this.#moveTabNextTo(tab, targetElement, false); + moveTabAfter(tab, targetElement, metricsContext) { + this.#moveTabNextTo(tab, targetElement, false, metricsContext); } /** * @param {MozTabbrowserTab|MozTabbrowserTabGroup[]} tabs * @param {MozTabbrowserTab|MozTabbrowserTabGroup} targetElement + * @param {TabMetricsContext} [metricsContext] */ - moveTabsAfter(tabs, targetElement) { - this.#moveTabsNextTo(tabs, targetElement, false); + moveTabsAfter(tabs, targetElement, metricsContext) { + this.#moveTabsNextTo(tabs, targetElement, false, metricsContext); } /** @@ -6004,9 +6030,10 @@ * The tab or tab group to move. Also accepts a tab group label as a * stand-in for its group. * @param {MozTabbrowserTab|MozTabbrowserTabGroup} targetElement - * @param {boolean} moveBefore + * @param {boolean} [moveBefore=false] + * @param {TabMetricsContext} [metricsContext] */ - #moveTabNextTo(tab, targetElement, moveBefore = false) { + #moveTabNextTo(tab, targetElement, moveBefore = false, metricsContext) { if (this.isTabGroupLabel(targetElement)) { targetElement = targetElement.group; if (!moveBefore) { @@ -6037,30 +6064,41 @@ return this.tabContainer; }; - this.#handleTabMove(tab, () => { - if (moveBefore) { - getContainer().insertBefore(tab, targetElement); - } else if (targetElement) { - targetElement.after(tab); - } else { - getContainer().appendChild(tab); - } - }); + this.#handleTabMove( + tab, + () => { + if (moveBefore) { + getContainer().insertBefore(tab, targetElement); + } else if (targetElement) { + targetElement.after(tab); + } else { + getContainer().appendChild(tab); + } + }, + metricsContext + ); } /** * @param {MozTabbrowserTab[]} tabs * @param {MozTabbrowserTab|MozTabbrowserTabGroup} targetElement - * @param {boolean} moveBefore + * @param {boolean} [moveBefore=false] + * @param {TabMetricsContext} [metricsContext] */ - #moveTabsNextTo(tabs, targetElement, moveBefore = false) { - this.#moveTabNextTo(tabs[0], targetElement, moveBefore); + #moveTabsNextTo(tabs, targetElement, moveBefore = false, metricsContext) { + this.#moveTabNextTo(tabs[0], targetElement, moveBefore, metricsContext); for (let i = 1; i < tabs.length; i++) { - this.#moveTabNextTo(tabs[i], tabs[i - 1]); + this.#moveTabNextTo(tabs[i], tabs[i - 1], false, metricsContext); } } - moveTabToGroup(aTab, aGroup) { + /** + * + * @param {MozTabbrowserTab} aTab + * @param {MozTabbrowserTabGroup} aGroup + * @param {TabMetricsContext} [metricsContext] + */ + moveTabToGroup(aTab, aGroup, metricsContext) { if (aTab.pinned) { return; } @@ -6069,19 +6107,80 @@ } aGroup.collapsed = false; - this.#handleTabMove(aTab, () => aGroup.appendChild(aTab)); + this.#handleTabMove(aTab, () => aGroup.appendChild(aTab), metricsContext); this.removeFromMultiSelectedTabs(aTab); this.tabContainer._notifyBackgroundTab(aTab); } + /** + * @typedef {object} TabMoveState + * @property {number} tabIndex + * @property {number} [elementIndex] + * @property {string} [tabGroupId] + */ + + /** + * @param {MozTabbrowserTab} tab + * @returns {TabMoveState|undefined} + */ + #getTabMoveState(tab) { + if (!this.isTab(tab)) { + return undefined; + } + + let state = { + tabIndex: tab._tPos, + }; + if (tab.visible) { + state.elementIndex = tab.elementIndex; + } + if (tab.group) { + state.tabGroupId = tab.group.id; + } + return state; + } + + /** + * @param {MozTabbrowserTab} tab + * @param {TabMoveState} [previousTabState] + * @param {TabMoveState} [currentTabState] + * @param {TabMetricsContext} [metricsContext] + */ + #notifyOnTabMove(tab, previousTabState, currentTabState, metricsContext) { + if (!this.isTab(tab) || !previousTabState || !currentTabState) { + return; + } + + let changedPosition = + previousTabState.tabIndex != currentTabState.tabIndex; + let changedTabGroup = + previousTabState.tabGroupId != currentTabState.tabGroupId; + + if (changedPosition || changedTabGroup) { + tab.dispatchEvent( + new CustomEvent("TabMove", { + bubbles: true, + detail: { + previousTabState, + currentTabState, + isUserTriggered: metricsContext?.isUserTriggered ?? false, + telemetrySource: + metricsContext?.telemetrySource ?? + this.TabMetrics.METRIC_SOURCE.UNKNOWN, + }, + }) + ); + } + } + /** * @param {MozTabbrowserTab} aTab * @param {function():void} moveActionCallback - * @returns + * @param {TabMetricsContext} [metricsContext] */ - #handleTabMove(aTab, moveActionCallback) { + #handleTabMove(aTab, moveActionCallback, metricsContext) { let wasFocused = document.activeElement == this.selectedTab; - let oldPosition = this.isTab(aTab) && aTab._tPos; + let previousTabState = this.#getTabMoveState(aTab); moveActionCallback(); @@ -6104,13 +6203,15 @@ if (aTab.pinned) { this.tabContainer._positionPinnedTabs(); } - // Pinning/unpinning vertical tabs, and moving tabs into tab groups, both bypass moveTabTo. - // We still want to check whether its worth dispatching an event. - if (this.isTab(aTab) && oldPosition != aTab._tPos) { - let evt = document.createEvent("UIEvents"); - evt.initUIEvent("TabMove", true, false, window, oldPosition); - aTab.dispatchEvent(evt); - } + + let currentTabState = this.#getTabMoveState(aTab); + + this.#notifyOnTabMove( + aTab, + previousTabState, + currentTabState, + metricsContext + ); } /** @@ -9056,8 +9157,16 @@ var TabContextMenu = { gTabsPanel.hideAllTabsPanel(); }, + /** + * @param {MozTabbrowserTabGroup} group + */ moveTabsToGroup(group) { - group.addTabs(this.contextTabs); + group.addTabs( + this.contextTabs, + gBrowser.TabMetrics.userTriggeredContext( + gBrowser.TabMetrics.METRIC_SOURCE.TAB_MENU + ) + ); group.ownerGlobal.focus(); }, diff --git a/browser/components/tabbrowser/content/tabgroup-menu.js b/browser/components/tabbrowser/content/tabgroup-menu.js index c9c5ccb724b1..1dfabf6b9c64 100644 --- a/browser/components/tabbrowser/content/tabgroup-menu.js +++ b/browser/components/tabbrowser/content/tabgroup-menu.js @@ -7,8 +7,8 @@ // This is loaded into chrome windows with the subscript loader. Wrap in // a block to prevent accidentally leaking globals onto `window`. { - const { TabGroupMetrics } = ChromeUtils.importESModule( - "moz-src:///browser/components/tabbrowser/TabGroupMetrics.sys.mjs" + const { TabMetrics } = ChromeUtils.importESModule( + "moz-src:///browser/components/tabbrowser/TabMetrics.sys.mjs" ); const { TabStateFlusher } = ChromeUtils.importESModule( "resource:///modules/sessionstore/TabStateFlusher.sys.mjs" @@ -448,10 +448,12 @@ document .getElementById("tabGroupEditor_deleteGroup") .addEventListener("command", () => { - gBrowser.removeTabGroup(this.activeGroup, { - isUserTriggered: true, - telemetrySource: TabGroupMetrics.METRIC_SOURCE.TAB_GROUP_MENU, - }); + gBrowser.removeTabGroup( + this.activeGroup, + TabMetrics.userTriggeredContext( + TabMetrics.METRIC_SOURCE.TAB_GROUP_MENU + ) + ); }); this.panel.addEventListener("popupshown", this); diff --git a/browser/components/tabbrowser/content/tabgroup.js b/browser/components/tabbrowser/content/tabgroup.js index e48f9392de0a..5c728dbef5bc 100644 --- a/browser/components/tabbrowser/content/tabgroup.js +++ b/browser/components/tabbrowser/content/tabgroup.js @@ -234,9 +234,11 @@ /** * add tabs to the group * - * @param tabs array of tabs to add + * @param {MozTabbrowserTab[]} tabs + * @param {TabMetricsContext} [metricsContext] + * Optional context to record for metrics purposes. */ - addTabs(tabs) { + addTabs(tabs, metricsContext) { for (let tab of tabs) { let tabToMove = this.ownerGlobal === tab.ownerGlobal @@ -245,7 +247,7 @@ tabIndex: gBrowser.tabs.at(-1)._tPos + 1, selectTab: tab.selected, }); - gBrowser.moveTabToGroup(tabToMove, this); + gBrowser.moveTabToGroup(tabToMove, this, metricsContext); } this.#lastAddedTo = Date.now(); } diff --git a/browser/components/tabbrowser/content/tabs.js b/browser/components/tabbrowser/content/tabs.js index b8f4cab12aa1..896f616954c9 100644 --- a/browser/components/tabbrowser/content/tabs.js +++ b/browser/components/tabbrowser/content/tabs.js @@ -9,6 +9,11 @@ // This is loaded into all browser windows. Wrap in a block to prevent // leaking to window scope. { + const lazy = {}; + ChromeUtils.defineESModuleGetters(lazy, { + TabMetrics: "moz-src:///browser/components/tabbrowser/TabMetrics.sys.mjs", + }); + const TAB_PREVIEW_PREF = "browser.tabs.hoverPreview.enabled"; const DIRECTION_BACKWARD = -1; @@ -1060,6 +1065,10 @@ var dropEffect = dt.dropEffect; var draggedTab; let movingTabs; + /** @type {TabMetricsContext} */ + const dropMetricsContext = lazy.TabMetrics.userTriggeredContext( + lazy.TabMetrics.METRIC_SOURCE.DRAG_AND_DROP + ); if (dt.mozTypesAt(0)[0] == TAB_DROP_TYPE) { // tab copy or move draggedTab = dt.mozGetDataAt(TAB_DROP_TYPE, 0); @@ -1084,7 +1093,7 @@ duplicatedDraggedTab = duplicatedTab; } } - gBrowser.moveTabsBefore(duplicatedTabs, dropTarget); + gBrowser.moveTabsBefore(duplicatedTabs, dropTarget, dropMetricsContext); if (draggedTab.container != this || event.shiftKey) { this.selectedItem = duplicatedDraggedTab; } @@ -1172,15 +1181,23 @@ let moveTabs = () => { if (dropIndex !== undefined) { for (let tab of movingTabs) { - gBrowser.moveTabTo(tab, { elementIndex: dropIndex }); + gBrowser.moveTabTo( + tab, + { elementIndex: dropIndex }, + dropMetricsContext + ); if (!directionForward) { dropIndex++; } } } else if (dropBefore) { - gBrowser.moveTabsBefore(movingTabs, dropElement); + gBrowser.moveTabsBefore( + movingTabs, + dropElement, + dropMetricsContext + ); } else { - gBrowser.moveTabsAfter(movingTabs, dropElement); + gBrowser.moveTabsAfter(movingTabs, dropElement, dropMetricsContext); } this.#expandGroupOnDrop(draggedTab); }; diff --git a/browser/components/tabbrowser/metrics.yaml b/browser/components/tabbrowser/metrics.yaml index bc66eac20c34..8dcec54d5ed3 100644 --- a/browser/components/tabbrowser/metrics.yaml +++ b/browser/components/tabbrowser/metrics.yaml @@ -196,6 +196,33 @@ tabgroup: type: string expires: never + add_tab: + type: event + disabled: true # To be controlled by server knobs during Firefox 138 launch due to expected high volume + description: > + Recorded when the user adds one or more ungrouped tabs to an existing tab group + notification_emails: + - dao@mozilla.com + - jswinarton@mozilla.com + - sthompson@mozilla.com + bugs: + - https://bugzil.la/1938424 + data_reviews: + - https://bugzil.la/1938424 + data_sensitivity: + - interaction + extra_keys: + source: + description: The system, surface, or control the user used to add the tab(s) to the tab group + type: string + tabs: + description: The number of tabs added to the tab group + type: quantity + layout: + description: The layout of the tab strip when the tabs were added (either "horizontal" or "vertical") + type: string + expires: never + active_groups: type: labeled_quantity description: > diff --git a/browser/components/tabbrowser/moz.build b/browser/components/tabbrowser/moz.build index ff81a8eb7a4a..aea973069427 100644 --- a/browser/components/tabbrowser/moz.build +++ b/browser/components/tabbrowser/moz.build @@ -13,7 +13,7 @@ MOZ_SRC_FILES += [ "NewTabPagePreloading.sys.mjs", "OpenInTabsUtils.sys.mjs", "SmartTabGrouping.sys.mjs", - "TabGroupMetrics.sys.mjs", + "TabMetrics.sys.mjs", "TabsList.sys.mjs", "TabUnloader.sys.mjs", ] diff --git a/browser/components/tabbrowser/test/browser/tabs/browser_tab_groups_telemetry.js b/browser/components/tabbrowser/test/browser/tabs/browser_tab_groups_telemetry.js index 903e525afbe5..87ece8b53540 100644 --- a/browser/components/tabbrowser/test/browser/tabs/browser_tab_groups_telemetry.js +++ b/browser/components/tabbrowser/test/browser/tabs/browser_tab_groups_telemetry.js @@ -353,39 +353,6 @@ async function closeTabsMenu() { await hidden; } -/** - * @param {XULToolbarButton} triggerNode - * @param {string} contextMenuId - * @returns {Promise} - */ -async function getContextMenu(triggerNode, contextMenuId) { - let nodeWindow = triggerNode.ownerGlobal; - triggerNode.scrollIntoView(); - const contextMenu = nodeWindow.document.getElementById(contextMenuId); - const contextMenuShown = BrowserTestUtils.waitForPopupEvent( - contextMenu, - "shown" - ); - - EventUtils.synthesizeMouseAtCenter( - triggerNode, - { type: "contextmenu", button: 2 }, - nodeWindow - ); - await contextMenuShown; - return contextMenu; -} - -/** - * @param {XULMenuElement|XULPopupElement} contextMenu - * @returns {Promise} - */ -async function closeContextMenu(contextMenu) { - let menuHidden = BrowserTestUtils.waitForPopupEvent(contextMenu, "hidden"); - contextMenu.hidePopup(); - await menuHidden; -} - /** * Returns a new basic, unnamed tab group that is fully loaded in the browser * and in session state. @@ -593,3 +560,66 @@ add_task(async function test_reopenSavedGroupTelemetry() { info("Perform reopen tests in vertical tabs mode"); await doReopenTests(true); }); + +add_task(async function test_tabContextMenu_addTabsToGroup() { + await resetTelemetry(); + + // `tabgroup.add_tab` is disabled by default and enabled by server knobs, + // so this test needs to enable it manually in order to test it. + Services.fog.applyServerKnobsConfig( + JSON.stringify({ + metrics_enabled: { + "tabgroup.add_tab": true, + }, + }) + ); + + info("set up a tab group to test with"); + let group = await makeTabGroup(); + let groupId = group.id; + + info("create 8 ungrouped tabs to test with"); + let moreTabs = Array.from({ length: 8 }).map(() => + BrowserTestUtils.addTab(win.gBrowser, "https://example.com") + ); + + info("select first ungrouped tab and multi-select three more tabs"); + win.gBrowser.selectedTab = moreTabs[0]; + moreTabs.slice(1, 4).forEach(tab => win.gBrowser.addToMultiSelectedTabs(tab)); + + await BrowserTestUtils.waitForCondition(() => { + return win.gBrowser.multiSelectedTabsCount == 4; + }, "Wait for Tabbrowser to update the multiselected tab state"); + + let menu = await getContextMenu(win.gBrowser.selectedTab, "tabContextMenu"); + let moveTabToGroupItem = win.document.getElementById( + "context_moveTabToGroup" + ); + let tabGroupButton = moveTabToGroupItem.querySelector( + `[tab-group-id="${groupId}"]` + ); + tabGroupButton.click(); + await closeContextMenu(menu); + + await BrowserTestUtils.waitForCondition(() => { + return Glean.tabgroup.addTab.testGetValue() !== null; + }, "Wait for a Glean event to be recorded"); + + let [addTabEvent] = Glean.tabgroup.addTab.testGetValue(); + Assert.deepEqual( + addTabEvent.extra, + { + source: "tab_menu", + tabs: "4", + layout: "horizontal", + }, + "should have recorded the correct event metadata" + ); + + for (let tab of moreTabs) { + BrowserTestUtils.removeTab(tab); + } + await removeTabGroup(group); + + await resetTelemetry(); +}); diff --git a/browser/components/tabbrowser/test/browser/tabs/head.js b/browser/components/tabbrowser/test/browser/tabs/head.js index a2c1f99a2376..e155a319aeb7 100644 --- a/browser/components/tabbrowser/test/browser/tabs/head.js +++ b/browser/components/tabbrowser/test/browser/tabs/head.js @@ -601,3 +601,36 @@ async function removeTabGroup(group) { await group.ownerGlobal.gBrowser.removeTabGroup(group, { animate: false }); await removePromise; } + +/** + * @param {Node} triggerNode + * @param {string} contextMenuId + * @returns {Promise} + */ +async function getContextMenu(triggerNode, contextMenuId) { + let win = triggerNode.ownerGlobal; + triggerNode.scrollIntoView({ behavior: "instant" }); + const contextMenu = win.document.getElementById(contextMenuId); + const contextMenuShown = BrowserTestUtils.waitForPopupEvent( + contextMenu, + "shown" + ); + + EventUtils.synthesizeMouseAtCenter( + triggerNode, + { type: "contextmenu", button: 2 }, + win + ); + await contextMenuShown; + return contextMenu; +} + +/** + * @param {XULMenuElement|XULPopupElement} contextMenu + * @returns {Promise} + */ +async function closeContextMenu(contextMenu) { + let menuHidden = BrowserTestUtils.waitForPopupEvent(contextMenu, "hidden"); + contextMenu.hidePopup(); + await menuHidden; +} diff --git a/browser/components/urlbar/ActionsProviderTabGroups.sys.mjs b/browser/components/urlbar/ActionsProviderTabGroups.sys.mjs index 121f1c61ab6d..66bee2a814c9 100644 --- a/browser/components/urlbar/ActionsProviderTabGroups.sys.mjs +++ b/browser/components/urlbar/ActionsProviderTabGroups.sys.mjs @@ -12,8 +12,7 @@ ChromeUtils.defineESModuleGetters(lazy, { BrowserWindowTracker: "resource:///modules/BrowserWindowTracker.sys.mjs", UrlbarPrefs: "resource:///modules/UrlbarPrefs.sys.mjs", SessionStore: "resource:///modules/sessionstore/SessionStore.sys.mjs", - TabGroupMetrics: - "moz-src:///browser/components/tabbrowser/TabGroupMetrics.sys.mjs", + TabMetrics: "moz-src:///browser/components/tabbrowser/TabMetrics.sys.mjs", }); const MIN_SEARCH_PREF = "tabGroups.minSearchLength"; @@ -77,7 +76,7 @@ class ProviderTabGroups extends ActionsProvider { savedGroup.id, window, { - source: lazy.TabGroupMetrics.METRIC_SOURCE.SUGGEST, + source: lazy.TabMetrics.METRIC_SOURCE.SUGGEST, } ); this.#switchToGroup(group); diff --git a/browser/modules/BrowserUsageTelemetry.sys.mjs b/browser/modules/BrowserUsageTelemetry.sys.mjs index cf07cd3a3e94..bcc22ca7bd71 100644 --- a/browser/modules/BrowserUsageTelemetry.sys.mjs +++ b/browser/modules/BrowserUsageTelemetry.sys.mjs @@ -18,8 +18,7 @@ ChromeUtils.defineESModuleGetters(lazy, { "moz-src:///browser/components/search/SearchSERPTelemetry.sys.mjs", SearchSERPTelemetryUtils: "moz-src:///browser/components/search/SearchSERPTelemetry.sys.mjs", - TabGroupMetrics: - "moz-src:///browser/components/tabbrowser/TabGroupMetrics.sys.mjs", + TabMetrics: "moz-src:///browser/components/tabbrowser/TabMetrics.sys.mjs", WindowsInstallsInfo: "resource://gre/modules/components-utils/WindowsInstallsInfo.sys.mjs", @@ -467,6 +466,20 @@ export let BrowserUsageTelemetry = { _inited: false, + /** + * @typedef {object} TabMovementsRecord + * @property {DeferredTask} deferredTask + * The `DeferredTask` that will report this record's metrics once all + * tab movement events with the same `telemetrySource` have been received + * in the current event loop. + * @property {number} numberAddedToTabGroup + * The number of tabs from `tabs` which started out as ungrouped tabs but + * moved into a tab group during the tab movement operation. + */ + + /** @type {Map} */ + _tabMovementsBySource: new Map(), + init() { this._lastRecordTabCount = 0; this._lastRecordLoadedTabCount = 0; @@ -598,6 +611,9 @@ export let BrowserUsageTelemetry = { case "TabGroupExpand": this._onTabGroupExpandOrCollapse(); break; + case "TabMove": + this._onTabMove(event); + break; case "TabGroupRemoveRequested": this._onTabGroupRemoveRequested(event); break; @@ -1143,11 +1159,13 @@ export let BrowserUsageTelemetry = { /** * Adds listeners to a single chrome window. + * @param {Window} win */ _registerWindow(win) { this._addUsageListeners(win); win.addEventListener("unload", this); + win.addEventListener("TabMove", this); win.addEventListener("TabOpen", this, true); win.addEventListener("TabPinned", this, true); win.addEventListener("TabGroupCreate", this); @@ -1167,6 +1185,7 @@ export let BrowserUsageTelemetry = { */ _unregisterWindow(win) { win.removeEventListener("unload", this); + win.removeEventListener("TabMove", this); win.removeEventListener("TabOpen", this, true); win.removeEventListener("TabPinned", this, true); win.removeEventListener("TabGroupCreate", this); @@ -1230,7 +1249,9 @@ export let BrowserUsageTelemetry = { if (event.detail.isUserTriggered) { Glean.tabgroup.createGroup.record({ id: event.target.id, - layout: lazy.sidebarVerticalTabs ? "vertical" : "horizontal", + layout: lazy.sidebarVerticalTabs + ? lazy.TabMetrics.METRIC_TABS_LAYOUT.VERTICAL + : lazy.TabMetrics.METRIC_TABS_LAYOUT.HORIZONTAL, source: event.detail.telemetryUserCreateSource, tabs: event.target.tabs.length, }); @@ -1325,7 +1346,7 @@ export let BrowserUsageTelemetry = { _onTabGroupRemoveRequested(event) { let { isUserTriggered = false, - telemetrySource = lazy.TabGroupMetrics.METRIC_SOURCE.UNKNOWN, + telemetrySource = lazy.TabMetrics.METRIC_SOURCE.UNKNOWN, } = event.detail; if (isUserTriggered) { @@ -1336,6 +1357,59 @@ export let BrowserUsageTelemetry = { } }, + /** + * Accumulates `TabMove` events in order to record 1 metrics event per frame + * per telemetry source. + * + * For example, dragging and dropping 4 tabs should listen for 4 `TabMove` + * events but result in 1 metrics event being recorded with a source of + * `drag` and a tab count of 4. + * + * @param {CustomEvent} event + */ + _onTabMove(event) { + let { isUserTriggered, telemetrySource } = event.detail; + + if (!isUserTriggered) { + return; + } + + let tabMovementsRecord = this._tabMovementsBySource.get(telemetrySource); + if (!tabMovementsRecord) { + let deferredTask = new lazy.DeferredTask(() => { + Glean.tabgroup.addTab.record({ + source: telemetrySource, + tabs: tabMovementsRecord.numberAddedToTabGroup, + layout: lazy.sidebarVerticalTabs ? "vertical" : "horizontal", + }); + this._tabMovementsBySource.delete(telemetrySource); + }, 0); + tabMovementsRecord = { + deferredTask, + numberAddedToTabGroup: 0, + }; + this._tabMovementsBySource.set(telemetrySource, tabMovementsRecord); + this._updateTabMovementsRecord(tabMovementsRecord, event); + deferredTask.arm(); + } else { + tabMovementsRecord.deferredTask.disarm(); + this._updateTabMovementsRecord(tabMovementsRecord, event); + tabMovementsRecord.deferredTask.arm(); + } + }, + + /** + * @param {TabMovementsRecord} record + * @param {CustomEvent} event + */ + _updateTabMovementsRecord(record, event) { + let { previousTabState, currentTabState } = event.detail; + + if (!previousTabState.tabGroupId && currentTabState.tabGroupId) { + record.numberAddedToTabGroup += 1; + } + }, + /** * Tracks the window count and registers the listeners for the tab count. * @param{Object} win The window object.