901887 - [RIL][Contacts] Saving to SIM doesn't work

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

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

+++ This bug was initially created as a clone of Bug #895886 +++

|var request = icc.updateContact('adn', aContact);|

And setting the |onsuccess| and |onerror| on the DOMRequest, we don't go through any of the callbacks.

When enabling the RIL logs we don't get much information, while performing the icc.updateContact we just get:

D/memalloc(  108): /dev/pmem: Allocated buffer base:0x48b00000 size:614400 offset:6144000 fd:80
D/memalloc(  487): /dev/pmem: Mapped buffer base:0x46100000 size:6758400 offset:6144000 fd:34
D/memalloc(  108): /dev/pmem: Allocated buffer base:0x48b00000 size:614400 offset:6758400 fd:104
D/memalloc(  487): /dev/pmem: Mapped buffer base:0x46864000 size:7372800 offset:6758400 fd:43
D/memalloc(  108): /dev/pmem: Allocated buffer base:0x48b00000 size:8192 offset:7372800 fd:111
D/memalloc(  487): /dev/pmem: Mapped buffer base:0x46f86000 size:7380992 offset:7372800 fd:47
D/memalloc(  108): /dev/pmem: Allocated buffer base:0x48b00000 size:8192 offset:7380992 fd:118
D/memalloc(  487): /dev/pmem: Mapped buffer base:0x476a0000 size:7389184 offset:7380992 fd:50

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Hi, Francisco 
Can you show me your gaia repository and branch should I can reproduce by myself?

Comment 2

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

Right, perhaps I'm doing something terrible wrong there :)

Here is how I'm testing:

https://github.com/arcturus/gaia/tree/contacts-export

Go to settings and choose the first export option.

Thanks!

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Hi, Francisco
I've tried your branch,
but seems the error is from your gaia app, 
"contact.email is undefined",

I try to fix the error,
then exporting is fine,
except it seems the Gaia app doesn't export the correct contact.
(for example, I select contact A, and from ril log, it seems contact B is exported).

Comment 4

[Gene Lian (I already quit Mozilla)]

I can reproduce this issue. Let me take this. :)

Comment 5

[Gene Lian (I already quit Mozilla)]

Attachment: Patch

This bug is really hard to catch because the console cannot display any error message when contact.anr is not available.

Anyway, we need to add this check to ensure it can be returned eventually because we don't want to update the anr for sure.

Comment 6

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 786980 [details] [diff] [review]
Patch

Review of attachment 786980 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/ril_worker.js
@@ +12593,5 @@
>  
>        // Check if contact has additional properties (email, anr, ...etc) need
>        // to be updated as well.
>        if ((field === USIM_PBR_EMAIL && !contact.email) ||
> +          (field === USIM_PBR_ANR0 && (!contact.anr || !contact.anr[0]))) {

I think using Array.isArray to check anr is better.

Comment 7

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

I think we also need to update xpcshell test case to test this.

I'll file another bug to test this.
Bug 902788

Comment 8

[Gene Lian (I already quit Mozilla)]

Attachment: Patch, V1.1

Thanks Yoshi for the review and providing the test!

Addressing comments. Ready to land.

Comment 9

[Gene Lian (I already quit Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/86091b087cb6

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/86091b087cb6