769245 - Contacts API: Add ContactEmail Type

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Gregor Wagner [:gwagner]]

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.

Comment 1

[Gregor Wagner [:gwagner]]

Attachment: Patch

first version

Comment 2

[Gregor Wagner [:gwagner]]

Attachment: patch

few more fixes

Comment 3

[Gregor Wagner [:gwagner]]

Attachment: patch

and without debug statements

Comment 4

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

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?

Comment 5

[Gregor Wagner [:gwagner]]

(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

Comment 6

[Ed Morley [:emorley]]

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

Comment 7

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c54c4e2f8058

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c54c4e2f8058

Comment 9

[Tantek Çelik]

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

[alberto.pastor]

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?