[Form Autofill] Move the strings in manageProfiles and editProfile dialog to .properties file

RESOLVED FIXED in Firefox 56

Status

()

Toolkit
Form Manager
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: scottwu, Assigned: scottwu)

Tracking

(Blocks: 1 bug)

unspecified
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [form autofill:M3])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

a year ago
The strings on manageProfiles.xhtml and editProfile.xhtml should be moved to a .properties file in locale.
(Assignee)

Updated

a year ago
Blocks: 1370763
No longer blocks: 1019471
Comment hidden (mozreview-request)
Before landing this we should discuss with l10n about system add-on localization
(Assignee)

Comment 3

a year ago
Hello Francesco, we are working on the form autofill feature and will be shipping it as a system addon[1] on 56. Is there anything we need to do so that the l10n team can translate the strings for release? For 56 we are only going to ship to en-US because we only allow the US in the country field for now.

[1] http://searchfox.org/mozilla-central/source/browser/extensions/formautofill/
Flags: needinfo?(francesco.lodolo)
You should get a good idea of what is needed from bug 1369291, where l10n was enabled for the onboarding system add-on. The first thing that I spot is that the folder for localization should be called "locales" (not "locale").

Compared to that bug, there's a new file to update too (we're in the middle of switching to TOML files for configurations, but right now we have both old and new system in place)
http://searchfox.org/mozilla-central/rev/cef8389c687203085dc6b52de2fbd0260d7495bf/browser/locales/l10n.toml#121-123

:pike would be the best reviewer for such a patch.

As for timing: the feature is shipping in 56 for en-US, 57 for all other locales? Do we already know how many strings there are to localize? Comment 0 talks about two files, but I'm pretty sure there's at least stuff in preferences, currently hard-coded.

