Re: [evolution-patches] Fix bugs in e-contact/e-vcard reordering attributes [#556061]

[Srinivasa Ragavan <sragavan novell com>]

You really don't have to post on bugzilla and post a mail. I will review
it over bugzilla.

-Srini.
On Sun, 2008-10-12 at 22:42 +0100, Matt Davey wrote:
> (see bug #556061)
> 
> 
> If you import a vcard with three EMAIL attributes and then export, you will
> find that the EMAIL attributes are exported in reverse order.
> 
> Also, if you create two email entries in the contact editor gui, you will find
> that they are put into the vcard in reverse order.
> 
> This may seem trivial, but it causes complications for the addressbook-conduit.
>  If the pda has two emails, in 'slot1' and 'slot2' it is problematic if they
> get written to 'email1' and 'email2' on the desktop and they are subsequently
> edited by the gui.  The edit can cause them to be reversed, which causes
> consistency problems when syncing.
> 
> I'm attaching two patches, one for e-d-s to fix e-contact and e-vcard, and one
> for the gui editor.  I have more extensive patches for the conduit itself so
> I'll raise that separately.
> 
> Matt
> 
> Index: addressbook/gui/contact-editor/e-contact-editor.c
> ===================================================================
> --- addressbook/gui/contact-editor/e-contact-editor.c	(revision 36574)
> +++ 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: e-contact.c
> ===================================================================
> --- e-contact.c	(revision 9652)
> +++ 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);
> @@ -1640,11 +1644,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 +1674,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 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: e-vcard.c
> ===================================================================
> --- e-vcard.c	(revision 9652)
> +++ 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: e-vcard.h
> ===================================================================
> --- e-vcard.h	(revision 9652)
> +++ 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);
> _______________________________________________
> Evolution-patches mailing list
> Evolution-patches gnome org
> http://mail.gnome.org/mailman/listinfo/evolution-patches