Closed Bug 701016 Opened 8 years ago Closed 8 years ago

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

Categories

(Thunderbird :: Mail Window Front End, defect)

10 Branch
x86
All
defect
Not set

Tracking

(thunderbird10+ fixed)

RESOLVED FIXED
Thunderbird 11.0
Tracking Status
thunderbird10 + fixed

People

(Reporter: mconley, Assigned: mconley)

References

Details

Attachments

(2 files, 1 obsolete file)

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."
Assignee: nobody → mconley
OS: Linux → All
Target Milestone: --- → Thunderbird 11.0
Attached patch Patch v1 (obsolete) — Splinter Review
Here's my first pass at a fix for this.
Attachment #573197 - Flags: review?(rq)
Attachment #573197 - Flags: review?(mbanner)
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…
Attachment #573197 - Flags: review?(rq) → review-
(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.
Attached patch Patch v2Splinter Review
Thanks for the review, RQ!  I've made the changes you suggested.
Attachment #573197 - Attachment is obsolete: true
Attachment #573197 - Flags: review?(mbanner)
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.
Attachment #573203 - Flags: review?(rq)
Attachment #573203 - Flags: review?(mbanner)
Attachment #573203 - Flags: feedback+
Attachment #573203 - Flags: approval-mozilla-aurora?
Attachment #573203 - Flags: review?(mbanner) → review?(sagarwal)
Attachment #573203 - Flags: review?(sagarwal) → review+
Attachment #573203 - Flags: approval-mozilla-aurora? → approval-comm-aurora+
-# 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... :(
RQ, will this work alright for you folks?
Attachment #573247 - Flags: review?(mbanner)
Attachment #573247 - Flags: feedback?(rq)
Attachment #573247 - Flags: approval-comm-aurora?
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.
Attachment #573247 - Flags: feedback?(rq) → feedback+
Rimas:

Ah, good catch.  Fixed locally.

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

Ok, this looks fine.
Attachment #573247 - Flags: review?(mbanner)
Attachment #573247 - Flags: review+
Attachment #573247 - Flags: approval-comm-aurora?
Attachment #573247 - Flags: approval-comm-aurora+
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
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.