New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 908767 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 0
Type: Bug-Regression

Blocked on:
issue 909997

Blocking:
issue 923755



Sign in to add a comment

Regression: No files are seen in File Manager when Chrome OS display language is Hindi

Project Member Reported by rkalavakuntla@chromium.org, Nov 27

Issue description

Chrome Version:71.0.3578.71/11151.45.0  beta-channel Kip, Daisy & Reks
OS:Chrome OS

What steps will reproduce the problem?
(1)Sign into user >> Set the Chrome OS display language to 'Hindi'
(2)Take a screenshot or download any file
(3)View them in Files app and observe(Please refer video)

Actual: No files are found when we try to view them in Files app
Expected: Instead,saved files should be seen in files app 

This is a Regression issue as same is working fine in M-66

Note: Issue is seen in M-67,68,69,M-70 stable 

Files are not getting reflected in File Manager when display language is Hindi. Hence adding Stable Blocker

 
Actual.mp4
12.5 MB View Download
Expected.mp4
5.7 MB View Download
Cc: adanilo@chromium.org amistry@chromium.org
Adding a couple people to CC to help find an owner.
Owner: noel@chromium.org
Status: Assigned (was: Untriaged)
noel@ - can you take a look at this one please?
Cc: dhadd...@chromium.org
OP mentioned: 

   > Note: Issue is seen in M-67,68,69,M-70 stable

This has been the state of things since M-67?  Don't see why its marked release block M-71 ...

+ dhaddock@ for advice, /me not sure what language display testing is done during Release QA but looks like we might need some.
Setup language:

  - select "Hindi" in chrome://settings/language
  - choose that chrome display in Hindi
      clich the check box: Chrome OS is displayed in this language
  - click "Restart" button

When chrome-os restarts:
  - right click the gear icon in lower right corner of chrome
    - confirm the displayed result is shown in "Hindi"
  - click the download button
    - Files app should be shown and its UI should be displayed
      in the "Hindi"

Repeated those steps 72.0.3624.0 M-72 Chrome dev, image result attached.


bug-908767-2018-11-28.png
50.9 KB View Download
Cc: -sashab@chromium.org
Seems fixed on M_72 at crrev.com/908114 which is where I tested. 
Cc: gov...@chromium.org
According to http://omahaproxy.appspot.com, the M71 release branch is 3578, with a release tag of 71.0.3578.49.

Re-built chrome-os chrome there using:
 
  % git checkout -b 3578 tags/71.0.3578.49
  % gclient sync --with_branch_heads --with_tags
  % autoninja -C out/chrome

and re-did the steps in #4, using a fresh --user-data-dir=/tmp/3578, and no reproduction, same result as #4 (good).

This look to be fixed on the M_71 release branch too. +govind as an FYI.



The M70 release branch is 3538, with a release tag of 70.0.3538.110. Re-built chrome-os chrome there:

  % git checkout -b 3538 tags/70.0.3538.110
  % gclient sync --with_branch_heads --with_tags
  % autoninja -C out/chrome

and again re-dd the steps: no reproduction on M-70 either.
Cc: kbleicher@chromium.org
+  kbleicher@ (Chrome OS M71 TPM), pls see comment #7.
Though the issue is seen in M-67,68,69,M-70 stable but as functionality wise issue seems to severe because files are not getting reflected in File Manager when display language is Hindi. Hence added Stable Blocker for M-71.

Feel free to adjust the milestone,if this shouldn't be blocking M-71 release. 
Thank you!

First step is to repro.

> "Though the issue is seen in M-67,68,69,M-70 stable

Maybe try M-72 dev on your devices (or even a clean device) if you have time?

Anyho, the bug report said M_71 tag 71.0.3578.71, so I assume you saw the issue there.  I can also try that using:

  % git checkout -b 3578.71 tags/71.0.3578.71
  % gclient sync --with_branch_heads --with_tags
  % autoninja -C out/chrome

and re-do the steps #4, but alas, no reproduction @ 71.0.3578.71
Reproduced.  Got an 'eve' device charged up, and pulled in release-dev and release-beta channels Chrome-OS onto the device.

Taking screen shots, after setting the device language to "Hindi" and restarting the device ...

Reproduced @ developer channel official release 71.0.3611.0
Reproduced @ beta channel official release 72.0.3578.49

(image attached).

bug-908767-files-app-no-files-in-hindi.png
186 KB View Download
I then typed chrome://inspect into the chrome browser, and inspected the files app.  The following is present in the console log ... (image attached).

bug-908767-console-log.png
434 KB View Download
[Deprecation] HTML Imports is deprecated and will be removed in M73, around March 2019. Please use ES modules instead. See 
... blah blah, elided
main_scripts.js:62 Error: Assertion failed: Unescaped $ found in localized string.
    at assert (assert.js:23)
    at main_scripts.js:1229
    at String.replace (<anonymous>)
    at LoadTimeData.substituteString (main_scripts.js:1228)
    at LoadTimeData.getStringF (main_scripts.js:1199)
    at strf (main_scripts.js:14835)
    at str (main_scripts.js:14678)
    at Object.util.bytesToString (main_scripts.js:14694)
    at FileMetadataFormatter.formatSize (main_scripts.js:46466)
    at HTMLDivElement.FileTable.updateSize_ (main_scripts.js:47353)
