The default bug view has changed. See this FAQ.

Contacts API: Add ContactEmail Type

RESOLVED FIXED in mozilla16

Status

()

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

People

(Reporter: gwagner, Assigned: gwagner)

Tracking

unspecified
mozilla16
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Currently we have a DOMString[] for email addresses.
We want to add a ContactEmail type that has a type for home, work, etc and the actual email address.

This is in the v1 requirements and in the UX design.
(Assignee)

Updated

5 years ago
Assignee: nobody → anygregor
(Assignee)

Comment 1

5 years ago
Created attachment 637512 [details] [diff] [review]
Patch

first version
(Assignee)

Comment 2

5 years ago
Created attachment 637583 [details] [diff] [review]
patch

few more fixes
Attachment #637512 - Attachment is obsolete: true
(Assignee)

Comment 3

5 years ago
Created attachment 637584 [details] [diff] [review]
patch

and without debug statements
Attachment #637583 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #637584 - Flags: review?(jonas)
Comment on attachment 637584 [details] [diff] [review]
patch

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

::: dom/contacts/ContactManager.js
@@ +161,5 @@
>          return new Array(aField);
> +      } else if (Array.isArray(aField)) {
> +        return aField;
> +      } else if (aField != null) {
> +        return [aField.toString()];

You're handling strings twice here. Just do something like:

if (Array.isArray(aField)) {
  ..convert each member to a string..
}
else if (aField != null) {
  return [String(aField)];
}

::: dom/contacts/fallback/ContactDB.jsm
@@ +116,5 @@
> +        objectStore.openCursor().onsuccess = function(event) {  
> +          let cursor = event.target.result;
> +          if (cursor) {
> +            debug("upgrade email1: " + JSON.stringify(cursor.value));
> +            for (let address in cursor.value.properties.email) {

It's a little scary to be iterating this array this way as you are modifying it. I'm not sure if JS guarantees that that will work.

Possibly doing something like

cursor.value.properties.email =
  cursor.value.properties.email.map(function(address) { return { address: address }; });

would be safer.

@@ +119,5 @@
> +            debug("upgrade email1: " + JSON.stringify(cursor.value));
> +            for (let address in cursor.value.properties.email) {
> +              cursor.value.properties.email[address] = {address: address};
> +            }
> +            cursor.update(cursor.value);

Don't you need to also fill search.email with data? Or is that already the case?
Attachment #637584 - Flags: review?(jonas) → review+
(Assignee)

Comment 5

5 years ago
(In reply to Jonas Sicking (:sicking) from comment #4)
> @@ +119,5 @@
> > +            debug("upgrade email1: " + JSON.stringify(cursor.value));
> > +            for (let address in cursor.value.properties.email) {
> > +              cursor.value.properties.email[address] = {address: address};
> > +            }
> > +            cursor.update(cursor.value);
> 
> Don't you need to also fill search.email with data? Or is that already the
> case?

That's already the case.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e29885f8f290
Backed out for M3 failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=e29885f8f290

{
12207 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_interfaces.html | Unexpected interface name in global scope: ContactEmail
}

https://hg.mozilla.org/integration/mozilla-inbound/rev/72a742ed3d20
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c54c4e2f8058
https://hg.mozilla.org/mozilla-central/rev/c54c4e2f8058
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16

Comment 9

5 years ago
https://wiki.mozilla.org/WebAPI/ContactsAPI updated with interface ContactEmail with a DOMString[] type to handle the listed requirements from https://wiki.mozilla.org/Gaia/Contacts#Gaia_v1 in particular:
"Email address - home (primary) and offers the ability to add work and other email addresses"

"(primary)" or favorite/preferred email address is indicated as *one* of the types of an email in vCard/hCard as the string "PREF".

home is "home", and work is "work".

Similarly for interface ContactTelephone and ContactAddress.

This usage of "PREF" for the 'favorite' or 'preferred' phone/email/address of a contact has been verified by exporting vCards from various mobile address books (Android, iOS), and noting the *array* of types that accompanies the tel/email/adr fields, including the string "PREF" for favorite/preferred, and as noted, additional values in the type array for "work", "home", "fax" etc.

Thus these updated interfaces represent a mapping that is compatible both with vCard and interop with the exports of existing implementations, which is a significant consideration for us, as users of B2G phones may be moving data from such devices and the better we preserve things like favorites/preferences and they "just work", the better experience we provide to our users.

Comment 10

5 years ago
I see in the wiki that tel and email have {type, value} instead {type, number} and {type, address}.

Is that wiki wrong or the code is going to change to use that value field?
You need to log in before you can comment on or make changes to this bug.