938261 - [Contacts] Accessing the tel property of a mozContact when handling the "update" activity throws an error

Status: RESOLVED DUPLICATE of bug 946294

(Core Graveyard :: DOM: Contacts, defect)

Description

[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 in an activity handling, the following error is thrown: [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}]

As additional info:
  - |'tel' in contact'| returns true.
  - Accessing the rest of the properties of a mozContact (id, category, name, email, adr, note, etc.) does not throw any error and seems to work fine.
  - Trying to access a not existent property (such as 'tel1') works as expected returning undefined.

Comment 1

[Reuben Morais [:reuben]]

What pick activity? What's the value of |type| for the phone number you picked?

Comment 2

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

what's the user impact? can you provide an STR?

Comment 3

[Julien Wajsberg [:julienw]]

Joe, see bug 938219.

Comment 4

[Julien Wajsberg [:julienw]]

German, would you please share your contact db with us?

Comment 5

[Julien Wajsberg [:julienw]]

WebIDL related -> 1.3?

Comment 6

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

Damn it, I don't know why I keep on setting the koi? when these issues are related to the recent contacts WebIDL updates ;-999 Sorry guys!

I'll try to get the DB and include it as a new attachment.

Comment 7

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

Attachment: Contacts_DB.zip

Julien, I am able to reproduce this bug on Gecko-4424186 and latest Gaia (as for now). You only have to have 1 contact in your contact list and launch the contact 'pick' activity from any other app. Anyhow, I include (what I think it is) the DB you need :-) If it not what you need, please do not hesitate to guide me through the steps to get the right one.

Comment 8

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

Hi Reuben, I am able to reproduce the bug creating a contact with just a given name and a phone number using the default "mobile" type, and of course selecting it once the contact pick activity is launched ;-) Everything works fine when selecting the contact from the contact list of the Contacts app. In this case, the contact details are properly shown.

Comment 9

[Julien Wajsberg [:julienw]]

Looks good, thanks German!

Reuben, do you plan to have a look on this today? I won't have time to look before tomorrow.

Comment 10

[Reuben Morais [:reuben]]

Yep.

Comment 11

[Reuben Morais [:reuben]]

(In reply to gtorodelvalle from comment #8)
> Hi Reuben, I am able to reproduce the bug creating a contact with just a
> given name and a phone number using the default "mobile" type, and of course
> selecting it once the contact pick activity is launched ;-)

What contact pick activity? It works for me on the SMS app.

Comment 12

[Reuben Morais [:reuben]]

See comment #11.

Comment 13

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

Hey Reuben, I have just re-tested it with Gecko-4424186 and latest Gaia from master and I am able to reproduce the issue according to the guidelines included in bug 938219. Of course, if using he SMS app the phone number where the SMS was sent can't be already associated to a contact to get the "Add to existing contact" option. Anyhow, I would say it is even more straightforward to reproduce it using the Dialer app. Just type some numbers in the Dialer app and click on the "Add contact" button on the lower left area of the Dialer. Do you get the selected contact to add that phone number to in edit mode after selecting it from the contact list? Thanks!

Comment 14

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

Please, see comment 13

Comment 15

[Tom Jao [:mbrsl]]

There's an app called "UI tests" if you use eng build of gaia. It may be helpful for you to test contact activities.
1. Launch UI tests app
2. Click on API tab at the bottom bar
3. Click on Contacts
4. If you don't have any contacts, click on "Insert fake contacts." This may take some time since it inserts 500 contacts.
5. Click on activity buttons you want to test. They're at the bottom

Comment 16

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

triage: blocks a blocker. 1.3+

Comment 17

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

Removing no longer needed need-info ;-) Thanks!

Comment 18

[Julien Wajsberg [:julienw]]

So, when using the use case from bug 938261, I get:

tel is {"value":"1235666","carrier":null,"type":"mobile"}

Whereas all other uses got me something like:

tel is {"type":[""],"value":"0267397915","carrier":null,"pref":null,"__exposedProps__":{"type":"rw","value":"rw","carrier":"rw","pref":"rw"}}

The log is in the validateArrayField callback.

So clearly there is something strange happening in this code path.

Should we run the validateArrayField the "type" property as well? I must admit I don't understand well what validateArrayField is doing.

Comment 19

[Reuben Morais [:reuben]]

validateArrayField is a wallpaper fix for not having arrays yet (bug 946294 will fix this). I'm not sure the problem is related to it.

This looks like the same type of bug with JSONifiying mozContact objects that I fixed in bug 929492 and bug 930241.

(In reply to Julien Wajsberg [:julienw] from comment #18)
> So, when using the use case from bug 938261, I get:
> 
> tel is {"value":"1235666","carrier":null,"type":"mobile"}

|"type": "mobile"| sounds like trouble. I thought the picker was fixed to not return scalars for the array properties.

Comment 20

[Julien Wajsberg [:julienw]]

So, does this come directly from the Contacts app?

Could we throw an earlier error when setting the tel.type to a non-array property?

Comment 21

[Julien Wajsberg [:julienw]]

See bug 938219 comment 11 for the root cause in the Contacts app.

Now, while the fix in Contacts is very easy, I'd like to have an early failure for array values, just like we do for other WebIDL values.

So Reuben, should we call validateArrayField on the "type" property? And shouldn't we call validateArrayField at setter time instead of getter time? I think earlier errors are better for everyone.

Comment 22

[Julien Wajsberg [:julienw]]

Bug 946294 will bring us wonderful WebIDL arrays that will let us do as much sanitization as possible. I'm duping there then.