anyone fancy testing this address-conduit patch?

[Matt Davey <mcdavey mrao cam ac uk>]

Dear code monkeys,

I'm attaching two patches, one for evolution-data-server and one for
evolution.  Together, they seem to fix some pretty bad bugs in the
address conduit.  I would appreciate it if anyone out there was willing
to test these patches (Tom, Nathan?).

The diffs are against Evolution branch 2.24 as the trunk would require
me to upgrade my glib and a bunch of other stuff, or build from
gargnome, neither of which I fancy just now.  2.24 was only branched a
couple of weeks ago, so it is pretty up to date.

I haven't yet attempted any support for the new Contact fields.  This
patch is just to fix existing addressbook bugs.  The sync logic,
particularly for the phone and email fields has changed considerably.

Bugs I've tried to fix:
     1. The 'first-name / family-name' split on the palm was not
        respected when writing to the desktop.  It went via 'full_name'.
        See bug #269342 for some more info.  (aside: IMO Evo full_name
        munging is far too fragile to be worth using and I wish they'd
        get rid of it.  It's never going to reliably separate first/last
        names when challenged by, say "Mary Anne Bloggs" and "John Mc
        Neill")
     2. If you erased a phone field on the Palm, it wouldn't be erased
        on the desktop.
     3. The 'OTHER' phone fields were broken in various ways.  This is
        actually already fixed on the 2.24 branch (and trunk) since
        revision 9380 (see bug #547223)
     4. There were many ways for phone fields to be re-ordered.  I've
        tried to avoid this wherever possible.
     5. Evolution was careless about ordering of multiple email/phone
        entries in the vcard.  For example, if you imported a 'vcard'
        with 3 emails, and then saved it immediately, you would find the
        3 entries would be reversed.  This sort of thing could upset the
        palm.
     6. If you had an entry on both desktop and palm, and say you had
        three phone numbers labelled 'home' on the palm.  Then that
        entry could be deleted if you changed the desktop record and
        synced again.

I will probably have to break down the patch into a few components
before submitting to the Evolution maintainers, but I'd appreciate it if
anyone was able to do some testing or take a look over the changes in
the meantime.

Thanks,

Matt

Matt Davey	I like to browse in occult bookshops if for no other reason than
mcdavey mrao cam ac uk   to refresh my commitment to science. -- Heinz Pagels

Index: addressbook/gui/contact-editor/e-contact-editor.c
===================================================================
--- addressbook/gui/contact-editor/e-contact-editor.c	(revision 36550)
+++ addressbook/gui/contact-editor/e-contact-editor.c	(working copy)
@@ -1078,7 +1078,7 @@
 	for (l = attr_list; l; l = g_list_next (l)) {
 		EVCardAttribute *attr = l->data;
 
-		e_vcard_add_attribute (vcard, e_vcard_attribute_copy (attr));
+		e_vcard_append_attribute (vcard, e_vcard_attribute_copy (attr));
 	}
 }
 
Index: addressbook/conduit/address-conduit.c
===================================================================
--- addressbook/conduit/address-conduit.c	(revision 36550)
+++ addressbook/conduit/address-conduit.c	(working copy)
@@ -64,6 +64,12 @@
 #define WARN g_warning
 #define INFO g_message
 
+/* note: evolution GUI contact editor does not preserve EVOLUTION_PILOT_SLOT_PARAM values.
+ *       that's okay, though, because we only need to remember these values if the
+ *       desktop record has not changed since last sync.
+ */
+#define EVOLUTION_PILOT_SLOT_PARAM "X-EVOLUTION-PILOT-SLOT"
+
 enum {
 	LABEL_WORK,
 	LABEL_HOME,
@@ -660,90 +666,112 @@
 	return FALSE;
 }
 