(anonymous) @ main_scripts.js:62
assert.js:23 Uncaught (in promise) Error: Assertion failed: Unescaped $ found in localized string.
    at assert (assert.js:23)
    at main_scripts.js:1229
    at String.replace (<anonymous>)
    at LoadTimeData.substituteString (main_scripts.js:1228)
    at LoadTimeData.getStringF (main_scripts.js:1199)
    at strf (main_scripts.js:14835)
    at str (main_scripts.js:14678)
    at Object.util.bytesToString (main_scripts.js:14694)
    at FileMetadataFormatter.formatSize (main_scripts.js:46466)
    at HTMLDivElement.FileTable.updateSize_ (main_scripts.js:47353)
2assert.js:31 Uncaught Error: Assertion failed: Unescaped $ found in localized string.
    at assert (assert.js:23)
    at main_scripts.js:1229
    at String.replace (<anonymous>)
    at LoadTimeData.substituteString (main_scripts.js:1228)
    at LoadTimeData.getStringF (main_scripts.js:1199)
    at strf (main_scripts.js:14835)
    at str (main_scripts.js:14678)
    at Object.util.bytesToString (main_scripts.js:14694)
    at FileMetadataFormatter.formatSize (main_scripts.js:46466)
    at HTMLDivElement.FileTable.updateSize_ (main_scripts.js:47353)

An exception throws util.bytesToString when processing file sizes for display in the UI.  This unhandled exception prevents Files app file list from being displayed (bug).

The files sizes are formatted and translated i18n, via the LoadTimeData, which finds an invalid string translation for the sizes util.bytesToString requests.

https://chromium.googlesource.com/chromium/src/+/b333835cd8f3139d0836baaac594070abdb3f679 added util.bytesToString, with i18n string message defines:

  <message name="IDS_FILE_BROWSER_SIZE_BYTES" desc="Size in bytes.">
    $1 bytes
  </message>
  <message name="IDS_FILE_BROWSER_SIZE_KB" desc="Size in kilo bytes.">
    $1 KB
  </message>
  <message name="IDS_FILE_BROWSER_SIZE_MB" desc="Size in mega bytes.">
    $1 MB
  </message>
  <message name="IDS_FILE_BROWSER_SIZE_GB" desc="Size in giga bytes.">
    $1 GB
  </message>
  <message name="IDS_FILE_BROWSER_SIZE_TB" desc="Size in tera bytes.">
    $1 TB
  </message>
  <message name="IDS_FILE_BROWSER_SIZE_PB" desc="Size in peta bytes.">
    $1 PB
  </message>

These IDS translate directly to symbols used for translation, we can see the calling code here:

https://cs.chromium.org/chromium/src/ui/file_manager/file_manager/common/js/util.js?type=cs&q=util.bytesToString&g=0&l=208

util.bytesToString = function(bytes) {
  // Translation identifiers for size units.
  var UNITS = ['SIZE_BYTES',
               'SIZE_KB',
               'SIZE_MB',
               'SIZE_GB',
               'SIZE_TB',
               'SIZE_PB'];
  ...

Looks to me like the Hindi translations values are invalid: exception unescaped $ in the translated value.  The should be no unescaped given the i18n string message definitions.
I printed out the translations for these values: loadTimeData.language = 'hi" and found ...
bug-908767-load-time-data-sizes-hindi.png
171 KB View Download
SIZE_KB and SIZE_MB are both busted, they start with $ followed by hindi characters.  

Wonder what those characters mean?  Google translate says they mean "one".  So $1 has been incorrectly translated to $One in Hindi !!
   

bug-908767-decode-hindi-chars-to-english-one.png
69.2 KB View Download
Labels: -Pri-1 Pri-0
Checking around, Object.util.bytesToString is used by FilesApp foreground and background pages, and related apps: Gallery, VideoPlayer, AudiPlayer, and the Chrome-OS SelectFileDialog.

When this bug springs, the above fall in a heap, making FilesApp and related entirely unusable for Hindi users.
Cc: fukino@chromium.org yamaguchi@chromium.org
Until we get translation console fixed, need to mitigate this issue with a simple focussed fix that could be merged.  Patch up ...

https://chromium-review.googlesource.com/c/chromium/src/+/1353044
Filed issue 909997 on Chrome UI>Localization re: translation console issue.

Blockedon: 909997
Project Member

Comment 22 by bugdroid1@chromium.org, Nov 29

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

commit 8f44d798f7731422935fdb8da90f3c112fe9c13b
Author: Noel Gordon <noel@chromium.org>
Date: Thu Nov 29 11:40:30 2018

FilesApp: file list fails to draw due to invalid Hindi translations

util.bytesToString fails with an exception that prevents the file list
from drawing for Hindi language users. The exception is due to invalid
translation console translations of the SIZE_KB, SIZE_MB strings [1].

Mitigation: process these loadTimeData values before use. Convert them
to valid values (prevents the JS exception) and do this once only.

[1] The $1 portion of these message strings should not be touched, but
translation console is effectively converting them to $one in Hindi.

Bug:  908767 
Change-Id: Ib5d741e501c3cf00e232b59366050778d732d906
Reviewed-on: https://chromium-review.googlesource.com/c/1353044
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612149}
[modify] https://crrev.com/8f44d798f7731422935fdb8da90f3c112fe9c13b/ui/file_manager/file_manager/common/js/util.js

