Based on the feedback[1] from :flod, some string IDs should be updated and more localization notes should be added to make localization easier. [1] https://github.com/mozilla-l10n/formautofill-l10n/pull/2

Comment on attachment 8917760 [details] Bug 1407530 - Update formautofill.properties string IDs and localization notes. https://reviewboard.mozilla.org/r/188330/#review194700 The code part looks good. Thanks. Should we ask L10N team to review IDs and notes if possible?

Hi :flod, the patch contains the string ID changes and localization notes we discussed on Github. Other changes in this patch are for updating string IDs and replacing hardcoded "Firefox" with brandShortname. Please let me know if the l10n part looks fine. Thanks again!

Comment on attachment 8917760 [details] Bug 1407530 - Update formautofill.properties string IDs and localization notes. https://reviewboard.mozilla.org/r/188330/#review194708 Strings look good. Mostly nits on comments, and one question. I also wonder if we should take bug 1399502 into account when working on new features, but that's a whole different story. ::: browser/extensions/formautofill/locales/en-US/formautofill.properties:8 (Diff revision 2) > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > -preferenceGroupTitle = Form Autofill > -enableAddressAutofill = Autofill addresses > -learnMore = Learn more > -savedAddresses = Saved Addresses… > +# LOCALIZATION NOTE (saveAddressesMessage): %S is brandShortName. This string is used on the doorhanger to > +# notify users that addresses are saved. > +saveAddressesMessage = %S now saves addresses so you can fill out forms faster. > +# LOCALIZATION NOTE (autofillOptionsLink, autofillOptionsLinkOSX): This string is used in the doorhanger for Nits: * this string is -> these strings are (there are two mentioned in the comment) * "leads user" -> "leads users" ::: browser/extensions/formautofill/locales/en-US/formautofill.properties:12 (Diff revision 2) > -saveAddressesMessage = Firefox now saves addresses so you can fill out forms faster. > autofillOptionsLink = Form Autofill Options > -autofillSecurityOptionsLink = Form Autofill & Security Options > -changeAutofillOptions = Change Form Autofill Options > autofillOptionsLinkOSX = Form Autofill Preferences > +# LOCALIZATION NOTE (autofillSecurityOptionsLink, autofillSecurityOptionsLinkOSX): This string is used this string is -> these strings are ::: browser/extensions/formautofill/locales/en-US/formautofill.properties:16 (Diff revision 2) > autofillOptionsLinkOSX = Form Autofill Preferences > +# LOCALIZATION NOTE (autofillSecurityOptionsLink, autofillSecurityOptionsLinkOSX): This string is used > +# in the doorhanger for saving credit card info. The link leads users to Form Autofill browser preferences. > +autofillSecurityOptionsLink = Form Autofill & Security Options > autofillSecurityOptionsLinkOSX = Form Autofill & Security Preferences > +# LOCALIZATION NOTE (changeAutofillOptions, changeAutofillOptionsOSX): This string is used on the doorhanger this string is -> these strings are ::: browser/extensions/formautofill/locales/en-US/formautofill.properties:20 (Diff revision 2) > autofillSecurityOptionsLinkOSX = Form Autofill & Security Preferences > +# LOCALIZATION NOTE (changeAutofillOptions, changeAutofillOptionsOSX): This string is used on the doorhanger > +# that notifies users that addresses are saved. The button leads users to Form Autofill browser preferences. > +changeAutofillOptions = Change Form Autofill Options > changeAutofillOptionsOSX = Change Form Autofill Preferences > +# LOCALIZATION NOTE (addressesSyncCheckbox): When users have sync account, on the doorhanger that notifies I think we can simplify this: If Sync is enabled, this checkbox is displayed on the doorhanger shown when saving addresses. ::: browser/extensions/formautofill/locales/en-US/formautofill.properties:43 (Diff revision 2) > +# drop down suggestion. Leads users to Form Autofill browser preferences. > autocompleteFooterOption = Form Autofill Options > -autocompleteFooterOptionShort = More Options > autocompleteFooterOptionOSX = Form Autofill Preferences > +# LOCALIZATION NOTE (autocompleteFooterOptionShort, autocompleteFooterOptionOSXShort): Narrow version of the > +# button at the bottom of the drop down suggestion. Leads users to Form Autofill browser preferences. I'm curious about this one: you have "More Options" and "Preferences", does it mean you have a maximum number of characters that can be displayed? It would be helpful to explain in the comments. I'm also not sure I can understand how it's used by reading the comment. Do you have a screenshot?

