824470 - Brazil Real currency is hardcoded in the Cost Control application

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Cost Control, defect, P3)

Description

[Axel Hecht [:Pike]]

I've just came across bug 816927, which hard-codes the brazil real as currency within gaia code. Not sure how wide-spread that is.

Comment 1

[chris hofmann]

bug 816927 seeks to remove that app as the fix.  I'm confused about if the app will make its return before shipping.  anyone have clarity there?

I guess the way to assess this bug is to do some greping over the source looking for anything Real or maybe Peso related can be found.    


The list is long with many false positives in this mxr search.

https://mxr.mozilla.org/gaia/search?string=real

I wonder if others have ideas on how to do some better searching?

Comment 2

[Axel Hecht [:Pike]]

Not really good to search for, but appearing at a few places is 'R$' for the Brazil real.

I think the backgroundservice doesn't have anything to do with removing the cost control app?

Comment 3

[Dietrich Ayala (:dietrich)]

We're shipping in multiple markets with different currencies, so requesting blocking on doing the audit described above.

Comment 4

[Axel Hecht [:Pike]]

I looked at this a bit more from the technical side, via
git grep -I 'R\$'
and it seems that to some extent build/applications-data.js takes care of things, though I'm not sure if that's as good as things should be. In particular if we're dealing with people using multiple SIM cards while traveling.

Apart from that, I found

apps/costcontrol/fte.html:            R$
apps/costcontrol/index.html:                <span id="settings-low-limit-currenc

The index.html one seems to be overwritten via apps/costcontrol/js/settings/settings.js, but I'm not sure in a good or timely fashion.

I couldn't track down the fte.html one.
neither of which I could track down to get configure

Comment 5

[Joe Cheng [:jcheng] (please needinfo)]

Triage: BB+ , in agreement with comment 3

Comment 6

[Guillermo López :willyaranda (probably SLOW response)]

Adding some guys responsible for Cost Control and FTE, could you guys check comment #4?

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

As far as I understand Cost Control this will be seen only if you use a Vivo prepaid card. Renominating based on that and let's ask more information to Salva.

Comment 8

[Salvador de la Puente González [:salva]]

The currency of Cost Control is set in OEM configuration. So settings module should load the actual currency and put where it is required. It should not be hardcoded in the HTML so I take the bug (but I'm on vacations yet).

Comment 9

[Staś Małolepszy :stas]

AFAIK, we're shipping 1.0 outside of Brazil as well.  We shouldn't hardocde any curencies at all.  Re-requesting basecamp-blocking.

Comment 10

[Salvador de la Puente González [:salva]]

It is not a matter of i18n but a matter of wich currency is used by your operator to take count of your credit. That is because it is your operator who is in charge of customize this value.

Comment 11

[Staś Małolepszy :stas]

Yes, understood.  But as it stands right now, R$ is hardcoded in the HTML.

https://github.com/mozilla-b2g/gaia/blob/d0c650db4a652803da5e7f5128097597e2e82747/apps/costcontrol/fte.html#L101
https://github.com/mozilla-b2g/gaia/blob/a51cc4e3e831e9fb328daa60249df57a1d327be3/apps/costcontrol/index.html#L297

I propose that the removal of these occurrences be marked as bb+.

Comment 12

[Salvador de la Puente González [:salva]]

Of course, as I said in comment 8 "It should not be hardcoded in the HTML so I take the bug (but I'm on vacations yet)."

Comment 13

[Staś Małolepszy :stas]

Cool, we're on the same page then :)

Comment 14

[Joe Cheng [:jcheng] (please needinfo)]

Triage: BB+

Comment 15

[Francisco Jordano [:arcturus] [:francisco]]

Stealing another one ;)

Comment 16

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: Pointer to GitHub PR 7346

Comment 17

[Staś Małolepszy :stas]

Comment on attachment 698904 [details]
Pointer to GitHub PR 7346

Why add currency to .properties at all?  If it's supposed to work as a fallback for when configuration.credit.currency is not set, then I don't think this is desirable.  Depending on the locale set by the user, we'd show different currencies, but the same number.  This can be harmful to the user.

If the currency is not given by the carrier, I think we should display an error message and possibly even not allow to use the cost control app (?).

Comment 18

[Axel Hecht [:Pike]]

Yeah, what stas said, the currency shouldn't show up in l10n.

Unless, you know, a few releases in the future, we'd had a currency database that'd allow us to have a localized name for the 'R$' brazilian real as well as euros, greek drachmen etc, so that you could get the localized currency name if you used your German phone with a Greek SIM card and cost-control communication.

Comment 19

[Francisco Jordano [:arcturus] [:francisco]]

Hi guys,

taking into account the feedback I'll just remove the fallback from l10n.

Thanks a lot for the feedback.

Comment 20

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: Pointer to GitHub PR 7346

New version without l10n requirements. Just based on configuration by partner.

Comment 21

[Francisco Jordano [:arcturus] [:francisco]]

Landed:

https://github.com/mozilla-b2g/gaia/commit/63b051fd78d7938f84bc369f16e267099a8d29f8