850098 - B2G RIL: Add nsIDOMIccContact

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: RIL, defect)

Description

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

For updating SIM contacts, we also need an identifier for the contact to be updated.
This identifier can be mapped to the physical location (record ID of EF_ADN/EF_FDN) for the SIM contacts stored inside the SIM.

Comment 1

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

Attachment: WIP - Part 1: Add nsIDOMIccContact.

Comment 2

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

Attachment: WIP - Part 2: DOM, update IccManager

Comment 3

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

Attachment: WIP - Part 3: return nsIDOMIccContact.

Comment 4

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

Hi, Jonas
In your previous comment, [1], you said we should create an ID for the SIM contact.
After I dicussed with Vicamo, 
he thinks we dont have to add another nsIDOMIccContact nor add a recordId property for this.

His idea is since Gaia won't care about this 'ID' of the SIM contact, we could still keep the 'ID' in the implementation without exposing it to the web.

Gaia should update the SIM contact which is retrieved from importing SIM contact.

// Gaia sample code
var req = icc.readContacts("adn");  // returns nsIDOMContact[]
req.onsucces = function() {
  icc.updateContact("adn", req.result[0]); // Update the 1st SIM contact
}

In implementation, we could store the 'ID' in content side, for example RILContentHelper.js, or in some properties (like catogory, as importing Facebook contacts do)


How do you think about this?

Thanks

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=847820#c36

Comment 5

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

Comment on attachment 731799 [details] [diff] [review]
WIP - Part 1: Add nsIDOMIccContact.

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

::: dom/icc/interfaces/nsIDOMIccManager.idl
@@ +13,3 @@
>  
> +[scriptable, builtinclass, uuid(620f09e1-f72c-4570-ab6d-5795c17d6c6a)]
> +interface nsIDOMMozIccContact : nsIDOMContactProperties

After discussed with Vicamo, he thinks I should try to avoid inheritance or at lease inherit nsIDOMContact.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'm somewhat worried about having objects that look just like ordinary DOMContact objects but that has some sort of internal id which means that it does special things when you save the contact.

It also means that you have to load a contact in order to overwrite it, just so that you get a reference to a DOMContact object.

What type of internal identifier would we have in the IccContact object? Could we expose that as an opaque string?

Also, is there a need to be able to add and remove contacts to the simcard? Is that supported in the current proposal?

Comment 7

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

