Closed Bug 838092 Opened 11 years ago Closed 11 years ago

B2G Contact: Have a more generic API for importing contacts from other device storages.

Categories

(Core :: DOM: Device Interfaces, defect)

Product:
Core
Component:
DOM: Device Interfaces
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED INVALID

People

(Reporter: edgar, Assigned: allstars.chh)

References

Details

Attachments

(5 files, 5 obsolete files)

Part 2: loadExternalContact impl.
7.37 KB, patch
Details | Diff | Splinter Review
Pull Request to support FDN contact on B2G Emulator
181 bytes, text/html
vicamo
: review+
Details
Part 1: IDL changed v3
2.72 KB, patch
Details | Diff | Splinter Review
Part 3: Use lower case for ADN and FDN. v2
7.92 KB, patch
Details | Diff | Splinter Review
Part 4: Marionette tests for loadExternalContact from icc.
2.62 KB, patch
Details | Diff | Splinter Review 
In Contacts APIs, we have getSimContacts() used for getting SIM card contact. But consider multi-SIM or other further feature, we need a more generic API, something like import() or load() having a type field which indicates the source. Otherwise we may need to add a new API for every new feature.
Summary: B2G Contacts: Have a more generic API for importing contacts → B2G Contact: Have a more generic API for importing contacts from other storages.
Component: General → DOM: Device Interfaces
Product: Boot2Gecko → Core
Summary: B2G Contact: Have a more generic API for importing contacts from other storages. → B2G Contact: Have a more generic API for importing contacts from other device storages.
Attached patch WIP - Part 1: IDL changed (obsolete) — — Splinter Review 
Hi, Gregor
This is still in draft,
but I would like to request for your feedback on this?
Also should we call this interface 'import' ?
Seems 'import' will imply those imported contacts will be saved into database in Gecko. But I think the 'save' action should be done in gaia.
Or should we rename it to 'loadExternalContact'...etc, but the longer naming doesn't fit into current style in Contact API.

Also I am not sure whether add a 'nsIDOMContactIccOptions' here is a good idea.

thanks
Assignee: nobody → allstars.chh
Attachment #715311 - Flags: feedback?(anygregor)
Yoshi: Can you explain this proposal a bit more? What is ADN, FDN and subscriptionId?
(In reply to Jonas Sicking (:sicking) from comment #2)
> Yoshi: Can you explain this proposal a bit more? What is ADN, FDN and
> subscriptionId?

We already support ADN and FDN with the current getSicmContacts function.
ADN=phonebook on SIM card
FDN=Fix Dialing Numbers

subscrirptionID will be used for multiple SIM cards. Every single one will have a subscriptionID.

Yoshi, this approach looks good to me. We should try to be consistent if we have a similar API already in place.
Load seems more correct than import in this case. I prefer correctness over previous naming styles :)
Yup, sounds like a good approach to me.

Though rather than using interfaces you could simply use a dictionary which contains a union of all properties that we need. That will make it a little less overhead.
Attached patch WIP - Part 1: IDL changed v2 (obsolete) — — Splinter Review 
Hi, Gregor
I've updated :
- rename it to loadExternalContact
- Use dictionary as Jonas suggested
- Add more comments about those properties in options.

Does this looks okay to you?

thanks
Attachment #715311 - Attachment is obsolete: true
Attachment #715311 - Flags: feedback?(anygregor)
Attachment #715840 - Flags: feedback?(anygregor)
Comment on attachment 715840 [details] [diff] [review]
WIP - Part 1: IDL changed v2

I also liked the old approach with different types. This might get messy if we have multiple sources but I don't care too much.
Attachment #715840 - Flags: feedback?(anygregor) → feedback+
Attached patch Part 1: IDL changed v3 (obsolete) — — Splinter Review 

        Attachment #715840 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 715932 [details] [diff] [review]
Part 3: Use lower case for ADN and FDN.

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

