1212363 - Add Telemetry to |Boomark Added| Dialog and its actions

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mark Capella [:capella]]

Currently when we bookmark a page, the "Bookmark Added" toast provides an "options" button that displays a dialog with |edit| and |Add to Home Screen| listitems.

I personally never add things to my homepage, so the dialog is a useless step for me when all I want to do is edit the Bookmarks auto-supplied |Name| field. Why not change the toast from "Options" to "Edit" and lose the dialog? We'll still have "Add to Home Page" available via. Menu -> Page.

To explore this, let's add telemetry to count:
1) # times the Dialog is displayed
2) # times the Edit option is selected
3) # times the Add to Home Page option is selected


http://hg.mozilla.org/mozilla-central/annotate/d1c5a7c5b433/mobile/android/base/BrowserApp.java#l563
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java?rev=3bbea41152bc&mark=1207-1207,1209-1209#1192

Comment 1

[:Margaret Leibovic]

We added this option to promote "Add to home screen", which can be hard for users to discover.

However, I think this point to add a telemetry probe is a good idea. I am definitely curious to know how often users select this option.

It may be scope creep here, but I would actually like to know how often users are using "Add to home screen" in general. I see we do have a probe for how often users load homescreen shortcuts:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java#3631

But we should make sure we have probes to understand how these shortcuts got there in the first place.

Comment 2

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

(In reply to :Margaret Leibovic from comment #1)

> It may be scope creep here, but I would actually like to know how often
> users are using "Add to home screen" in general. I see we do have a probe
> for how often users load homescreen shortcuts:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.
> java#3631
> 
> But we should make sure we have probes to understand how these shortcuts got
> there in the first place.

We already have "add_to_launcher" probes for menus and contextmenus.

Comment 3

[Mark Capella [:capella]]

Attachment: bug1212363.diff

Maybe something like this.

I broke out TelemetryContract.Method.BOOKMARK_OPTIONS instead of a generic TelemetryContract.Method.DIALOG.

I also added in a new "add_to_launcher" probe for "Add to Home Screen" contextmenu action taken after longPress of the awesomebar/toolbar for completeness.

Comment 4

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

Comment on attachment 8673076 [details] [diff] [review]
bug1212363.diff


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

>                     public void onButtonClicked() {
>+                        Telemetry.sendUIEvent(TelemetryContract.Event.SHOW,
>+                            TelemetryContract.Method.TOAST, "bookmark_options");

You're a "wrapper". If your IDE did this it's OK, but don't feel compelled to always wrap lines.

>     private void showBookmarkDialog() {

>+                    final String extrasId = res.getResourceEntryName(R.string.contextmenu_edit_bookmark);
>+                    final String extrasId = res.getResourceEntryName(R.string.contextmenu_add_to_launcher);

I suppose it's OK to have these show up in Telemetry as "context_*" Extra strings. Currently, we have "add_to_launcher" and "home_add_to_launcher". Also, "home_edit_bookmark".

>+            Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.CONTEXT_MENU,
>+                getResources().getResourceEntryName(itemId));

Good catch. I noticed the other cases in this method just hard code the Extra string. I think some of that started because this method is shared with the main menu for a few items. I don't mind either way.

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

>+        // Action occurred via bookmark options dialog.
>+        BOOKMARK_OPTIONS("bookmark_options"),
>+

Let's just keep using DIALOG. We don't like the Methods to become too specific. We try to reuse them.


r+ with the DIALOG change. The others are nits.

Comment 5

[Mark Capella [:capella]]

Great thanks! ...
https://treeherder.mozilla.org/#/jobs?repo=try&revision=08a0ab5b40a5

I don't mind long-lines. Working in c++ with the 80 char limit is really annoying. iir, Java style line limit is 100 so I try to stay inside that, while noting that there are some *really* long lines in that module :-)

Comment 6

[Pulsebot]

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

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c03346d41db6