Hi, Jonas
Thanks for your comments,
(In reply to Jonas Sicking (:sicking) from comment #6)
> I'm somewhat worried about having objects that look just like ordinary
> DOMContact objects but that has some sort of internal id which means that it
> does special things when you save the contact.
> 

Yes, if the contact has the internal id, we would 'update' the existing contact according to the internal id.

> It also means that you have to load a contact in order to overwrite it, just
> so that you get a reference to a DOMContact object.
> 

Yes.
Or do you think we still need to expose the id to Web, so Gaia can choose which location it wants to store the contact?

> What type of internal identifier would we have in the IccContact object?
> Could we expose that as an opaque string?
> 

Currently it should be an 'unsigned long', but we could change that in this bug.
(The patches are still in WIP, I'll update it later.)

> Also, is there a need to be able to add and remove contacts to the simcard?
> Is that supported in the current proposal?

Yes, the requirement is from partners, FirefoxOS needs to update(add, modify or remove) SIM contact.

So in order to 'modify' or 'remove' the contact, we need some 'ID' to identify it, that's what we thought in the first place and created this bug.

My idea is to add an identifier like the id attribute in nsIDOMContact of Contact API.
But Vicamo's comment is does Gaia really need this 'ID', do we need to expose this ID to the Web?

If we pass the same object (which we got from icc.readContacts) to updateContact, then we can do 'modify' or 'remote' according to the hidden internal ID, in that case we don't have to add another attribute ID to IDL.

I am not sure if this internal hidden ID is okay from Web API perspective, 
so I'd like to ask your opinion on this.

How do you think? 

Thanks


Thanks

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Is the ID a real id that's actually stored on the card? Would it remain consistent if the SIM card is removed and then re-inserted? What about if other contacts are added and removed? If yes, I think it would be ok to expose it.

I think simply using the normal nsIDOMContact interface and setting the .id to something like "12345" might work?

Comment 9

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

(In reply to Jonas Sicking (:sicking) from comment #8)
> Is the ID a real id that's actually stored on the card? Would it remain
> consistent if the SIM card is removed and then re-inserted? What about if
> other contacts are added and removed? If yes, I think it would be ok to
> expose it.

Limited number of SIM contacts can be stored in the SIM card. The ID we talk about here is actually the index of the SIM contact stored in a continuous array. So that ID persists after being re-inserted if it itself is not removed.

To UPDATE the SIM contact, we need the INDEX. So no matter you're going to expose it as a string or not, you have to read it out first. Or, we allow blind update -- create a contact, specify that INDEX and write to SIM card. This action certainly need knowledge to the SIM card, for example, the maximum capacity. I find no reason one would actually ever want to write a contact to a specified indexed record because the usual use cases in updating SIM contacts are: 1) add a new one, 2) remove an existing one, 3) update an existing one. In these use cases, Gaia never touches this INDEX field, so I don't think we have to expose it as a public attribute. Then the API means:

  1. updateContact(A contact with INDEX) -- update ICC contact, of course. If phone
                                            numbers are missing, the record will be
                                            cleared instead.

  2. updateContact(A contact without INDEX) -- add a new ICC contact.

> I think simply using the normal nsIDOMContact interface and setting the .id
> to something like "12345" might work?

One of my idea. We can pre-gen a share UUID as prefix, and replace the last few characters with the INDEX of each retrieved ICC contact. But still have some internal technical problems though.

Comment 10

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

(In reply to Jonas Sicking (:sicking) from comment #8)
> Is the ID a real id that's actually stored on the card?
The ID can be mapped to the actual location stored on SIM card.

The background is that for SIM cards, it uses two levels of indirections, like 
"Phonebook set 1, record Id 120", 
"Phonebook set 2, record Id 31", ..
each phonebook set can have at most 255 entries.

> Would it remain consistent if the SIM card is removed and then re-inserted?
Yes.

But two SIM contacts on two difference SIM cards might have the same 'recordId'.
Do we need to make the recordId 'unique' ?

> What about if other contacts are added and removed? 
Yes, still remains consistent.

>If yes, I think it would be ok to
> expose it.
> 
> I think simply using the normal nsIDOMContact interface and setting the .id
> to something like "12345" might work?

The .id attribute is used by Contact API implementation.
In mozContacts.save, it would check the contact has the 'id' property, if it does, it will update the existing contact stored in the DB, otherwise it just store a new contact in DB.

See http://mxr.mozilla.org/mozilla-central/source/dom/contacts/ContactManager.js#587

So if we're going to reuse the .id attribute, unless we modify the ContactManager, otherwise we cannot save the contact imported from SIM to ContactDB.

Comment 11

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I think we should expose the low-level primitives as to not prevent any particular UIs here.

I don't think we need to create anything complicated though, lets try to keep it simple.

So when reading contacts from the sim card, I think you should get an object which contains the phonebook-set, the recordId and a nsIDOMContactProperties. I.e. something like:

interface nsIDOMSIMContact {
  attribute DOMString phoneBookSet;
  attribute DOMString recordId;
  attribute nsIDOMContactProperties properties;
}

or, to follow the style of nsIDOMContact

interface nsIDOMSIMContact : nsIDOMContactProperties {
  attribute DOMString phoneBookSet;
  attribute DOMString recordId;
}

When writing a contact, you should be able to pass in a phonebook-set, a recordId and a nsIDOMContactProperties.

Comment 12

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

Now we use mozContact, and the id property in mozContact can be mapped to the correct phonebook set and recorId. Hence we don't need another nsIDOMIccContact anymore,
close as WONTFIX.