1201374 - Add a telemetry probe to track how often F11 fullscreen mode is used (browser-fullscreen)

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

F11 fullscreen aka browser fullscreen is a bit outdated in its look-and-feel. We may want to update it or remove support for it, but first we should see how often it is used. This isn't the only metric for determining the future of it, but it should be included in the decision.

This bug is to add the necessary probe(s).

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

Comment 2

[Allison Naaktgeboren :ally]

Comment on attachment 8656690 [details] [diff] [review]
Patch

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

Looks good from my end. p=ally

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8656690 [details] [diff] [review]
Patch

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

::: browser/base/content/browser-fullScreen.js
@@ +99,5 @@
> +    if (enterFS) {
> +      try {
> +        Services.telemetry.getHistogramById("FX_BROWSER_FULLSCREEN_USED")
> +                          .add(1);
> +      } catch (ex) { /* Don't break if Telemetry is acting up. */ }

I don't see us doing this anywhere else, why are you assuming telemetry could throw? If it would be broken we surely had a lot more breakage all over the place, no?

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

url:        https://hg.mozilla.org/integration/fx-team/rev/e40065dd77cebf8e6f3ed02a68ad496da313fdf4
changeset:  e40065dd77cebf8e6f3ed02a68ad496da313fdf4
user:       Jared Wein <[e-mail]>
date:       Tue Sep 08 08:53:05 2015 -0400
description:
Bug 1201374 - Add a telemetry probe to track how often F11 fullscreen mode is used (browser-fullscreen). r=ttaubert p=ally

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8656690 [details] [diff] [review]
Patch

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

::: browser/base/content/browser-fullScreen.js
@@ +98,5 @@
> +
> +    if (enterFS) {
> +      try {
> +        Services.telemetry.getHistogramById("FX_BROWSER_FULLSCREEN_USED")
> +                          .add(1);

If this telemetry is only for the F11 fullscreen mode, then the condition here is not correct. You should also check "!document.mozFullScreen" to ensure it is not for the DOM Fullscreen. DOM Fullscreen is triggered via the Fullscreen API and it is widely used in video sites.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Follow-up patch

Comment 7

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

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

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

url:        https://hg.mozilla.org/integration/fx-team/rev/70c8b3ee5ad0dec85b374e8f410d7b71cdd08fa3
changeset:  70c8b3ee5ad0dec85b374e8f410d7b71cdd08fa3
user:       Jared Wein <[e-mail]>
date:       Tue Sep 08 16:56:13 2015 -0400
description:
Bug 1201374 - followup, check for DOM fullscreen. r=xidorn