Implement about:healthreport wrapper on Android

RESOLVED FIXED in Firefox 23

Status

defect
P1
normal
RESOLVED FIXED
6 years ago
9 months ago

People

(Reporter: nalexander, Assigned: nalexander)

Tracking

unspecified
Firefox 23
All
Android
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(1 attachment, 1 obsolete attachment)

There are a few parts to this ticket:

* copy-and-modify browser/base/content/abouthealthreport/* to mobile/android/chrome/content
* ensure Gecko prefs code is round-tripped through Java preferences so that background service and front-end display are kept in sync
* implement postMessage API backed by Java for fetching latest document, triggering document generation, etc

There's a strong chance that the existing content served for desktop will not gracefully degrade on mobile devices, but we'll file additional tickets as and when that is observed.
Component: Metrics and Firefox Health Report → Client: Desktop
Product: Mozilla Services → Firefox Health Report
Component: Client: Desktop → about:healthreport
Depends on: 858828
Progress update: first version that loads the iframe correctly posted to github.  Next up: wire up communication between Javascript and Java.
Component: about:healthreport → Client: Android
Summary: Implement about:healthreport on Android → Implement about:healthreport wrapper on Android
Depends on: 866271
Assignee: nobody → nalexander
Priority: -- → P3
Priority: P3 → P1
Depends on: 870371
Here's a try build exercising Javascript robocop tests:

https://tbpl.mozilla.org/?tree=Try&rev=63977c0b128c
This try build includes about:healthreport, providing I haven't made an error in mobile/android/confvars.sh.

This lands one new string in

mobile/android/locales/en-US/chrome/aboutHealthReport.dtd

but this is a direct copy of

browser/locales/en-US/chrome/browser/aboutHealthReport.dtd

which has already been translated for desktop.  How do we inform l10n team that this work is already done and just needs to be duplicated?
(In reply to Nick Alexander :nalexander from comment #4)
> This try build includes about:healthreport, providing I haven't made an
> error in mobile/android/confvars.sh.
> 
> This lands one new string in
> 
> mobile/android/locales/en-US/chrome/aboutHealthReport.dtd
> 
> but this is a direct copy of
> 
> browser/locales/en-US/chrome/browser/aboutHealthReport.dtd
> 
> which has already been translated for desktop.  How do we inform l10n team
> that this work is already done and just needs to be duplicated?

The only way I know how. Hi, Axel! :D

This should be landing in 23.
Things I'd like QA to manually verify:

1. about:healthreport is viewable.

There is no data yet, so the data display should be empty -- but it should show the content page, and it shouldn't fail noisily.

2. page title and favicon look correct.

3. tapping the "upload enabled" button works, and preserves state across application restarts.

Once https://bugzilla.mozilla.org/show_bug.cgi?id=833625 lands:

4. "upload enabled" button plays nicely with Settings checkbox -- changes are reflected in both places, etc.

5. on a fresh profile, about:healthreport shows enabled by default.  That is, the data reporting notification is shown on first run, and then subsequently navigating to about:healthreport shows the "upload is enabled" green flag.
Mercurial 2.0.2 seems to have a Mac OS X filename case bug.  Try

https://tbpl.mozilla.org/?tree=Try&rev=85dad1ab2455
Comment on attachment 748290 [details] [diff] [review]
Bug 857419 - Implement about:healthreport on Android. r=rnewman

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

::: mobile/android/chrome/content/aboutHealthReport.js
@@ +23,5 @@
> +let sharedPrefs = new SharedPreferences();
> +
> +// Name of Android SharedPreference controlling whether to upload
> +// health reports.
> +let uploadEnabledPref = "android.not_a_preference.healthreport.uploadEnabled";

const.

@@ +26,5 @@
> +// health reports.
> +let uploadEnabledPref = "android.not_a_preference.healthreport.uploadEnabled";
> +
> +// Action sent via Android Ordered Broadcast to background service.
> +let healthReportBroadcastAction = "@ANDROID_PACKAGE_NAME@" + ".healthreport.request";

const, and move these things to the top.

@@ +29,5 @@
> +// Action sent via Android Ordered Broadcast to background service.
> +let healthReportBroadcastAction = "@ANDROID_PACKAGE_NAME@" + ".healthreport.request";
> +
> +// Name of Gecko Pref specifying report content location.
> +let reportUrlPref = "datareporting.healthreport.about.reportUrl";

const.

@@ +72,5 @@
> +
> +  recordHealthReportUploadEnabled: function(enabled) {
> +    enabled = !!enabled;
> +
> +    sharedPrefs.setBoolPref(uploadEnabledPref, enabled);

Just inline the !!.

::: mobile/android/chrome/jar.mn
@@ +38,4 @@
>    content/SelectionHandler.js          (content/SelectionHandler.js)
>    content/HelperApps.js                (content/HelperApps.js)
>    content/dbg-browser-actors.js        (content/dbg-browser-actors.js)
> +  content/WebAppRT.js                  (content/WebAppRT.js)

Good catch.
Attachment #748290 - Flags: review?(rnewman) → review+
Waiting to land this in services after Bug 833625.
Nits addressed.
Attachment #748290 - Attachment is obsolete: true
Attachment #748355 - Flags: review+
Comment on attachment 748355 [details] [diff] [review]
Implement about:healthreport on Android. v2

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

::: mobile/android/components/AboutRedirector.js
@@ +76,5 @@
> +  healthreport: {
> +    uri: "chrome://browser/content/aboutHealthReport.xhtml",
> +    privileged: true
> +  },
> +#ifdef MOZ_SERVICES_HEALTHREPORT

WHOOPS
(In reply to Richard Newman [:rnewman] from comment #5)

> This should be landing in 23.

We should have a no-string impact patch for aurora.
It's landing in m-c; the only l10n note is that these are strings that already exist for desktop.
https://hg.mozilla.org/mozilla-central/rev/dec449c6ac15
https://hg.mozilla.org/mozilla-central/rev/7130e5134a6e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in services]
Target Milestone: --- → Firefox 23
Build: Firefox for Android 23.0a1(2013-05-14) 
Device: Asus Transformer TF 101
OS: Android 4.0.3

Based on comment #8 
- about:healthreport is viewable.
- page title and favicon look correct
- the preference is controllable via Settings > Data Choices > Nightly Health Report
- changing the preference in Settings > Data Choices > Nightly Health Report  changes correctly in about:healthreport
- with a fresh profile, health report preference is enabled by default. Going to about:healthreport shows the "Data Sharing" green flag.
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in before you can comment on or make changes to this bug.