Improve plural form for AndXMoreFiles string

RESOLVED FIXED in mozilla26

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: tchevalier, Assigned: tchevalier)

Tracking

Trunk
mozilla26
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Posted patch more-files.diff (obsolete) — Splinter Review
I think many locales can't use plural form here.

I first localized the string like:
AndXMoreFiles=et %S supplémentaire;et %S supplémentaires

And got an error on the l10n dashboard because of the second %S.
Unless I'm missing something about arguments in plural forms, I think we should use #1 instead.

I fixed it for French this way, but it won't work for locales using other plural rules
AndXMoreFiles=et 1 supplémentaire;et %S supplémentaires

So here is a patch (untested) that should fix that.
Francesco, I think you know plural forms more than I do, did I miss something?
Attachment #791160 - Flags: feedback?(francesco.lodolo)
Comment on attachment 791160 [details] [diff] [review]
more-files.diff

Thanks Théo, I saw this string yesterday and meant to look into it.

compare-locales determines if a string is using plural forms by searching the link in the associated l10n comment (specifically it searches for "Localization_and_Plurals"), so the problem is not #1 vs %S.

Example of a string using %S
http://hg.mozilla.org/mozilla-central/file/1ed5a88cd4d0/browser/locales/en-US/chrome/browser/downloads/downloads.properties#l79

I wouldn't go with AndXMoreFiles2, but with a meaningful name and a more explicit localization comment (e.g. AndNumMoreFiles, AndNMoreFiles, etc.). Note: if you change the string, you need to update the reference in the comment too.

If the string is using plural forms but has no singular case, it should be ";String for plural", with an empty string for the singular case in English.

See for example
http://hg.mozilla.org/mozilla-central/file/1ed5a88cd4d0/browser/locales/en-US/chrome/browser/browser.properties#l346

But in this case there should be a singular case, since it's not displaying the total count of files but total-20. You would notice it if the string was "and %s more files" (failing for 21).

# LOCALIZATION NOTE (AndNMoreFiles): Semi-colon list of plural forms. 
# See: http://developer.mozilla.org/en/docs/Localization_and_Plurals 
# This string is shown at the end of the tooltip text for <input type='file'
# multiple> when there are more than 21 files selected (when we will only list
# the first 20, plus an "and X more" line). %S will be the number of files
# minus 20.
AndNMoreFiles=and %S more;and %S more


Also this old string would definitely need plural form support
XFilesSelected=%S files selected.

Problem is that is used in a .cpp file, so no idea how to deal with that.
Attachment #791160 - Flags: feedback?(francesco.lodolo) → feedback-
(In reply to Francesco Lodolo [:flod] from comment #1)
> Also this old string would definitely need plural form support
> XFilesSelected=%S files selected.
> 
> Problem is that is used in a .cpp file, so no idea how to deal with that.

Filed bug 905943.
Component: General → DOM: Core & HTML
Product: Firefox → Core
Version: 26 Branch → Trunk
(In reply to Francesco Lodolo [:flod] from comment #1)
> But in this case there should be a singular case, since it's not displaying
> the total count of files but total-20. You would notice it if the string was
> "and %s more files" (failing for 21).

When I read the code, it seems like there isn't a singular case:
20 files chosen - 20 files shown - AndXMoreFiles string isn't shown
21 files chosen - 21 files shown - AndXMoreFiles string isn't shown
22 files chosen - 20 files shown - AndXMoreFiles string with 2 substituted
23 files chosen - 20 files shown - AndXMoreFiles string with 3 substituted

Basically, the tooltip is capped based on the number of lines to display (21), rather than the number of files chosen.
Assignee: nobody → theo.chevalier11
Status: NEW → ASSIGNED
I think you're right, I based my comment on the l10n comment instead of the code. Can't verify yet since my nightly still doesn't include the change.

files.length = 21 -> complete list
files.length = 22 -> list plus tooltip with 2

At this point I think the string should mimic what happens for Show all tabs (you can also use #1 instead of %S, since it's the most common pattern in plural forms).

# LOCALIZATION NOTE (AndNMoreFiles): Semi-colon list of plural forms. 
# See: http://developer.mozilla.org/en/docs/Localization_and_Plurals 
# This string is shown at the end of the tooltip text for <input type='file'
# multiple> when there are more than 21 files selected (when we will only list
# the first 20, plus an "and X more" line). #1 represents the number of files
# minus 20 and will always be a number equal to or greater than 2.
AndNMoreFiles=;and #1 more
I can confirm from a text that the singular form is never used. 

For clarity (i.e. to be sure that localizers don't overlook the starting ";") I would leave the singular form in the string anyway, with the explanation in the preceding l10n comment about it never being used.

AndNMoreFiles=and one more;and #1 more

Théo, are you still on this?
Flags: needinfo?(theo.chevalier11)
Posted patch patch v2Splinter Review
Sorry for the delay :\
Thanks Francesco for your useful comments! I was almost sure I missed a lot of things.

If that's okay for you, I will ask enndeakin for review, then land it if that's a r+
Attachment #792304 - Flags: feedback?(francesco.lodolo)
Flags: needinfo?(theo.chevalier11)
Attachment #791160 - Attachment is obsolete: true
Comment on attachment 792304 [details] [diff] [review]
patch v2

Review of attachment 792304 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks Théo, patch looks good to me.
Attachment #792304 - Flags: feedback?(francesco.lodolo) → feedback+
(In reply to Francesco Lodolo [:flod] from comment #7)
> Comment on attachment 792304 [details] [diff] [review]
> patch v2
> 
> Review of attachment 792304 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks Théo, patch looks good to me.

Great, thanks!
Attachment #792304 - Flags: review?(enndeakin)
Attachment #792304 - Flags: review?(enndeakin) → review+
https://hg.mozilla.org/mozilla-central/rev/2cec08a1b933
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.