1501571 - Update String for <Product and feature tips> setting summary when disabled

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Settings and Preferences, enhancement, P1)

Description

[Petru-Mugurel Lingurar [:petru]]

This is a continuation of bug 1459864

As mentioned in bug 1459864 comment 17, bug 1459864 comment 20 and bug 1459864 comment 23 the initial solution is not optimal for different localizations.

To improve on that we can use 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.

Comment 1

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1501571 - Update <Product and feature tips> setting summary when disabled; r?sdaswani

Needed to allow localizers more control over what needs to be translated in a
String that contains another already translated String that should be bolded.

Comment 2

[Natalia Csoregi [:nataliaCs]]

Land: https://hg.mozilla.org/integration/autoland/rev/ef5037b7a3cdabb2bee19adcc36186e9d279ea8e

Comment 3

[Raul Gurzau (:RaulG)]

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

Comment 4

[Ton [:Tonnes]]

In bug 1459864, flod asked for instructions for localizers on how to see this screen. Moreover, I asked to include a trailing period for this string 2 times, and for what I think are valid reasons. That request simply seems to be ignored and not even worth commenting on.

Adding a trailing period for any string intended to be imperative (instructive) such as the one here would have the same effect as prefixing it with "Please", and hence vital for some locales. If not, such a string is most likely considered to be a selectable option (as indicated by referring to https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices.)

Some locales may not really care - Italian is probably a nice example, as it uses active mood for anything (such as "Attiva" for "Activate" or "Turn on") - regardless of them being imperative or infinitive. Locales such as Dutch, German, French, Spanish etc. and probably more however _do_ care. As indicated, this can be the difference between "Schakel x in" [instruction] / x inschakelen" [choice] for nl/de, and "Activez/Activa" [instructive] vs. "Activer/Activar" [choice] for fr/es. See the current wrong infinitive forms in localizations for pref_feature_tips_notification_enabling_hint and tab_queue_prompt_permit_drawing_over_apps for evidence (simply search transvision for "turn on" for these locales) - indicating these strings are considered to be options to be chosen rather than instructions to the user, as intended.

I won’t be surprised if localizations for pref_feature_tips_notification_enabling_hint2 will get full verbs (infinitive) for fr/es as a result of the missing period when localizations trickle in.

I hope that explains the concerns.

Comment 5

[Petru-Mugurel Lingurar [:petru]]

Hey Ton,
Thanks for chiming in and sorry if the current patch still has issues. I've asked the sheriffs to back it out.

Currently what is used is
> <!ENTITY pref_feature_tips_notification_enabling_hint2 "Turn on &pref_feature_tips_notification_enabling_path;"
How should this be phrased so that it will be easier to localize it?

Comment 6

[Daniel Varga [:dvarga]]

Backed out changeset ef5037b7a3cd (Bug 1501571) requested by dev for concerns regarding localization. a=backout

Comment 7

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/fd2afb2034d1
Backed out changeset ef5037b7a3cd requested by dev for concerns regarding localization. a=backout

Comment 8

[Ton [:Tonnes]]

Hi Petru,

The simple solution would be to just include a period to the string already suggested.

> <!ENTITY pref_feature_tips_notification_enabling_hint2 "Turn on &pref_feature_tips_notification_enabling_path;."

(... provided that and assuming this would not introduce an additional space between &pref_feature_tips_notification_enabling_path; and the period for specific coding reasons - something that could be overcome for non-English localizations requiring additonal words at the end of the string, but nevertheless.)

The final string would end up in a similar construction as appMenuRemoteTabs.tabsnotsyncing.label and syncedTabs.sidebar.tabsnotsyncing.label in browser.dtd for Firefox desktop - see e.g. https://transvision.mozfr.org/?recherche=Turn+on&repo=gecko_strings&sourcelocale=en-US&locale=fr&search_type=strings for French.

Side notes:

1) For anyone who thinks the period issue is not important for English: do note that strings are completely identical without a period for both moods and may cause just as much confusion to users (if not more), regardless of verb or other requirements.
2) Fwiw, I do agree with flod about comments and string IDs that should clarify, but unfortunately sometimes localizers simply overlook them, and/or they aren’t always that clear either.
3) For pref_feature_tips_notification_enabling_hint and assuming that can or will not be changed, I circumvented that by translating it as "Turn on this feature using [x]".

Comment 9

[Petru-Mugurel Lingurar [:petru]]

Attachment: FHR disabled product tips.png

Thanks Ton!

I've added a trailing period. It doesn't impact the code in any way. It looks good.
Because this patch was backed out I presume there's no need to increment `pref_feature_tips_notification_enabling_hint2` right ?

Comment 10

[Petru-Mugurel Lingurar [:petru]]

Attachment: FHR disabled product tips 2.png

