1023331 - Telemetry for three-dot menu in tabs tray

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

Follow-up to bug 817716.

Comment 1

[:Margaret Leibovic]

Attachment: Telemetry for three-dot menu in tabs tray

This depends on the patch for bug 817716, but I can land them at the same time.

Also, with the patch in that bug, the new_tab and new_private_tab items will call BrowserApp.onOptionsItemSelected, so they will be recorded in with the regular main menu telemetry. Are we okay with that? If not, I could give these unique ids, but currently the shared ids allow us to just re-use existing logic.

I decided to add telemetry events for the menu items that won't get recorded in BrowserApp. Ideally we could just put one event at the top of this onMenuItemClick method, but then the new_tab/new_private_tab actions would get recorded twice.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8441408 [details] [diff] [review]
Telemetry for three-dot menu in tabs tray

>diff --git a/mobile/android/base/tabspanel/TabsPanel.java b/mobile/android/base/tabspanel/TabsPanel.java

>         mMenuButton.setOnClickListener(new Button.OnClickListener() {
>             @Override
>             public void onClick(View view) {

>+
>+                Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.BUTTON, "tabs_menu");

I don't think we need this probe. We have not added any probes for opening menus, which I assume this would track. If we want to add this in the future, for all menus, we can do it in a new bug.

>         if (itemId == R.id.close_all_tabs) {
>             if (mCurrentPanel == Panel.NORMAL_TABS) {
>+                Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.MENU, "close_all_tabs");

There is probably nothing wrong with this hardcoded Extra, but you could use | getResources().getResourceEntryName(itemId) | in it's place, if you thought keeping the id/text in sync was an issue.

>         if (itemId == R.id.close_private_tabs) {
>             if (mCurrentPanel == Panel.PRIVATE_TABS) {
>+                Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.MENU, "close_private_tabs");

Same

---

Since you're in here and adding telemetry, could you add a probe in the mAddTab onClick handler? Something like this should work:

>         mAddTab.setOnClickListener(new Button.OnClickListener() {
>             @Override
>             public void onClick(View v) {
>+                final String extra = (mCurrentPanel == Panel.NORMAL_TABS ? "new_tab" : "new_private_tab");
>+                Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.ACTIONBAR, extra);
>+
>                 TabsPanel.this.addTab();
>             }
>         });

Also fine if you want to handle that in a new bug.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

> Since you're in here and adding telemetry, could you add a probe in the
> mAddTab onClick handler? Something like this should work:
> 
> >         mAddTab.setOnClickListener(new Button.OnClickListener() {
> >             @Override
> >             public void onClick(View v) {
> >+                final String extra = (mCurrentPanel == Panel.NORMAL_TABS ? "new_tab" : "new_private_tab");
> >+                Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.ACTIONBAR, extra);
> >+
> >                 TabsPanel.this.addTab();
> >             }
> >         });
> 
> Also fine if you want to handle that in a new bug.

... and that is already filed as bug 1018432

Comment 4

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/563bd023c060

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/563bd023c060