Closed Bug 938261 Opened 6 years ago Closed 6 years ago

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

Categories

(Core Graveyard :: DOM: Contacts, defect)

x86
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.3+)

RESOLVED DUPLICATE of bug 946294
1.3 Sprint 6 - 12/6
blocking-b2g 1.3+

People

(Reporter: gtorodelvalle, Assigned: julienw)

References

Details

Attachments

(1 file)

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.
Blocks: 938219
blocking-b2g: --- → koi?
What pick activity? What's the value of |type| for the phone number you picked?
what's the user impact? can you provide an STR?
Flags: needinfo?(gtorodelvalle)
Joe, see bug 938219.
Flags: needinfo?(gtorodelvalle)
German, would you please share your contact db with us?
Flags: needinfo?(gtorodelvalle)
WebIDL related -> 1.3?
blocking-b2g: koi? → 1.3?
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.
Flags: needinfo?(gtorodelvalle)
Attached file 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.
Flags: needinfo?(felash)
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.
Looks good, thanks German!

Reuben, do you plan to have a look on this today? I won't have time to look before tomorrow.
Flags: needinfo?(felash)
Yep.
Flags: needinfo?(reuben.bmo)
(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.
Flags: needinfo?(reuben.bmo)
See comment #11.
Flags: needinfo?(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!
Flags: needinfo?(gtorodelvalle)
Please, see comment 13
Flags: needinfo?(reuben.bmo)
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
triage: blocks a blocker. 1.3+
blocking-b2g: 1.3? → 1.3+
Assignee: nobody → felash
Depends on: 931711
Target Milestone: --- → 1.3 Sprint 6 - 12/6
Removing no longer needed need-info ;-) Thanks!
Flags: needinfo?(reuben.bmo)
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.
Flags: needinfo?(reuben.bmo)
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.
Flags: needinfo?(reuben.bmo)
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?
Flags: needinfo?(reuben.bmo)
Summary: [Contacts] Accessing the tel property of a mozContact when handling the pick activity throws an error → [Contacts] Accessing the tel property of a mozContact when handling the "update" activity throws an error
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.
Bug 946294 will bring us wonderful WebIDL arrays that will let us do as much sanitization as possible. I'm duping there then.
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(reuben.bmo)
Resolution: --- → DUPLICATE
Duplicate of bug: 946294
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.