Closed Bug 1459864 Opened 2 years ago Closed Last year

Modify <Product and feature tips> setting summary when disabled

Categories

(Firefox for Android :: Settings and Preferences, defect, P5)

Unspecified
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 64
Tracking Status
firefox64 --- verified

People

(Reporter: petru, Assigned: petru)

References

Details

Attachments

(3 files, 2 obsolete files)

Attached image setting disabled.png (obsolete) —
As a follow-up to bug 1454686, it might be nice to update this setting's summary when it is disabled (as a result of the user toggling off Health Report) as an indication to the users for why this setting is disabled (cannot be acted upon) and how can it be enabled.

This is a small change to implement if UX agrees and provides the new string to be used only when that setting is disabled.
Depends on: 1454686
Flags: needinfo?(sdaswani)
NI'ing Bram.
Flags: needinfo?(sdaswani) → needinfo?(bram)
Hi Petru, I didn’t know that these two toggles are connected to each other.

Normally, we don’t recommend changing the entire summary text. What we can do is add to it.

So, maybe it can say something like:

> Product and feature tips
>
> Learn more about using Fennec and other Mozilla products
>
> Turn on Privacy > Data Choices > Firefox Health Report to enable this option

NI brjones for advise.
Flags: needinfo?(bram) → needinfo?(brjones)
This is what the disabled setting would look like.

I suggested modifying it's summary to make it clear for the user why something on the screen cannot be interacted with.

I saw the summary being updated for a setting in the case of
> Advanced > Remote debugging via Wi-Fi
if there is no QR code reader app installed.
Bram how does this look?
Flags: needinfo?(bram)
This looks good. Just needs a tiny bit of modification.

Petru, these changes will make the UI even better:

1. Add a space between the two sentences (as if you’re tapping the “Return” key twice to create a new paragraph)
2. Bold the “Privacy > Data Choices > Firefox Health Report” part of the sentence, so it’s clearer
3. Slightly reorder the sentence structure: “To enable this option, turn on Privacy > Data Choices > Firefox Health Report”.

Three small changes, as above.
Flags: needinfo?(bram)
Attached image disabled tips notifications summary.png (obsolete) —
Thanks Bram!
If this looks good to you, as soon as the patch for bug 1454686 gets merged I'll push the patch for this.
Petru, do we have to land new strings for localization here?
Flags: needinfo?(petru.lingurar)
Indeed, if everything looks good to Bram and so we will be using "To enable this option, turn on Privacy > Data Choices > Firefox Health Report", the
> "To enable this option, turn on"
part will need to be localized.
Because the rest of the string refers to other settings titles I think we should use their already localized values (which would also ensure that if those change our string that contains them would also get updated, and not be forgotten)

We can either push the string first and the feature later (I think this would need another ticket), or push them together.
From what I see in the commit history for the strings file and because Lint is set to fail the build if unused strings are found I think normally the strings and the feature are to be pushed together.
Flags: needinfo?(petru.lingurar) → needinfo?(bram)
Thanks Petru, let's hope Bram and Brian are ok with the strings and you can push this for 62.
I’m okay with these changes.
Flags: needinfo?(bram)
Brian told me we should simplify the last sentence to 'Turn on Privacy > Data choices > Fennec Health Report' (no need for the 'To enable this option')_
Great!
As soon as the feature in bug 1454686, on which this depends on, is merged I can work on this. Should be a simple a simple fix.
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.
Priority: -- → P5
Inform the user about why is this feature disabled and what to do to enable it.
Keywords: checkin-needed
Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6eaed80ae36f
Modify <Product and feature tips> setting summary when disabled; r=jchen
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6eaed80ae36f
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Assignee: nobody → petru.lingurar
Looks like this patch is concatenating "Turn on" with " " and then something else. This is an extremely poor approach to localization, and won't work for a lot of languages.
Can someone provide a screenshot of the feature, or instructions on how to see the strings in Fennec? I see screenshots here, but I assume they're outdated, given how old they are and the following discussions.
This is what the summary of that disabled setting would look like.

I went with "concatenating "Turn on" with " " and then something else" because after "Turn on" (and the space) what follows are settings paths for which the names can be changed not taking into account what this ticket wants to achieve, changes which will automatically appear here also.
If you have any suggestions on how to improve this I'd be glad to make the changes.
Attachment #8973964 - Attachment is obsolete: true
Attachment #8974657 - Attachment is obsolete: true
Flags: needinfo?(brjones) → needinfo?(francesco.lodolo)
Thanks for the screenshot, it helps.

Consider a language where the only way to translate this is "Select X to turn on this feature", or "X turn on". There's no way to do it, you're imposing the English grammar to all languages. It should be "Turn on %S", so that localizers can move the second part around. If it's a limitation of the code (e.g. injecting markup), then it needs a different copy.

Side question: is "turn on" intended as a description of the command, explaining what happens when you toggle the switch?
Flags: needinfo?(francesco.lodolo)
I think "turn on" - just as a very small and simple description of the needed action allows localizers some wiggle room while also allowing the markup for the path that follows.

