898241 - [Buri][Contacts][Language]"Mobile" can't be translated correctly

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Contacts, defect, P1)

Description

[sync-1]

AU_LINUX_GECKO_ICS_STRAWBERRY.01.01.00.019.164
 Firefox os  v1.1
 Mozilla build ID:20130715070218
 
 
 
 
 Created an attachment (id=470246)
 pic
 
 DEFECT DESCRIPTION:
  "Mobile" can't be translated correctly
  REPRODUCING PROCEDURES:
  1.New contact A,with phone numbers(such as:123456),Set language as English
  2.Dial screen->input some numbers(such as:654321)->Add to existing contact->Select Contact A->Update
  3.Change language to Francais->Back to Contacts screen->Edit Contact A->It displayed "Mobile" as usual---->KO
  EXPECTED BEHAVIOUR:
  It should stay the same
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
  Mid
  REPRODUCING RATE:
  5/5
  For FT PR, Please list reference mobile's behavior:

Comment 1

[sync-1]

Clone from brother

Comment 2

[sync-1]

Attachment: log

Comment 3

[sync-1]

Clone from brother

Comment 4

[sync-1]

Attachment: pic

Comment 5

[buri.blff]

please update status,thanks.

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

Isn't 'mobile' also what we want to show when in French?

Comment 7

[Julien Wajsberg [:julienw]]

That's what I'd say but will ask Théo as he does the translation.

BTW the 'expected behavior' seems to be wrong.

Also, does it work for other locales ?

Comment 8

[Théo Chevalier]

Attachment: Bug fr > es

This bug seems not related to French, I updated the contact while in French, then I changed language to Spanish, and as you can see, on three fields, only one has been updated to the current locale.

But, for French we could use "mobile" too, yes. We use both: http://transvision.mozfr.org/?repo=central&sourcelocale=en-US&locale=fr&search_type=strings&recherche=mobile

Comment 9

[Jason Smith [:jsmith]]

Sounds like comment 8 concludes this probably related to a language change while the contacts app is open is causing certain text fields to not get updated to the new language.

Comment 10

[Julien Wajsberg [:julienw]]

Another possibility is that the reporter does not use a multilocale build, and therefore gets incomplete locales from Gaia repository.

I don't know enough about this app. Jose, could you take this and see if this should be correctly translated with the current code ?

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

I think I see the problem.

The "type" string is being created as "Mobile" instead of "mobile" when the tel field is updated from an activity.  This in turn causes the code to think we are dealing with a custom type entry which should not be translated.

This is in gaia/master and looks like it will effect email addresses updated via activity as well.

I'll have a pull request ready shortly.  I just want to add a test case.

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Pull request at https://github.com/mozilla-b2g/gaia/pull/11233

Here is a pull request that fixes the issue.  It uses the field type string instead of the current translation for that type when creating new tel and email fields.

I investigated writing a unit test, but I did not see an easy way to trigger the activity path with our currently infrastructure.  The input I got from #gaia was to wait for the new integration test framework that should be available soon.

I tested both the update activity for both the tel and email fields on my device.

Comment 13

[Julien Wajsberg [:julienw]]

we have unit tests for the SMS activity handler there : https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/test/unit/activity_handler_test.js#L89

This mocks the setMessageHandler function so that we can programmatically trigger an activity. Works quite well :)

Comment 14

[buri.blff]

