New issue
Advanced search Search tips

Issue 919700 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 9
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Fix i18n message placeholder use in Android

Project Member Reported by noel@chromium.org, Jan 8

Issue description

i18n message placeholder 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, sadly, see  issue 908767 .

There are ~5 places in the android <message> that need fixing.  Once that is done, issue 915681 can resolve, and lock down use to prevent developers submitting invalid i18n messages.

 
chrome/app/settings_strings.grdp has placeholders elements with % chars in the placeholder content.  

<message name="IDS_SETTINGS_PERCENTAGE" desc="A number displayed ...">
  <ph name="PERCENTAGE">$1<ex>120</ex>%</ph>
</message>

which can confuse with Android placeholder vetting code.  We should move the % outside of the <ph> placeholder.

./chrome/android/java/strings/android_chrome_strings.grd

        {FILES, plural, =1 {%1$d file downloaded} other {%1$d files downloaded}}
        {BOOKMARKS_COUNT, plural, =1 {%1$d bookmark} other {%1$d bookmarks}}
        %1$d/%2$d
        {OPEN_TABS, plural, =1 {%1$d open tab, tap to switch tabs} other {%1$d open tabs, tap to switch tabs}}

Only 4 cases to fix: these placeholders should be wrapped in <ph> ...
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7329151faa2f059b07ab3cbc1056f988fe03ad4e

commit 7329151faa2f059b07ab3cbc1056f988fe03ad4e
Author: Noel Gordon <noel@chromium.org>
Date: Tue Jan 08 07:51:40 2019

app/settings_strings.grdp: move placeholder % chars outside of <ph>

To automatically vet i18n message placeholder content on all ports, it
would help if Android / GRIT special characters for placeholders, like
%, are outside of the <ph> element content. All other messages in this
file follow that rule: only three cases that did not, fix them.

Bug:  919700 
Change-Id: I8841ab7fa4d3375397a08f07aeb2036c1e9796dc
Reviewed-on: https://chromium-review.googlesource.com/c/1399682
Reviewed-by: Dan Beam <dbeam@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620649}
[modify] https://crrev.com/7329151faa2f059b07ab3cbc1056f988fe03ad4e/chrome/app/settings_strings.grdp

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a3e42c96f9a825eed88539b3584f0365c40c32d9

commit a3e42c96f9a825eed88539b3584f0365c40c32d9
Author: Noel Gordon <noel@chromium.org>
Date: Tue Jan 08 23:59:40 2019

Wrap IDS_BOOKMARKS_COUNT placeholders in a <ph> element

Bug:  919700 
Change-Id: I542dedbab90fb1764b4e957083c73a1d5e3589ca
Reviewed-on: https://chromium-review.googlesource.com/c/1399762
Reviewed-by: Becky Zhou <huayinz@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620949}
[modify] https://crrev.com/a3e42c96f9a825eed88539b3584f0365c40c32d9/chrome/android/java/strings/android_chrome_strings.grd

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e24ccf2862d7527f06006ec15d39f0741c669ece

commit e24ccf2862d7527f06006ec15d39f0741c669ece
Author: Noel Gordon <noel@chromium.org>
Date: Wed Jan 09 00:34:52 2019

Wrap IDS_ACCESSIBILITY_TOOLBAR_BTN_TABSWITCHER_TOGGLE placeholders in a <ph> element

Bug:  919700 
Change-Id: I5d2bab64bcad711ce5d0cd324ed9e10e2a7777f9
Reviewed-on: https://chromium-review.googlesource.com/c/1399764
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620962}
[modify] https://crrev.com/e24ccf2862d7527f06006ec15d39f0741c669ece/chrome/android/java/strings/android_chrome_strings.grd

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dbf9a8b458473516226c68267aa2abcc01f608a2

commit dbf9a8b458473516226c68267aa2abcc01f608a2
Author: Noel Gordon <noel@chromium.org>
Date: Wed Jan 09 00:44:27 2019

Wrap IDS_DOWNLOAD_UI_FILES_DOWNLOADED placeholders in a <ph> element

No-presubmit: true
Bug:  919700 
Change-Id: I5ce055c15b8689048015fd2295d756814a7b3bb2
Reviewed-on: https://chromium-review.googlesource.com/c/1399683
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620967}
[modify] https://crrev.com/dbf9a8b458473516226c68267aa2abcc01f608a2/chrome/android/java/strings/android_chrome_strings.grd

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b07863b47aebd4379734808db84322d8c71df525

commit b07863b47aebd4379734808db84322d8c71df525
Author: Noel Gordon <noel@chromium.org>
Date: Wed Jan 09 00:46:02 2019

Wrap IDS_FIND_IN_PAGE_COUNT placeholders in a <ph> element

No-presubmit: true
Bug:  919700 
Change-Id: Ie46575ed09b6c86666d0858738cc60f2ee88029d
Reviewed-on: https://chromium-review.googlesource.com/c/1399765
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620969}
[modify] https://crrev.com/b07863b47aebd4379734808db84322d8c71df525/chrome/android/java/strings/android_chrome_strings.grd

Status: Fixed (was: Started)

Sign in to add a comment