Closed Bug 1201374 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 43
Tracking Status
firefox43 --- fixed

People

(Reporter: jaws, Assigned: jaws)

Details

Attachments

(2 files)

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).
Attached patch PatchSplinter Review
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attachment #8656690 - Flags: review?(ttaubert)
Attachment #8656690 - Flags: feedback?(ally)
Comment on attachment 8656690 [details] [diff] [review]
Patch

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

Looks good from my end. p=ally
Attachment #8656690 - Flags: feedback?(ally) → feedback+
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?
Attachment #8656690 - Flags: review?(ttaubert) → review+
url:        https://hg.mozilla.org/integration/fx-team/rev/e40065dd77cebf8e6f3ed02a68ad496da313fdf4
changeset:  e40065dd77cebf8e6f3ed02a68ad496da313fdf4
user:       Jared Wein <jwein@mozilla.com>
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 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.
Flags: needinfo?(jaws)
Attached patch Follow-up patchSplinter Review
Flags: needinfo?(jaws)
Attachment #8658369 - Flags: review?(quanxunzhen)
Attachment #8658369 - Flags: review?(quanxunzhen) → review+
https://hg.mozilla.org/mozilla-central/rev/e40065dd77ce
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
url:        https://hg.mozilla.org/integration/fx-team/rev/70c8b3ee5ad0dec85b374e8f410d7b71cdd08fa3
changeset:  70c8b3ee5ad0dec85b374e8f410d7b71cdd08fa3
user:       Jared Wein <jwein@mozilla.com>
date:       Tue Sep 08 16:56:13 2015 -0400
description:
Bug 1201374 - followup, check for DOM fullscreen. r=xidorn
You need to log in before you can comment on or make changes to this bug.