Add 'key' a'la hCard to Contacts API

RESOLVED FIXED in mozilla24

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: ddahl, Assigned: stully)

Tracking

(Blocks: 1 bug)

Trunk
mozilla24
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed-in-birch], URL)

Attachments

(2 attachments, 9 obsolete attachments)

(Reporter)

Description

5 years ago
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
(Reporter)

Comment 1

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

Comment 2

5 years ago
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
Yes, please file in Boot2Gecko->Gaia.
(Reporter)

Updated

5 years ago
Blocks: 807717

Updated

5 years ago
Component: DOM: Apps → DOM: Mozilla Extensions

Comment 4

5 years ago
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
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.
Whiteboard: [good first bug][mentor=gwagner][lang=js]
(Reporter)

Updated

4 years ago
Assignee: nobody → ddahl
(Reporter)

Comment 6

4 years ago
Created attachment 719256 [details] [diff] [review]
Added publicKey property to MozContacts
Attachment #719256 - Flags: feedback?(anygregor)
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.
Attachment #719256 - Flags: feedback?(anygregor) → feedback+

Comment 8

4 years ago
Are you going to submit a new patch, David?
Flags: needinfo?(ddahl)
(Reporter)

Comment 9

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

Yes I will be soon.
Flags: needinfo?(ddahl)
Component: DOM: Mozilla Extensions → DOM
Product: Core → Core

Comment 10

4 years ago
Adding Reuben.
(Assignee)

Comment 11

4 years ago
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?
(Reporter)

Comment 12

4 years ago
(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.
(Assignee)

Comment 13

4 years ago
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.
Attachment #758219 - Flags: feedback?(anygregor)
(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
(Assignee)

Comment 15

4 years ago
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!)
Assignee: ddahl → stully
Attachment #719256 - Attachment is obsolete: true
Attachment #758219 - Attachment is obsolete: true
Attachment #758219 - Flags: feedback?(anygregor)
Attachment #758251 - Flags: review?(anygregor)
(Assignee)

Updated

4 years ago
Attachment #758251 - Attachment description: Add "publicKey" field → 1/2 Add "publicKey" field
(Assignee)

Comment 16

4 years ago
Created attachment 758264 [details] [diff] [review]
2/2 Add "publicKey" field tests

Add "publicKey" field tests
Attachment #758264 - Flags: review?(anygregor)
(Assignee)

Comment 17

4 years ago
(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.
(Assignee)

Updated

4 years ago
Attachment #758251 - Flags: review?(anygregor)
(Assignee)

Updated

4 years ago
Attachment #758264 - Flags: review?(anygregor)
(Assignee)

Comment 18

4 years ago
Created attachment 758320 [details] [diff] [review]
1/2 Add "publicKey" field

Add "publicKey" field

Updated to reflect changes in bug 869615.
Attachment #758251 - Attachment is obsolete: true
Attachment #758320 - Flags: review?(anygregor)
(Assignee)

Comment 19

4 years ago
Created attachment 758321 [details] [diff] [review]
2/2 Add "publicKey" field tests

Add "publicKey" field tests

Updated to reflect changes in bug 869615.
Attachment #758264 - Attachment is obsolete: true
Attachment #758321 - Flags: review?(anygregor)
(Assignee)

Updated

4 years ago
Attachment #758321 - Attachment description: Add "publicKey" field tests → 2/2 Add "publicKey" field tests
(Assignee)

Updated

4 years ago
OS: Linux → All
(Assignee)

Updated

4 years ago
Component: DOM → Canvas: WebGL
Hardware: x86_64 → All
(Assignee)

Updated

4 years ago
Component: Canvas: WebGL → DOM
(Assignee)

Comment 20

4 years ago
Created attachment 758379 [details] [diff] [review]
1/2 Add "publicKey" field

Add "publicKey" field 

Fixed typo in previous patch. (sorry for all the revisions!)
Attachment #758320 - Attachment is obsolete: true
Attachment #758320 - Flags: review?(anygregor)
Attachment #758379 - Flags: review?(anygregor)
(Assignee)

Comment 21

4 years ago
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[]
> };
(Assignee)

Comment 22

4 years ago
Created attachment 758386 [details] [diff] [review]
1/2 Add "publicKey" field

Add "publicKey" field 

Fixed typo in previous patch. (sorry for all the revisions!)
Attachment #758379 - Attachment is obsolete: true
Attachment #758379 - Flags: review?(anygregor)
Attachment #758386 - Flags: review?(anygregor)

Updated

4 years ago
Component: DOM → DOM: Device Interfaces
Comment on attachment 758386 [details] [diff] [review]
1/2 Add "publicKey" field

Interface changes require superreview as well.
Attachment #758386 - Flags: superreview?(jonas)
Comment on attachment 758386 [details] [diff] [review]
1/2 Add "publicKey" field

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

Thanks!
Attachment #758386 - Flags: review?(anygregor) → review+
Attachment #758321 - Flags: review?(anygregor) → review+
Oh the spec says 'key' and not 'publicKey'. Please change.
(Assignee)

Comment 26

4 years ago
Created attachment 758434 [details] [diff] [review]
1/2 Add "key" field

Add "key" field

Revised: Changed "publicKey" to "key"
Attachment #758386 - Attachment is obsolete: true
Attachment #758386 - Flags: superreview?(jonas)
Attachment #758434 - Flags: review?(anygregor)
(Assignee)

Comment 27

4 years ago
Created attachment 758435 [details] [diff] [review]
2/2 Add "key" field to tests

Add "key" field to tests

Revised: CHanged "publicKey" to "key"
Attachment #758321 - Attachment is obsolete: true
Attachment #758435 - Flags: review?(anygregor)
Attachment #758434 - Flags: superreview?(jonas)
Attachment #758434 - Flags: review?(anygregor)
Attachment #758434 - Flags: review+
Attachment #758435 - Flags: review?(anygregor) → review+
(Assignee)

Updated

4 years ago
Blocks: 857730
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.
Attachment #758434 - Flags: superreview?(jonas) → superreview+
(Assignee)

Comment 29

4 years ago
Created attachment 762760 [details] [diff] [review]
1/2 Add "key" field

Added commas as requested.
Attachment #758434 - Attachment is obsolete: true
Attachment #762760 - Flags: review?(jonas)
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!
Attachment #762760 - Flags: review?(jonas) → review+
https://tbpl.mozilla.org/?tree=Birch&rev=2e810d6c8779
Whiteboard: [good first bug][mentor=gwagner][lang=js] → [fixed-in-birch]
(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
https://hg.mozilla.org/mozilla-central/rev/2e810d6c8779
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.