Ton raised an important issue above in comment 8. He needed to translate "Turn on X" to "Turn on this feature using X"
This was also raised by Bram in bug 1459864 comment 5 but as per bug 1459864 comment 11 we decided to use the short form.
Should we use a longer form (which?) to better support localization and also improve on the clarity of this summary?

Comment 11

[Ton [:Tonnes]]

Petru,

That looks better in my opinion. I can’t tell about the requirement for incrementing the ID, but think there is none since the strings had not landed yet (cmiiw).

Admitted, I also prefer to see the last part ("to enable this feature.") included in order to notify why users should enable what is requested. Please do note that it would be good to think about whether "it" refers to "feature" or "option" here.

Comment 12

[Petru-Mugurel Lingurar [:petru]]

I think "it" refers to "option".
In that if the user enables FHR (as instructed)
That toggle we see on the screen will become actionable.
So he will have the option to act on that toggle
Which when toggled on will turn the feature on.

Comment 13

[Bram Pitoyo [:bram]]

I agree with the change you’ve proposed about making the strings a bit longer and more conditional, in order to better support localisation. It will help make our message understandable.

I would still defer to Brian for string advise.

If we really want users to be able to act on this item quickly without digging through the menu (and potentially forgetting where to look), we can turn “Privacy > Data Choices > ...” into a tappable link. When tapped, it links to the relevant menu item, that can be enabled straight away.

We can even make this process easier by making the link do a “Toggle ON” action on the “Fennec Health Report”, without even going to the page. Why make the user jump through hoops, if we can do the action for them?

What do you think?

Comment 14

[Petru-Mugurel Lingurar [:petru]]

(In reply to Bram Pitoyo [:bram] from comment #13)
> I agree with the change you’ve proposed about making the strings a bit
> longer and more conditional, in order to better support localisation. It
> will help make our message understandable.
> 
> I would still defer to Brian for string advise.
> 
> If we really want users to be able to act on this item quickly without
> digging through the menu (and potentially forgetting where to look), we can
> turn “Privacy > Data Choices > ...” into a tappable link. When tapped, it
> links to the relevant menu item, that can be enabled straight away.
> 
> We can even make this process easier by making the link do a “Toggle ON”
> action on the “Fennec Health Report”, without even going to the page. Why
> make the user jump through hoops, if we can do the action for them?
> 
> What do you think?

Thanks Bram.
Waiting for the final saying from Brian then if we should replace the current
"Turn on X." with "Turn on to enable this option." (like in the attached images) or any other text.

I like your suggestion about directing the user straight to the Privacy screen to enable FHR but this is not possible if this preference is disabled. Android disables this view and all its descendants so no touch/click events are received.

Comment 15

[Brian Jones]

I apologize in advance if I'm missing some of the nuances of this conversation. Is there a reason we can't turn "Privacy > Data Choices>..."into a tappable link with this string:

Enable Privacy > Data Choices > ...

(We're already using "Enable" in multiple places with no localization issues?)

Comment 16

[Petru-Mugurel Lingurar [:petru]]

(In reply to Brian Jones from comment #15)
> I apologize in advance if I'm missing some of the nuances of this
> conversation. Is there a reason we can't turn "Privacy > Data
> Choices>..."into a tappable link with this string:
> 
> Enable Privacy > Data Choices > ...
> 
> (We're already using "Enable" in multiple places with no localization
> issues?)

In bug 1454686 was decided that this "Product and feature tips" is a subset of Health Report. When Health Report is disabled, this "Product and feature tips" is also disabled.
Being disabled users cannot toggle it on or off. Allowing them to toggle it on would cause confusion as it would not actually do anything (because FHR is turned off) despite it appearing enabled.

But this has the side effect that no click/touch event are passed to the disabled view. So a tappable link in a disabled view cannot be tapped.

Comment 17

[Brian Jones]

Let's look at changing the content structure to make it more obvious that P > DC > FHR must be enabled for this toggle to be active.

Product and feature tips
(To enable this option, Privacy > Data choices > Firefox Health Report must already be enabled.)
Learn more about using Firefox and other Mozilla products.

Comment 18

[Petru-Mugurel Lingurar [:petru]]

Attachment: FHR disabled product tips - updated.png

Thanks Brian!
So if this looks ok to you I'll update the patch accordingly.

Comment 19

[Brian Jones]

Hi Petru. Looks good to me...thank you!

Comment 20

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1501571 - Inform user that FHR must be enabled for 'Product and feature tips' to work; r?sdaswani

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/08e578f6f4ca
Inform user that FHR must be enabled for 'Product and feature tips' to work; r=geckoview-reviewers,snorp

Comment 22

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/08e578f6f4ca

Comment 23

[Sorina Florean [:sflorean]]

Verified as fixed in build 65.0a1 - 18/11.
Device: Google Pixel (Android 9).