915987 - B2G RIL: Map recordId from ICC Contact to mozContact.id

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

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

In Bug 850098 we're trying to create another DOM object IccContact, and we need the id mapping when we try to edit or delete the SIM contact. But it seems we can reuse the id property from nsIDOMContact, and this id property can be used to identify the record Id (or index) for the contact on SIM.

Comment 1

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

Attachment: Part 1: reuse nsIDOMContact.id.

Comment 2

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

Attachment: Part 2: Update recordId for USIM contact

Comment 3

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

Attachment: Part 3: test case for contact.id

Comment 4

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 804254 [details] [diff] [review]
Part 1: reuse nsIDOMContact.id.

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

::: dom/system/gonk/ril_worker.js
@@ +882,5 @@
>        options.contactType,
>        function onsuccess(contacts) {
>          // Reuse 'options' to get 'requestId' and 'contactType'.
>          options.contacts = contacts;
> +        options.iccid = RIL.iccInfo.iccid;

nit: please also update that comment line to include 'recordId' as well.

Comment 5

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 804255 [details] [diff] [review]
Part 2: Update recordId for USIM contact

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

Can't this patch merge into part 1 line 922?

Comment 6

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

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #5)
> Comment on attachment 804255 [details] [diff] [review]
> Part 2: Update recordId for USIM contact
> 
> Review of attachment 804255 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can't this patch merge into part 1 line 922?

You can see ICCContactHelper.readUSimContact, the recordId and the phonebook set id should be handled inside that function.
I don't think we should do this in Part 1.

Comment 7

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 804255 [details] [diff] [review]
Part 2: Update recordId for USIM contact

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

After clarification from yoshi, we may have two phone book set and each of them have |ICC_MAX_LINEAR_FIXED_RECORDS| records.  The |recordId| attribute could be 0..(2*ICC_MAX_LINEAR_FIXED_RECORDS - 1) and we only need the offset part before diving into |ICCContactHelper.updateUSimContact()|.

Comment 8

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

https://hg.mozilla.org/integration/b2g-inbound/rev/bb2c8956f37b
https://hg.mozilla.org/integration/b2g-inbound/rev/1a4154829e67
https://hg.mozilla.org/integration/b2g-inbound/rev/72c352e6ee38

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bb2c8956f37b
https://hg.mozilla.org/mozilla-central/rev/1a4154829e67
https://hg.mozilla.org/mozilla-central/rev/72c352e6ee38