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

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: aryx, Assigned: mihai)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.
Created attachment 791907 [details]
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.
Attachment #791907 - Flags: review?(etienne)
(Assignee)

Updated

5 years ago
Assignee: nobody → mihai
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 :)
Attachment #791907 - Flags: review?(etienne) → review?(bkelly)
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?
(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.
Flags: needinfo?(bkelly)
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.
Attachment #791907 - Flags: review?(bkelly) → review+
Looks like travis is having issues.  Trying to run them locally now.

Note, you will need to rebase since bug 905771 landed.
Flags: needinfo?(bkelly)
Ok.  I verified the tests run locally since travis is having problems today.  I say go ahead and land after rebasing.

Thanks again!
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.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.