Bug 1899352 Comment 7 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

asd(In reply to :Gijs (he/him) from comment #6)
> I think I missed something when reviewing this.
> 
> https://hg.mozilla.org/mozilla-central/rev/97a51d1d23ba48ea4f2c913a451c140b9a586d7c#l2.78
> 
> ```diff
> --- a/browser/modules/BrowserUsageTelemetry.sys.mjs
> +++ b/browser/modules/BrowserUsageTelemetry.sys.mjs
> @@ -80,16 +109,17 @@ const UI_TARGET_COMPOSED_ELEMENTS_MAP = 
>  
>  // The containers of interactive elements that we care about and their pretty
>  // names. These should be listed in order of most-specific to least-specific,
>  // when iterating JavaScript will guarantee that ordering and so we will find
>  // the most specific area first.
>  const BROWSER_UI_CONTAINER_IDS = {
>    "toolbar-menubar": "menu-bar",
>    TabsToolbar: "tabs-bar",
> +  "vertical-tabs": "vertical-tabs-container",
>    PersonalToolbar: "bookmarks-bar",
>    "appMenu-popup": "app-menu",
>    tabContextMenu: "tabs-context",
>    contentAreaContextMenu: "content-context",
>    "widget-overflow-list": "overflow-menu",
>    "widget-overflow-fixed-list": "pinned-overflow-menu",
>    "page-action-buttons": "pageaction-urlbar",
>    pageActionPanel: "pageaction-panel",
> ```
> 
> These containers are meant to correspond to telemetry scalars. See e.g. [this comparable patch](https://hg.mozilla.org/mozilla-central/rev/fd0c3627998852d28181d8db439371d452e99c18) which added the extensions panel. So I think this needs a follow-up, either for the area to be removed (if we don't care about the interactions) or for the scalar to be added, in order for the interactions with buttons in this area to be recorded in telemetry (and, in that case, to add an automated test).

This was added for the [special case](https://searchfox.org/mozilla-central/rev/9fe73403523732f57cd82d30590ecc272fb0b165/browser/modules/BrowserUsageTelemetry.sys.mjs#975-986) of handling `tabs_context` from the vertical tabs bar. We want to track the entrypoint for the tabs context menu commands (e.g. right click tab -> click "New Tab Below"), and it will only track vertical tabs container if it is added to `BROWSER_UI_CONTAINER_IDS`. My patch had a [test](https://searchfox.org/mozilla-central/rev/9fe73403523732f57cd82d30590ecc272fb0b165/browser/components/sidebar/tests/browser/browser_vertical_tabs.js#80-91) for this as well:

```javascript
info("Open a new tab using the context menu.");
await openAndWaitForContextMenu(contextMenu, gBrowser.selectedTab, () => {
  document.getElementById("context_openANewTab").click();
});

const keyedScalars = TelemetryTestUtils.getProcessScalars("parent", true);
TelemetryTestUtils.assertKeyedScalar(
  keyedScalars,
  "browser.ui.interaction.tabs_context_entrypoint",
  "vertical-tabs-container",
  1
);
```

If we still want to document `vertical-tabs-container` and add it as a scalar, or handle this a different way somehow, I'm happy to do that in a follow-up bug.
(In reply to :Gijs (he/him) from comment #6)
> I think I missed something when reviewing this.
> 
> https://hg.mozilla.org/mozilla-central/rev/97a51d1d23ba48ea4f2c913a451c140b9a586d7c#l2.78
> 
> ```diff
> --- a/browser/modules/BrowserUsageTelemetry.sys.mjs
> +++ b/browser/modules/BrowserUsageTelemetry.sys.mjs
> @@ -80,16 +109,17 @@ const UI_TARGET_COMPOSED_ELEMENTS_MAP = 
>  
>  // The containers of interactive elements that we care about and their pretty
>  // names. These should be listed in order of most-specific to least-specific,
>  // when iterating JavaScript will guarantee that ordering and so we will find
>  // the most specific area first.
>  const BROWSER_UI_CONTAINER_IDS = {
>    "toolbar-menubar": "menu-bar",
>    TabsToolbar: "tabs-bar",
> +  "vertical-tabs": "vertical-tabs-container",
>    PersonalToolbar: "bookmarks-bar",
>    "appMenu-popup": "app-menu",
>    tabContextMenu: "tabs-context",
>    contentAreaContextMenu: "content-context",
>    "widget-overflow-list": "overflow-menu",
>    "widget-overflow-fixed-list": "pinned-overflow-menu",
>    "page-action-buttons": "pageaction-urlbar",
>    pageActionPanel: "pageaction-panel",
> ```
> 
> These containers are meant to correspond to telemetry scalars. See e.g. [this comparable patch](https://hg.mozilla.org/mozilla-central/rev/fd0c3627998852d28181d8db439371d452e99c18) which added the extensions panel. So I think this needs a follow-up, either for the area to be removed (if we don't care about the interactions) or for the scalar to be added, in order for the interactions with buttons in this area to be recorded in telemetry (and, in that case, to add an automated test).

This was added for the [special case](https://searchfox.org/mozilla-central/rev/9fe73403523732f57cd82d30590ecc272fb0b165/browser/modules/BrowserUsageTelemetry.sys.mjs#975-986) of handling `tabs_context` from the vertical tabs bar. We want to track the entrypoint for the tabs context menu commands (e.g. right click tab -> click "New Tab Below"), and it will only track vertical tabs container if it is added to `BROWSER_UI_CONTAINER_IDS`. My patch had a [test](https://searchfox.org/mozilla-central/rev/9fe73403523732f57cd82d30590ecc272fb0b165/browser/components/sidebar/tests/browser/browser_vertical_tabs.js#80-91) for this as well:

```javascript
info("Open a new tab using the context menu.");
await openAndWaitForContextMenu(contextMenu, gBrowser.selectedTab, () => {
  document.getElementById("context_openANewTab").click();
});

const keyedScalars = TelemetryTestUtils.getProcessScalars("parent", true);
TelemetryTestUtils.assertKeyedScalar(
  keyedScalars,
  "browser.ui.interaction.tabs_context_entrypoint",
  "vertical-tabs-container",
  1
);
```

If we still want to document `vertical-tabs-container` and add it as a scalar, or handle this a different way somehow, I'm happy to do that in a follow-up bug.

Back to Bug 1899352 Comment 7