Last Comment Bug 701016 - Remove hard-coded "Free" string, and add plural form support in Account Provisioner
: Remove hard-coded "Free" string, and add plural form support in Account Provi...
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Mail Window Front End (show other bugs)
: 10 Branch
: x86 All
: -- normal (vote)
: Thunderbird 11.0
Assigned To: Mike Conley (:mconley) - (Needinfo me!)
:
Mentors:
Depends on:
Blocks: 686347
  Show dependency treegraph
 
Reported: 2011-11-09 07:29 PST by Mike Conley (:mconley) - (Needinfo me!)
Modified: 2011-11-13 14:42 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed


Attachments
Patch v1 (2.69 KB, patch)
2011-11-09 07:55 PST, Mike Conley (:mconley) - (Needinfo me!)
rimas: review-
Details | Diff | Splinter Review
Patch v2 (2.85 KB, patch)
2011-11-09 08:09 PST, Mike Conley (:mconley) - (Needinfo me!)
sid.bugzilla: review+
rimas: feedback+
mozilla: approval‑comm‑aurora+
Details | Diff | Splinter Review
Follow-up patch v1 (1.57 KB, patch)
2011-11-09 10:28 PST, Mike Conley (:mconley) - (Needinfo me!)
standard8: review+
rimas: feedback+
standard8: approval‑comm‑aurora+
Details | Diff | Splinter Review

Description Mike Conley (:mconley) - (Needinfo me!) 2011-11-09 07:29:08 PST
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) - (Needinfo me!) 2011-11-09 07:55:16 PST
Created attachment 573197 [details] [diff] [review]
Patch v1

Here's my first pass at a fix for this.
Comment 2 Rimas Kudelis 2011-11-09 08:02:47 PST
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 2011-11-09 08:06:38 PST
(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) - (Needinfo me!) 2011-11-09 08:09:37 PST
Created attachment 573203 [details] [diff] [review]
Patch v2

Thanks for the review, RQ!  I've made the changes you suggested.
Comment 5 Rimas Kudelis 2011-11-09 08:13:35 PST
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 2011-11-09 10:21:29 PST
-# 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) - (Needinfo me!) 2011-11-09 10:28:37 PST
Created attachment 573247 [details] [diff] [review]
Follow-up patch v1

RQ, will this work alright for you folks?
Comment 8 Rimas Kudelis 2011-11-09 10:30:39 PST
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) - (Needinfo me!) 2011-11-09 10:31:51 PST
Rimas:

Ah, good catch.  Fixed locally.

-Mike
Comment 10 Mike Conley (:mconley) - (Needinfo me!) 2011-11-09 10:37:39 PST
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) (afk until 26th July) 2011-11-09 10:45:20 PST
Comment on attachment 573247 [details] [diff] [review]
Follow-up patch v1

Ok, this looks fine.
Comment 12 Mike Conley (:mconley) - (Needinfo me!) 2011-11-09 10:51:30 PST
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

Note You need to log in before you can comment on or make changes to this bug.