Closed Bug 988635 Opened 8 years ago Closed 8 months ago

Telemetry: Device orientation

Categories

(Firefox for Android Graveyard :: General, defect, P2)

ARM
Android
defect

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: liuche, Assigned: oogunsakin)

References

Details

Attachments

(1 file, 1 obsolete file)

Pretty self-explanatory: portrait or landscape.

This would especially be interesting to know especially for tablets (both large and small).
Attached patch bug-988635.patch (obsolete) — Splinter Review
Attachment #8400400 - Flags: feedback?(liuche)
Comment on attachment 8400400 [details] [diff] [review]
bug-988635.patch

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

>+    public String telemetrySessionForCurrentOrientation() {

How about screenOrientationToTelemetryOrientation ? It kinda matches the other code already in the file.
You'd then pass mScreenOrientation to the method.
(In reply to Mark Finkle (:mfinkle) from comment #2)
> How about screenOrientationToTelemetryOrientation ? It kinda matches the
> other code already in the file.
> You'd then pass mScreenOrientation to the method.

sounds good!
Attached patch bug-988635.patchSplinter Review
-create screenOrientationToTelemetryOrientation(ScreenOrientation)
Attachment #8400400 - Attachment is obsolete: true
Attachment #8400400 - Flags: feedback?(liuche)
Attachment #8400898 - Flags: feedback?(liuche)
Assignee: nobody → oogunsakin
Comment on attachment 8400898 [details] [diff] [review]
bug-988635.patch

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

It looks like this uses the GeckoApp inheritance to handle configuration changes (see GeckoApp.onConfigurationChanged). This means that anything that doesn't subclass GeckoApp won't get these tracked (e.g. Settings). It's possible that we don't care about the screen orientation in Settings though, but in that case, we need to explicitly close the session because this will be an "unmanaged" session.

::: mobile/android/base/TelemetryContract.java
@@ +33,5 @@
>          // Started when a user enters a given home panel.
>          // Session name is dynamic, encoded as "homepanel.1:<panel_id>"
>          public static final String HOME_PANEL = "homepanel.1:";
> +
> +        // Started when the device enters portrait mode.

Comment style, see comments on previous bugs.
Attachment #8400898 - Flags: feedback?(liuche) → feedback+
Adding a dependency on the session-closing bug, because that will allow us to track application-level sessions, instead of just GeckoApp level sessions (and omitting Settings).
Depends on: 994273

Downgrading priority from P1 to P2

Priority: P1 → P2
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → INCOMPLETE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.