New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 12
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug



Sign in to add a comment
link

Issue 920577: Wrap i18n message GRIT placeholders in a <ph> element

Reported by noel@google.com, Jan 10 Project Member

Issue description

GRIT i18n message placeholder have the form %d or %s, and 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

When this is not done, applications can fail to work when reading the i18n translated strings, see  issue 908767  for example.

There are only 3 chrome i18n <message>'s that need fixing.  They all use %s, and only as a literal, not a real placeholder.  These should be easy to fix.

Once that is done, issue 915681 can resolve, and lock down use to prevent developers submitting invalid i18n messages.
 

Comment 1 by bugdroid1@chromium.org, Jan 10

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/146bf5ff1885c11650da4b2eb711f1362e712dca

commit 146bf5ff1885c11650da4b2eb711f1362e712dca
Author: Noel Gordon <noel@chromium.org>
Date: Thu Jan 10 13:32:23 2019

[ChromeOS] Remove webcal: and mailto: protocol handler i18n strings

These i18n strings contain a URL, and the URLs do not change when they
are translated i18n [1]. So there is no reason to define them in a grd
<message> string. The URLs also contain the special character sequence
%s which is an invalid i18n placeholder per issue 915681.

To help resolve bug 915681, remove these IDS-defined URL from the .grd
file and in-line the URLs where used instead.

[1] Note that l10n_util::GetStringUTF8(IDS_XX) is allowed to re-format
an IDS string for RTL locales if there is evidence that the string has
RTL characters. These URL strings have no RTL characters, so they were
identical in both LTR and RTL locales when translated. It is therefore
safe to not use l10n_util::GetStringUTF8 to pre-process these URLs.

No change in behavior, no new tests.

Bug:  920577 
Change-Id: Ib11576e2f2185f96f571300d120901d7c6578548
Reviewed-on: https://chromium-review.googlesource.com/c/1402388
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621564}
[modify] https://crrev.com/146bf5ff1885c11650da4b2eb711f1362e712dca/chrome/app/generated_resources.grd
[modify] https://crrev.com/146bf5ff1885c11650da4b2eb711f1362e712dca/chrome/browser/custom_handlers/protocol_handler_registry.cc

Comment 2 by bugdroid1@chromium.org, Jan 12

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/468cd7506daf3a55436cd7bfeeb392c2873e020b

commit 468cd7506daf3a55436cd7bfeeb392c2873e020b
Author: Noel Gordon <noel@chromium.org>
Date: Sat Jan 12 03:10:08 2019

Wrap IDS_SETTINGS_SEARCH_ENGINES_QUERY_URL_EXPLANATION %s in a <ph> element

This IDS string contains a "%s" substring. Ensure the substring is not
localized: wrap the "%s" substring in an i18n <ph> placeholder element,
which ensures translators can't modify it.

No change in behavior, no new tests.

Bug:  920577 
Change-Id: I1afafd2d943c20cd7aa9c418e9309d3f54d2b7b8
Reviewed-on: https://chromium-review.googlesource.com/c/1404905
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622279}
[modify] https://crrev.com/468cd7506daf3a55436cd7bfeeb392c2873e020b/chrome/app/settings_strings.grdp

Comment 3 by noel@google.com, Jan 12

Status: Fixed (was: Started)

Comment 4 by noel@chromium.org, Jan 22

Cc: dbeam@chromium.org dpa...@chromium.org
Still waiting for the translations of the message changed in #2 to be rolled into chrome:

<message name="IDS_SETTINGS_SEARCH_ENGINES_QUERY_URL_EXPLANATION" desc="...">
  URL with <ph name="SPECIAL_SYMBOL">%s</ph> in place of query
</message>

The current (old) translation id is 1545786162090505744 and there was no <ph> wrapper around the "%s".  Code searching for the translations

https://cs.chromium.org/search/?q=1545786162090505744&sq=package:chromium&type=cs

showed that, yeap, i18n translators sometimes decided to fiddle with the %s in the message:

chrome/app/resources/generated_resources_el.xtb
<translation id="1545786162090505744">URL με % στη θέση του ερωτήματος</translation>

chrome/app/resources/generated_resources_mr.xtb
<translation id="1545786162090505744">क्वेरीच्या जागेवर % सह URL</translation>

chrome/app/resources/generated_resources_ar.xtb
<translation id="1545786162090505744">‏عنوان URL الذي يحتوي على % بدلاً من طلب البحث</translation>

chrome/app/resources/generated_resources_lv.xtb
<translation id="1545786162090505744">URL ar zīmēm % vaicājuma vietā</translation>

chrome/app/resources/generated_resources_ko.xtb
<translation id="1545786162090505744">URL(검색어 자리에 % 입력)</translation>

chrome/app/resources/generated_resources_fil.xtb
<translation id="1545786162090505744">URL na may mga % sa lugar ng query</translation>

When the new translations for this meesage rolls, the <ph> added in #2 will protect the %s, and so the errors listed above will be gone.

Comment 5 by noel@chromium.org, Jan 26

Translated strings rolled https://chromium-review.googlesource.com/c/chromium/src/+/1435214

New translations for this <message> id = 7063311912041006059
  https://cs.chromium.org/search/?q=7063311912041006059&p=5&sq=package:chromium&type=cs
and they look good to me.

Sign in to add a comment