896060 - [contacts] trim whitespaces at start or end of constructed names provided to other apps, e.g. if firstname or lastname is empty

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Contacts, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

From bug 893579:

Unagi with Gaia 1.1.0.0-prerelease 20130714070208

When replying to a mail from a stored contact whose firstname or lastname is empty, there will be two whitespaces between {{name}} and 'wrote' in the reply.

Something like that is going to affect other consumers of mozContacts too.

Comment 1

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Attachment: Pull Request #11591 - Avoid whitespaces in contact names

Contact names were generated when saving a contact (i.e. in the contact form) by concatenating the givenName with the familyName, however, the whitespace between them was still added if one of the names was empty. This patch fixes that.

Comment 2

[Etienne Segonzac (:etienne)]

Comment on attachment 791907 [details]
Pull Request #11591 - Avoid whitespaces in contact names

Forwarding to Ben, Francisco tells me he's a good contacts app reviewer :)

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

This change looks good to me, but it would be nice to have a unit test.  Mihai, can you add a test case to contacts_form_test.js?

Comment 4

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

(In reply to Ben Kelly [:bkelly] from comment #3)
> This change looks good to me, but it would be nice to have a unit test. 
> Mihai, can you add a test case to contacts_form_test.js?

Hi Ben, just updated the PR with tests for contact name generation. Let me know if it looks good now.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 791907 [details]
Pull Request #11591 - Avoid whitespaces in contact names

Looks good!  Thanks for adding the tests.

Assuming we can get travis to pass the contacts tests (there have been persistent errors in dialer and system), then I think this is good to land.  I just restarted the travis build to see if it will run.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Looks like travis is having issues.  Trying to run them locally now.

Note, you will need to rebase since bug 905771 landed.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Ok.  I verified the tests run locally since travis is having problems today.  I say go ahead and land after rebasing.

Thanks again!

Comment 8

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Thanks for the review, Ben!

Landed on master:
https://github.com/mozilla-b2g/gaia/commit/bfe4868321b006115076fba520a4745206ed905f

Note: the unit tests for contact form are currently not run because of the refactoring brought by the patch for bug 905771, for which I have filed bug 907459.