Closed Bug 1023331 Opened 6 years ago Closed 6 years ago

Telemetry for three-dot menu in tabs tray

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 33

People

(Reporter: Margaret, Assigned: Margaret)

References

Details

Attachments

(1 file)

Follow-up to bug 817716.
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.
Attachment #8441408 - Flags: review?(mark.finkle)
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.
Attachment #8441408 - Flags: review?(mark.finkle) → review+
> 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
https://hg.mozilla.org/mozilla-central/rev/563bd023c060
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
You need to log in before you can comment on or make changes to this bug.