Comment 11 for bug 600755

Comment on attachment 164315
Patch to 97294

Thanks for the nice patch, sorry its taken so long for someone to look at it.
Works well, but could do with a couple of tweaks:

+ switch( fieldNum) {
...
+ }

Please add in a default: break; option to the switch. Although not strictly
required, I believe its better practice to have one.

+ displayName.Append(PRUnichar(' '));

It should be:

displayName.Append(NS_LITERAL_STRING(" ");

this aviods run-time conversions.

+ if (!firstName.IsEmpty()) {
+ displayName = firstName;
+ }
+ if (!firstName.IsEmpty() && !lastName.IsEmpty()) {
+ displayName.Append(PRUnichar(' '));
+ }

Move the second if inside the first if statement, and that way you can drop the
second check for !firstName.IsEmpty().

If you could update the patch with those changes, and re-request review from
myself, that would be good (hence I assigned the bug to you). If you can't do
that, reassign the bug to the default owner, and let us know.