GNOME Bugzilla – Bug 645570
Problems in _update_structured_name ()
Last modified: 2011-03-30 16:06:23 UTC
Created attachment 184129 [details] [review] Fix checks for empty and equal Structured Names There is 2 problems with _update_structured_name () in folks/individual.vala: 1) We check if a Persona has a different StructuredName than the one currently hold by the Individual by doing: if (new_value != this.structured_name) this.structured_name = new_value; This comparison may be bogus because it rests upon Personas replacing entirely there structured_name property when updating it (its not the case for Tracker, since each member of StructuredName is updated in granular fashion so it makes no sense to create a new StructuredName each time we get a change). What we could do instead is use the equal method in StructuredName proposed here: https://bugzilla.gnome.org/show_bug.cgi?id=645569 to correctly discriminate identical StructuredName(s). 2) _update_structured_name () assumes that persona.structured_name == NULL means an empty StructuredName. Again, for Tracker this is not the case. This should instead be checked with the is_empty method available in StructuredName. Both problems are addressed by the attached patch.
Created attachment 184153 [details] [review] Fix checks for empty and equal Structured Names Updated to deal with this.structured_name not being initialized.
You posted a more-updated version of the patch here yesterday in response to my comments about the property never getting unset again, but since Bugzilla went down it got lost. Could you re-attach it please?
(In reply to comment #2) > You posted a more-updated version of the patch here yesterday in response to my > comments about the property never getting unset again, but since Bugzilla went > down it got lost. Could you re-attach it please? Can you not see the currently attached patch to this report? That is the latest version of the patch.
Created attachment 184261 [details] [review] Fix checks for empty and equal Structured Names Actually - bugzilla did ate the last patch posted.
Review of attachment 184261 [details] [review]: Looks good. The same changes need making to _update_full_name(), _update_nickname(), _update_gender() and _update_im_addresses() (though this last one's slightly different).
Merged: commit 45a0b1dd7b4384b2f11555c7dfb61dbfacb30de1 Author: Raul Gutierrez Segales <raul.gutierrez.segales@collabora.co.uk> Date: Tue Mar 22 21:35:57 2011 +0000 Fix checks for empty and equal StructuredNames NEWS | 1 + folks/individual.vala | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-)
(In reply to comment #5) > The same changes need making to _update_full_name(), > _update_nickname(), _update_gender() and _update_im_addresses() (though this > last one's slightly different). What about this?
I am targeting those in a new bug report: https://bugzilla.gnome.org/show_bug.cgi?id=646244