As a description for the "Turn on" string I've added [1]
> Localization note (pref_feature_tips_notification_enabling_hint):
> Describe the action the user should do to enable this preference
> Will be used in the context "<Turn on> ... to enable this feature"
`&brandShortName; Health Report` (the path's destination) being something that can be turned on/off by toggling it's switch.

[1] https://hg.mozilla.org/mozilla-central/rev/6eaed80ae36f#l2.12
NIing Delphine to ask her opinion.
Francesco has a fair point but I'm not sure how to improve the current solution which, as detailed in comment 21 still think is the best that can be done. 
Open to any suggestions, of course.
Flags: needinfo?(lebedel.delphine)
The main issue is, as flod pointed out, that "turn on" may need 2 parts that include the feature in the middle of it. Examples of German/ Dutch: "Schalte x ein" / "Schakel x in", which is not currently possible. Just imagine the original English string would be "Set x to on". 

Additionally, one may wonder whether the string is a description which usually means the verbs are infinitive, or instructive (to the user) which means it’s imperative - the latter seems to be the case here. Without looking at the context (and please realize the Notifications screen is hard to find in Nightly), localizers may assume the string to be a description or option, hence requiring infinitive mood in general, leading to translations like e.g. "x einschalten" or "x inschakelen" or "Activer x"  or "Activar x" for German, Dutch, French and Spanish respectively instead of ones using imperative mood with different forms for verbs and perhaps more.

And that brings us to the known story of trailing periods that are commonly used in imperative (instructive) strings and left out in others. In other words, similar strings in English - doubly interpretable - periods do matter for other locales, and in some cases even for English. Please do not rely on localizers to look at parts like "hint" in string IDs to find out what mood is intended.

The string may be comparable to tab_queue_prompt_permit_drawing_over_apps. Regardless of the fact that string uses no variable, I’d say that string should use a period too since a few locales (fr, es) currently localized them as infinitive, as if they were an option or description.

Also see the Tooltips and Menu and control labels section in https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices.
Thanks Ton!

How about the following in android_strings.dtd
> <!-- Localization note (pref_feature_tips_notification_enabling_path):
>      Nothing to translate. Simple concatenation of already localized strings. Result is used below -->
> <!ENTITY pref_feature_tips_notification_enabling_path "&pref_category_privacy_short; > &pref_category_datareporting; > &datareporting_fhr_title;">
> <!-- Localization note (pref_feature_tips_notification_enabling_hint):
>      Describe the action the user should do to enable this preference -->
> <!ENTITY pref_feature_tips_notification_enabling_hint "Turn on &pref_feature_tips_notification_enabling_path;">

What would need to be localized is still "Turn on" but the context is there now and the string for the path is immutable while still allowing localizers to compose the entire resulting string however they see fit.

This would also still allow to apply any needed styling from Java side for the path string because that block is known.
I’m not aware of whether this will be the perfect solution - that’s more flod’s territory :) - yet am afraid using &pref***; IDs in l10n strings are uncommon.

However I would still end the final string with a period, which would immediately indicate the proper form without even looking at the l10n comment - note some localizers may overlook these comments, unfortunately.
Talked a bit with Flod about this, and it seems that :petru's proposed solution could work.
Flod just pointed out though that you would need to rename the string IDs, since now it's already in beta and central
Flags: needinfo?(lebedel.delphine)
Depends on: 1501571
Would you let Brian and I know what the final string looks like?

As I proposed on comment 5, saying something conditional such as:

> Turn on x to enable y

or

> To enable y, turn on x


Will make it easier for users to understand what to do to “unlock” this feature.
(In reply to Bram Pitoyo [:bram] from comment #27)
> Would you let Brian and I know what the final string looks like?
> 
> As I proposed on comment 5, saying something conditional such as:
> 
> > Turn on x to enable y
> 
> or
> 
> > To enable y, turn on x
> 
> 
> Will make it easier for users to understand what to do to “unlock” this
> feature.

from comment #11
> Brian told me we should simplify the last sentence to 'Turn on Privacy >
> Data choices > Fennec Health Report' (no need for the 'To enable this
> option')
so that's what I went with - attachment 9018958 [details]

Should this be changed to "Turn on X to enable this feature" ?
Flags: needinfo?(bram)
(In reply to Petru-Mugurel Lingurar[:petru] from comment #28)
> 
> from comment #11
> > Brian told me we should simplify the last sentence to 'Turn on Privacy >
> > Data choices > Fennec Health Report' (no need for the 'To enable this
> > option')
> so that's what I went with - attachment 9018958 [details]
> 
> Should this be changed to "Turn on X to enable this feature" ?

Sorry, I didn’t read the thread carefully. If Brian has approved with the string, then I’m also okay with it. No change to make here.

Sorry for the mixup!
Flags: needinfo?(bram)
Verified as fixed on build 64.0b7.
Device: Nokia 6(Android 7.1.1).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.