1417843 - List countries Form Autofill supports in a pref

Status: RESOLVED FIXED

(Toolkit :: Form Autofill, enhancement)

Description

[Scott Wu [:scottwu]]

In order to support more than just the US, we need a place to specify which countries are supported. The pref would be a comma separated string like (CA,DE,US). This data would be used in bootstrap phase to determine if Form Autofill is available to the user, and ensure the countries could be saved into the profile storage, as well as having the countries show up in preferences.

Comment 1

[Scott Wu [:scottwu]]

Attachment: Bug 1417843 - Add supportedCountries pref to configure which countries are supported.

Review commit: https://reviewboard.mozilla.org/r/201200/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/201200/

Comment 2

[Steve Chung [:steveck]]

Comment on attachment 8930009 [details]
Bug 1417843 - Add supportedCountries pref to configure which countries are supported.

https://reviewboard.mozilla.org/r/201200/#review206614

r+ if Mark agreed with the string changes. Just a reminder that Bug 1022925 will need this pref as well and please update if 1022925 land first.

::: browser/extensions/formautofill/bootstrap.js:64
(Diff revision 1)
>    } else if (availablePref == "detect") {
>      let locale = Services.locale.getRequestedLocale();
>      let region = Services.prefs.getCharPref("browser.search.region", "");
> -    return locale == "en-US" && region == "US";
> +    let supportedCountries = Services.prefs.getCharPref("extensions.formautofill.supportedCountries")
> +                                           .split(",");
> +    return locale == "en-US" && supportedCountries.includes(region);

This reminds me that we should consider the locale as well(maybe not in this patch). Do you think we should even remove the locale checking once we settle down l10n(but users might be confused if they need to travel across countries)?

::: browser/extensions/formautofill/locales/en-US/formautofill.properties:128
(Diff revision 1)
>  country = Country or Region
>  tel = Phone
>  email = Email
>  cancelBtnLabel = Cancel
>  saveBtnLabel = Save
> -countryWarningMessage = Form Autofill is currently available only for US addresses
> +countryWarningMessage2 = Form Autofill is currently available only for certain countries.

Did Mark notice the string changes and update on the spec?

Comment 3

[Steve Chung [:steveck]]

Comment on attachment 8930009 [details]
Bug 1417843 - Add supportedCountries pref to configure which countries are supported.

https://reviewboard.mozilla.org/r/201200/#review206614

> Did Mark notice the string changes and update on the spec?

Seems like he already update the spec: https://mozilla.invisionapp.com/share/RWEH3FMM5#/screens

Comment 4

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8930009 [details]
Bug 1417843 - Add supportedCountries pref to configure which countries are supported.

https://reviewboard.mozilla.org/r/201200/#review206614

> This reminds me that we should consider the locale as well(maybe not in this patch). Do you think we should even remove the locale checking once we settle down l10n(but users might be confused if they need to travel across countries)?

Yeah, we'll probably need another pref for supported locales but it's not the scope of this bug indeed. Maybe we can file a bug for tracking.

Comment 5

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8930009 [details]
Bug 1417843 - Add supportedCountries pref to configure which countries are supported.

https://reviewboard.mozilla.org/r/201200/#review206766

Looks good. Thanks.

Comment 6

[Scott Wu [:scottwu]]

Comment on attachment 8930009 [details]
Bug 1417843 - Add supportedCountries pref to configure which countries are supported.

https://reviewboard.mozilla.org/r/201200/#review206614

> Yeah, we'll probably need another pref for supported locales but it's not the scope of this bug indeed. Maybe we can file a bug for tracking.

Since localizers are working on string translation already, can we just remove the locale restriction altogether? I don't think we should prevent any user who live in supported countries from having this feature. I've opened bug 1419312 for this and we can discuss there.

Comment 7

[Scott Wu [:scottwu]]

Comment on attachment 8930009 [details]
Bug 1417843 - Add supportedCountries pref to configure which countries are supported.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/201200/diff/1-2/

Comment 8

[Scott Wu [:scottwu]]

Rebased to central and checking in. Thanks Steve and Luke!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/00b541dc416d
Add supportedCountries pref to configure which countries are supported. r=lchang,steveck

Comment 10

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/00b541dc416d