(In reply to Ben Kelly [:bkelly] from comment #11)
> I think I see the problem.
> 
> The "type" string is being created as "Mobile" instead of "mobile" when the
> tel field is updated from an activity.  This in turn causes the code to
> think we are dealing with a custom type entry which should not be translated.
> 
> This is in gaia/master and looks like it will effect email addresses updated
> via activity as well.
> 
> I'll have a pull request ready shortly.  I just want to add a test case.

we change the "Mobile" to "mobile" with function "toLowercase", but it still can't be translated.

Comment 15

[Ben Kelly [:bkelly, not reviewing]]

(In reply to buri.blff from comment #14)
> (In reply to Ben Kelly [:bkelly] from comment #11)
> > I think I see the problem.
> > 
> > The "type" string is being created as "Mobile" instead of "mobile" when the
> > tel field is updated from an activity.  This in turn causes the code to
> > think we are dealing with a custom type entry which should not be translated.
> > 
> > This is in gaia/master and looks like it will effect email addresses updated
> > via activity as well.
> > 
> > I'll have a pull request ready shortly.  I just want to add a test case.
> 
> we change the "Mobile" to "mobile" with function "toLowercase", but it still
> can't be translated.

If you mean you changed "Mobile" to "mobile" by editing the contact in the app, then this is inadequate.  It requires code changes.  See the pull request in comment 13.

Comment 16

[Ben Kelly [:bkelly, not reviewing]]

Sorry, I meant the pull request in comment 12.  Also, I apologize if my earlier comment was unclear.  Thanks for trying to resolve on your end.

Comment 17

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Julien Wajsberg [:julienw] (PTO 15th -> 28th July) from comment #13)
> we have unit tests for the SMS activity handler there :
> https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/test/unit/
> activity_handler_test.js#L89
> 
> This mocks the setMessageHandler function so that we can programmatically
> trigger an activity. Works quite well :)

Thanks.  I'll check it out, time permitting.  (I'm supposed to be working other perf related bugs...)

Comment 18

[Jose Manuel Cantera]

Comment on attachment 782784 [details]
Pull request at https://github.com/mozilla-b2g/gaia/pull/11233

review positive, but I believe we should open a follow-up bug to report that (as Jason said in another comment) if the language changes and you are on the contact details page, the labels for email or telephone are not translated.

thanks, Ben

Comment 19

[Ben Kelly [:bkelly, not reviewing]]

Thanks Jose.  I believe bug 898992 probably already captures the failure to update when the language is changed while the app is running.

Comment 20

[Ben Kelly [:bkelly, not reviewing]]

Merged:

  https://github.com/mozilla-b2g/gaia/commit/9a802a4b2e6737b914bcab73376f097761875081

By the way, I should also probably note that this fix will only correct the translation for new phone numbers added via the activity update.  If you have a phone number that was added with the previous, broken code it will remain mis-translated.  To test the new, fixed code you need to delete the phone number from the contact and re-add it from the dialer app.  Sorry for the inconvenience.

Comment 21

[bhavana bajaj [:bajaj]]

IS this a regression ? Adding qawanted to see what the behaviour is in 1.0.1.

Comment 22

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

https://github.com/mozilla-b2g/gaia/blob/v1.0.1/apps/communications/contacts/js/contacts.js#L333 shows that 1.0.1 should be affected as well.

Comment 23

[Natalya Kot [:nkot]]

agreed with comment 22, same behavior on v1.0.1

retested on Unagi 7/29 v1.0.1 build, tested with "Deutsch" - "Mobile" is untranslated

Build ID: 20130729070220
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/9c62297d11b0
Gaia: 054cdc27404e2daca91d3065d9783681032b2151
Platform Version: 18.0

Comment 24

[Lukas Blakk [:lsblakk] use ?needinfo]

Blocking since it seems low risk and reduces confusion, let's uplift to v1.1

Comment 25

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 9a802a4b2e6737b914bcab73376f097761875081 to:
v1-train: feed8ab34e5fea20ac8f2f33af883123797f7278

Comment 26

[buri.blff]

(In reply to John Ford [:jhford] -- please use 'needinfo?' instead of a CC from comment #25)
> Uplifted 9a802a4b2e6737b914bcab73376f097761875081 to:
> v1-train: feed8ab34e5fea20ac8f2f33af883123797f7278

Dear John Ford:
   As the commit 20 say, the patch can't resolve this pr completely.
   is the merged patch diffrent from commit 20?

Comment 27

[Ben Kelly [:bkelly, not reviewing]]

(In reply to buri.blff from comment #26)
> (In reply to John Ford [:jhford] -- please use 'needinfo?' instead of a CC
> from comment #25)
> > Uplifted 9a802a4b2e6737b914bcab73376f097761875081 to:
> > v1-train: feed8ab34e5fea20ac8f2f33af883123797f7278
> 
> Dear John Ford:
>    As the commit 20 say, the patch can't resolve this pr completely.
>    is the merged patch diffrent from commit 20?

Answering for John since he just took care of the uplift.

Are you asking for additional changes to avoid the need to delete and re-add phone numbers?  It was not clear this was needed.  Writing something to detect and correct the condition could be problematic as the user can legitimately add any text for a custom field.  Also, it raises the issue of when is the correction done and how long do we need to maintain the fixup heuristic.  Is it possible to simply have a support FAQ providing the steps to correct the condition for affected contacts?

Or are you asking about the additional work I bug 898992?

Comment 28

[Julien Wajsberg [:julienw]]

The uplifted patch is quite never different than the master patch.

As Ben, I think it's not worth the work correcting the problem after the fact. Even if there is a bug the user will get used to this, and we can't change it automatically, it may disturb him. I really think a manual action is preferable here.

Ben, we can probably ask UX though, and file a new bug if necessary ?

Comment 29

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Julien Wajsberg [:julienw] from comment #28)
> The uplifted patch is quite never different than the master patch.

I had difficulty parsing this sentence.  :-)  I did go back and verify the uplift is the same as the original master commit.  I also tested on v1-train to verify that the fix does indeed work there.

> Ben, we can probably ask UX though, and file a new bug if necessary ?

I agree.  If manually correcting the data in the contacts DB is not acceptable, please open a new bug to indicate we need a data cleanup procedure.

Thank you.

Comment 30

[Tim Guan-tin Chien [:timdream] (please needinfo)]

v1.1.0hd: feed8ab34e5fea20ac8f2f33af883123797f7278

Comment 31

[Deepa Subramanian]

The issue does not reproduce in Buri v1.1. But reproduces on leo device for v1.1.
Please see screenshot attached. 

Buri Build ID: 20130809041203
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/45480132106b
Gaia: c9d0901564cf6f50e375ab48e4124b8378a2e246
Platform Version: 18.1

Leo Build ID: 20130809041203
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/45480132106b
Gaia: c9d0901564cf6f50e375ab48e4124b8378a2e246
Platform Version: 18.1

Comment 32

[Deepa Subramanian]

Attachment: screenshot - 8/9/2013

Comment 33

[Julien Wajsberg [:julienw]]

Deepa: we don't do any migration in this patch. Therefore you need to test with adding contacts _after_ the patch is applied. Is it what you did on the leo device ?

Comment 34

[Deepa Subramanian]

Hello Julien, 

Today, I tested both on leo and buri. Manually creating the contact and edit them. The string "Mobile" is translated in French as "Portable". 

Leo Build ID: 20130812041203
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/36bbc5943448
Gaia: c60e3507d9df160e006d2e25967d26df2dc543cb
Platform Version: 18.1

Other notes:
This is what I did the other day with leo device. When importing contacts from the SIM, the user sees the string "Mobile" untranslated. LEt me know if you want me to report a separate bug for this issue.

Comment 35

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Deepa Subramanian from comment #34)
> Other notes:
> This is what I did the other day with leo device. When importing contacts
> from the SIM, the user sees the string "Mobile" untranslated. LEt me know if
> you want me to report a separate bug for this issue.

I think that is worth writing a separate bug.  It looks like when we import a contact we simply copy whatever the vcard type is over into the contact.  If the vcard type is "Mobile", then it will not get translated as the magic l10n type is "mobile".

Comment 36

[Deepa Subramanian]

Please see this bug https://bugzilla.mozilla.org/show_bug.cgi?id=904261