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.