Closed Bug 769245 Opened 9 years ago Closed 9 years ago

Contacts API: Add ContactEmail Type

Categories

(Core :: DOM: Device Interfaces, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: gwagner, Assigned: gwagner)

References

()

Details

Attachments

(1 file, 2 obsolete files)

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: nobody → anygregor
Attached patch Patch (obsolete) — Splinter Review
first version
Attached patch patch (obsolete) — Splinter Review
few more fixes
Attachment #637512 - Attachment is obsolete: true
Attached patch patchSplinter Review
and without debug statements
Attachment #637583 - Attachment is obsolete: true
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+
(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
https://hg.mozilla.org/mozilla-central/rev/c54c4e2f8058
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
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.
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.