Issue metadata
Sign in to add a comment
|
Regression: No files are seen in File Manager when Chrome OS display language is Hindi |
||||||||||||||||||||||
Issue descriptionChrome 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
,
Nov 27
noel@ - can you take a look at this one please?
,
Nov 27
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.
,
Nov 28
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.
,
Nov 28
,
Nov 28
Seems fixed on M_72 at crrev.com/908114 which is where I tested.
,
Nov 28
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.
,
Nov 28
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.
,
Nov 28
+ kbleicher@ (Chrome OS M71 TPM), pls see comment #7.
,
Nov 28
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!
,
Nov 28
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
,
Nov 28
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).
,
Nov 28
I then typed chrome://inspect into the chrome browser, and inspected the files app. The following is present in the console log ... (image attached).
,
Nov 28
[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).
,
Nov 28
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.
,
Nov 28
I printed out the translations for these values: loadTimeData.language = 'hi" and found ...
,
Nov 28
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 !!
,
Nov 29
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.
,
Nov 29
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
,
Nov 29
Filed issue 909997 on Chrome UI>Localization re: translation console issue.
,
Nov 29
,
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
,
Nov 29
Is there a pending CL for verification / merge consideration yet? Approaching M71 stable.
,
Nov 30
Yeap, patch in #22. Next steps?
,
Nov 30
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
,
Dec 3
#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 ...
,
Dec 3
#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.
,
Dec 3
,
Dec 3
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
,
Dec 3
,
Dec 3
We crossed.
,
Dec 3
Merge approved for M71 Chrome OS.
,
Dec 4
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
,
Dec 4
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}
,
Dec 4
,
Jan 21
(2 days ago)
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by dgagnon@chromium.org
, Nov 27