885582 - Append version number to about:healthreport URL

Status: RESOLVED FIXED

(Firefox Health Report Graveyard :: Client: Android, defect)

Description

[Nick Alexander :nalexander [he/him]]

rnewman and I were talking, and we'd like to version the mobile about:healthreport wrapper.  The idea would be to request /mobile/1 for version 1 (or /mobile/?version=1) to allow the about:healthreport wrapper to grow capabilities.  That is, this will let us advertise what messages the wrapper understands from the content.

The first set of capabilities will likely be showing the data reporting settings (Bug 879555) and launching an updater (Bug 882745).

Let's make this ticket the content ticket; after discussion, I'll open the Android client ticket.

Comment 1

[Nick Alexander :nalexander [he/him]]

Attachment: Append version number query parameter to Android about:healthreport URL. r=rnewman

Tested against no trailing slash, trailing slash, and included query
parameter:

    328:E GeckoConsole(20202)         uri.spec: https://fhr.cdn.mozilla.net/en-US?v=1
    427:E GeckoConsole(20202)         uri.spec: https://fhr.cdn.mozilla.net/en-US/mobile/?v=1
    468:E GeckoConsole(20202)         uri.spec: https://fhr.cdn.mozilla.net/en-US/mobile/?x=y&v=1

Comment 2

[Richard Newman [:rnewman]]

Comment on attachment 766128 [details] [diff] [review]
Append version number query parameter to Android about:healthreport URL. r=rnewman

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

Approval request: we would like the FHR page URL to have a version parameter, so we can update the capabilities of the wrapper without confusing the web content. This tiny patch does so.

::: mobile/android/chrome/content/aboutHealthReport.js
@@ +60,5 @@
>  
>    _getReportURI: function () {
>      let url = Services.urlFormatter.formatURLPref(PREF_REPORTURL);
> +    // This handles URLs that already have query parameters.
> +    let uri = Services.io.newURI(url, null, null).QueryInterface(Ci.nsIURL);

This kinda rings my "should have a try block" bell, but if anyone does something stupid like jams a mailto: URI into the report URL pref, they deserve what they get.

@@ +64,5 @@
> +    let uri = Services.io.newURI(url, null, null).QueryInterface(Ci.nsIURL);
> +    if (uri.query != "") {
> +      uri.query += "&";
> +    }
> +    uri.query += "v=" + WRAPPER_VERSION;

Every little helps: avoid a concatenation.

  uri.query += ((uri.query != "") ? "&v=" : "v=") + WRAPPER_VERSION;

Comment 3

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/services/services-central/rev/ec25f53bf993

Comment 4

[Nick Alexander :nalexander [he/him]]

I stole this one for the Android Client.  Webdev can open another as and when they see fit.

Comment 5

[Phil Ringnalda (:philor)]

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

Comment 6

[bhavana bajaj [:bajaj]]

Comment on attachment 766128 [details] [diff] [review]
Append version number query parameter to Android about:healthreport URL. r=rnewman

[Triage Comment]

Switching the aurora nom to beta and approving as this is needed for Fx23 beta 1 and confirmed its low risk.

Comment 7

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/releases/mozilla-beta/rev/48c44c0dbab7