New issue
Advanced search Search tips

Issue 856113 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Review CurrencyFormatter (especially removing currency code from the formatted output)

Project Member Reported by js...@chromium.org, Jun 25 2018

Issue description


CurrencyFormatter is a wrapper over ICU's currency formatter (NumberFormat - currencyInstance), but it's more than a wrapper. It manipulates the ICU output in a rather interesting way probably because of the following requirement. 

------cut------here-------

https://cs.chromium.org/chromium/src/components/payments/core/currency_formatter.h?rcl=d2ae696f6db291360f2b4fce6a5f368b818eb2f6&l=30

  // Formats the |amount| according to the currency code that was set. The
  // result will NOT contain the currency code, nor a subset of it. Rather, the
  // caller of this function should display the currency code separately. The
  // return value may contain non-breaking space and is ready for display. In
  // the case of a failure in initialization of the formatter or during
  // formatter, this method will return |amount|.
  base::string16 Format(const std::string& amount);


https://cs.chromium.org/chromium/src/components/payments/core/currency_formatter.cc?rcl=d2ae696f6db291360f2b4fce6a5f368b818eb2f6&l=101

 // Explicitly removes the currency code (truncated to its 3-letter, 2-letter
  // and 1-letter versions) from the output, because callers are expected to
  // display the currency code alongside this result.
  //
  // 3+ letters: If currency code is "ABCDEF" or "BTX", this code will
  // transform "ABC55.00"/"BTX55.00" to "55.00".
  // 2 letters: If currency code is "CAD", this code will transform "CA$55.00"
  // to "$55.00" (en_US) or "55,00 $ CA" to "55,00 $" (fr_FR).
  // 1 letter: If currency code is "AUD", this code will transform "A$55.00"
  // to "$55.00" (en_US).

--------------

I'm not sure why callers have to put the currency code separately instead of just using the output as a whole (does it have anything to do with web payment RFC??)   The currency code or currency sign (depending on locales and currency, either of them or a combination thereof can be used as a part of the ICU's currency formatter per CLDR ) is an integral part of the currency format output. 

Manipulating the ICU output to keep only a part of it while chopping off others would not work very well and can lead to an unexpected surprise when CLDR data changes. 

For instance, with ICU 62.1, CurrencyAmounts/PaymentsCurrencyFormatterTest.IsValidCurrencyFormat/40 test fails. Why? Because in the latest version of CLDRs, some locales that put a minus sign before the currency code/sign have an extra space between the currency code and the numeric amount.

For instance, in en-AU locale, it used to be 

-USD45.35

Now, it is 

-USD 45.35.

When USD is removed by Chrome's CurrencyFormatter, it becomes "- 45.35", but the expected result is "-45.35".  

It can be worked around for now by replacing "-\u0020" and "=\u00a0" with "-", which is what I'm doing for  bug 850334 , but that's only a work around to pass the test. 

Also, it has to be noted that in RTL locales, Bidi control characters (such as U+200E, U+200F) can be inserted around currency code or currency sign. Removing the currency code  as done by CurrencyFormatter::format can break Bidi output in an unexpected way. 


 

Comment 1 by js...@chromium.org, Jun 25 2018

> It can be worked around for now by replacing "-\u0020" and "=\u00a0" with "-", which is what I'm doing for  bug 850334 , but that's only a work around to pass the test.

See 
https://chromium-review.googlesource.com/c/chromium/src/+/1111818/4/components/payments/core/currency_formatter.cc

for my temporary work-around. That's for  bug 850334 . 
If using the new NumberFormatter API, there is an option to suppress the currency symbol, UnitWidth.HIDDEN.

If stuck using the old API, field positions are your best bet for removing the currency symbol, but that is not a perfect science because of impacts you discovered such as the sign and currency spacing.

Sign in to add a comment