New issue
Advanced search Search tips

Issue 870338 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 31
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Number.prototype.toLocaleString() does not return sterling currency symbol for locale en-GB

Reported by mike.h...@digitaldesignlabs.com, Aug 2

Issue description

Steps to reproduce the problem:
Number(10).toLocaleString("en-GB",  {style: "currency", currency: "GBP"});

What is the expected behavior?
£10.00

What went wrong?
GBP10.00

Did this work before? Yes 67

Does this work in other browsers? Yes

Chrome version: 68.0.3440.84  Channel: stable
OS Version: 5.1.1
Flash Version: 

Returns correctly if the locale is changed from en-GB to simply en.  Also seems to only affect Chrome for Android.
 
Components: -Blink>JavaScript Blink>JavaScript>Internationalization
Labels: Needs-triage-Mobile
Cc: chelamcherla@chromium.org
Labels: -Pri-2 hasbisect-per-revision M-68 Target-69 Target-70 RegressedIn-68 Triaged-Mobile FoundIn-70 Target-68 FoundIn-68 FoundIn-69 Pri-1
Owner: js...@chromium.org
Status: Assigned (was: Unconfirmed)
Tested the issue in Android and able to reproduce the issue. 

Steps Followed:
1. Opened chrome and pasted Number(10).toLocaleString("en-GB",  {style: "currency", currency: "GBP"}); in console
2. Observed output as GBP10.00 instead of £10.00

Chrome versions tested:
68.0.3440.85, 70.0.3415.4

OS:
Android 9.0

Android Devices:
Pixel 2 XL

Issue is seen in latest stable and latest canary.

Using the per-revision bisect providing the bisect results,
Good Build - 68.0.3423.0
Bad Build - 68.0.3425.0 

CL: https://chromium.googlesource.com/chromium/src/+/359c346836e261c50fb4d3168f5e0a3ce4edb436

@ jshin: Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to owner concerned. Adding RB-Stable for M-68, Please remove if this not the case. 

Please navigate to below link for log's  --
go/chrome-androidlogs/870338

Thanks!

Cc: jbanavatu@chromium.org
 Issue 872215  has been merged into this issue.
Labels: -M-68 ReleaseBlock-Stable M-70
Recently regressed, please have a fix before M70 hits stable.
Hi, could I clarify what this means in terms of an expected timeline for a fix? 

We are considering whether we need to put something in place to work around the issue until the fix lands, so it would be useful to know roughly how long that might be.

Thanks!
#6 - if this is indeed fixed in Chrome 70 (it is currently not fixed), then it will be released in about 12 weeks.
Labels: -ReleaseBlock-Stable
This is not a release blocker, not sure why this was added. Removing label.
Desktop is fine. So, the ICU data for Android has an issue. 
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/deps/icu.git/+/a191af9d025859e8368b8b469120d78006e9f5f6

commit a191af9d025859e8368b8b469120d78006e9f5f6
Author: Jungshik Shin <jshin@chromium.org>
Date: Wed Aug 22 06:57:36 2018

Fix ICU data file for Android

In some locales, currency data turns empty after currency list filtering
on Android. This leads v8's currency formatter to fall back to 3-letter
currency code instead of currency symbol (e.g. 'GBP' in en-GB).

Fix the ICU data trimming script to remove an empty Currency block after
filtering and rebuild ICU data files.

Bug:  870338 
Test: See the bug
TBR=gsathya@chromium.org
Change-Id: Idf7af0a5f00b05b7f26d2ad42b21abd1aca47d07
Reviewed-on: https://chromium-review.googlesource.com/1184505
Reviewed-by: Jungshik Shin <jshin@chromium.org>

[modify] https://crrev.com/a191af9d025859e8368b8b469120d78006e9f5f6/android/icudtl.dat
[modify] https://crrev.com/a191af9d025859e8368b8b469120d78006e9f5f6/cast/icudtl.dat
[modify] https://crrev.com/a191af9d025859e8368b8b469120d78006e9f5f6/cast/patch_locale.sh
[modify] https://crrev.com/a191af9d025859e8368b8b469120d78006e9f5f6/common/icudtb.dat
[modify] https://crrev.com/a191af9d025859e8368b8b469120d78006e9f5f6/common/icudtl.dat
[modify] https://crrev.com/a191af9d025859e8368b8b469120d78006e9f5f6/ios/icudtl.dat

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 22

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

commit 9fa9364c9aab685f22490b2035012a56a9fcebc3
Author: Jungshik Shin <jshin@chromium.org>
Date: Wed Aug 22 22:54:18 2018

Roll ICU to a191af9d

This roll has only one change as recorded in
     https://chromium.googlesource.com/chromium/deps/icu/+/a191af9d .

TBR=gsathya@chromium.org

Bug: chromium: 870338
Test: See the bug
Change-Id: Iceaa255ede13ef3f9e94fdcf6ab74268c1940427
Reviewed-on: https://chromium-review.googlesource.com/1184508
Reviewed-by: Jungshik Shin <jshin@chromium.org>
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585291}
[modify] https://crrev.com/9fa9364c9aab685f22490b2035012a56a9fcebc3/DEPS

This is fixed now in ToT for M70. 

A fix is trivial (requires only ICU deps roll). After baking in trunk for a while, ICU deps roll can be cherry-picked for M69. 

For M68, it's too late. I don't think there will be any more release in M68 branch before M69 turns stable. 
Labels: M-69 Merge-Request-69
Status: Fixed (was: Assigned)
Verified in 70.x canary on Android. 

I missed 69 stable cut by 3 days. 69 stable release is still a few days away. Trying it for M-69 (maybe 2nd stable release of 69 branch). 

Project Member

Comment 14 by sheriffbot@chromium.org, Aug 31

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

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-69 Merge-Approved-69
Approved for merge into 69, branch 3497.
Labels: -Merge-Approved-69 Merge-Merged-69
It's merged. DOn't know why bugdroid hasn't recorded it here, yet.

https://chromium-review.googlesource.com/c/chromium/src/+/1208250



Project Member

Comment 17 by bugdroid1@chromium.org, Sep 5

Labels: merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8a7148be5419aea65accc3bb2108b2518d51e1b5

commit 8a7148be5419aea65accc3bb2108b2518d51e1b5
Author: Jungshik Shin <jshin@chromium.org>
Date: Wed Sep 05 18:46:10 2018

[M69 branch] Roll ICU to a191af9d

This roll has only one change as recorded in
     https://chromium.googlesource.com/chromium/deps/icu/+/a191af9d .

This is to fix the currency format with GB Pound in en-GB locale on
Android.

TBR=gsathya@chromium.org,benmason@chromium.org

Bug:  chromium:870338 
Test: See the bug
Change-Id: Iceaa255ede13ef3f9e94fdcf6ab74268c1940427
Reviewed-on: https://chromium-review.googlesource.com/1184508
Reviewed-by: Jungshik Shin <jshin@chromium.org>
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#585291}(cherry picked from commit 9fa9364c9aab685f22490b2035012a56a9fcebc3)
Reviewed-on: https://chromium-review.googlesource.com/1208250
Cr-Commit-Position: refs/branch-heads/3497@{#880}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/8a7148be5419aea65accc3bb2108b2518d51e1b5/DEPS

Sign in to add a comment