938219 - [Contacts] Unable to select a contact via the contact "update" activity

Status: VERIFIED FIXED

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

Description

[Germán Toro del Valle (:gtorodelvalle)]

TO REPRODUCE:
1. Create at least one contact in case there is no contacts created.
2. Go to any application which may launch the Contacts app "pick" activity such as the Dialer app (via the "add contact" button on the lower left area), the Email app (via clicking on any email address) or SMS app (clicking on the header of a SMS thread). Launch the contact pick activity.
  2.1. In case of the Dialer app:
    2.1.1. Type a phone number.
    2.1.2. Click on the "add contact" button.
    2.1.3. Select the "Add to existing contact" option.
  2.2. In case of the Email app:
    2.2.1. Open any email.
    2.2.2. Click on the email address of the sender or any of the addressees.
    2.2.3. Select the "Add to existing contact" option.
  2.3. In case of the SMS app:
    2.2.1. Open any SMS thread.
    2.2.2. Click on the header of the SMS thread.
    2.2.3. Select the "Add to existing contact" option.
3. The contact list is shown letting the user to select any contact.
4. Click on any contact from the list.

EXPECTED RESULT:
5. The details of the selected contact are shown in edit mode.

ACTUAL RESULT:
5. Nothing happens.

Comment 1

[Germán Toro del Valle (:gtorodelvalle)]

For some strange reason, when accessing the 'tel' property of a contact (no matter if using dot notation or Array notation) from the Contacts app when handling the pick activity throws the following error: [JavaScript Error: "Argument 1 of ContactTelField.initialize can't be converted to a sequence." {file: "jar:file:///system/b2g/omni.ja!/components/ContactManager.js" line: 245}]

I just created bug 938261 for DOM:Contacts.

Comment 2

[Preeti Raghunath(:Preeti)]

Is contact manager failing? What device is in question when tested?

Comment 3

[Jose Manuel Cantera]

are we sure this reproduces on v1.2? it seems a WebIDL regression and that code has not landed on v1.2

Comment 4

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

+1 to Jose comment, the error looks like it's related to webidl changes.

Comment 5

[Germán Toro del Valle (:gtorodelvalle)]

Yeah guys, you are right ;-) Rafa from QA just confirmed that, obviously, it does not reproduce in 1.2 since it is related to the latest WebIDL changes ;-) Thanks for pointing that out ;)

Comment 6

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

traige: regression 1.3+

Comment 7

[Julien Wajsberg [:julienw]]

Correction for comment 1: it doesn't fail for "pick", nor for "open", nor for "new". It fails only for the "update" activity.

It also doesn't fail when showing a contact from the contacts app outside of an activity.

German, I'm trying to debug bug 938261. Can you please show me the place where the "update" activity fails in the contacts code?

Comment 8

[Germán Toro del Valle (:gtorodelvalle)]

Hi Julien! For example, when trying the Dialer case (2.1.1 to 2.1.3), I get:

E/GeckoConsole(  713): [JavaScript Error: "Argument 1 of ContactTelField.initialize can't be converted to a sequence." {file: "jar:file:///system/b2g/omni.ja!/components/ContactManager.js" line: 245}]
E/GeckoConsole(  713): [JavaScript Error: "NS_ERROR_UNEXPECTED: " {file: "app://communications.gaiamobile.org/contacts/js/views/form.js" line: 256}]

So, regarding the Contacts app, the line is https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/views/form.js#L256 which is the place where the 'tel' property is being accessed. Anyhow, as you see in the logs, there is a problem with ContactManager.js in line 245 :-(

Hope that helps ;-)

Comment 9

[Julien Wajsberg [:julienw]]

Yep, I've found the issue in the contacts API, but I try to understand the whole picture to see where something should be fixed.

Thanks, I should have found this line myself! :)

Comment 10

[Germán Toro del Valle (:gtorodelvalle)]

No no, no problem ;-)

Comment 11

[Julien Wajsberg [:julienw]]

So, the problem initially comes from [1]: the type is not an array here. Then the 'extras' parameter is eventually added to the contact in [2].

I suspect the same issue will happen for email just below [1] too.

So the fix in the Contacts app is easy, but I'll see in bug 938261 if we can get an early error instead of finding the error later in the code.

German, can I let you fix the contacts code?

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/contacts.js#L284-L300
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/contacts.js#L83

Comment 12

[Julien Wajsberg [:julienw]]

Filed bug 948473 and bug 948472 to add integration tests around these activities.

Comment 13

[Julien Wajsberg [:julienw]]

German, by the way, for next time, it's way easier to have a reduced testcase (that is: a very simple page and code that demonstrates the problem and the problem only) to fix issues. In case of a Gecko issue, it saves time for the Gecko developer, and in this case, the coding issue would have been made obvious. So it's a Win in all situations! :-)

Comment 14

[Germán Toro del Valle (:gtorodelvalle)]

Hi Julien, thanks for the hint :-)

Regarding the provided test case, just wanted to provide as many information as possible at the time of the finding of the issue. But totally agree with you when possible ;-)

Comment 15

[Germán Toro del Valle (:gtorodelvalle)]

Attachment: 14581.html

Comment 16

[Jose Manuel Cantera]

Comment on attachment 8345963 [details]
14581.html

looks good German

thanks!

Comment 17

[Germán Toro del Valle (:gtorodelvalle)]

Merged in master: https://github.com/mozilla-b2g/gaia/commit/390b313a254a947d12e3cdbcde19d7d1619ff63c

Comment 18

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

Uplifted 390b313a254a947d12e3cdbcde19d7d1619ff63c to:
v1.3: 202c1506782525ace1cc006feddf428e2ba8512f

Comment 19

[isabelrios]

Verified on latest buri 1.3 build (01/08):

Gecko-e499b1f
Gaia-d578331