Open Bug 988635 Opened 6 years ago Updated 4 months ago

Telemetry: Device orientation

Categories

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

ARM
Android
defect

Tracking

()

People

(Reporter: liuche, Assigned: oogunsakin)

References

(Depends on 1 open bug, Blocks 1 open bug)

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
You need to log in before you can comment on or make changes to this bug.