If the target is indeed 57, and the number of strings is not too high, I would target the very beginning of the 57 cycle to enable localization in m-c. We could do it before, but if you need to update any message in Beta 56, approval would become a lot harder with strings already exposed for localization.
Flags: needinfo?(francesco.lodolo)
One more question: are you expecting this add-on to ride the trains, or to have updates out of cycle involving strings?
(In reply to Francesco Lodolo [:flod] (PTO until Jul 17) from comment #4)
> As for timing: the feature is shipping in 56 for en-US, 57 for all other
> locales?

Hi Francesco,

We currently have no plan to ship for all other locales in Fx57. However, we do have discussed the possibility to support Spanish as soon as possible, so it should be the only locale at most we might support by Fx57.

(In reply to Francesco Lodolo [:flod] (PTO until Jul 17) from comment #5)
> One more question: are you expecting this add-on to ride the trains, or to
> have updates out of cycle involving strings?

Form Autofill will ship through Go Faster. We're not expecting it to ride the trains.
(In reply to Luke Chang [:lchang] from comment #6)
> We currently have no plan to ship for all other locales in Fx57. However, we
> do have discussed the possibility to support Spanish as soon as possible, so
> it should be the only locale at most we might support by Fx57.

Can you explain how Spanish was chosen? Also Spanish as es-ES, or Spanish as es-* (we support four of them)? What's the plan for other locales?

> Form Autofill will ship through Go Faster. We're not expecting it to ride
> the trains.

Shipping out of trains, together with the fact that we're not going to support all languages, changes the scenario drastically. 

Right now, the only way we can support this is by putting strings in an external repository, out of mozilla-central. An example is Pocket:
* Code: https://github.com/mozilla-partners/pocket
* Strings: https://github.com/mozilla-l10n/pocket-l10n

Code and strings are periodically copied into m-c (or a different branch), localization happens via Pontoon or GitHub directly, and it's not bound to trains.

Note that the add-on won't be part of the build system, so you need to ensure on your side that the localization is usable (no errors) and complete. That's relatively easy for one locale, but it needs to scale if you add more.
(In reply to Francesco Lodolo [:flod] from comment #7)
> (In reply to Luke Chang [:lchang] from comment #6)
> > We currently have no plan to ship for all other locales in Fx57. However, we
> > do have discussed the possibility to support Spanish as soon as possible, so
> > it should be the only locale at most we might support by Fx57.
> 
> Can you explain how Spanish was chosen? Also Spanish as es-ES, or Spanish as
> es-* (we support four of them)? What's the plan for other locales?

Joe C. can clarify this but my understanding is that it was for Spanish speaking people in the United States since we will only support US addresses for the MVP and he saw data showing we have a large enough population of ES speakers in the U.S. We plan to use geoip to selectively enable the feature (like the search service), not just the build locale.

> > Form Autofill will ship through Go Faster. We're not expecting it to ride
> > the trains.
> 
> Shipping out of trains, together with the fact that we're not going to
> support all languages, changes the scenario drastically. 
> 
> Right now, the only way we can support this is by putting strings in an
> external repository, out of mozilla-central. An example is Pocket:
> * Code: https://github.com/mozilla-partners/pocket
> * Strings: https://github.com/mozilla-l10n/pocket-l10n
> 
> Code and strings are periodically copied into m-c (or a different branch),
> localization happens via Pontoon or GitHub directly, and it's not bound to
> trains.
> 
> Note that the add-on won't be part of the build system, so you need to
> ensure on your side that the localization is usable (no errors) and
> complete. That's relatively easy for one locale, but it needs to scale if
> you add more.

Right, my plan was to follow the Pocket model which was why I didn't want this patch to land in m-c and why we didn't initially localize this file in m-c.

How do we make that happen? Can we get a formautofill-l10n repo setup on mozilla-l10n on GitHub? How do we indicate in m-c that strings we copy into m-c don't need to be localized (as they already are)?

Thanks
Flags: needinfo?(francesco.lodolo)
(In reply to Francesco Lodolo [:flod] from comment #7)
> (In reply to Luke Chang [:lchang] from comment #6)
> > We currently have no plan to ship for all other locales in Fx57. However, we
> > do have discussed the possibility to support Spanish as soon as possible, so
> > it should be the only locale at most we might support by Fx57.
> Can you explain how Spanish was chosen? Also Spanish as es-ES, or Spanish as
> es-* (we support four of them)? What's the plan for other locales?

Ping Cindy for the clarification.
Flags: needinfo?(chsiang)

Comment 10

a year ago
Hi Luke,
Thanks for the message. For 56, the plan is to 

-Filter based on geoip in US
-Filter again based on browser version
  EN-US 
  ES-MX (Support Spanish speaking population in US)
Flags: needinfo?(chsiang)
(In reply to Matthew N. [:MattN] (huge backlog; PM if requests are blocking you) from comment #8)
> How do we make that happen? Can we get a formautofill-l10n repo setup on
> mozilla-l10n on GitHub? 

Yes, I can set up that and give admin access to developers who need it.
Then I'll need to set up Pontoon for es-MX when the en-US files are ready.

One thing that we might need is to figure out how to test this localization.

> How do we indicate in m-c that strings we copy into
> m-c don't need to be localized (as they already are)?

Strings are exposed to localizers when you add the path to filter.py (or l10n.toml). As long as you put the .properties files in paths unknown to filter.py, localizers won't be aware of them.

For example, if you land the current patch with strings in browser/extensions/formautofill/locale/en-US, they won't be exposed for localization.
Flags: needinfo?(francesco.lodolo)
(In reply to Francesco Lodolo [:flod] from comment #11)
> (In reply to Matthew N. [:MattN] (huge backlog; PM if requests are blocking
> you) from comment #8)
> > How do we make that happen? Can we get a formautofill-l10n repo setup on
> > mozilla-l10n on GitHub? 
> 
> Yes, I can set up that and give admin access to developers who need it.

Thanks. I landed an initial import https://github.com/mozilla-l10n/formautofill-l10n/commit/ad5dd7f9762d5e00fd3882208fc954f4abcb4c3f

> Then I'll need to set up Pontoon for es-MX when the en-US files are ready.

We can let you know when it's ready. We still have some string changes and new strings like this bug and credit card stuff to land.

> One thing that we might need is to figure out how to test this localization.

Perhaps we can use taskcluster-github integration on the l10n repo to run tests against the latest m-c somehow?

> > How do we indicate in m-c that strings we copy into
> > m-c don't need to be localized (as they already are)?
> 
> Strings are exposed to localizers when you add the path to filter.py (or
> l10n.toml). As long as you put the .properties files in paths unknown to
> filter.py, localizers won't be aware of them.
> 
> For example, if you land the current patch with strings in
> browser/extensions/formautofill/locale/en-US, they won't be exposed for
> localization.

OK, and from my skimming of those files it would be fine to rename "locale" to "locales" (as you requested) and it still won't be picked up for l10n-central localization, right?

Thanks again
Flags: needinfo?(francesco.lodolo)
(In reply to Matthew N. [:MattN] (huge backlog; PM if requests are blocking you) from comment #12)
> (In reply to Francesco Lodolo [:flod] from comment #11)
> > (In reply to Matthew N. [:MattN] (huge backlog; PM if requests are blocking
> > you) from comment #8)
> > > How do we make that happen? Can we get a formautofill-l10n repo setup on
> > > mozilla-l10n on GitHub? 
> > 
> > Yes, I can set up that and give admin access to developers who need it.
> 
> Thanks. I landed an initial import
> https://github.com/mozilla-l10n/formautofill-l10n/commit/
> ad5dd7f9762d5e00fd3882208fc954f4abcb4c3f
> 
> > Then I'll need to set up Pontoon for es-MX when the en-US files are ready.
> 
> We can let you know when it's ready. We still have some string changes and
> new strings like this bug and credit card stuff to land.
> 
> > One thing that we might need is to figure out how to test this localization.
> 
> Perhaps we can use taskcluster-github integration on the l10n repo to run
> tests against the latest m-c somehow?

Pike might have some ideas about that.

> OK, and from my skimming of those files it would be fine to rename "locale"
> to "locales" (as you requested) and it still won't be picked up for
> l10n-central localization, right?

Yes, even renaming it to locales won't make the folder visible to localizers.

I'll wait for your ping to enable Pontoon for es-MX. As said, we'll need to figure out a way for them to test the localization (or have screenshots of the localized feature).
Flags: needinfo?(francesco.lodolo)
Comment on attachment 8882443 [details]
Bug 1375799 - (Part 1) [Form Autofill] Use localized strings instead of hardcoded strings.

https://reviewboard.mozilla.org/r/153562/#review163932

::: browser/extensions/formautofill/FormAutofillUtils.jsm:14
(Diff revision 1)
>  const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
>  
>  const ADDRESS_REFERENCES = "chrome://formautofill/content/addressReferences.js";
>  
>  Cu.import("resource://gre/modules/XPCOMUtils.jsm");
> +XPCOMUtils.defineLazyModuleGetter(this, "Services", "resource://gre/modules/Services.jsm");

You can just use a regular import

::: browser/extensions/formautofill/content/editProfile.js:28
(Diff revision 1)
> +    FormAutofillUtils.localizeMarkup(REGIONS_BUNDLE_URI, this._elements.country);
> +    FormAutofillUtils.localizeMarkup(AUTOFILL_BUNDLE_URI, document);

Can we do this earlier before DCL in both cases?

::: browser/extensions/formautofill/locale/en-US/formautofill.properties:31
(Diff revision 1)
> +addressLevel2 = City/Town
> +addressLevel1 = State/Province
> +postalCode = Zip/Postal

City
State
Zip Code
I created a PR to add a config file to run the (released yesterday) current version of compare-locales against the repo, https://github.com/mozilla-l10n/formautofill-l10n/pull/1.

I think it'd be interesting to run that via tc, but you'd be the first to try that with compare-locales, so I would expect that we need some fixes.

This already support l10n-merge, too, so it's probably useful for you to integrate that into your build system.
(Assignee)

Comment 16

a year ago
mozreview-review-reply
Comment on attachment 8882443 [details]
Bug 1375799 - (Part 1) [Form Autofill] Use localized strings instead of hardcoded strings.

https://reviewboard.mozilla.org/r/153562/#review163932

Thanks Matt. I address the issues and also implemented methods to get the proper labels for address-level1 (State/Province) and postal-code (ZIP Code/Postal Code). We can expand the method to get the `fmt` data to order fields and show/hide fields based on countries (Bug 1383687).

> Can we do this earlier before DCL in both cases?

OK. I'm moving it to the end of the file.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 21

a year ago
mozreview-review
Comment on attachment 8889714 [details]
Bug 1375799 - (Part 2) [Form Autofill] Rename locale folder to locales.

https://reviewboard.mozilla.org/r/160762/#review166648
Attachment #8889714 - Flags: review?(lchang) → review+
Comment on attachment 8882443 [details]
Bug 1375799 - (Part 1) [Form Autofill] Use localized strings instead of hardcoded strings.

https://reviewboard.mozilla.org/r/153562/#review167228

From a quick skim I don't see major issues though I wouldn't mind if Luke still looks at it. I also didn't review how the country data is loaded.
Attachment #8882443 - Flags: review?(MattN+bmo) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 26

a year ago
mozreview-review
Comment on attachment 8882443 [details]
Bug 1375799 - (Part 1) [Form Autofill] Use localized strings instead of hardcoded strings.

https://reviewboard.mozilla.org/r/153562/#review167722
Attachment #8882443 - Flags: review?(lchang) → review+

Comment 27

a year ago
Pushed by lchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9bdc922716e8
(Part 1) [Form Autofill] Use localized strings instead of hardcoded strings. r=lchang,MattN
https://hg.mozilla.org/integration/autoland/rev/e52caa844728
(Part 2) [Form Autofill] Rename locale folder to locales. r=lchang

Comment 28

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9bdc922716e8
https://hg.mozilla.org/mozilla-central/rev/e52caa844728
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.