New issue
Advanced search Search tips

Issue 911101 link

Starred by 3 users
Status: Fixed
Owner:
joelhockey@chromium.org
Closed: Dec 18
Cc:
noel@chromium.org
ajha@chromium.org
gov...@chromium.org
slangley@chromium.org
songsuk@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 908888



Sign in to add a comment

Files app i18n message file $1 $2 placeholders should be wrapped in a <ph> element

Project Member Reported by noel@chromium.org, Dec 3 
Some files app messages contain bare $1 $2 place holders that is not <ph></ph> wrapped, which can cause incorrect language translations.  They can cause Files app not to draw its files list, for example  issue 908767 

Current message:
<message name="IDS_FILE_BROWSER_SIZE_BYTES" desc="Size in bytes.">
  $1 bytes
</message>

Message should be:
<message name="IDS_FILE_BROWSER_SIZE_BYTES" desc="Size in bytes.">
  <ph name="...">$1</ph> bytes
</message>

We should go through all the Files app i18n messages, and add <ph> wrapping to any $1 $2 etc place holders.

Comment 2 by noel@chromium.org, Dec 3

Labels: -CrOSFilesCategory-Testing CrOSFilesCategory-CodeHealth

Comment 3 by slangley@chromium.org, Dec 14

Owner: joelhockey@chromium.org

Comment 4 by noel@chromium.org, Dec 17 

% xmllint --xpath "//message/text()" chrome/app/file_manager_strings.grdp \
 2>/dev/null | grep "\$[0-9][^$]*$" | wc -l

suggests we have 32 errors in chrome/app/file_manager_strings.grdp, FTR. When I looked over all of chrome, the total number of errors is ~50.

Comment 5 by noel@chromium.org, Dec 17 

Issue 908888 seems somewhat related.

Comment 6 by noel@chromium.org, Dec 17

Labels: bad-i18n-placeholders

Comment 7 by noel@chromium.org, Dec 17

Labels: -Pri-3 Pri-1
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 18 

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

commit 1be413f79a1110c26ef027b957c62e0c4a1a8e0c
Author: Joel Hockey <joelhockey@chromium.org>
Date: Tue Dec 18 00:28:03 2018

FilesApp: Use <ph name="...">$1<ex>...</ex></ph> placeholders

Wrap $1 in i18n <message> strings with <ph> placholders.

Bug:  911101 
Change-Id: Ife378f24afe224f11dff20394a679456ac004507
Reviewed-on: https://chromium-review.googlesource.com/c/1377952
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617312}
[modify] https://crrev.com/1be413f79a1110c26ef027b957c62e0c4a1a8e0c/chrome/app/file_manager_strings.grdp

Comment 9 by joelhockey@chromium.org, Dec 18

Status: Fixed (was: Available)

Comment 11 by slangley@chromium.org, Dec 18 

Thanks for landing that fix so quickly Joel.

Comment 12 by noel@google.com, Dec 18 

^^^ Indeed.

Comment 13 by noel@chromium.org, Dec 18

Blocking: 908888

Comment 14 by noel@chromium.org, Dec 19

Summary: Files app i18n message file $1 $2 placeholders should be wrapped in a <ph> element (was: Files app i18n message file $1 $2 place holders should be wrapped in a <ph> element )

Comment 15 by noel@chromium.org, Jan 14

Cc: noel@chromium.org gov...@chromium.org ajha@chromium.org
Issue 918332 has been merged into this issue.
►

Sign in to add a comment