701016 - Remove hard-coded "Free" string, and add plural form support in Account Provisioner

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

From RQ:

"# LOCALIZATION NOTE (more):
#   %S will be the number of email addresses minus two.  (So, we'll show
#   two, and say that there are "(n-2) more…"
more=+%S more…

Would it be possible to make this line support plural forms?

Also, the word "Free" in the table header for e.g. AOL, is not localizable.
Please make it such."

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1

Here's my first pass at a fix for this.

Comment 2

[Rimas Kudelis [use needinfo to ping]]

Comment on attachment 573197 [details] [diff] [review]
Patch v1

@@ -1,10 +1,13 @@
+free=Free

This probably lacks an L10n comment. Something like:
# LOCALIZATION NOTE (free):
# This will be shown instead of price in table heading for free email providers
free=Free

Make it sound better, if you can. :)


+#   #1 will be the number of email addresses minus two.  (So, we'll show
+#   two, and say that there are "(n-2) more…" - uses PluralForm, so the
+#   string before the semi-colon is singular, and the string after is
+#   for plural.
+more=+#1 more…;+#1 more…

The comment is a bit incorrect. Why not say something like:

# LOCALIZATION NOTE (more): Semi-colon list of plural forms.
# See: http://developer.mozilla.org/en/Localization_and_Plurals
# #1 is the number of additional email addresses available for registration.
# This line is shown when there are more than two suggested email
# addresses available
more=+#1 more…;+#1 more…

Comment 3

[Rimas Kudelis [use needinfo to ping]]

(In reply to Rimas Kudelis from comment #2)
> The comment is a bit incorrect. Why not say something like:

Just in case, the incorrect part is in the fact that there can be more than two plural forms. The example comment I pasted you on IRC and used here is what is often used elsewhere in the source.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v2

Thanks for the review, RQ!  I've made the changes you suggested.

Comment 5

[Rimas Kudelis [use needinfo to ping]]

Comment on attachment 573203 [details] [diff] [review]
Patch v2

Thanks! It looks fine now. But Mark should review the JS part, cause I don't know whether or not it's correct.

Comment 6

[Rimas Kudelis [use needinfo to ping]]

-# LOCALIZATION NOTE (more):
-#   %S will be the number of email addresses minus two.  (So, we'll show
-#   two, and say that there are "(n-2) more…"
-more=+%S more…
+# LOCALIZATION NOTE (more): Semi-colon list of plural forms.
+# See: http://developer.mozilla.org/en/Localization_and_Plurals
+# #1 is the number of additional email addresses available for registration.
+# This line is shown when there are more than two suggested email
+# addresses available.
+more=+#1 more…;+#1 more…

Crap, I just realized that I forgot you should change the string ID when changing the string or it may not be noticed by localizers otherwise.

I suggest to rename 'more' to 'moreOptions' or something. Sorry for not mentioning it sooner... :(

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Attachment: Follow-up patch v1

RQ, will this work alright for you folks?

Comment 8

[Rimas Kudelis [use needinfo to ping]]

Comment on attachment 573247 [details] [diff] [review]
Follow-up patch v1

Feedback+, but please change the LOCALIZATION NOTE for moreOptions to reflect the string ID change.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Rimas:

Ah, good catch.  Fixed locally.

-Mike

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Patch v2 checked into comm-central here:  http://hg.mozilla.org/comm-central/rev/92b24860c777

And into comm-aurora here:  http://hg.mozilla.org/releases/comm-aurora/rev/2c75e2e2702d

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 573247 [details] [diff] [review]
Follow-up patch v1

Ok, this looks fine.

Comment 12

[Mike Conley (:mconley) (:⚙️)]

Follow-up patch (including Rimas's suggestion) landed on comm-central here:

http://hg.mozilla.org/comm-central/rev/6db37beee5dc

and comm-aurora here:

http://hg.mozilla.org/releases/comm-aurora/rev/3a9ca8ed436e