Closed
Bug 905933
Opened 9 years ago
Closed 9 years ago
Improve plural form for AndXMoreFiles string
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: tchevalier, Assigned: tchevalier)
References
Details
Attachments
(1 file, 1 obsolete file)
3.07 KB,
patch
|
enndeakin
:
review+
flod
:
feedback+
|
Details | Diff | 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 1•9 years ago
|
||
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-
Comment 2•9 years ago
|
||
(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
Comment 3•9 years ago
|
||
(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
Comment 4•9 years ago
|
||
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
Comment 5•9 years ago
|
||
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)
Assignee | ||
Comment 6•9 years ago
|
||
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)
Updated•9 years ago
|
Attachment #791160 -
Attachment is obsolete: true
Comment 7•9 years ago
|
||
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+
Assignee | ||
Comment 8•9 years ago
|
||
(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!
Assignee | ||
Updated•9 years ago
|
Attachment #792304 -
Flags: review?(enndeakin)
Updated•9 years ago
|
Attachment #792304 -
Flags: review?(enndeakin) → review+
Assignee | ||
Comment 9•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/2cec08a1b933
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2cec08a1b933
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in
before you can comment on or make changes to this bug.
Description
•