::: dom/contacts/ContactManager.js
@@ +3,5 @@
>   * You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  "use strict";
>  
> +const DEBUG = true;

accidentally turn on.

    
    

    
  
(In reply to Gregor Wagner [:gwagner] from comment #3)
> (In reply to Jonas Sicking (:sicking) from comment #2)
> > Yoshi: Can you explain this proposal a bit more? What is ADN, FDN and
> > subscriptionId?
> 
> We already support ADN and FDN with the current getSicmContacts function.
> ADN=phonebook on SIM card
> FDN=Fix Dialing Numbers
> 
> subscrirptionID will be used for multiple SIM cards. Every single one will
> have a subscriptionID.
> 
> Yoshi, this approach looks good to me. We should try to be consistent if we
> have a similar API already in place.
> Load seems more correct than import in this case. I prefer correctness over
> previous naming styles :)

We should change getSimContacts() in a way that prevents passing anything to it to get anything from it. In other words, we should make sure we do not end up doing that:
http://robert.ocallahan.org/2012/05/canvas-getcontext-mistake.html

I'm afraid that, with the API you are building, some vendors might implement: loadExternalContacts({ storage: "foobar" }) without any specification for it.

Given that we want to handle multi-SIM scenarios, and that for many APIs, I wouldn't be surprised to have a SIM object. I think it would be better to have that instead of a "subscriptionId", assuming that parameter is actually the SIM id but this is a bit cryptic to me... Actually I think it should not because this API should be usable by people that don't know much about the low level mobile technologies. In other words, regular developers like me should be able to write an application using that API without googling for "ICC" and trying to understand the meaning of "APN", "FDN" and "subscriptionId".

The API I would like to see would look like this:
  DOMRequest getSIMContacts([optional] Object options);
with Object looking like:
  {
    sim: ASimObject, // default to the default SIM card
    adn: true,       // could be set to false, default to true
    fdn: false,      // could be set to true, default to false
  }
That way, regular developers will just do loadSIMContacts() and get the SIM contacts.

This said, there is still the issue of multi-SIM. We still haven't had a broad discussion about the general policy we want to have regarding that. Do we want to use a default SIM by default for all APIs taking the risk to have applications mis-behaving when there is more than one SIM card. Do we want to make developers life a bit harder and require them to explicitly get a correct SIM object? We could also provide them a method named .getDefaultSIM() that would return the default SIM object. Anyway, the method could be changed to:
  DOMRequest getSIMContacts(SIMObject sim, [optional] Object options);
with Object looking like:
  {
    adn: true,  // as above
    fdn: false, // as above
  }

In summary, we shouldn't make this API too generic. Unless there are really that many way to store contacts in a SIM card? (I don't know anything about that) I realise that I might have mis-interpreted the intent of that API...

    
    

    
  
I agree if we only talk about importing contacts from SIM cards. How about importing contacts from files/bluetooth/cloud/.... Should we add another importFileContact, importBluetoothContact... function?

    
    

    
  