Is there a pending CL for verification / merge consideration yet?  Approaching M71 stable.  
Yeap, patch in #22.  Next steps?
Summarizing offline discussion.

When using placeholders in strings, they should be wrapped by <ph>$1</ph> XML tags, otherwise translators don't know that they should not modify these. The strings at [1] are incorrect to begin with (which is the root of the issue)

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

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


See [2] for proper placeholder usage. File Manager app already uses correctly formatted strings at [3].

[1] https://cs.chromium.org/chromium/src/chrome/app/file_manager_strings.grdp?l=93-110
[2] https://www.chromium.org/developers/tools-we-use-in-chromium/grit/grit-users-guide
[3] https://cs.chromium.org/chromium/src/chrome/app/file_manager_strings.grdp?l=162
#25 Thank you for the details.  Looking over other messages files today, it seems that Files app is not the only component with message place holders without <ph> wrappings.   

Perhaps a pre-submit would help prevent that, and I had hoped to use issue 909997 to resolve.  Anyho, that bug is closed.

At least for Files app, I filed  issue 911101  to fix any unwrapped place holders.  With that done, going back to the business of the current bug ...
#23 > Is there a pending CL for verification / merge consideration yet?  Approaching M71 stable.  

The patch in #22.  I checked the Canary Chrome that came out over the w/end on Chrome-OS today.  It contained #22.

When I set the device language to Hindi, the files list now displays correctly.
I check files that had KB and MB sizes.  Looks fixed to me.
Labels: Merge-Request-71
Project Member

Comment 29 by sheriffbot@chromium.org, Dec 3

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: We are only 0 days from stable.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-71 Merge-Request-71
Labels: -Merge-Request-71 Merge-Review-71
We crossed.
Labels: -Merge-Review-71 Merge-Approved-71
Merge approved for M71 Chrome OS.
Project Member

Comment 33 by bugdroid1@chromium.org, Dec 4

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/82e46eabf6858e3a2db372aea2afcf125cf850a0

commit 82e46eabf6858e3a2db372aea2afcf125cf850a0
Author: Noel Gordon <noel@chromium.org>
Date: Tue Dec 04 01:18:03 2018

[M71] FilesApp: file list fails to draw due to invalid Hindi translations

util.bytesToString fails with an exception that prevents the file list
from drawing for Hindi language users. The exception is due to invalid
translation console translations of the SIZE_KB, SIZE_MB strings [1].

Mitigation: process these loadTimeData values before use. Convert them
to valid values (prevents the JS exception) and do this once only.

[1] The $1 portion of these message strings should not be touched, but
translation console is effectively converting them to $one in Hindi.

Bug:  908767 
Change-Id: Ib5d741e501c3cf00e232b59366050778d732d906
Reviewed-on: https://chromium-review.googlesource.com/c/1353044
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612149}(cherry picked from commit 8f44d798f7731422935fdb8da90f3c112fe9c13b)
Reviewed-on: https://chromium-review.googlesource.com/c/1360032
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#869}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/82e46eabf6858e3a2db372aea2afcf125cf850a0/ui/file_manager/file_manager/common/js/util.js

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/82e46eabf6858e3a2db372aea2afcf125cf850a0

Commit: 82e46eabf6858e3a2db372aea2afcf125cf850a0
Author: noel@chromium.org
Commiter: noel@chromium.org
Date: 2018-12-04 01:18:03 +0000 UTC

[M71] FilesApp: file list fails to draw due to invalid Hindi translations

util.bytesToString fails with an exception that prevents the file list
from drawing for Hindi language users. The exception is due to invalid
translation console translations of the SIZE_KB, SIZE_MB strings [1].

Mitigation: process these loadTimeData values before use. Convert them
to valid values (prevents the JS exception) and do this once only.

[1] The $1 portion of these message strings should not be touched, but
translation console is effectively converting them to $one in Hindi.

Bug:  908767 
Change-Id: Ib5d741e501c3cf00e232b59366050778d732d906
Reviewed-on: https://chromium-review.googlesource.com/c/1353044
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612149}(cherry picked from commit 8f44d798f7731422935fdb8da90f3c112fe9c13b)
Reviewed-on: https://chromium-review.googlesource.com/c/1360032
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#869}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Status: Fixed (was: Assigned)

Comment 36 by noel@chromium.org, Jan 21 (2 days ago)

Blocking: 923755

Sign in to add a comment