Improve plural support for the matches counter #10078
Conversation
|
/botio-windows preview |
l10n/en-US/viewer.properties
Outdated
| find_matches_count[other]={{current}} of {{total}} matches | ||
| # LOCALIZATION NOTE (find_matches_count_limit): The supported plural forms are | ||
| find_match_count={[ plural(n) ]} | ||
| find_match_count[zero]={{current}} of {{total}} matches |
There was a problem hiding this comment.
This particular case should never happen, since when there's no matches the counter is hidden.
There was a problem hiding this comment.
Doesn't it happen when https://github.com/mozilla/pdf.js/blob/master/web/pdf_find_controller.js#L514 is triggered then?
There was a problem hiding this comment.
Nope; please note that total must be non-zero for the message to be displayed in the first place, see https://github.com/mozilla/pdf.js/blob/master/web/pdf_find_bar.js#L162 and https://searchfox.org/mozilla-central/source/toolkit/content/widgets/findbar.js#1131.
I'll admit that this was perhaps not as clear as it could have been, given that the line in pdf_find_bar.js probably should have been e.g. if (total > 0) { instead.
There was a problem hiding this comment.
Done in the new commit.
l10n/en-US/viewer.properties
Outdated
| find_matches_count[one]={{current}} of {{total}} match | ||
| find_matches_count[other]={{current}} of {{total}} matches | ||
| # LOCALIZATION NOTE (find_matches_count_limit): The supported plural forms are | ||
| find_match_count={[ plural(n) ]} |
There was a problem hiding this comment.
Having looked at the Firefox findbar implementation again, see here, I'm no longer convinced that there's any attempt to format these numbers according to locale.
So it might be possible to simplify this by removing the toLocaleString() calls from the JS code, and change this to just {[ plural(total) ]} respectively {[ plural(limit) ]} for for the other string below.
There was a problem hiding this comment.
I'll also check in the browser and if it doesn't do the formatting either, I'll indeed remove it and use total/limit in the plural.
There was a problem hiding this comment.
Done in the new commit. The browser find bar indeed also doesn't do any formatting according to locale, so I've removed that here too.
timvandermeij
force-pushed
the
l10n-fix
branch
from
e52f615
to
f711dbc
Compare
|
/botio-windows preview |
From: Bot.io (Windows)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/1781dcbe329b133/output.txt |
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/1781dcbe329b133/output.txt Total script time: 4.30 mins Published |
|
@rvandermeulen Could you do a new upstream merge that contains these changes? This is necessary for a correct localization of these strings. |
Fixes #10076.