(In reply to Mounir Lamouri (:mounir) from comment #16)
> We should change getSimContacts() in a way that prevents passing anything to
> it to get anything from it. In other words, we should make sure we do not
> end up doing that:
> http://robert.ocallahan.org/2012/05/canvas-getcontext-mistake.html

Good point.

(In reply to Gregor Wagner [:gwagner] from comment #17)
> I agree if we only talk about importing contacts from SIM cards. How about
> importing contacts from files/bluetooth/cloud/.... Should we add another
> importFileContact, importBluetoothContact... function?

Yup. Adding functions is cheap. Not meaningfully more expensive than adding additional supported value for the "storage" property.

    
    

    
  
(In reply to Jonas Sicking (:sicking) from comment #18)
> (In reply to Mounir Lamouri (:mounir) from comment #16)
> > We should change getSimContacts() in a way that prevents passing anything to
> > it to get anything from it. In other words, we should make sure we do not
> > end up doing that:
> > http://robert.ocallahan.org/2012/05/canvas-getcontext-mistake.html
> 
> Good point.
> 
> (In reply to Gregor Wagner [:gwagner] from comment #17)
> > I agree if we only talk about importing contacts from SIM cards. How about
> > importing contacts from files/bluetooth/cloud/.... Should we add another
> > importFileContact, importBluetoothContact... function?
> 
> Yup. Adding functions is cheap. Not meaningfully more expensive than adding
> additional supported value for the "storage" property.

That's why I wanted different types. I don't really like the idea with creating new functions for every new backend.

    
    

    
  
Comment on attachment 715938 [details] [diff] [review]
Part 1: IDL changed v3

Cancelling r? for Mounir has different opinions.

        Attachment #715938 -
        Flags: review?(anygregor)

    
    

    
  
Comment on attachment 715931 [details] [diff] [review]
Part 2: loadExternalContact impl.

Cancelling r? for Mounir has different opinions.

        Attachment #715931 -
        Flags: review?(anygregor)

    
    

    
  
Comment on attachment 715941 [details] [diff] [review]
Part 4: Marionette tests for loadExternalContact from icc.

Cancelling r? for Mounir has different opinions.

        Attachment #715941 -
        Flags: review?(htsai)

    
    

    
  
Comment on attachment 715940 [details] [diff] [review]
Part 3: Use lower case for ADN and FDN. v2

Cancelling r? for Mounir has different opinions.

        Attachment #715940 -
        Flags: review?(htsai)

    
    

    
  
(In reply to Mounir Lamouri (:mounir) from comment #16)
> 
> I'm afraid that, with the API you are building, some vendors might
> implement: loadExternalContacts({ storage: "foobar" }) without any
> specification for it.
> 
But what about the original getSimContacts in terms of Web API?
For example, what if that device doesn't have a SIM?

> Given that we want to handle multi-SIM scenarios, and that for many APIs, I
> wouldn't be surprised to have a SIM object.
>
But To Contact API, SIM card means a kind of storage,
it won't care will that storage can make a phone call.
So even for Multi-SIM, to Contact API it just means 'Multi-Storages' for contacts.

> I think it would be better to
> have that instead of a "subscriptionId", assuming that parameter is actually
> the SIM id but this is a bit cryptic to me... 
subscriptonId is the identifier we use to identifity SIM cards in multi-SIM.
You can also discuss this in dev-webapi.

> 
> In summary, we shouldn't make this API too generic. Unless there are really
> that many way to store contacts in a SIM card? (I don't know anything about
> that) I realise that I might have mis-interpreted the intent of that API...

I think this API 'loadExtenralContact' could be used not only for SIM but for SD card as well.
As Gregor said, 
should we add loadSDCardContact for SD card?

    
    

    
  
(In reply to Mounir Lamouri (:mounir) from comment #16)
>   DOMRequest getSIMContacts(SIMObject sim, [optional] Object options);
> with Object looking like:
>   {
>     adn: true,  // as above
>     fdn: false, // as above
>   }
> 

Those properties are just boolean,
should we really need to create an object to enumerate all possibilities?

What about user set both members to true?
Should we need to make our implementation so complicated to retrieve all of them?
What's the use case when user needs all kinds of SIM contact?

If we'd use this style to pass parameters, I think many APIs will also need to change as well, for example in MobileConnection,
nsIDOMDOMRequest selectNetwork(in nsIDOMMozMobileNetworkInfo network);

The API should be changed to 
nsIDOMDOMRequest selectNetwork(in jsval networks);  
where networks is an object containing properties which are all networks returned from getNetworks() and only one of them has value 'true'.
for example

networks = {
  network1:  false,
  network2:  false,
  ...
  networkK:  true,
  ...
  networkN:  false
};

    
    

    
  
(In reply to Gregor Wagner [:gwagner] from comment #19)
> (In reply to Jonas Sicking (:sicking) from comment #18)
> > Yup. Adding functions is cheap. Not meaningfully more expensive than adding
> > additional supported value for the "storage" property.
> 
> That's why I wanted different types. I don't really like the idea with
> creating new functions for every new backend.

I think you misunderstood my comment. I'm saying that adding additional functions *is* cheap.

And that the difference in cost between adding a new function and adding a new value for the "storage" property is very low. So we might as well add new functions as we add support for more storage backends since that allows pages to do feature detection to see which backends we support.

See http://robert.ocallahan.org/2012/05/canvas-getcontext-mistake.html

    
    

    
  
(In reply to Jonas Sicking (:sicking) from comment #26)
> (In reply to Gregor Wagner [:gwagner] from comment #19)
> > (In reply to Jonas Sicking (:sicking) from comment #18)
> > > Yup. Adding functions is cheap. Not meaningfully more expensive than adding
> > > additional supported value for the "storage" property.
> > 
> > That's why I wanted different types. I don't really like the idea with
> > creating new functions for every new backend.
> 
> I think you misunderstood my comment. I'm saying that adding additional
> functions *is* cheap.
> 
> And that the difference in cost between adding a new function and adding a
> new value for the "storage" property is very low. So we might as well add
> new functions as we add support for more storage backends since that allows
> pages to do feature detection to see which backends we support.
> 
> See http://robert.ocallahan.org/2012/05/canvas-getcontext-mistake.html

It might be cheap but I think it's a mess.

So we should also change navigator.getDeviceStorage(type) to getSDCardDeviceStorage, getPhotoDeviceStorage, getVideoDeviceStorage, getSharedDeviceStorage, getMusicDeviceSotrage....?

    
    

    
  
> It might be cheap but I think it's a mess.

I agree it feels uncomfortable, and it's certainly not how I'd design a C++ API. But it seems to work quite well in JS. Can you give an example of the downsides?

> So we should also change navigator.getDeviceStorage(type) to
> getSDCardDeviceStorage, getPhotoDeviceStorage, getVideoDeviceStorage,
> getSharedDeviceStorage, getMusicDeviceSotrage....?

That is indeed a very good question! Quite possibly so.

    
    

    
  
(In reply to Jonas Sicking (:sicking) from comment #28)
> > So we should also change navigator.getDeviceStorage(type) to
> > getSDCardDeviceStorage, getPhotoDeviceStorage, getVideoDeviceStorage,
> > getSharedDeviceStorage, getMusicDeviceSotrage....?
> 
> That is indeed a very good question! Quite possibly so.
Hi, Jonas
I though Web API should be a more high level of API. So in the examples above, some API (getXXXDeviceStorage) will work only on some platforms, is this okay with Web API?

And in that case, in the specification(or IDL), there should be some documentations indicates some interfaces will only work on some platforms while some other interfaces will only work on other platforms.
Is this okay?

    
    

    
  
If we use the syntax: getDeviceStorage("music") or the syntax getMusicDeviceStorage won't affect which platforms it works on, so I'm not sure I understand the question?

The fact that not all platforms have a music storage is in fact the reason that getMusicDeviceStorage() is better than getDeviceStorage("music") since the former lets pages do:

if ("getMusicDeviceStorage" in navigator) {
  <use music storage>
} else if ("getSDCardDeviceStorage" in navigator) {
  <use sdcard storage>
} else {
  <use local IndexedDB database>
}

    
    

    
  
Comment on attachment 715934 [details]
Pull Request to support FDN contact on B2G Emulator

Merged on github :)

        Attachment #715934 -
        Flags: review?(vyang) → review+

    
    

    
  
(In reply to Jonas Sicking (:sicking) from comment #30)
> The fact that not all platforms have a music storage is in fact the reason
> that getMusicDeviceStorage() is better than getDeviceStorage("music") since
> the former lets pages do:
> 
> if ("getMusicDeviceStorage" in navigator) {
>   <use music storage>
> } else if ("getSDCardDeviceStorage" in navigator) {
>   <use sdcard storage>
> } else {
>   <use local IndexedDB database>
> }
Got it, thank you.

So we enable getMusicDeviceStorage on platforms with supporting 'Music', right?
Like how we did it in WebMobileConnection/WebTelephony/WebSMS.
These object are available only on B2G.

So that means when we design some *platform-dependant* Web API,
we should not make that interface not too high level, nor too generic,
(Like we should name it getMusicDeviceStorage, instead of getDeviceStorage('music'))

So in this bug, we should name it loadSimContact instead of loadExternalContact("sim"),
And make this API only available for B2G platform.

Is my understanding correct?

    
    

    
  
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #32)
Also my concern for 'getSimContacts' is that on W3C contact API discussion, someone said that 'getSimContacts' is not well suit for Web as not every device would have SIM card on it, so we would like to rename it in this bug in the beginning.

    
    

    
  
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #25)
> (In reply to Mounir Lamouri (:mounir) from comment #16)
> >   DOMRequest getSIMContacts(SIMObject sim, [optional] Object options);
> > with Object looking like:
> >   {
> >     adn: true,  // as above
> >     fdn: false, // as above
> >   }
> > 
> 
> Those properties are just boolean,
> should we really need to create an object to enumerate all possibilities?
> 
> [...]

I don't know more about those values. We can also do an enum with "adn" and "fdn" and have it being "adn" by default. Though, we should have better names I believe.

(In reply to Gregor Wagner [:gwagner] from comment #27)
> It might be cheap but I think it's a mess.
> 
> So we should also change navigator.getDeviceStorage(type) to
> getSDCardDeviceStorage, getPhotoDeviceStorage, getVideoDeviceStorage,
> getSharedDeviceStorage, getMusicDeviceSotrage....?

Why do we need so many of those? AFAIK, on my phone, if we except the online services, I can import contacts from my SIM card or from a file. The application can get the file in different ways (devicestorage, intent, <input type='file'>, XHR, ...), then the file has to be transformed to actual Contact objects. I don't know if we need an actual API for that. If content can create new Contacts (and I don't see why not), parsing the file should be doable by the application itself.

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #33)
> (In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #32)
> Also my concern for 'getSimContacts' is that on W3C contact API discussion,
> someone said that 'getSimContacts' is not well suit for Web as not every
> device would have SIM card on it, so we would like to rename it in this bug
> in the beginning.

I don't think renaming it is a good way to solve that issue. A solution could be to move that method to a SIM API that we actually might need anyway. We could have something like:
  sim[0].getContacts();
If we do that, we could even have two methods, one for ADN, the other for FDN.

    
    

    
  
(In reply to Mounir Lamouri (:mounir) from comment #34)
> I don't think renaming it is a good way to solve that issue. A solution
> could be to move that method to a SIM API that we actually might need
> anyway. We could have something like:
>   sim[0].getContacts();
> If we do that, we could even have two methods, one for ADN, the other for
> FDN.
Hi, Mounir
Thanks for the comments, as I read my concerns in Comment 33 again, I found I already answered myself in Comment 32.(If my understanding to Jonas in Comment 30 is correct).
getSimContacts will only be available on platforms with SIM card supported.

But since you mentioned,
If we move the API to icc, what's the returned Contact?
Should I create a new interface called nsIDOMIccContact?
In that case how does this nsIDOMIccContact converted to nsIDOMContact?
If returning nsIDOMContact, seems this interface should be in mozContacts, not in icc.

    
    

    
  
Hi, Gregor and Jonas

After Comment 27, Comment 28, Comment 29, Comment 30 and Comment 32
I think we should still keep this API (getSimContacts),
If you agree, then I'll close this bug as INVALID

or should we just rename it to loadSimContacts?

For load SIM contacts from multi-SIM, I'll file another bug for this.

    
    

    
  
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #32)
> So we enable getMusicDeviceStorage on platforms with supporting 'Music',
> right?
> Like how we did it in WebMobileConnection/WebTelephony/WebSMS.
> These object are available only on B2G.
> 
> So that means when we design some *platform-dependant* Web API,
> we should not make that interface not too high level, nor too generic,
> (Like we should name it getMusicDeviceStorage, instead of
> getDeviceStorage('music'))
> 
> So in this bug, we should name it loadSimContact instead of
> loadExternalContact("sim"),
> And make this API only available for B2G platform.
> 
> Is my understanding correct?

Yup. Exactly.

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #33)
> Also my concern for 'getSimContacts' is that on W3C contact API discussion,
> someone said that 'getSimContacts' is not well suit for Web as not every
> device would have SIM card on it, so we would like to rename it in this bug
> in the beginning.

This same problem exists with loadExternalContacts("sim").

There is no way that we can create APIs that are as powerful as native APIs and so expose hardware specific APIs, and at the same time only create APIs that work on all types of hardware.

So the best thing we can do for very low-level hardware API is to ensure that webpages can detect when they can interact with that hardware and when they can't. I don't see any other solution unfortunately.

    
    

    
  
(In reply to Jonas Sicking (:sicking) from comment #37)
> (In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #32)
> > So we enable getMusicDeviceStorage on platforms with supporting 'Music',
> > right?
> > Like how we did it in WebMobileConnection/WebTelephony/WebSMS.
> > These object are available only on B2G.
> > 
> > So that means when we design some *platform-dependant* Web API,
> > we should not make that interface not too high level, nor too generic,
> > (Like we should name it getMusicDeviceStorage, instead of
> > getDeviceStorage('music'))
> > 
> > So in this bug, we should name it loadSimContact instead of
> > loadExternalContact("sim"),
> > And make this API only available for B2G platform.
> > 
> > Is my understanding correct?
> 
> Yup. Exactly.
> 

Making this API available in B2G is not always correct. You suggested that users test for navigator.getSimContacts. What should the behavior be for phones with no sim cards at all? getSimContacts() returns not available or null?

So going back to the original problem, supporting multiple SIM cards. How should this API look like? getSimContacts(subscriptionID) or getSimContactsSubscriptionID1(), getSimContactsSubscriptionID2().... ?

    
    

    
  
I see much more of an argument that a page would want to iterate through all sim-cards so getSimContacts(subscriptionID) seems better.

Additionally the page can presumably detect which simcards are available through whichever API is used to get subscriptionIDs.

    
    

    
  
(In reply to Gregor Wagner [:gwagner] from comment #38) 
> 
> Making this API available in B2G is not always correct. You suggested that
> users test for navigator.getSimContacts. What should the behavior be for
> phones with no sim cards at all? getSimContacts() returns not available or
> null?
> 
Hi, Gregor:

navigator.mozContacts.getSimContacts -> undefined
Like currently on Desktop, navigator.mozMobileConnection is undefined.
See Bug785942

But in our case, we still need mozContacts on all platforms except for the interface getSimContacts.
For implementation, maybe 'partial interface' cannot work on XPIDL, I haven't tried yet. If so, I'll still put getSimContacts in nsIDOMContactManager.idl for now and file another bug to address this.

How do you think?

    
    

    
  
(In reply to Gregor Wagner [:gwagner] from comment #38)
> Making this API available in B2G is not always correct. You suggested that
> users test for navigator.getSimContacts. What should the behavior be for
> phones with no sim cards at all? getSimContacts() returns not available or
> null?
> 
> So going back to the original problem, supporting multiple SIM cards. How
> should this API look like? getSimContacts(subscriptionID) or
> getSimContactsSubscriptionID1(), getSimContactsSubscriptionID2().... ?

Both problems would be solved if we had a SIM object on which we could call .getContacts().

If you do so, we could imagine an API like:
if (navigator.sims[0] !== null) {
  navigator.sims[0].getContacts();
}

And for multi-sim:
for (var i = 0; i < navigator.sims.length; ++i) {
  navigator.sims[i].getContacts();
}

I'm not sure to understand why this solution seems to be ignored.

    
    

    
  
(In reply to Mounir Lamouri (:mounir) from comment #41)
> Both problems would be solved if we had a SIM object on which we could call
> .getContacts().
> 
> If you do so, we could imagine an API like:
> if (navigator.sims[0] !== null) {
>   navigator.sims[0].getContacts();
> }
> 
> And for multi-sim:
> for (var i = 0; i < navigator.sims.length; ++i) {
>   navigator.sims[i].getContacts();
> }
> 
> I'm not sure to understand why this solution seems to be ignored.

If we move the API to icc, what's the type of returned Contact?
Should I create a new interface called nsIDOMIccContact?
In that case how does this nsIDOMIccContact converted to nsIDOMContact?
If returning nsIDOMContact, seems this interface should be in mozContacts, not in icc.

    
    

    
  
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #42)
> (In reply to Mounir Lamouri (:mounir) from comment #41)
> > Both problems would be solved if we had a SIM object on which we could call
> > .getContacts().
> > 
> > If you do so, we could imagine an API like:
> > if (navigator.sims[0] !== null) {
> >   navigator.sims[0].getContacts();
> > }
> > 
> > And for multi-sim:
> > for (var i = 0; i < navigator.sims.length; ++i) {
> >   navigator.sims[i].getContacts();
> > }
> > 
> > I'm not sure to understand why this solution seems to be ignored.
> 
> If we move the API to icc, what's the type of returned Contact?
> Should I create a new interface called nsIDOMIccContact?
> In that case how does this nsIDOMIccContact converted to nsIDOMContact?
> If returning nsIDOMContact, seems this interface should be in mozContacts,
> not in icc.

The SIM object should be able to return nsIDOMContact. I do not see why this can't or shouldn't happen.

    
    

    
  
Do we have a navigator.sims object though? It seems strange to introduce that if the only API available on it is for contact importing.

The part that I'm not sure about is if the security model for reading contact data is different from reading other data from the sim card. I.e. I wonder if contact information from the sim card is less sensitive than other information from the sim card?

    
    

    
  
(In reply to Mounir Lamouri (:mounir) from comment #41)
> Both problems would be solved if we had a SIM object on which we could call
> .getContacts().
> 
...
> 
> I'm not sure to understand why this solution seems to be ignored.

Agreed with Mounir, pushing this out to a separate SIM object that supports the ContactsAPI makes more sense and separates out all the questions (does the device have a SIM? does it have more than one SIM) that are orthogonal to the ContactsAPI.

    
    

    
  
(In reply to Tantek Çelik from comment #45)
> Agreed with Mounir, pushing this out to a separate SIM object that supports
> the ContactsAPI makes more sense and separates out all the questions (does
> the device have a SIM? does it have more than one SIM) that are orthogonal
> to the ContactsAPI.

So Contact API doesn't have to handle SIM contacts? Nor contacts(VCARD) from SD card?

    
    

    
  
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #46)
> (In reply to Tantek Çelik from comment #45)
> > Agreed with Mounir, pushing this out to a separate SIM object that supports
> > the ContactsAPI makes more sense and separates out all the questions (does
> > the device have a SIM? does it have more than one SIM) that are orthogonal
> > to the ContactsAPI.
> 
> So Contact API doesn't have to handle SIM contacts? Nor contacts(VCARD) from
> SD card?

So ContactAPI doesn't have to handle Bluetooth/USB/Wifi/SD/(insert-new-hardware-here) contacts either.

No. Contact API doesn't have to handle SIM contacts. The SIM[s] object should merely support the ContactsAPI itself.

Put management of hardware I/O devices in a *separate* API please. Nothing about those are "Contacts.API" specific, so they don't belong anywhere near the ContactsAPI.

The navigator.[moz]Contacts object should handle the default contacts store built-into the device. Nothing more. 

For additional sources of contacts, create APIs for accessing those additional I/O ports/peripherals etc., including checking if they're "ready", how many they have (how many SIMs?), iterating (picking through SIM or SD cards when there are multiple slots), handling unexpected disconnection (e.g. user removes an SD card while an operation is happening, unplugs a USB drive). etc.

All of these separable/detachable forms of hardware I/O have their own API requirements, don't conflate them with ContactsAPI.

    
    

    
  
Thanks for Tantek for explanation.
Gregor, do you also agree on this?

    
    

    
  
(In reply to Jonas Sicking (:sicking) from comment #44)
> Do we have a navigator.sims object though? It seems strange to introduce
> that if the only API available on it is for contact importing.

We will very likely want to get some information about the SIM cards (status, network, number, etc.). I doubt such object would only have a .getContacts() method. Also, we will have to make other APIs multi-sim capable. Using a SIM object could be a solution.

> The part that I'm not sure about is if the security model for reading
> contact data is different from reading other data from the sim card. I.e. I
> wonder if contact information from the sim card is less sensitive than other
> information from the sim card?

That's a good question. We can have a 'sim-contacts' permission if that is not the case.

    
    

    
  
(In reply to Mounir Lamouri (:mounir) from comment #49)
> (In reply to Jonas Sicking (:sicking) from comment #44)
> > Do we have a navigator.sims object though? It seems strange to introduce
> > that if the only API available on it is for contact importing.
> 
> We will very likely want to get some information about the SIM cards
> (status, network, number, etc.). I doubt such object would only have a
> .getContacts() method. Also, we will have to make other APIs multi-sim
> capable. Using a SIM object could be a solution.
> 
Currently we have navigator.mozMobileConnection.icc, which is used to handle STK related functions.

    
    

    
  
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #48)
> Thanks for Tantek for explanation.
> Gregor, do you also agree on this?

Gregor, ping?

    
    

    
  
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #51)
> (In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #48)
> > Thanks for Tantek for explanation.
> > Gregor, do you also agree on this?
> 
> Gregor, ping?

Sorry I couldn't keep up with bugmail during MWC. getSimContacts was a quick hack for a feature that we wanted to drop in the first place for v1 as you remember :)
I like the idea of moving import functions outside of the actual contacts API.
We already have getICCContacts and now we just need 'some' service that puts the contacts in our DB. The remaining question is where this service should live.

    
    

    
  
(In reply to Gregor Wagner [:gwagner] from comment #52)
> We already have getICCContacts and now we just need 'some' service that puts
> the contacts in our DB. The remaining question is where this service should
> live.

What do you mean?

    
    

    
  
(In reply to Gregor Wagner [:gwagner] from comment #52)
> We already have getICCContacts and now we just need 'some' service that puts
> the contacts in our DB. The remaining question is where this service should
> live.

Gregor, the SIM contacts are stored to DB in gaia

https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/utilities/import_sim_contacts.js#L86

Is this what you're talking about?

    
    

    
  
(In reply to Mounir Lamouri (:mounir) from comment #53)
> (In reply to Gregor Wagner [:gwagner] from comment #52)
> > We already have getICCContacts and now we just need 'some' service that puts
> > the contacts in our DB. The remaining question is where this service should
> > live.
> 
> What do you mean?

Our getSimContacts is basically just a wrapper around getICCContacts http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/RadioInterfaceLayer.js#2853

I don't know if getICCContacts is exposed to content but we could just use this API directly.

    
    

    
  
Since now all agree to remove getSimContacts from Contact API,
so I will close this bug as INVALID and file another bug to expose 'getICCContacts' to content.

Thanks for all your great comments here. :)

    
    

    
  
Filed Bug 847741 to address the conclusion.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → INVALID

    
  
Blocks: 1157082

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: