per Arun: we would like to centralize FHR control in Settings > Data choices on Android. This would be a link or a button of some type in the Settings panel.

Arun and I settled on changing the title into a link, like in the attached mockup

(Arun: I follow this whole product, so no need to CC me! Thanks though.)

Styling the link is simple. Making that link clickable is the hard part, because clicks get swallowed up by the Preference, and end up changing the preference state rather than launching the link. Will poke around this later. Alternatively, we might be able to use a long-click, if that's acceptable.

Thanks, Chenxia. Long tap might be very difficult for people to understand particularly in this context (they would assume that the link is not working after tapping). We are shipping on June 25 to the beta release (just fyi) Let us know how it goes :)

Is this underway, Chenxia? Need to know if it can hit 23 or not.

Getting a click to register on the link is going to require a lot more work, along the lines of making a custom preference layout, which is not going to get done by merge. To get into 23, I can add another item into the list called "View health report" to launch about:healthreport, would that work?

test apk: http://people.mozilla.com/~liuche/bug-879558/ (You need to hit back one more time after clicking "View health report", but I'll fix that after lunch.) A few notes: - We probably need some summary text. Feel free to suggest title changes too. - Will it be confusing to have the link if FHR is not enabled? We should include that in the summary text.

paging the grammar nitpicking police :-) (no idea who that would be, who does enforce text UX?) Should "View health report" have the same capitalization as "Firefox Health Report" ?

(In reply to Roland Tanglao :rolandtanglao from comment #10) > paging the grammar nitpicking police :-) (no idea who that would be, who > does enforce text UX?) Should "View health report" have the same > capitalization as "Firefox Health Report" ? IMO yes, but this is Ian's call.

Proposing to go with this: https://bugzilla.mozilla.org/attachment.cgi?id=765534 with the following changes: (1) title change from "Firefox Health Report" to "Firefox Health Report sharing" (data sharing would make it too long?) (2) title change from "View health report" to "View Firefox Health Report" (3) Also, include following subtext to "View Firefox Health Report" only when "Firefox Health Report sharing" is turned off: option #1 "You can view this even if you have data sharing turned off." option #2 "You can view this even if data sharing is turned off."

needs?info'ing Ian & Verdi to make the calls on this.

Hm, I don't really like "FHR sharing" - it seems inconsistent with the other data choices strings (and they're all under Data Choices anyways), and it's likely the string will be too long to display for a lot of phones. I understand that there might be some confusion between "View FHR" and simply "FHR." What about changing "View FHR" to "My FHR"? May be inconsistent with the other data strings. For subtext strings, I also like: "You can always view this, even if you have sharing turned off."

The other option is to visually distinguish the "view" option -- make it vertically shorter, and align the text to the right, say. ---------------------------------- Telemetry Shares performance… [ ] ---------------------------------- Nightly Health Report Helps you understand… [ ] ---------------------------------- View your Health Report >> ---------------------------------- Crash Reporter Nightly submits… [ ] ---------------------------------- Re appending "sharing": better, I think, to reword the *explanatory* text to put "Shares" first, just like Telemetry: ----- Firefox Health Report Shares data with Mozilla about your browser health to help you fix problems and understand your browser's performance. ----- After that, showing "View your Health Report" seems to make plenty of sense. I should note that we cannot add strings to 23, so if we're adding a new string for viewing, we can reword the rest. Given that, Ian: is it enough for 23 to show: ---------------------------------- Nightly Health Report Helps you understand… [ ] ---------------------------------- Nightly Health Report >> ---------------------------------- ?

