1030546 - [Emergency call] "Cancel" button is not localized

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Dialer, defect)

Description

[Théo Chevalier]

STR:
1. Enable passcode
2. Switch to French
3. On lockscreen, tap on "Appel d’urgence" (Emergency call)
4. The dial pad displays a "Cancel" button that is not localized.

Added qawanted to test on 2.0

Tested on Flame 2.1
Gaia      1bcd355855626640b2532f2ccb1f814711f7a6ad
Gecko     https://hg.mozilla.org/mozilla-central/rev/da1dbcff9493
BuildID   20140625160202
Version   33.0a1
ro.build.version.incremental=109
ro.build.date=Mon Jun 16 16:51:29 CST 2014

Comment 1

[Francesco Lodolo [:flod]]

Confirm: it's not localized, and probably the design is way too easy to break ("Cancel" barely fits).

I see l10n was changed in bug 999195, not sure if there's a relation.

Comment 2

[Francesco Lodolo [:flod]]

Code seems correct, and app should have access to system.properties, which has "cancel"
https://github.com/mozilla-b2g/gaia/blob/master/apps/emergency-call/index.html#L88

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

It's a regression from bug 960915. As emergency-call code has been forked from system, locale resources have never been.

It affects 2.0 and 2.1.

:gweng, you were the owner of that bug. Can you confirm my analysis?

Comment 4

[Greg Weng [:snowmantw][:gweng][:λ]]

I think you're right, and this can be solved by copying the necessary resource to resolve this bug.
If this is not an emergent issue, I may be able to solve it at next week since this bug was caused by the bug I'd resolved, or we need to NI emergency-call owner like Etienne on the Wiki page.

Comment 5

[Doug Sherk (:drs) (inactive)]

Attachment: Share keypad localization data across the dialer and emergency-call apps.

PR: https://github.com/mozilla-b2g/gaia/pull/21241

I grepped for /l10n/i within ./apps/emergency-call/ and moved all of the strings that it uses from the dialer to shared.

It's kind of weird that none of the strings in ./shared/locales/ are localized in anything other than English. It's also weird that only the "cancel" key is localized within the dialer (e.g. "phoneNumberInput", "keypadDelete", etc. should be too).

Comment 6

[Francesco Lodolo [:flod]]

(In reply to Doug Sherk (:drs) from comment #5)
> It's kind of weird that none of the strings in ./shared/locales/ are
> localized in anything other than English. It's also weird that only the
> "cancel" key is localized within the dialer (e.g. "phoneNumberInput",
> "keypadDelete", etc. should be too).

The sooner we get rid of those locales on Github, the faster developers will understand that those are not real locales ;-)

Real locales live here
https://hg.mozilla.org/gaia-l10n/

Comment 7

[Etienne Segonzac (:etienne)]

Comment on attachment 8448945 [details] [diff] [review]
Share keypad localization data across the dialer and emergency-call apps.

Review of attachment 8448945 [details] [diff] [review]:
-----------------------------------------------------------------

r+ for the move to shared/locales/keypad.ini

But I'd like to ask Francesco what's the best way to format this patch.
Just touching the .en-US.properties files maybe?

Comment 8

[Francesco Lodolo [:flod]]

Personally I wouldn't touch locales files outside of en-US: they're not used, patch has less noise, and final target is to get rid of them completely. 

So far they've been synced periodically with the actual translation by :tchevalier

Comment 9

[Doug Sherk (:drs) (inactive)]

Attachment: Share keypad localization data across the dialer and emergency-call apps.

(In reply to Francesco Lodolo [:flod] from comment #8)
> Personally I wouldn't touch locales files outside of en-US: they're not
> used, patch has less noise, and final target is to get rid of them
> completely. 
> 
> So far they've been synced periodically with the actual translation by
> :tchevalier

I spoke with Francesco on IRC and he told me how to structure my patch correctly, so I made those changes and I'm carrying r+. I also updated the PR.

Comment 10

[Doug Sherk (:drs) (inactive)]

Attachment: Share keypad localization data across the dialer and emergency-call apps.

(In reply to Francesco Lodolo [:flod] from comment #8)
> Personally I wouldn't touch locales files outside of en-US: they're not
> used, patch has less noise, and final target is to get rid of them
> completely. 
> 
> So far they've been synced periodically with the actual translation by
> :tchevalier

I spoke with Francesco on IRC and he told me how to structure my patch correctly, so I made those changes and I'm carrying r+. I also updated the PR.

Comment 11

[Doug Sherk (:drs) (inactive)]

https://github.com/mozilla-b2g/gaia/commit/f9110ee004442fdfca84f8cc9562fbb379fc387c

Comment 12

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8449588 [details] [diff] [review]
Share keypad localization data across the dialer and emergency-call apps.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 960915
[User impact] if declined: The "Cancel" button on the emergency call screen won't be localized.
[Testing completed]: I tried it on a Flame, and Etienne probably did too.
[Risk to taking this patch] (and alternatives if risky): Low, other than needing to deal with the strings being moved.
[String changes made]: Added keypad.ini file which is a manifest for a refactor of some strings from the dialer. This file is shared between the dialer and the emergency-call apps. Unfortunately, there's no way to structure this patch that I can think of that doesn't, at the very least, involve moving around strings.

Comment 13

[Doug Sherk (:drs) (inactive)]

Attachment: Import dialer.xxx.properties into emergency call app to fix cancel button.

For the v2.0 uplift.

Comment 14

[Etienne Segonzac (:etienne)]

Comment on attachment 8451884 [details] [diff] [review]
Import dialer.xxx.properties into emergency call app to fix cancel button.

Review of attachment 8451884 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm, just confirming: this is compatible with the build time magic making the emergency-call app part of the system app right?

Comment 15

[Doug Sherk (:drs) (inactive)]

(In reply to Etienne Segonzac (:etienne) from comment #14)
> Comment on attachment 8451884 [details] [diff] [review]
> Import dialer.xxx.properties into emergency call app to fix cancel button.
> 
> Review of attachment 8451884 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> lgtm, just confirming: this is compatible with the build time magic making
> the emergency-call app part of the system app right?

Yes.

v2.0: https://github.com/mozilla-b2g/gaia/commit/6b2c470734da5fe4fa261cc345fd9a0ae5535420

Comment 16

[Shally Li（leave from Mozilla）]

This issue has been successfully verified on Flame v2.1&v2.0.
See attachment: verified_v2.1.mp4
Reproducing rate: 0/5

Flame2.0 build:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141127000203
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141127.034818
FW-Date         Thu Nov 27 03:48:29 EST 2014
Bootloader      L1TC00011880

Flame2.1 build:
Gaia-Rev        5372b675e018b6aac97d95ff5db8d4bd16addb9b
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/f34377ae402b
Build-ID        20141127001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141127.035005
FW-Date         Thu Nov 27 03:50:16 EST 2014
Bootloader      L1TC00011880

Comment 17

[Shally Li（leave from Mozilla）]

Attachment: verified_v2.1.MP4