807688 - Add 'key' a'la hCard to Contacts API

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[David Dahl :ddahl]

We should add the "key" property to the Contacts API - a placeholder here would be great for experimentation on signing and encrypting messages via Firefox OS Apps, also see: http://microformats.org/wiki/key-examples

Comment 1

[David Dahl :ddahl]

Dietrich: I assume we will need a gaia front-end bug for this as well, correct?

Comment 2

[Tantek Çelik]

Added URL.

(In reply to David Dahl :ddahl from comment #1)
> Dietrich: I assume we will need a gaia front-end bug for this as well,
> correct?

Yes, we need to get 'Contact key' added to the list of fields the user has the ability to add to here:

https://wiki.mozilla.org/Gaia/Contacts#Gaia_v2

Comment 3

[Dietrich Ayala (:dietrich)]

Yes, please file in Boot2Gecko->Gaia.

Comment 4

[Tantek Çelik]

Gregor, how hard would this be to add to our B2G ContactsAPI implementation?

specifically, add to interface ContactProperties : 

attribute DOMString key[]; // takes a key URI per RFC6350 6.8.1

Comment 5

[Gregor Wagner [:gwagner]]

It's not hard but I don't think I have time before the B2G v1 deadline (Jan 15th).
We should mark it as good first bug.

Comment 6

[David Dahl :ddahl]

Attachment: Added publicKey property to MozContacts

Comment 7

[Gregor Wagner [:gwagner]]

Comment on attachment 719256 [details] [diff] [review]
Added publicKey property to MozContacts

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

You also have to add it to http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/contacts/nsIDOMContactProperties.idl#51 and change the UUID.

::: dom/contacts/fallback/ContactDB.jsm
@@ +17,5 @@
>  Cu.import("resource://gre/modules/IndexedDBHelper.jsm");
>  Cu.import("resource://gre/modules/PhoneNumberUtils.jsm");
>  
>  const DB_NAME = "contacts";
> +const DB_VERSION = 9;

I don't think you have to increase the DB version. You don't actually have to update existing entries.

Comment 8

[Josh Matthews [:jdm]]

Are you going to submit a new patch, David?

Comment 9

[David Dahl :ddahl]

(In reply to Josh Matthews [:jdm] from comment #8)
> Are you going to submit a new patch, David?

Yes I will be soon.

Comment 10

[Tantek Çelik]

Adding Reuben.

Comment 11

[Shane Tully (:stully:]

ddahl, are you still working on this?

The attached patch has the field called "publicKey", but the API spec (https://wiki.mozilla.org/WebAPI/ContactsAPI#API) has it as just called "key". Which of these is correct?

Comment 12

[David Dahl :ddahl]

(In reply to Shane Tully (:stully: from comment #11)
> ddahl, are you still working on this?
> 
Yeah, if you are interested, please take it, otherwise I'll get back to it soon.

> The attached patch has the field called "publicKey", but the API spec
> (https://wiki.mozilla.org/WebAPI/ContactsAPI#API) has it as just called
> "key". Which of these is correct?

I am using publicKey in this patch as it is more explicit. "key" is in the spec, but is too generic.

Comment 13

[Shane Tully (:stully:]

Attachment: Add "publicKey" field

Add "publicKey" field

This is a slightly updated version of ddahl's patch. Problem is that in the save function, the field is always undefined for some unknown reason. Is there somewhere else the new field needs added? I'm working on bug 857730 (contacts API for Android) so I'd like to get this added there.

Comment 14

[Reuben Morais [:reuben]]

(In reply to Shane Tully (:stully: from comment #13)
> This is a slightly updated version of ddahl's patch. Problem is that in the
> save function, the field is always undefined for some unknown reason. Is
> there somewhere else the new field needs added? I'm working on bug 857730
> (contacts API for Android) so I'd like to get this added there.

Yes, you have to update the interface as well.

See: http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/contacts/nsIContactProperties.idl

Don't forget to change the interface's UUID, then update the manifest and the implementation:

https://mxr.mozilla.org/mozilla-central/source/dom/contacts/ContactManager.manifest
https://mxr.mozilla.org/mozilla-central/source/dom/contacts/ContactManager.js#49

Comment 15

[Shane Tully (:stully:]

Attachment: 1/2 Add "publicKey" field

Add "publicKey" field

Revised with modifcations to the IDL files as suggested by reuben (thanks!)

Comment 16

[Shane Tully (:stully:]

Attachment: 2/2 Add "publicKey" field tests

Add "publicKey" field tests

Comment 17

[Shane Tully (:stully:]

(In reply to Shane Tully (:stully: from comment #16)
> Created attachment 758264 [details] [diff] [review]
> 2/2 Add "publicKey" field tests
> 
> Add "publicKey" field tests

I wouldn't trust them to encrypt anything, but they look like the random gibberish of public keys.

Comment 18

[Shane Tully (:stully:]

Attachment: 1/2 Add "publicKey" field

Add "publicKey" field

Updated to reflect changes in bug 869615.

Comment 19

[Shane Tully (:stully:]

Attachment: 2/2 Add "publicKey" field tests

Add "publicKey" field tests

Updated to reflect changes in bug 869615.

Comment 20

[Shane Tully (:stully:]

Attachment: 1/2 Add "publicKey" field

Add "publicKey" field 

Fixed typo in previous patch. (sorry for all the revisions!)

Comment 21

[Shane Tully (:stully:]

Comment on attachment 758379 [details] [diff] [review]
1/2 Add "publicKey" field

>Date: 1370387242 25200
>From: Shane Tully <[e-mail]>
>Bug 857730 - Add "publicKey" field
>
>diff --git a/dom/contacts/ContactManager.js b/dom/contacts/ContactManager.js
>--- a/dom/contacts/ContactManager.js
>+++ b/dom/contacts/ContactManager.js
>@@ -41,17 +41,17 @@ function stringOrBust(aObj) {
> function sanitizeStringArray(aArray) {
>   if (!Array.isArray(aArray)) {
>     aArray = [aArray];
>   }
>   return aArray.map(stringOrBust).filter(function(el) { return el != undefined; });
> }
> 
> const nsIClassInfo            = Ci.nsIClassInfo;
>-const CONTACTPROPERTIES_CID   = Components.ID("{6cb78b21-4218-414b-8a84-3b7bf0088b34}");
>+const CONTACTPROPERTIES_CID   = Components.ID("{35ad8a4e-9486-44b6-883d-550f14635e49}");
> const nsIContactProperties    = Ci.nsIContactProperties;
> 
> // ContactProperties is not directly instantiated. It is used as interface.
> 
> function ContactProperties(aProp) { if (DEBUG) debug("ContactProperties Constructor"); }
> 
> ContactProperties.prototype = {
> 
>@@ -260,17 +260,18 @@ Contact.prototype = {
>                       tel: 'rw',
>                       org: 'rw',
>                       jobTitle: 'rw',
>                       bday: 'rw',
>                       note: 'rw',
>                       impp: 'rw',
>                       anniversary: 'rw',
>                       sex: 'rw',
>-                      genderIdentity: 'rw'
>+                      genderIdentity: 'rw',
>+                      publicKey: 'rw'
>                      },
> 
>   set name(aName) {
>     this._name = sanitizeStringArray(aName);
>   },
> 
>   get name() {
>     return this._name;
>@@ -455,16 +456,24 @@ Contact.prototype = {
>       this._genderIdentity = null;
>     }
>   },
> 
>   get genderIdentity() {
>     return this._genderIdentity;
>   },
> 
>+  set publicKey(aPublicKey) {
>+    this._publicKey = sanitizeStringArray(aPublicKey);
>+  },
>+
>+  get publicKey() {
>+    return this._publicKey;
>+  },
>+
>   init: function init(aProp) {
>     this.name =            aProp.name;
>     this.honorificPrefix = aProp.honorificPrefix;
>     this.givenName =       aProp.givenName;
>     this.additionalName =  aProp.additionalName;
>     this.familyName =      aProp.familyName;
>     this.honorificSuffix = aProp.honorificSuffix;
>     this.nickname =        aProp.nickname;
>@@ -477,16 +486,17 @@ Contact.prototype = {
>     this.org =             aProp.org;
>     this.jobTitle =        aProp.jobTitle;
>     this.bday =            aProp.bday;
>     this.note =            aProp.note;
>     this.impp =            aProp.impp;
>     this.anniversary =     aProp.anniversary;
>     this.sex =             aProp.sex;
>     this.genderIdentity =  aProp.genderIdentity;
>+    this.publicKey =       aProp.publicKey;
>   },
> 
>   get published () {
>     return this._published;
>   },
> 
>   set published(aPublished) {
>     this._published = aPublished;
>@@ -747,17 +757,18 @@ ContactManager.prototype = {
>       tel:             [],
>       org:             [],
>       jobTitle:        [],
>       bday:            null,
>       note:            [],
>       impp:            [],
>       anniversary:     null,
>       sex:             null,
>-      genderIdentity:  null
>+      genderIdentity:  null,
>+      publicKey:       []
>     };
>     for (let field in newContact.properties) {
>       newContact.properties[field] = aContact[field];
>     }
> 
>     let reason;
>     if (aContact.id == "undefined") {
>       // for example {25c00f01-90e5-c545-b4d4-21E2ddbab9e0} becomes
>diff --git a/dom/contacts/ContactManager.manifest b/dom/contacts/ContactManager.manifest
>--- a/dom/contacts/ContactManager.manifest
>+++ b/dom/contacts/ContactManager.manifest
>@@ -1,10 +1,10 @@
>-component {6cb78b21-4218-414b-8a84-3b7bf0088b34} ContactManager.js
>-contract @mozilla.org/contactProperties;1 {6cb78b21-4218-414b-8a84-3b7bf0088b34}
>+component {35ad8a4e-9486-44b6-883d-550f14635e49} ContactManager.js
>+contract @mozilla.org/contactProperties;1 {35ad8a4e-9486-44b6-883d-550f14635e49}
> 
> component {9cbfa81c-bcab-4ca9-b0d2-f4318f295e33} ContactManager.js
> contract @mozilla.org/contactAddress;1 {9cbfa81c-bcab-4ca9-b0d2-f4318f295e33}
> 
> component {ad19a543-69e4-44f0-adfa-37c011556bc1} ContactManager.js
> contract @mozilla.org/contactField;1 {ad19a543-69e4-44f0-adfa-37c011556bc1}
> 
> component {4d42c5a9-ea5d-4102-80c3-40cc986367ca} ContactManager.js
>diff --git a/dom/contacts/fallback/ContactDB.jsm b/dom/contacts/fallback/ContactDB.jsm
>--- a/dom/contacts/fallback/ContactDB.jsm
>+++ b/dom/contacts/fallback/ContactDB.jsm
>@@ -431,17 +431,18 @@ ContactDB.prototype = {
>       tel:             [],
>       org:             [],
>       jobTitle:        [],
>       bday:            null,
>       note:            [],
>       impp:            [],
>       anniversary:     null,
>       sex:             null,
>-      genderIdentity:  null
>+      genderIdentity:  null,
>+      publicKey:       []
>     };
> 
>     contact.search = {
>       givenName:       [],
>       familyName:      [],
>       email:           [],
>       category:        [],
>       tel:             [],
>diff --git a/dom/interfaces/contacts/nsIContactProperties.idl b/dom/interfaces/contacts/nsIContactProperties.idl
>--- a/dom/interfaces/contacts/nsIContactProperties.idl
>+++ b/dom/interfaces/contacts/nsIContactProperties.idl
>@@ -41,17 +41,17 @@ interface nsIContactFindSortOptions : ns
> interface nsIContactFindOptions : nsIContactFindSortOptions
> {
>   attribute DOMString filterValue;  // e.g. "Tom"
>   attribute DOMString filterOp;     // e.g. "startsWith"
>   attribute jsval filterBy;         // DOMString[], e.g. ["givenName", "nickname"]
>   attribute unsigned long filterLimit;
> };
> 
>-[scriptable, uuid(6cb78b21-4218-414b-8a84-3b7bf0088b34)]
>+[scriptable, uuid(35ad8a4e-9486-44b6-883d-550f14635e49)]
> interface nsIContactProperties : nsISupports
> {
>   attribute jsval         name;               // DOMString[]
>   attribute jsval         honorificPrefix;    // DOMString[]
>   attribute jsval         givenName;          // DOMString[]
>   attribute jsval         additionalName;     // DOMString[]
>   attribute jsval         familyName;         // DOMString[]
>   attribute jsval         honorificSuffix;    // DOMString[]
>@@ -63,11 +63,12 @@ interface nsIContactProperties : nsISupp
>   attribute jsval         adr;                // ContactAddress[]
>   attribute jsval         tel;                // ContactTelField[]
>   attribute jsval         org;                // DOMString[]
>   attribute jsval         jobTitle;           // DOMString[]
>   attribute jsval         bday;               // Date
>   attribute jsval         note;               // DOMString[]
>   attribute jsval         impp;               // ContactField[]
>   attribute jsval         anniversary;        // Date
>-  attribute DOMString     sex;
>-  attribute DOMString     genderIdentity;
>+  attribute DOMString     sex;                // DOMString
>+  attribute DOMString     genderIdentity;     // DOMString
>+  attribute jsval         publicKey;          // DOMString[]
> };

Comment 22

[Shane Tully (:stully:]

Attachment: 1/2 Add "publicKey" field

Add "publicKey" field 

Fixed typo in previous patch. (sorry for all the revisions!)

Comment 23

[Gregor Wagner [:gwagner]]

Comment on attachment 758386 [details] [diff] [review]
1/2 Add "publicKey" field

Interface changes require superreview as well.

Comment 24

[Gregor Wagner [:gwagner]]

Comment on attachment 758386 [details] [diff] [review]
1/2 Add "publicKey" field

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

Thanks!

Comment 25

[Gregor Wagner [:gwagner]]

Oh the spec says 'key' and not 'publicKey'. Please change.

Comment 26

[Shane Tully (:stully:]

Attachment: 1/2 Add "key" field

Add "key" field

Revised: Changed "publicKey" to "key"

Comment 27

[Shane Tully (:stully:]

Attachment: 2/2 Add "key" field to tests

Add "key" field to tests

Revised: CHanged "publicKey" to "key"

Comment 28

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

Comment on attachment 758434 [details] [diff] [review]
1/2 Add "key" field

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

I'll defer to Tantek here. And since he has said yes, then I'll say yes too.

::: dom/contacts/ContactManager.js
@@ +265,5 @@
>                        impp: 'rw',
>                        anniversary: 'rw',
>                        sex: 'rw',
> +                      genderIdentity: 'rw',
> +                      key: 'rw'

Add a comma after this so that future diffs look cleaner. It's perfectly valid JS to have a comma at the end. Same elsewhere in this patch.

Comment 29

[Shane Tully (:stully:]

Attachment: 1/2 Add "key" field

Added commas as requested.

Comment 30

[Gregor Wagner [:gwagner]]

Comment on attachment 762760 [details] [diff] [review]
1/2 Add "key" field

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

Looks good! Land it with r=gwagner, sr=sicking.
Great work!

Comment 31

[Reuben Morais [:reuben]]

https://tbpl.mozilla.org/?tree=Birch&rev=2e810d6c8779

Comment 32

[Reuben Morais [:reuben]]

(In reply to Reuben Morais [:reuben] from comment #31)
> https://tbpl.mozilla.org/?tree=Birch&rev=2e810d6c8779

Heh. https://hg.mozilla.org/projects/birch/rev/2e810d6c8779

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2e810d6c8779