Closed Bug 784646 Opened 8 years ago Closed 8 years ago

B2G: Cannot import SIM contacts

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-basecamp:+)

RESOLVED FIXED
blocking-basecamp +

People

(Reporter: allstars.chh, Assigned: allstars.chh)

References

Details

Attachments

(2 files)

STR:
Insert a SIM card with contacts in it 
Launch Contact app
Press 'Import SIM Contacts'

From log, I see 

E/GeckoConsole( 1122): [JavaScript Error: "cyclic object value" {file: "jar:file:///system/b2g/omni.ja!/components/ContactManager.js" line: 517}]
E/GeckoConsole( 1122): [JavaScript Error: "NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: '[JavaScript Error: "cyclic object value" {file: "jar:file:///system/b2g/omni.ja!/components/ContactManager.js" line: 517}]' when calling method: [nsIRILContactCallback::receiveContactsList]" {file: "jar:file:///system/b2g/omni.ja!/components/RadioInterfaceLayer.js" line: 427}]

This seems to be a regression from Bug 779794

Also I am wondering how we store the phone number? 

From IDL, that should be tel[index].value, 
http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/contacts/nsIDOMContactProperties.idl#25

but the patch from 779794 uses 'number'
http://mxr.mozilla.org/mozilla-central/source/dom/contacts/ContactManager.js#514

Also from gaia, it also says 'number'
https://github.com/mozilla-b2g/gaia/blob/master/apps/contacts/js/utilities/import_sim_contacts.js#L52

whereas other files use 'value'
https://github.com/mozilla-b2g/gaia/blob/master/apps/contacts/js/contacts_list.js
CC to kaze and gwanger for comments
FTR, the number/value confusion should not affect gaia, as the code involved in gaia only stores mozContact elements without digging through its properties.
Does this still happen with a new gecko build? It should be fixed now.
Works for me now.
It works now,
Should I change to state to WORKSFORME?

Thanks
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
Reopening on Francisco Jordano’s request.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Hi,

unfortunately the import contacts from sim (actually reading sim from contacts) is not working with last builds.
When performing the dom request we always get the onerror callback.

I'm attaching a small gaia app that just tries to call :

var request = navigator.mozContacts.getSimContacts('ADN');

and shows the result in the screen.

Cheers!
blocking-basecamp: --- → ?
Despite being called 'simimport' this is not importing anything, just testing the call to the method:

navigator.mozContacts.getSimContacts('ADN')
Importing SIM contacts is a blocking issue.
blocking-basecamp: ? → +
(In reply to Fabien Cazenave (:kaze) from comment #7)
> Reopening on Francisco Jordano’s request.

Yoshi, can you take a look? The new error callback landed yesterday.

In the future please don't reopen a bug. Please file a new one because the old comments are most likely wrong for the current bug.
(In reply to francisco.jordano from comment #8)
> unfortunately the import contacts from sim (actually reading sim from
> contacts) is not working with last builds.

Note that this may also be a SIM-card specific issue.

> When performing the dom request we always get the onerror callback.

Can you please turn on DEBUG_ALL in gecko/dom/system/gonk/ril_consts.js [1], reflash Gecko, run your tests again, and attach the output of adb logcat as a file to this bug? Thanks!

[1] https://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_consts.js#17
Attached patch PatchSplinter Review
Hi, gwanger
Could you help to review this?
The cause is undefined will become 'undefined' (string) after converted to json.

BTW I still see a gaia error after this patch.

E/GeckoConsole(14195): [JavaScript Error: "TypeError: this is undefined" {file: "app://communications.gaiamobile.org/contacts/js/contacts_list.js" line: 84}]

Thanks
Attachment #665534 - Flags: review?(anygregor)
Comment on attachment 665534 [details] [diff] [review]
Patch

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

::: dom/contacts/fallback/ContactService.jsm
@@ +151,5 @@
>        case "Contacts:GetSimContacts":
>          mRIL.getICCContacts(
>            msg.options.contactType,
>            function (aErrorMsg, aType, aContacts) {
> +            if (aErrorMsg !== 'undefined') {

Wuuut? How would aErrorMsg be the *string* 'undefined'?
(In reply to Philipp von Weitershausen [:philikon] from comment #14)
> Comment on attachment 665534 [details] [diff] [review]
> Patch
> 
> Review of attachment 665534 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/contacts/fallback/ContactService.jsm
> @@ +151,5 @@
> >        case "Contacts:GetSimContacts":
> >          mRIL.getICCContacts(
> >            msg.options.contactType,
> >            function (aErrorMsg, aType, aContacts) {
> > +            if (aErrorMsg !== 'undefined') {
> 
> Wuuut? How would aErrorMsg be the *string* 'undefined'?

Some cross process/thread json stringify magic. I also saw it with the message manager that just declared properties become 'undefined' on the other side.
Ah actually that comes from nsIRILContactCallback calling with an undefined value. Lets check first that this behavior will stay the same in the future.
(In reply to Gregor Wagner [:gwagner] from comment #16)
> Ah actually that comes from nsIRILContactCallback calling with an undefined
> value. Lets check first that this behavior will stay the same in the future.

Yeah, I feel like there's an actual bug lurking in the RIL here. Let's fix it at the root.
Comment on attachment 665534 [details] [diff] [review]
Patch

I talked to bz about it and it turns out that the proper fix is to annotate our IDL like [Undefined(Null)] in DOMString errorMsg. But this also doesn't work because there is some double wrapping going on.

Once we switch to webIDL it should just work :)
Attachment #665534 - Flags: review?(anygregor) → review+
Assignee: nobody → allstars.chh
I cannot push this to inbound now, will do later.

remote: Tree mozilla-inbound is CLOSED! (https://treestatus.mozilla.org/mozilla-inbound?format=json) - Power outage in MTV
remote: To push despite the closed tree, include "CLOSED TREE" in your push comment
remote: transaction abort!
remote: rollback completed
remote: abort: pretxnchangegroup.a_treeclosure hook failed
https://hg.mozilla.org/mozilla-central/rev/c637ee14bccd
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Blocks: 1157082
You need to log in before you can comment on or make changes to this bug.