1069817 - [sms] localization missing for unknown contacts

Status: RESOLVED FIXED

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

Description

[ankit93040]

User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/37.0.2062.120 Safari/537.36

Steps to reproduce:

go to sms->new sms->To Field

Type an unknown number


Actual results:

unknown contact seen in english irrespective of language


Expected results:

unknown contact should be localized

Comment 1

[Julien Wajsberg [:julienw]]

[Blocking Requested - why for this release]:

Comment 2

[Maria Angeles Oteo (:oteo)]

Comms triage: too late for Late-l10n changes...

Comment 3

[ankit93040]

Attachment: Pull Request

Please merge to master. Thanks

Comment 4

[Francesco Lodolo [:flod]]

Ankit, have you tried this on a phone? Because I'd assume it breaks the view for known existing contacts.
Also, localization comment is partial and completely unrelated to the string.

Having said that, it's definitely a bug but it can probably be fixed without a new string and using the existing unknown-contact (which seems currently unused), so it might be 2.1 material.

Comment 5

[Julien Wajsberg [:julienw]]

We have a "unknown-contact" string but it doesn't have the right value :( 

Not sure what's the good path forward: is it better to have a weird text, or an english text?

Comment 6

[Francesco Lodolo [:flod]]

The key was 'unknown-contact', so it's clear that "Unknown" was referring to a contact. 

We can keep "Unknown" on 2.1 and fix it properly on master with a new string, it's definitely better than showing an English string.

Comment 7

[Steve Chung [:steveck]]

Comment on attachment 8492907 [details] [review]
Pull Request

Hi Ankit, since flod though using "Unknown" should be enough for 2.1, please left the localization as it is and reuse it for unknown contacts per comment 6 if you want to uplift to previous version.

Hi julien, sorry I 'm not sure about the 'weird text' you mentioned here, could you give an example for it? or maybe we could simply set content empty?

Comment 8

[Julien Wajsberg [:julienw]]

I meant that showing the string "Unknown" at that moment would be strange. Also I don't know what's the state of the translations and if they still can be changed at that time, because they probably were used without much context in that case. Since we don't ship the phone in any english-speaking country as far as I know, so our paying customers would only get translations, and maybe we can still get translations right?

About keeping the content empty, I think this is a good option. Then we'd have only the phone number and an empty line above. What do you think Jenny, is it good enough for v2.1? In v2.2 we'd have the proper translated string.

Comment 9

[Jenny Lee]

Hmmm I am ok with showing only number, but can we make the number would be at half of the line height? Otherwise it'd look unbalanced. Thanks!

Comment 10

[Julien Wajsberg [:julienw]]

Steve, can we land ankit's patch on master and do another patch for v2.1?

Comment 11

[Steve Chung [:steveck]]

(In reply to Julien Wajsberg [:julienw] from comment #10)
> Steve, can we land ankit's patch on master and do another patch for v2.1?

Ok, but the original patch got a conflict here. Will solved it and re-commit patch later.

Comment 12

[Steve Chung [:steveck]]

Attachment: Link to github

Hi Oleg, this patch is for master. I'll create another patch for 2.1 which only reuse the existing string.

Comment 13

[Steve Chung [:steveck]]

Attachment: Link to github(for 2.1)

It's the 2.1 patch that reuse the existing "unknown" string like comment 6 said. Although I proposed removing string before, inventing another layout at this moment seems not safe even the changes might be trivial. So I choose the method with minimal changes here.

Comment 14

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8500217 [details] [review]
Link to github

Looks good to me, thanks!

Comment 15

[Steve Chung [:steveck]]

Comment on attachment 8500220 [details] [review]
Link to github(for 2.1)

Redirect to Oleg since Julien is on PTO.

Comment 16

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8500220 [details] [review]
Link to github(for 2.1)

r=me, thanks!

Comment 17

[Steve Chung [:steveck]]

In master: e6c0dfd544380a07b7281379eb23e0731ac5fb16

Comment 18

[Steve Chung [:steveck]]

Comment on attachment 8500220 [details] [review]
Link to github(for 2.1)

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 959018
[User impact] if declined: 'unknown contact' string will not translate
[Testing completed]: N/A
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: N/A

Comment 19

[bhavana bajaj [:bajaj]]

Comment on attachment 8500220 [details] [review]
Link to github(for 2.1)

Approving the low risk work around to improve the localization experience.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/14bb86d2109b7b6b8a83040bc97e42c1ff9c3b02

Comment 21

[Lance（leave from Mozilla）]

This issue has been verified successfully on Flame 2.1. But verified to fail on Flame 2.2.
See attachment: 1659.MP4 and logcat_1659.txt
Flame 2.1 Reproducing rate: 0/5
Flame 2.2 Reproducing rate: 5/5

Step:
1.Launch Message.
2.Create a new message, input the phone number in receiver box.
3.Switch the system language to other(ex:Japanese, Korean, Simplified Chinese, isiZulu...).
4.Then back to edit message page.

Actual result:
Flame 2.1: The "Unknown" will be localized.
Flame 2.2: When you switch the language to Korean/isiZulu or other, the "Unknown Contact" can not be localized.

Flame 2.1 version:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-2g34_v2_1/rev/18fb67530b22
Build-ID        20141202001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141202.034824
FW-Date         Tue Dec  2 03:48:34 EST 2014
Bootloader      L1TC00011880

Flame 2.2 version:
Gaia-Rev        725685831f5336cf007e36d9a812aad689604695
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/2c9781c3e9b5
Build-ID        20141202040207
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141202.072347
FW-Date         Tue Dec  2 07:23:58 EST 2014
Bootloader      L1TC00011880

Comment 22

[Lance（leave from Mozilla）]

Attachment: logcat_1659.txt

Comment 23

[Lance（leave from Mozilla）]

Attachment: 1659.MP4

Comment 24

[Julien Wajsberg [:julienw]]

Hey Steve, can you check this?

It's likely because the l10n files are not uptodate for v2.2 yet but better be sure :)

FTR http://hg.mozilla.org/gaia-l10n/ is the place where authoritative locales are located.

Comment 25

[Steve Chung [:steveck]]

(In reply to Julien Wajsberg [:julienw] from comment #24)
> Hey Steve, can you check this?
> 
> It's likely because the l10n files are not uptodate for v2.2 yet but better
> be sure :)
> 
> FTR http://hg.mozilla.org/gaia-l10n/ is the place where authoritative
> locales are located.

Yes, it's clearly that some language's l10n files are not sync with latest changes.
In this video, we can see the language in Japanese is correct but not translated in Korean, and the latest ko l10n file changes are browser deletion on 10/14, and this patch's l10n changes is still not applied yet.

Comment 26

[Francesco Lodolo [:flod]]

http://transvision.mozfr.org/string/?entity=apps/sms/sms.properties:unknown-contact-title&repo=gaia

Don't test localization on master unless you know what you're checking. Usually French or Italian and good locales to pick for that ;-)