Delete Hosted Apps i18n plural message $1 $2 $3 should be wrapped in <ph> elements |
||||
Issue descriptionSplit out from https://bugs.chromium.org/p/chromium/issues/detail?id=915656 components/browsing_data_strings.grdp contains an i18n message <message name="IDS_DEL_HOSTED_APPS_COUNTER" desc="A counter showing how many hosted apps the user has. ..."> {COUNT, plural, =0 {None} =1 {1 app ($1)} =2 {2 apps ($1, $2)} other {# apps ($1, $2, $3)}} </message> Placeholder elements like $1 $2, should be wrapped in <ph> elements according to the docs [1] [2] [1] https://sites.google.com/a/chromium.org/dev/developers/design-documents/ui-localization#TOC-How-to-add-a-string [2] https://sites.google.com/a/chromium.org/dev/developers/tools-we-use-in-chromium/grit/grit-users-guide#Adding_Resources but the docs do not mention how/what to do with plurals. I noticed that components/payments_strings.grdp <message name="IDS_PAYMENT_REQUEST_ORDER_SUMMARY_MORE_ITEMS" desc="The label in the Order Summary section of the Payment Sheet that indicates how many display items are hidden. [ICU Syntax]"> {MORE_ITEMS, plural, =1 {<ph name="ITEM_COUNT">#<ex>1</ex></ph> more item} other {<ph name="ITEM_COUNT">#<ex>2</ex></ph> more items}} </message> wrapped them. I also noticed how the translated IDS_DEL_HOSTED_APPS_COUNTER plural message appeared (badly) in language=bn ./components/strings/components_strings_bn.xtb <translation id="262424810616849754">{COUNT,plural, =0{কিছুই নয়}=1{১টি অ্যাপ ($১)}=2{২টি অ্যাপ ($১, $২)}one{#টি অ্যাপ ($১, $২, $৩)}other{#টি অ্যাপ ($১, $২, $৩)}}</translation> Suggests to me that placeholders in plurals should be wrapped in <ph>. Thoughts? https://codereview.chromium.org/2228913003 touched the code last, so reaching out to those folks for help ...
,
Dec 17
I probably have to go over all ClearBrowsingData strings. I guess there are a few other cases as well. Does this actually lead to incorrect translations? Did the bn translation translate $1 into the localized version of a 1?
,
Dec 17
,
Dec 17
#2 Thanks for taking this one. I went over the the translated message strings (the .xtb files of chrome) and extracted bogus translations involving the $ placeholder character. The results are listed here: https://bugs.chromium.org/p/chromium/issues/detail?id=908888#c6 Seems that some bn translators have turned $1 $2 $3 into $১, $২, $৩ so yes, bad translations do happen. If such symbols are decoded using l10 string C++ tools, they just get ignored silently in production, resulting in missing bits in the chrome UI. If they are decoded using chrome's WebUI JS tools, the result is an exception which can stop apps working ( issue 908767 ). I have worked backwards from the results, to go over all the .grd and .grdp files in chrome to find the sources of the bad placeholders, and filed bugs for them all under the label https://bugs.chromium.org/p/chromium/issues/list?q=label:bad-i18n-placeholders and assigned them. Once they are fixed, we probably need a presubmit or some such to prevent this type of error, filed issue 915681 for that discussion.
,
Dec 17
Thanks for going through these issues. I worried that this will trigger a lot of retranslation but if there are actual errors in the translations, there is probably no other way.
,
Dec 17
No problem, and I think there's no other way than to fix. You case involves 'plurals', and the chrome i18n docs I quoted in the OP don't provide examples of how to handle them when place holders are present. Recommend that in review for this bug, you add dpapad@ too, to us help sort out how plurals with placeholder $1, $2 ... should be done correctly.
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f0c5cf83303bad6b3bd148ea83bc8327308c12d8 commit f0c5cf83303bad6b3bd148ea83bc8327308c12d8 Author: Christian Dullweber <dullweber@chromium.org> Date: Wed Dec 19 10:57:43 2018 Fix HostedApps counter placeholders Placeholders in translated strings should be surrounded by <ph> tags. The HostedAppsCounter didn't have this and the $1 got tranlated into $১. Bug: 915663 Change-Id: I6dbdbf2cd86a7d8154caf05954911efebfac8611 Reviewed-on: https://chromium-review.googlesource.com/c/1382444 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Christian Dullweber <dullweber@chromium.org> Cr-Commit-Position: refs/heads/master@{#617796} [modify] https://crrev.com/f0c5cf83303bad6b3bd148ea83bc8327308c12d8/components/browsing_data_strings.grdp
,
Dec 19
The other browsing_data strings look good.
,
Dec 19
Good to know=, and thank you for fixing this issue so quickly Christian. |
||||
►
Sign in to add a comment |
||||
Comment 1 by ioanap@chromium.org
, Dec 17