(In reply to Chenxia Liu [:liuche] from comment #14) > Hm, I don't really like "FHR sharing" - it seems inconsistent with the other > data choices strings (and they're all under Data Choices anyways), and it's > likely the string will be too long to display for a lot of phones. > > I understand that there might be some confusion between "View FHR" and > simply "FHR." What about changing "View FHR" to "My FHR"? May be > inconsistent with the other data strings. > > For subtext strings, I also like: > "You can always view this, even if you have sharing turned off." Yup… Thanks for coming up with the strings – I like this one too!

(In reply to Arun Balachandran Ganesan [:abc] from comment #12) > Proposing to go with this: > https://bugzilla.mozilla.org/attachment.cgi?id=765534 > > with the following changes: > > (1) title change from "Firefox Health Report" to "Firefox Health Report > sharing" (data sharing would make it too long?) This makes sense to me because it is the sharing that you are turning on or off. FHR is still there (and viewable) no matter the state of the check box. That makes the explanatory text a little strange though. It may be better to say something like, "Shares data with Mozilla about your browser health." > > (2) title change from "View health report" to "View Firefox Health Report" Sounds good. > > (3) Also, include following subtext to "View Firefox Health Report" only > when "Firefox Health Report sharing" is turned off: > > option #1 "You can view this even if you have data sharing turned off." > option #2 "You can view this even if data sharing is turned off." If we were to change the explanatory text under "Firefox Health Report sharing" like I suggest above, then I think you don't have to explain that you can view this at any time. Instead you can include the rest of the original text, "Understand your browser performance." But to answer the question, I think option #2 is better. So to recap, what about something like this? ---------------------------------- Nightly Health Report sharing Shares data with Mozilla about your browser health [ ] ---------------------------------- View Nightly Health Report >> Understand your browser performance ----------------------------------

Patch for option 2: Adding a new "View FHR" list item. Downsides: we won't be able to uplift this to 23 because new strings.

Do we have an icon for the ">" in a list item? I can't think of or find any examples in Android in the wild; I assume we want it to look like the "<" in the Android ActionBar.

(In reply to Chenxia Liu [:liuche] from comment #19) > Do we have an icon for the ">" in a list item? I can't think of or find any > examples in Android in the wild; I assume we want it to look like the "<" in > the Android ActionBar. themes/core/images/arrowright-16.png base/resources/drawable*/{menu_item_more,ic_menu_forward,ic_awesomebar_go}.png

Comment on attachment 766138 [details] Screenshot: [alt 2] right-align View on 4.0 phone Can we right-align the text, too?

Needs tweaking on padding.

Added a custom pref with an icon. Needs some pixel pushing.

Pike, some input from you about strings: can I add a "»" in strings.xml.in instead of the localized strings without breaking string freeze? See <Screenshot: [alt 3] no subtext, aurora strings>: I'd like to use a "»" instead of the single arrow. Since I'd also like to leave open the option of uplifting to 23 aurora without breaking strings, I was thinking of something like the following: <!ENTITY datareporting_fhr_title "&brandShortName; Health Report"> (already exists in 23) and adding the "»" in strings.xml.in <string name="datareporting_viewfhr_title">&datareporting_fhr_title;&#x00A0;»</string> We'll be changing the strings for 24 to be slightly clearer about what FHR is, and viewfhr_title will probably become "View my Health Report", so up to you if you'd prefer the chevron to be in the localizable string or not.

Hardcoding both glyphs and whitespace is not a good idea. For some languages, » might look like the default quote, and that's odd. Also, IIRC, at least the Chinese languages don't use whitespace for separation of latin text.

So it looks like the we won't be able to uplift to aurora if we want to use » in the strings. I'm going to use the preference with a > icon then, so that we can uplift to 23, and for consistency, stick that into 24. We can file a bug later if desired to change it to something else.

This patch adds a small list item into the preferences for navigating to about:healthreport. Also not, it makes a small change to the subtext string for Data Choices > FHR

No string changes. https://tbpl.mozilla.org/?tree=Try&rev=9c1f9984c184

Comment on attachment 766252 [details] [diff] [review] Patch: "View my Health Report" about:healthreport entry v1 try: https://tbpl.mozilla.org/?tree=Try&rev=802fe055a078

Haven't done tablet testing with this, so please take a look and make sure all the pixels are playing nicely.

Screenshot for Firefox 24, with updated strings. Specifically, small changes to the FHR subtext, and new strings for the about:healthreport entry.

No new strings.

Chenxia: Thank you! If it's still possible to change, I would like the arrow to be center aligned with the checkbox. Looks fine to me otherwise. Have a good weekend! Arun

Comment on attachment 766253 [details] [diff] [review] Aurora patch: "* Health Report" about:healthreport entry v1 Review of attachment 766253 [details] [diff] [review]: ----------------------------------------------------------------- I think this is good… testing shortly. ::: mobile/android/base/GeckoPreferences.java @@ +73,5 @@ > private static String PREFS_MP_ENABLED = "privacy.masterpassword.enabled"; > private static String PREFS_UPDATER_AUTODOWNLOAD = "app.update.autodownload"; > + private static String PREFS_HEALTHREPORT_LINK = NON_PREF_PREFIX + "healthreport.link"; > + > + private static int PREF_SCREEN_REQCODE = 5; Would prefer REQUEST_CODE_PREF_SCREEN. @@ +285,5 @@ > continue; > + } else if (PREFS_HEALTHREPORT_LINK.equals(key) && !AppConstants.MOZ_SERVICES_HEALTHREPORT) { > + preferences.removePreference(pref); > + i--; > + continue; Indenting is wrong for Fennec. And we could actually roll this into the previous clause.

Comment on attachment 766253 [details] [diff] [review] Aurora patch: "* Health Report" about:healthreport entry v1 This looks great on tablet and phone. [Approval Request Comment] Bug caused by (feature/regressing bug #): New work to provide discoverable access to FHR. User impact if declined: Need to know the secret "about:healthreport" incantation. Testing completed (on m-c, etc.): Tested by hand on m-a. Risk to taking this patch (and alternatives if risky): Slim: new code path and new pref fragment. String or IDL/UUID changes made by this patch: None.

Running a build for the inbound bit now. Will flag r+ when I'm ready to land my modified version.

(In reply to Richard Newman [:rnewman] from comment #39) > Comment on attachment 766253 [details] [diff] [review] > Aurora patch: "* Health Report" about:healthreport entry v1 > > This looks great on tablet and phone. Yaay!

Comment on attachment 766252 [details] [diff] [review] Patch: "View my Health Report" about:healthreport entry v1 https://hg.mozilla.org/integration/mozilla-inbound/rev/38244599c282

Richard, you're not supposed to change strings once they landed on mozilla-central (datareporting_fhr_summary) https://developer.mozilla.org/en-US/docs/Making_String_Changes Unfortunately this happened way to close to merge day. Pike, suggestions? Should we leave the string as it is and warn localizers?

(In reply to Francesco Lodolo [:flod] from comment #44) > Richard, you're not supposed to change strings once they landed on > mozilla-central (datareporting_fhr_summary) Drat, sorry I didn't spot that in Chenxia's patch. Didn't expect it _not_ to have changed, if that makes sense, because we've been through this before! > Pike, suggestions? Should we leave the string as it is and warn localizers? This is an easy thing for me to fix, if 8 hours on m-c isn't too long. (And of course this doesn't affect the stringless Aurora patch that I hope to land tomorrow.)

Let's fix this, and if it misses the merge, get it uplifted. akeybl is doing the merge, flagging this to be on his radar.

Thanks Pike. Richard: I opened bug 886231 to keep track of this change, at least we can set flags or keywords there if needed.

(In reply to Richard Newman [:rnewman] from comment #11) > (In reply to Roland Tanglao :rolandtanglao from comment #10) > > paging the grammar nitpicking police :-) (no idea who that would be, who > > does enforce text UX?) Should "View health report" have the same > > capitalization as "Firefox Health Report" ? > > IMO yes, but this is Ian's call. Agree with everyone's decision to capitalize View Health Report. Nice design solutions, too, for 23 and 24 :)

Comment on attachment 766253 [details] [diff] [review] Aurora patch: "* Health Report" about:healthreport entry v1 [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Testing completed (on m-c, etc.): Risk to taking this patch (and alternatives if risky): String or IDL/UUID changes made by this patch:

Comment on attachment 766253 [details] [diff] [review] Aurora patch: "* Health Report" about:healthreport entry v1 Needed for fhr on android, the aurora/beta does not have string changes.