-static gboolean
-is_syncable (EAddrConduitContext *ctxt, EAddrLocalRecord *local)
+static EVCardAttributeParam *
+get_pilot_slot_param (EVCardAttribute *attr)
 {
-	EContactField next_mail, next_home, next_work, next_fax;
-	EContactField next_other, next_main, next_pager, next_mobile;
-	gboolean syncable = TRUE;
-	int i, l = 0;
+        EVCardAttributeParam *param = NULL;
+        GList                *param_list;
+        GList                *l;
 
-	/* See if there are fields we can't sync or not in priority order */
-	get_next_init (&next_mail, &next_home, &next_work, &next_fax,
-		       &next_other, &next_main, &next_pager, &next_mobile);
+        param_list = e_vcard_attribute_get_params (attr);
 
-	for (i = entryPhone1; i <= entryPhone5 && syncable; i++) {
-		int phonelabel = local->addr->phoneLabel[i - entryPhone1];
-		const char *phone_str = local->addr->entry[i];
-		gboolean empty = !(phone_str && *phone_str);
+        for (l = param_list; l; l = g_list_next (l)) {
+                const gchar *str;
 
-		if (empty)
-			continue;
+                param = l->data;
 
-		for ( ; priority_label[l] != -1; l++)
-			if (phonelabel == priority_label[l])
-				break;
+                str = e_vcard_attribute_param_get_name (param);
+                if (!g_ascii_strcasecmp (str, EVOLUTION_PILOT_SLOT_PARAM))
+                        break;
 
-		if (priority_label[l] == -1) {
-			syncable = FALSE;
-			continue;
-		}
+                param = NULL;
+        }
 
-		if (phonelabel == LABEL_EMAIL) {
-			if (is_next_done (next_mail) || next_mail != priority[l]) {
-				syncable = FALSE;
-				break;
-			}
-			next_mail = get_next_mail (&next_mail);
-		} else if (phonelabel == LABEL_HOME) {
-			if (is_next_done (next_home) || next_home != priority[l]) {
-				syncable = FALSE;
-				break;
-			}
-			next_home = get_next_home (&next_home);
-		} else if (phonelabel == LABEL_WORK) {
-			if (is_next_done (next_work) || next_work != priority[l]) {
-				syncable = FALSE;
-				break;
-			}
-			next_work = get_next_work (&next_work);
-		} else if (phonelabel == LABEL_FAX) {
-			if (is_next_done (next_fax) || next_fax != priority[l]) {
-				syncable = FALSE;
-				break;
-			}
-			next_fax = get_next_fax (&next_fax);
-		} else if (phonelabel == LABEL_OTHER) {
-			if (is_next_done (next_other) || next_other != priority[l]) {
-				syncable = FALSE;
-				break;
-			}
-			next_other = get_next_other (&next_other);
-		} else if (phonelabel == LABEL_MAIN) {
-			if (is_next_done (next_main) || next_main != priority[l]) {
-				syncable = FALSE;
-				break;
-			}
-			next_main = get_next_main (&next_main);
-		} else if (phonelabel == LABEL_PAGER) {
-			if (is_next_done (next_pager) || next_pager != priority[l]) {
-				syncable = FALSE;
-				break;
-			}
-			next_pager = get_next_pager (&next_pager);
-		} else if (phonelabel == LABEL_MOBILE) {
-			if (is_next_done (next_mobile) || next_mobile != priority[l]) {
-				syncable = FALSE;
-				break;
-			}
-			next_mobile = get_next_mobile (&next_mobile);
-		}
+        return param;
+}
+
+/* Given an e-card attribute, return the numerical value of the
+ * associated pilot slot, or -1 if it is not set.
+ */
+static gint
+get_pilot_slot (EVCardAttribute *attr)
+{
+        EVCardAttributeParam *param;
+        gint                  slot = -1;
+
+        param = get_pilot_slot_param (attr);
+
+        if (param) {
+                GList *value_list;
+
+                value_list = e_vcard_attribute_param_get_values (param);
+                slot = atoi (value_list->data);
+        }
+
+        return slot;
+}
+
+/* Associate a pilot slot with the supplied attribute.
+ * Use the special value -1 to remove slot tag from attribute
+ */
+static void
+set_pilot_slot (EVCardAttribute *attr, gint slot)
+{
+        EVCardAttributeParam *param;
+        gchar                *slot_str;
+
+        param = get_pilot_slot_param (attr);
+	if (slot == -1) {
+		if (param)
+			e_vcard_attribute_remove_param (attr, EVOLUTION_PILOT_SLOT_PARAM);
+		return;
 	}
+        if (!param) {
+                param = e_vcard_attribute_param_new (EVOLUTION_PILOT_SLOT_PARAM);
+                e_vcard_attribute_add_param (attr, param);
+        }
 
-	return syncable;
+        e_vcard_attribute_param_remove_values (param);
+
+        slot_str = g_strdup_printf ("%d", slot);
+        e_vcard_attribute_param_add_value (param, slot_str);
+        g_free (slot_str);
 }
 
+
+/* retrieve the pilot slot associated with a contact field, or -1 if it
+ * is not set.
+ */
+static int
+get_contact_field_pilot_slot(EContact *contact, EContactField field)
+{
+	EVCardAttribute *attr;
+	gpointer data;
+
+        data = e_contact_get_field_and_attribute (contact, field, &attr);
+	if (attr) {
+		return get_pilot_slot(attr);
+	} else {
+		return -1;
+	}
+}
+/* set the pilot slot associated with a contact field, or remove it
+ * if -1 is supplied.
+ */
 static void
+set_contact_field_pilot_slot(EContact *contact, EContactField field, gint slot)
+{
+	EVCardAttribute *attr;
+	gpointer data;
+
+        data = e_contact_get_field_and_attribute (contact, field, &attr);
+	if (attr)
+		set_pilot_slot(attr, slot);
+	else
+		LOG(g_message("couldn't find field %d for writing pilot slot %d\n", field, slot));
+}
+
+static void
 set_contact_text (EContact *contact, EContactField field, struct Address address, int entry)
 {
 	char *text = NULL;
@@ -854,8 +882,11 @@
 	EContactAddress *address = NULL;
 	int phone = entryPhone1;
 	EContactField field;
-	gboolean syncable;
 	int i;
+	gboolean field_used[E_CONTACT_FIELD_LAST];
+	gboolean slot_used[5];
+	EContactField next_mail, next_home, next_work, next_fax;
+	EContactField next_other, next_main, next_pager, next_mobile, current_field;
 
 	g_return_if_fail (local != NULL);
 	g_return_if_fail (contact != NULL);
@@ -867,8 +898,10 @@
 
 	local->addr = g_new0 (struct Address, 1);
 
-	/* Handle the fields and category we don't sync by making sure
-         * we don't overwrite them
+	/* First load the current phone and custom fields from the pda, if a
+	 * record exists. This ensures we don't lose data that we can't map to
+	 * the Desktop record, such as 'Custom' fields or third and subsequent
+	 * Home phone numbers.
 	 */
 	if (local->local.ID != 0) {
 		struct Address addr;
@@ -960,71 +993,124 @@
 	}
 
 	/* Phone numbers */
+	/* There are 5 'phone' slots on the pda, that take entries of type
+	 * home/work/email/pager/fax, etc.  They could all be of the same type,
+	 * for example.  These don't map cleanly to Evolution addressbook
+	 * fields, so we need to be careful to avoid reordering, duplicating or
+	 * losing data.  The logic we use is as follows: for each slot on the
+	 * pda, fetch a corresponding entry from the Desktop record, if it
+	 * exists.  Second pass, we find any blank slots on the pda and write
+	 * in highest 'priority' fields not yet written to pda.
+	 */
+	for (i = 0; i < E_CONTACT_FIELD_LAST; i++)
+		field_used[i] = FALSE;
+	get_next_init (&next_mail, &next_home, &next_work, &next_fax,
+	    &next_other, &next_main, &next_pager, &next_mobile);
 
-	/* See if everything is syncable */
-	syncable = is_syncable (ctxt, local);
+	/* First pass, read Desktop fields to fill in pda slots, as
+	 * per the 'labels' assigned to each pda slot
+	 */
+	for (i = entryPhone1; i <= entryPhone5; i++) {
+		int phonelabel = local->addr->phoneLabel[i - entryPhone1];
+		const char *phone_str = NULL;
 
-	if (syncable) {
-		INFO ("Syncable");
+		current_field = E_CONTACT_FIELD_LAST;
 
-		/* Sync by priority */
-		for (i = 0, phone = entryPhone1;
-		     priority[i] != E_CONTACT_FIELD_LAST && phone <= entryPhone5; i++) {
-			const char *phone_str;
-
-			phone_str = e_contact_get_const (contact, priority[i]);
+		if (phonelabel == LABEL_EMAIL && !is_next_done (next_mail)) {
+			phone_str = e_contact_get_const (contact, next_mail);
+			current_field = next_mail;
+			next_mail = get_next_mail (&next_mail);
+		} else if (phonelabel == LABEL_HOME && !is_next_done (next_home)) {
+			phone_str = e_contact_get_const (contact, next_home);
+			current_field = next_home;
+			next_home = get_next_home (&next_home);
+		} else if (phonelabel == LABEL_WORK && !is_next_done (next_work)) {
+			phone_str = e_contact_get_const (contact, next_work);
+			current_field = next_work;
+			next_work = get_next_work (&next_work);
+		} else if (phonelabel == LABEL_FAX && !is_next_done (next_fax)) {
+			phone_str = e_contact_get_const (contact, next_fax);
+			current_field = next_fax;
+			next_fax = get_next_fax (&next_fax);
+		} else if (phonelabel == LABEL_OTHER && !is_next_done (next_other)) {
+			phone_str = e_contact_get_const (contact, next_other);
+			current_field = next_other;
+			next_other = get_next_other (&next_other);
+		} else if (phonelabel == LABEL_MAIN && !is_next_done (next_main)) {
+			phone_str = e_contact_get_const (contact, next_main);
+			current_field = next_mail;
+			next_main = get_next_main (&next_main);
+		} else if (phonelabel == LABEL_PAGER && !is_next_done (next_pager)) {
+			phone_str = e_contact_get_const (contact, next_pager);
+			current_field = next_pager;
+			next_pager = get_next_pager (&next_pager);
+		} else if (phonelabel == LABEL_MOBILE && !is_next_done (next_mobile)) {
+			phone_str = e_contact_get_const (contact, next_mobile);
+			current_field = next_mobile;
+			next_mobile = get_next_mobile (&next_mobile);
+		}
+		if (current_field != E_CONTACT_FIELD_LAST) {
+			clear_entry_text (*local->addr, i);
 			if (phone_str && *phone_str) {
-				clear_entry_text (*local->addr, phone);
-				local->addr->entry[phone] = e_pilot_utf8_to_pchar (phone_str);
-				local->addr->phoneLabel[phone - entryPhone1] = priority_label[i];
-				phone++;
+				local->addr->entry[i] = e_pilot_utf8_to_pchar (phone_str);
+			} else {
+				/* empty field on desktop, so clear field on pda */
+				local->addr->entry[i] = NULL;
 			}
+			field_used[current_field] = TRUE;
 		}
-		for ( ; phone <= entryPhone5; phone++)
-			local->addr->phoneLabel[phone - entryPhone1] = phone - entryPhone1;
-		local->addr->showPhone = 0;
-	} else {
-		EContactField next_mail, next_home, next_work, next_fax;
-		EContactField next_other, next_main, next_pager, next_mobile;
+		if (local->addr->entry[i] && *local->addr->entry[i])
+			slot_used[i - entryPhone1] = TRUE;
+		else
+			slot_used[i - entryPhone1] = FALSE;
+	}
 
-		INFO ("Not Syncable");
-		get_next_init (&next_mail, &next_home, &next_work, &next_fax,
-			       &next_other, &next_main, &next_pager, &next_mobile);
-
-		/* Not completely syncable, so do the best we can */
-		for (i = entryPhone1; i <= entryPhone5; i++) {
-			int phonelabel = local->addr->phoneLabel[i - entryPhone1];
-			const char *phone_str = NULL;
-
-			if (phonelabel == LABEL_EMAIL && !is_next_done (next_mail)) {
-				phone_str = e_contact_get_const (contact, next_mail);
-				next_mail = get_next_mail (&next_mail);
-			} else if (phonelabel == LABEL_HOME && !is_next_done (next_home)) {
-  				phone_str = e_contact_get_const (contact, next_home);
-				next_home = get_next_home (&next_home);
-			} else if (phonelabel == LABEL_WORK && !is_next_done (next_work)) {
-				phone_str = e_contact_get_const (contact, next_work);
-				next_work = get_next_work (&next_work);
-			} else if (phonelabel == LABEL_FAX && !is_next_done (next_fax)) {
-				phone_str = e_contact_get_const (contact, next_fax);
-				next_fax = get_next_fax (&next_fax);
-			} else if (phonelabel == LABEL_OTHER && !is_next_done (next_other)) {
-				phone_str = e_contact_get_const (contact, next_other);
-				next_other = get_next_other (&next_other);
-			} else if (phonelabel == LABEL_MAIN && !is_next_done (next_main)) {
-				phone_str = e_contact_get_const (contact, next_main);
-				next_main = get_next_main (&next_main);
-			} else if (phonelabel == LABEL_PAGER && !is_next_done (next_pager)) {
-				phone_str = e_contact_get_const (contact, next_pager);
-				next_pager = get_next_pager (&next_pager);
-			} else if (phonelabel == LABEL_MOBILE && !is_next_done (next_mobile)) {
-				phone_str = e_contact_get_const (contact, next_mobile);
-				next_mobile = get_next_mobile (&next_mobile);
+	/* Second pass: float all empty slots to the bottom.  This ensures that
+	 * if we add additional entries that they are after any existing entries
+	 * of the same type.
+	 */
+	for (i = phone = 0; phone < 5 && i < 5; phone++) {
+		if (!slot_used[phone]) {
+			for (i = phone + 1; i < 5; i++) {
+				if (slot_used[i]) {
+					clear_entry_text(*local->addr, phone + entryPhone1);
+					local->addr->entry[phone + entryPhone1] =
+					    local->addr->entry[i + entryPhone1];
+					local->addr->phoneLabel[phone] = 
+					    local->addr->phoneLabel[i];
+					local->addr->entry[i + entryPhone1] = NULL;
+					slot_used[phone] = TRUE;
+					slot_used[i] = FALSE;
+					if (local->addr->showPhone == i)
+						local->addr->showPhone = phone;
+					break;
+				}
 			}
+		}
+	}
 
+	/* Third pass: for each empty field on pda, write in next unwritten field 
+	 * on Desktop in priority order.
+	 */
+	for (phone = entryPhone1, i = 0;
+	     priority[i] != E_CONTACT_FIELD_LAST && phone <= entryPhone5;
+	     phone++) {
+		const char *phone_str = get_entry_text (*local->addr, phone);
+		
+		if (phone_str && *phone_str)
+			continue;
+		for (; priority[i] != E_CONTACT_FIELD_LAST; i++) {
+			if (field_used[priority[i]])
+				continue;
+			/* we have a desktop field not yet written to pda */
+			phone_str = e_contact_get_const (contact, priority[i]);
 			if (phone_str && *phone_str) {
-				clear_entry_text (*local->addr, i);
-				local->addr->entry[i] = e_pilot_utf8_to_pchar (phone_str);
+				clear_entry_text (*local->addr, phone);
+				local->addr->entry[phone] = e_pilot_utf8_to_pchar (phone_str);
+				local->addr->phoneLabel[phone - entryPhone1] =
+				    priority_label[i];
+				field_used[priority[i]] = TRUE;
+				break;
 			}
 		}
 	}
@@ -1077,7 +1163,9 @@
 	char *txt, *find, *full_name;
 	EContactField next_mail, next_home, next_work, next_fax;
 	EContactField next_other, next_main, next_pager, next_mobile;
-	int i;
+	char *new_field_str[E_CONTACT_FIELD_LAST];
+	EContactField slot_to_field[5];
+	int i, slot;
 #ifdef PILOT_LINK_0_12
 	pi_buffer_t * buffer;
 #endif
@@ -1109,6 +1197,9 @@
 	name->given = get_entry_text (address, entryFirstname);
 	name->family = get_entry_text (address, entryLastname);
 
+	/* set the name, respecting the pilot's given/family names */
+	e_contact_set (contact, E_CONTACT_NAME, name);
+	/* now set the full_name */
 	full_name = e_contact_name_to_string (name);
 	e_contact_set (contact, E_CONTACT_FULL_NAME, full_name);
 	e_contact_name_free (name);
@@ -1124,6 +1215,8 @@
 	set_contact_text (contact, E_CONTACT_ORG, address, entryCompany);
 
 	/* Address */
+	/* look for a non-empty address, starting with the configured default
+	 * address */
 	mailing_address = -1;
 	if ((eaddress = e_contact_get (contact, ctxt->cfg->default_address))) {
 		mailing_address = ctxt->cfg->default_address;
@@ -1162,41 +1255,115 @@
 	e_contact_address_free (eaddress);
 
 	/* Phone numbers */
+	/* logic:
+	 * first pass: examine slots on pilot and decide which ones
+	 *             will be replicated on the desktop record, and
+	 *             what their desktop field ids will be.
+	 * second pass:delete all fields in the desktop record that currently
+	 *             have a pilot 'slot id' but were not marked (by the
+	 *             first pass) to be included in the sync.  Then remove
+	 *             all existing 'slot ids' in the desktop record.
+	 * third pass: go through list created in the first pass and write
+	 *             records to the Desktop, including current slot ids.
+	 */
+	for (i = 0; i < E_CONTACT_FIELD_LAST; i++)
+		/* new_field_str maps contact fields to the string value they
+		 * should have, from the pilot, post sync.  NULL means the
+		 * contact field is not taken from a pilot slot.
+		 */
+		new_field_str[i] = NULL;
+	for (i = 0; i < 5; i++)
+		/* slot_to_field maps pilot slots to the contact fields they
+		 * should be stored in post sync. E_CONTACT_FIELD_LAST means
+		 * the slot is not stored on the desktop.
+		 */
+		slot_to_field[i] = E_CONTACT_FIELD_LAST;
+
 	get_next_init (&next_mail, &next_home, &next_work, &next_fax,
 		       &next_other, &next_main, &next_pager, &next_mobile);
 
+	/* first pass */
 	for (i = entryPhone1; i <= entryPhone5; i++) {
 		int phonelabel = address.phoneLabel[i - entryPhone1];
 		char *phonenum = get_entry_text (address, i);
 
+		/* empty records won't get synced */
+		if (!phonenum || !*phonenum) {
+			if (phonenum)
+				g_free(phonenum);
+			continue;
+		}
+
 		if (phonelabel == LABEL_EMAIL && !is_next_done (next_mail)) {
-			e_contact_set (contact, next_mail, phonenum);
-			next_mail = get_next_mail (&next_mail);
+			slot_to_field[i - entryPhone1] = next_mail;
+			new_field_str[next_mail] = phonenum;
+			if (phonenum && *phonenum)
+				next_mail = get_next_mail (&next_mail);
 		} else if (phonelabel == LABEL_HOME && !is_next_done (next_home)) {
-			e_contact_set (contact, next_home, phonenum);
-			next_home = get_next_home (&next_home);
+			slot_to_field[i - entryPhone1] = next_home;
+			new_field_str [next_home] = phonenum;
+			if (phonenum && *phonenum)
+				next_home = get_next_home (&next_home);
 		} else if (phonelabel == LABEL_WORK && !is_next_done (next_work)) {
-			e_contact_set (contact, next_work, phonenum);
-			next_work = get_next_work (&next_work);
+			slot_to_field[i - entryPhone1] = next_work;
+			new_field_str [next_work] = phonenum;
+			if (phonenum && *phonenum)
+				next_work = get_next_work (&next_work);
 		} else if (phonelabel == LABEL_FAX && !is_next_done (next_fax)) {
-			e_contact_set (contact, next_fax, phonenum);
-			next_fax = get_next_fax (&next_fax);
+			slot_to_field[i - entryPhone1] = next_fax;
+			new_field_str [next_fax] = phonenum;
+			if (phonenum && *phonenum)
+				next_fax = get_next_fax (&next_fax);
 		} else if (phonelabel == LABEL_OTHER && !is_next_done (next_other)) {
-			e_contact_set (contact, next_other, phonenum);
-			next_other = get_next_other (&next_other);
+			slot_to_field[i - entryPhone1] = next_other;
+			new_field_str [next_other] = phonenum;
+			if (phonenum && *phonenum)
+				next_other = get_next_other (&next_other);
 		} else if (phonelabel == LABEL_MAIN && !is_next_done (next_main)) {
-			e_contact_set (contact, next_main, phonenum);
-			next_main = get_next_main (&next_main);
+			slot_to_field[i - entryPhone1] = next_main;
+			new_field_str [next_main] = phonenum;
+			if (phonenum && *phonenum)
+				next_main = get_next_main (&next_main);
 		} else if (phonelabel == LABEL_PAGER && !is_next_done (next_pager)) {
-			e_contact_set (contact, next_pager, phonenum);
-			next_pager = get_next_pager (&next_pager);
+			slot_to_field[i - entryPhone1] = next_pager;
+			new_field_str [next_pager] = phonenum;
+			if (phonenum && *phonenum)
+				next_pager = get_next_pager (&next_pager);
 		} else if (phonelabel == LABEL_MOBILE && !is_next_done (next_mobile)) {
-			e_contact_set (contact, next_mobile, phonenum);
-			next_mobile = get_next_mobile (&next_mobile);
+			slot_to_field[i - entryPhone1] = next_mobile;
+			new_field_str [next_mobile] = phonenum;	
+			if (phonenum && *phonenum)
+				next_mobile = get_next_mobile (&next_mobile);
 		}
-
-		g_free (phonenum);
 	}
+	/* second pass */
+	for (i = 0; priority[i] != E_CONTACT_FIELD_LAST; i++) {
+		slot = get_contact_field_pilot_slot (contact, priority[i]);
+		if (slot != -1) {
+			if (new_field_str[priority[i]] == NULL) {
+				/*LOG (g_message ("  removing field %d (%s) (priority %d)\n", priority[i], e_contact_get(contact, priority[i]), i)); */
+				e_contact_set (contact, priority[i], NULL);
+			} else {
+				/* LOG (g_message ("  removing slot %d from field  %d (priority %d)\n", slot, priority[i], i));*/
+				set_contact_field_pilot_slot (contact, priority[i], -1);
+			}
+		}
+	}
+	/* third pass */
+	for (i = 0; i < 5; i++) {
+		if (slot_to_field[i] != E_CONTACT_FIELD_LAST) {
+			/* LOG (g_message ("  writing text %s to field  %d\n",
+			   new_field_str[slot_to_field[i]], slot_to_field[i])); */
+			e_contact_set(contact, slot_to_field[i], new_field_str[slot_to_field[i]]);
+			/* LOG (g_message ("contact: %s\n", e_vcard_to_string
+			   ((EVCard *)contact, EVC_FORMAT_VCARD_30))); */
+			/*LOG (g_message ("  now writing pilot slot %d (%s) to field  %d\n",
+			  i, new_field_str[slot_to_field[i]], slot_to_field[i])); */
+			set_contact_field_pilot_slot(contact, slot_to_field[i], i);
+			/* LOG (g_message ("contact: %s\n", e_vcard_to_string
+			   ((EVCard *)contact, EVC_FORMAT_VCARD_30))); */
+		}
+	}
 
 	/* Note */
 	set_contact_text (contact, E_CONTACT_NOTE, address, entryNote);
Index: calendar/conduits/common/libecalendar-common-conduit.c
===================================================================
--- calendar/conduits/common/libecalendar-common-conduit.c	(revision 36550)
+++ calendar/conduits/common/libecalendar-common-conduit.c	(working copy)
@@ -26,7 +26,7 @@
 #include <string.h>
 
 #include <libedataserver/e-categories.h>
-#include <e-util/e-pilot-util.h>
+#include <e-pilot-util.h>
 #include <pi-appinfo.h>
 #include <glib.h>
 
Index: calendar/conduits/common/Makefile.am
===================================================================
--- calendar/conduits/common/Makefile.am	(revision 36550)
+++ calendar/conduits/common/Makefile.am	(working copy)
@@ -1,4 +1,5 @@
 INCLUDES = 					\
+	-I$(top_srcdir)			\
 	-I$(top_srcdir)/e-util			\
 	-I$(top_builddir)/e-util		\
 	$(EVOLUTION_CALENDAR_CONDUIT_CFLAGS)

Index: addressbook/libebook/e-contact.c
===================================================================
--- addressbook/libebook/e-contact.c	(revision 9639)
+++ addressbook/libebook/e-contact.c	(working copy)
@@ -808,7 +808,8 @@
 								e_vcard_attribute_param_new (EVC_TYPE),
 								"OTHER");
 					}
-					e_vcard_add_attribute (E_VCARD (contact), attr);
+					/* append, so that first attr is still first attr */
+					e_vcard_append_attribute (E_VCARD (contact), attr);
 				}
 
 				e_vcard_attribute_add_value (attr, sval);
@@ -829,7 +830,7 @@
 			gboolean found = FALSE;
 			int num_left = info->list_elem;
 			GList *attrs = e_vcard_get_attributes (E_VCARD (contact));
-			GList *l;
+			GList *l, *last_attr;
 
 			for (l = attrs; l && !found; l = l->next) {
 				const char *name;
@@ -880,10 +881,12 @@
 						}
 
 						if (found_needed1 && found_needed2) {
+							last_attr = l; /* remember last occurrence, even if it's not the one we want */
 							if (num_left-- == 0) {
 								found = TRUE;
-								break;
 							}
+							break; /* break here, otherwise we'd match again if there
+								  are any more params in this attribute */
 						}
 					}
 				}
@@ -896,7 +899,8 @@
 			else {
 				/* we didn't find it - add a new attribute */
 				attr = e_vcard_attribute_new (NULL, info->vcard_field_name);
-				e_vcard_add_attribute (E_VCARD (contact), attr);
+				/* append attribute, to ensure we don't change HOME into HOME_1, for example */
+				e_vcard_append_attribute (E_VCARD (contact), attr);
 				if (info->attr_type1)
 					e_vcard_attribute_add_param_with_value (attr, e_vcard_attribute_param_new (EVC_TYPE),
 										info->attr_type1);
@@ -1372,18 +1376,25 @@
 }
 
 /**
- * e_contact_get:
+ * e_contact_get_field_and_attribute:
  * @contact: an #EContact
  * @field_id: an #EContactField
+ * @attrp: pointer to an #EVCardAttribute
  *
- * Gets the value of @contact's field specified by @field_id.
+ * Gets the value of @contact's field specified by @field_id, and
+ * returns a pointer to the corresponding #EVCardAttribute in @attrp,
+ * if the field is backed by a single attribute.  @attrp is set to NULL
+ * if the field is not backed by a single attribute, or the field is
+ * not found.
  *
  * Return value: Depends on the field's type, owned by the caller.
  **/
 gpointer
-e_contact_get (EContact *contact, EContactField field_id)
+e_contact_get_field_and_attribute (EContact *contact, EContactField field_id,
+    EVCardAttribute **attrp)
 {
 	const EContactFieldInfo *info = NULL;
+	*attrp = NULL;
 
 	g_return_val_if_fail (contact && E_IS_CONTACT (contact), NULL);
 	g_return_val_if_fail (field_id >= 1 && field_id <= E_CONTACT_FIELD_LAST, NULL);
@@ -1391,20 +1402,20 @@
 	info = &field_info[field_id];
 
 	if (info->t & E_CONTACT_FIELD_TYPE_BOOLEAN) {
-		EVCardAttribute *attr = e_contact_get_first_attr (contact, info->vcard_field_name);
+		*attrp = e_contact_get_first_attr (contact, info->vcard_field_name);
 		gboolean rv = FALSE;
 
-		if (attr) {
-			GList *v = e_vcard_attribute_get_values (attr);
+		if (*attrp) {
+			GList *v = e_vcard_attribute_get_values (*attrp);
 			rv = v && v->data && !g_ascii_strcasecmp ((char*)v->data, "true");
 			return rv ? "1" : NULL;
 		}
 	}
 	else if (info->t & E_CONTACT_FIELD_TYPE_LIST) {
-		EVCardAttribute *attr = e_contact_get_first_attr (contact, info->vcard_field_name);
+		*attrp = e_contact_get_first_attr (contact, info->vcard_field_name);
 
-		if (attr) {
-			GList *list = g_list_copy (e_vcard_attribute_get_values (attr));
+		if (*attrp) {
+			GList *list = g_list_copy (e_vcard_attribute_get_values (*attrp));
 			GList *l;
 			for (l = list; l; l = l->next)
 				l->data = g_strdup (l->data);
@@ -1413,12 +1424,12 @@
 	}
 	else if (info->t & E_CONTACT_FIELD_TYPE_LIST_ELEM) {
 		if (info->t & E_CONTACT_FIELD_TYPE_STRING) {
-			EVCardAttribute *attr = e_contact_get_first_attr (contact, info->vcard_field_name);
+			*attrp = e_contact_get_first_attr (contact, info->vcard_field_name);
 
-			if (attr) {
+			if (*attrp) {
 				GList *v;
 
-				v = e_vcard_attribute_get_values (attr);
+				v = e_vcard_attribute_get_values (*attrp);
 				v = g_list_nth (v, info->list_elem);
 
 				return v ? g_strdup (v->data) : NULL;
@@ -1427,6 +1438,8 @@
 	}
 	else if (info->t & E_CONTACT_FIELD_TYPE_MULTI_ELEM) {
 		if (info->t & E_CONTACT_FIELD_TYPE_STRING) {
+			/* more than one attribute, so do not set *attrp until
+			 * we find the requested one */
 			GList *attrs, *l;
 			int num_left = info->list_elem;
 
@@ -1441,6 +1454,7 @@
 				if (!g_ascii_strcasecmp (name, info->vcard_field_name)) {
 					if (num_left-- == 0) {
 						GList *v = e_vcard_attribute_get_values (attr);
+						*attrp = attr;
 
 						return v ? g_strdup (v->data) : NULL;
 					}
@@ -1449,11 +1463,11 @@
 		}
 	}
 	else if (info->t & E_CONTACT_FIELD_TYPE_ATTR_TYPE) {
-		EVCardAttribute *attr = e_contact_find_attribute_with_types (contact, info->vcard_field_name, info->attr_type1, info->attr_type2, info->list_elem);
+		*attrp = e_contact_find_attribute_with_types (contact, info->vcard_field_name, info->attr_type1, info->attr_type2, info->list_elem);
 
 		if (info->t & E_CONTACT_FIELD_TYPE_STRING) {
-			if (attr) {
-				GList *p = e_vcard_attribute_get_values (attr);
+			if (*attrp) {
+				GList *p = e_vcard_attribute_get_values (*attrp);
 				return g_strdup (p->data);
 			}
 			else {
@@ -1461,21 +1475,21 @@
 			}
 		}
 		else { /* struct */
-			return info->struct_getter (contact, attr);
+			return info->struct_getter (contact, *attrp);
 		}
 
 	}
 	else if (info->t & E_CONTACT_FIELD_TYPE_STRUCT) {
-		EVCardAttribute *attr = e_contact_get_first_attr (contact, info->vcard_field_name);
-		if (attr)
-			return info->struct_getter (contact, attr);
+		*attrp = e_contact_get_first_attr (contact, info->vcard_field_name);
+		if (*attrp)
+			return info->struct_getter (contact, *attrp);
 	}
 	else if (info->t & E_CONTACT_FIELD_TYPE_GETSET) {
-		EVCardAttribute *attr = e_contact_get_first_attr (contact, info->vcard_field_name);
+		*attrp = e_contact_get_first_attr (contact, info->vcard_field_name);
 		void *rv = NULL;
 
-		if (attr)
-			rv = info->struct_getter (contact, attr);
+		if (*attrp)
+			rv = info->struct_getter (contact, *attrp);
 
 		if (info->t & E_CONTACT_FIELD_TYPE_STRUCT)
 			return (gpointer)info->boxed_type_getter();
@@ -1504,12 +1518,12 @@
 			return g_strdup (str);
 		}
 		case E_CONTACT_CATEGORIES: {
-			EVCardAttribute *attr = e_contact_get_first_attr (contact, EVC_CATEGORIES);
+			*attrp = e_contact_get_first_attr (contact, EVC_CATEGORIES);
 			char *rv = NULL;
 
-			if (attr) {
+			if (*attrp) {
 				GString *str = g_string_new ("");
-				GList *v = e_vcard_attribute_get_values (attr);
+				GList *v = e_vcard_attribute_get_values (*attrp);
 				while (v) {
 					g_string_append (str, (char*)v->data);
 					v = v->next;
@@ -1528,6 +1542,7 @@
 		}
 	}
 	else {
+		/* no single attribute, so don't set *attrp */
 		GList *attrs, *l;
 		GList *rv = NULL; /* used for multi attribute lists */
 
@@ -1557,6 +1572,23 @@
 }
 
 /**
+ * e_contact_get:
+ * @contact: an #EContact
+ * @field_id: an #EContactField
+ *
+ * Gets the value of @contact's field specified by @field_id.
+ *
+ * Return value: Depends on the field's type, owned by the caller.
+ **/
+gpointer
+e_contact_get (EContact *contact, EContactField field_id)
+{
+	EVCardAttribute *attr;
+	return e_contact_get_field_and_attribute(contact, field_id, &attr);
+}
+
+
+/**
  * e_contact_get_const:
  * @contact: an #EContact
  * @field_id: an #EContactField
@@ -1640,11 +1672,12 @@
 		name = e_vcard_attribute_get_name (attr);
 
 		if (!g_ascii_strcasecmp (name, info->vcard_field_name)) {
-			l = g_list_prepend (l, e_vcard_attribute_copy (attr));
+			l = g_list_append (l, e_vcard_attribute_copy (attr));
 		}
 	}
 
-	return g_list_reverse(l);
+	return l;
+
 }
 
 /**
@@ -1669,7 +1702,11 @@
 	e_vcard_remove_attributes (E_VCARD (contact), NULL, info->vcard_field_name);
 
 	for (l = attributes; l; l = l->next)
-		e_vcard_add_attribute (E_VCARD (contact),
+		/* append the attributes, to keep ensure synthetic
+		 * attributes such as EMAIL_1, EMAIL_2 are in the same
+		 * order as the vcard entries appear.
+		 */
+		e_vcard_append_attribute (E_VCARD (contact),
 				       e_vcard_attribute_copy ((EVCardAttribute*)l->data));
 }
 
Index: addressbook/libebook/e-vcard.c
===================================================================
--- addressbook/libebook/e-vcard.c	(revision 9639)
+++ addressbook/libebook/e-vcard.c	(working copy)
@@ -1149,6 +1149,22 @@
 }
 
 /**
+ * e_vcard_append_attribute:
+ * @evc: an #EVCard
+ * @attr: an #EVCardAttribute to add to end of list of attributes
+ *
+ * Adds @attr to @evc.
+ **/
+void
+e_vcard_append_attribute (EVCard *evc, EVCardAttribute *attr)
+{
+	g_return_if_fail (E_IS_VCARD (evc));
+	g_return_if_fail (attr != NULL);
+
+	evc->priv->attributes = g_list_append (evc->priv->attributes, attr);
+}
+
+/**
  * e_vcard_add_attribute_with_value:
  * @evcard: an #EVCard
  * @attr: an #EVCardAttribute to add
Index: addressbook/libebook/e-contact.h
===================================================================
--- addressbook/libebook/e-contact.h	(revision 9639)
+++ addressbook/libebook/e-contact.h	(working copy)
@@ -294,6 +294,7 @@
 EContact*               e_contact_duplicate        (EContact *contact);
 
 gpointer                e_contact_get              (EContact *contact, EContactField field_id);
+gpointer		e_contact_get_field_and_attribute (EContact *contact, EContactField field_id, EVCardAttribute **attrp);
 gconstpointer		e_contact_get_const        (EContact *contact, EContactField field_id);
 void                    e_contact_set              (EContact *contact, EContactField field_id, const gpointer value);
 
Index: addressbook/libebook/e-vcard.h
===================================================================
--- addressbook/libebook/e-vcard.h	(revision 9639)
+++ addressbook/libebook/e-vcard.h	(working copy)
@@ -146,6 +146,7 @@
 void             e_vcard_remove_attributes           (EVCard *evc, const char *attr_group, const char *attr_name);
 void             e_vcard_remove_attribute            (EVCard *evc, EVCardAttribute *attr);
 void             e_vcard_add_attribute               (EVCard *evc, EVCardAttribute *attr);
+void             e_vcard_append_attribute            (EVCard *evc, EVCardAttribute *attr);
 void             e_vcard_add_attribute_with_value    (EVCard *evcard, EVCardAttribute *attr, const char *value);
 void             e_vcard_add_attribute_with_values   (EVCard *evcard, EVCardAttribute *attr, ...);
 void             e_vcard_attribute_add_value         (EVCardAttribute *attr, const char *value);