928866 - contacts import can fail with "TypeError: aEmail.type is null"

Status: RESOLVED FIXED

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

Description

[Ben Kelly [:bkelly, not reviewing]]

While trying to test the import path today I ran into this error:

E/GeckoConsole( 1130): [JavaScript Error: "TypeError: aEmail.type is null" {file: "app://communications.gaiamobile.org/contacts/js/contacts_merger.js" line: 239}]

Looking at this code in contacts_merger.js, I think the second line should be using `type` instead of `aEmail.type`.

        var type = Array.isArray(aEmail.type) ? aEmail.type : [aEmail.type];
        aEmail.type[0] = aEmail.type[0] || DEFAULT_EMAIL_TYPE;

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

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

Jose, this is a one line change to fix importing of contacts with a single email value.  I think this is broken in v1.2, so we should get this in soon if we can.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

From visual inspection this problem exists in v1.2:

  https://github.com/mozilla-b2g/gaia/blob/v1.2/apps/communications/contacts/js/contacts_merger.js#L239

Nom koi?

Comment 3

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

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

Stealling review.

Looking good to me, could you wait till travis gives the ok? (I know it won't do, but at least the job related to test-agent)

Thanks a lot Ben!

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

FTP server is down so travis can't get firefox.  I ran the tests locally, though, and they look good.

Merged:

  https://github.com/mozilla-b2g/gaia/commit/92731c43532c8205b81be2c9602dd451bcd1c4ed

Comment 5

[Preeti Raghunath(:Preeti)]

QA,

Please check if this issue is a regression from 1.1 or even within the 1.2 dev time frame.

Thanks.

Comment 6

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

can we also understand the steps to reproduce? thanks

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Contacts merging / de-duplication did not exist in v1.1.  So this is new code as of v1.2.  I don't know if you want to call that a regression or not.

I don't have exact steps to reproduce.  I can't remember if I hit this by importing from sim, google, facebook, or sdcard.

The code currently in v1.2 is provably wrong, though, and this is a very low risk change.

If this is inadequate, then just remove the koi nom.

Comment 8

[Jason Smith [:jsmith]]

Sounds like Ben's comment addresses the QA Wanted request.

Comment 9

[Preeti Raghunath(:Preeti)]

QA,

Please help with understanding what the root cause is.

Comment 10

[Jason Smith [:jsmith]]

I'll have a look at this. I'll start a discussion w/Ben & Francisco offline to reverse engineer the root cause here.

Comment 11

[Jason Smith [:jsmith]]

Ben & I discussed this offline. We think this is a fallout from the WebIDL changes for the Contacts API, so we don't have conclusive evidence this will even have impact on 1.2. So moving this to a minus, although this is definitely going to be in 1.3.