1026003 - UI telemetry for opening pages in new tabs from web content contextmenu

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Richard Newman [:rnewman]]

09:43:17 <@mfinkle> we still don't cover loading urls via "open in new/private tab" from web context menu

Let's get it done!

Comment 1

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

Attachment: uitelemetry-loadurl-webcontext v0.1

Uses the same scheme we use in other loadurl telemetry

Comment 2

[Chenxia Liu [:liuche]]

Comment on attachment 8440812 [details] [diff] [review]
uitelemetry-loadurl-webcontext v0.1

Review of attachment 8440812 [details] [diff] [review]:
-----------------------------------------------------------------

One comment, o/w lgtm!

::: mobile/android/chrome/content/browser.js
@@ +502,5 @@
>  
>      NativeWindow.contextmenus.add(Strings.browser.GetStringFromName("contextmenu.openInPrivateTab"),
>        NativeWindow.contextmenus.linkOpenableContext,
>        function(aTarget) {
> +        UITelemetry.addEvent("loadurl.1", "contextmenu", null);

I'd like to see us differentiate between private and non-private tabs, so maybe an extra here (or for consistency/if you're feeling pedantic, extras in both events).

Comment 3

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

(In reply to Chenxia Liu [:liuche] from comment #2)
> Comment on attachment 8440812 [details] [diff] [review]
> uitelemetry-loadurl-webcontext v0.1
> 
> Review of attachment 8440812 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> One comment, o/w lgtm!
> 
> ::: mobile/android/chrome/content/browser.js
> @@ +502,5 @@
> >  
> >      NativeWindow.contextmenus.add(Strings.browser.GetStringFromName("contextmenu.openInPrivateTab"),
> >        NativeWindow.contextmenus.linkOpenableContext,
> >        function(aTarget) {
> > +        UITelemetry.addEvent("loadurl.1", "contextmenu", null);
> 
> I'd like to see us differentiate between private and non-private tabs, so
> maybe an extra here (or for consistency/if you're feeling pedantic, extras
> in both events).

We have the same situation with the home pane context menu. It has "open new" and "open private" actiosn on it. we skipped adding extras to it for tracking normal and private.

We current use "action" for track private tabs, but we could revisit adding extras to the right places for "loadurl". I'll file a different bug for that, since it affects more than this patch.

Comment 4

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

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

Filed bug 1026338

Comment 5

[Carsten Book [:Tomcat]]

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

Comment 6

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

Comment on attachment 8440812 [details] [diff] [review]
uitelemetry-loadurl-webcontext v0.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Allows collecting more telemetry data
Testing completed (on m-c, etc.): Working on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 7

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/122393de469c

Comment 8

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/4de57e02872e