Comment on attachment 8917760 [details] Bug 1407530 - Update formautofill.properties string IDs and localization notes. https://reviewboard.mozilla.org/r/188330/#review194708 > I think we can simplify this: > > If Sync is enabled, this checkbox is displayed on the doorhanger shown when saving addresses. Thanks. Much more concise. > I'm curious about this one: you have "More Options" and "Preferences", does it mean you have a maximum number of characters that can be displayed? It would be helpful to explain in the comments. > > I'm also not sure I can understand how it's used by reading the comment. Do you have a screenshot? Here's the visual spec: https://mozilla.invisionapp.com/share/MQ8J8K4CE#/screens/246435182 When the input field width is <= 185px, the short form is used, so there's no exact character count.

(In reply to Scott Wu [:scottwu] from comment #6) > > I'm curious about this one: you have "More Options" and "Preferences", does it mean you have a maximum number of characters that can be displayed? It would be helpful to explain in the comments. > > > > I'm also not sure I can understand how it's used by reading the comment. Do you have a screenshot? > > Here's the visual spec: > https://mozilla.invisionapp.com/share/MQ8J8K4CE#/screens/246435182 > When the input field width is <= 185px, the short form is used, so there's > no exact character count. The problem with this approach is that the logic is tailored for English. For example, the Chinese "large" text might fit all widths, while the German short version might be too long for that width. What happens to the label if it doesn't fit? I guess this is a good subject for a follow up bug. As for the comment, I would suggest something more detailed: Used as a label for the button, displayed at the bottom of the drop down suggestion, to open Form Autofill browser preferences. This version is used instead of autocompleteFooterOption* when the menu width is below 185px.

The label will wrap if it's too long, but doesn't have left & right margin, which could be fixed easily. I'll open a bug to address this scenario so that at least it doesn't look broken if it happens. Thanks for the comment suggestion. I've updated them. As for Bug 1399502, should there be a linux version as well? And should I change Windows strings to `Settings`?

(In reply to Scott Wu [:scottwu] from comment #9) > As for Bug 1399502, should there be a linux version as well? And should I > change Windows strings to `Settings`? Let's stick to Options and Preferences for now, I've asked there, but it seems like a long term goal.

I've filed Bug 1409250 to address the case when the string for dropdown footer is too long, and I've updated the patch with your suggestions: https://reviewboard.mozilla.org/r/188330/diff/2-3/

Comment on attachment 8917760 [details] Bug 1407530 - Update formautofill.properties string IDs and localization notes. https://reviewboard.mozilla.org/r/188330/#review195300 Thanks, it looks good. Let's try landing it, so we can land the build change.

Pushed by francesco.lodolo@mozillaitalia.org: https://hg.mozilla.org/integration/autoland/rev/e3bf74eeaecb Update formautofill.properties string IDs and localization notes. r=flod,lchang

> +insecureFieldWarningDescription = %S has detected an insecure site. Form Autofill is temporarily disabled Shouldn’t this end with a trailing period?

(In reply to Ton from comment #15) > > +insecureFieldWarningDescription = %S has detected an insecure site. Form Autofill is temporarily disabled > > Shouldn’t this end with a trailing period? You are right, there should be a period at the end of the sentence. Filed Bug 1412217 for it. Thanks!