Last Comment Bug 807688 - Add 'key' a'la hCard to Contacts API
: Add 'key' a'la hCard to Contacts API
Status: RESOLVED FIXED
[fixed-in-birch]
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla24
Assigned To: Shane Tully (:stully:
:
Mentors:
https://wiki.mozilla.org/WebAPI/Conta...
Depends on:
Blocks: 807717 857730
  Show dependency treegraph
 
Reported: 2012-11-01 08:52 PDT by David Dahl :ddahl
Modified: 2013-06-18 16:24 PDT (History)
8 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Added publicKey property to MozContacts (3.65 KB, patch)
2013-02-27 16:59 PST, David Dahl :ddahl
anygregor: feedback+
Details | Diff | Review
Add "publicKey" field (2.80 KB, patch)
2013-06-04 14:49 PDT, Shane Tully (:stully:
no flags Details | Diff | Review
1/2 Add "publicKey" field (5.53 KB, patch)
2013-06-04 15:13 PDT, Shane Tully (:stully:
no flags Details | Diff | Review
2/2 Add "publicKey" field tests (2.99 KB, patch)
2013-06-04 15:35 PDT, Shane Tully (:stully:
no flags Details | Diff | Review
1/2 Add "publicKey" field (6.78 KB, patch)
2013-06-04 17:23 PDT, Shane Tully (:stully:
no flags Details | Diff | Review
2/2 Add "publicKey" field tests (3.02 KB, patch)
2013-06-04 17:24 PDT, Shane Tully (:stully:
anygregor: review+
Details | Diff | Review
1/2 Add "publicKey" field (6.78 KB, patch)
2013-06-04 21:02 PDT, Shane Tully (:stully:
no flags Details | Diff | Review
1/2 Add "publicKey" field (6.78 KB, patch)
2013-06-04 21:25 PDT, Shane Tully (:stully:
anygregor: review+
Details | Diff | Review
1/2 Add "key" field (6.73 KB, patch)
2013-06-05 00:37 PDT, Shane Tully (:stully:
anygregor: review+
jonas: superreview+
Details | Diff | Review
2/2 Add "key" field to tests (2.99 KB, patch)
2013-06-05 00:38 PDT, Shane Tully (:stully:
anygregor: review+
Details | Diff | Review
1/2 Add "key" field (7.14 KB, patch)
2013-06-14 10:23 PDT, Shane Tully (:stully:
anygregor: review+
Details | Diff | Review

Description David Dahl :ddahl 2012-11-01 08:52:31 PDT
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 2012-11-01 08:54:22 PDT
Dietrich: I assume we will need a gaia front-end bug for this as well, correct?
Comment 2 Tantek Çelik 2012-11-01 09:14:40 PDT
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) 2012-11-01 10:03:49 PDT
Yes, please file in Boot2Gecko->Gaia.
Comment 4 Tantek Çelik 2012-12-06 15:35:41 PST
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] 2012-12-06 15:52:14 PST
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 2013-02-27 16:59:04 PST
Created attachment 719256 [details] [diff] [review]
Added publicKey property to MozContacts
Comment 7 Gregor Wagner [:gwagner] 2013-03-02 07:44:41 PST
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] 2013-04-04 11:33:59 PDT
Are you going to submit a new patch, David?
Comment 9 David Dahl :ddahl 2013-04-04 13:28:01 PDT
(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 2013-04-10 17:34:05 PDT
Adding Reuben.
Comment 11 Shane Tully (:stully: 2013-06-04 10:36:40 PDT
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 2013-06-04 10:42:09 PDT
(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: 2013-06-04 14:49:21 PDT
Created attachment 758219 [details] [diff] [review]
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] 2013-06-04 14:52:37 PDT
(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: 2013-06-04 15:13:38 PDT
Created attachment 758251 [details] [diff] [review]
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: 2013-06-04 15:35:11 PDT
Created attachment 758264 [details] [diff] [review]
2/2 Add "publicKey" field tests

Add "publicKey" field tests
Comment 17 Shane Tully (:stully: 2013-06-04 15:37:03 PDT
(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: 2013-06-04 17:23:59 PDT
Created attachment 758320 [details] [diff] [review]
1/2 Add "publicKey" field

Add "publicKey" field

Updated to reflect changes in bug 869615.
Comment 19 Shane Tully (:stully: 2013-06-04 17:24:08 PDT
Created attachment 758321 [details] [diff] [review]
2/2 Add "publicKey" field tests

Add "publicKey" field tests

Updated to reflect changes in bug 869615.
Comment 20 Shane Tully (:stully: 2013-06-04 21:02:37 PDT
Created attachment 758379 [details] [diff] [review]
1/2 Add "publicKey" field

Add "publicKey" field 

Fixed typo in previous patch. (sorry for all the revisions!)
Comment 21 Shane Tully (:stully: 2013-06-04 21:22:32 PDT
Comment on attachment 758379 [details] [diff] [review]
1/2 Add "publicKey" field

>Date: 1370387242 25200
>From: Shane Tully <stully@mozilla.com>
>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: 2013-06-04 21:25:06 PDT
Created attachment 758386 [details] [diff] [review]
1/2 Add "publicKey" field

Add "publicKey" field 

Fixed typo in previous patch. (sorry for all the revisions!)
Comment 23 Gregor Wagner [:gwagner] 2013-06-05 00:03:08 PDT
Comment on attachment 758386 [details] [diff] [review]
1/2 Add "publicKey" field

Interface changes require superreview as well.
Comment 24 Gregor Wagner [:gwagner] 2013-06-05 00:06:04 PDT
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] 2013-06-05 00:26:23 PDT
Oh the spec says 'key' and not 'publicKey'. Please change.
Comment 26 Shane Tully (:stully: 2013-06-05 00:37:10 PDT
Created attachment 758434 [details] [diff] [review]
1/2 Add "key" field

Add "key" field

Revised: Changed "publicKey" to "key"
Comment 27 Shane Tully (:stully: 2013-06-05 00:38:19 PDT
Created attachment 758435 [details] [diff] [review]
2/2 Add "key" field to tests

Add "key" field to tests

Revised: CHanged "publicKey" to "key"
Comment 28 Jonas Sicking (:sicking) PTO Until July 5th 2013-06-13 22:08:57 PDT
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: 2013-06-14 10:23:33 PDT
Created attachment 762760 [details] [diff] [review]
1/2 Add "key" field

Added commas as requested.
Comment 30 Gregor Wagner [:gwagner] 2013-06-14 10:31:49 PDT
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] 2013-06-18 13:56:30 PDT
https://tbpl.mozilla.org/?tree=Birch&rev=2e810d6c8779
Comment 32 Reuben Morais [:reuben] 2013-06-18 13:56:57 PDT
(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] 2013-06-18 16:24:17 PDT
https://hg.mozilla.org/mozilla-central/rev/2e810d6c8779

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