1249873 - Use makeMailboxObject to construct 'name <email>' format of sender in bug 1209565

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, enhancement)

Description

[:aceman]

Sorry for looking at the patch in bug 1209565 after landing.
Is there a reason that the construction of the sender address display is hardcoded to:
+      return addresses[0].name ?
+             addresses[0].name + " <" + addresses[0].email + ">" :
+             addresses[0].email;
?

If not, please use makeMailboxObject().
It is used like as MailServices.headerParser.makeMailboxObject(name, email).toString()

There is also a helper function when you are in C++, see bug 318495 .

Please look around in the file mail/base/modules/displayNameUtils.js if more places could be converted.

Comment 1

[:aceman]

Or if the place where this is used is performance-cricital, we can surely make a helper function that does not create an object like makeMailboxObject(), as we do not need the object itself, just the address string.

Comment 2

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #0)
> Is there a reason that the construction of the sender address display is
> hardcoded to:

Ignorance ;-) Something similar was done a few lines above:
  // Make sure we have an unambiguous name if there are multiple identities
  if (MailServices.accounts.allIdentities.length > 1)
    displayName += " <" + identity.email + ">";

If you have a better solution, please submit it.

Comment 3

[:aceman]

Attachment: patch

OK, this should do it. Can you try it on the testcases from bug 1209565? Thanks

Comment 4

[Jorg K (CEST = GMT+2)]

Oof, that's a mouthful of text ;-)

Any change needs to pass
mozmake SOLO_TEST=folder-display/test-summarization.js mozmill-one
mozmake SOLO_TEST=multiple-identities/test-display-names.js mozmill-one

That's the two I had to fix. I'll try it out right now; can you please manually run those tests.

Comment 5

[Jorg K (CEST = GMT+2)]

Comment on attachment 8721698 [details] [diff] [review]
patch

Review of attachment 8721698 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not sure I have review power here ;-)
Anyway, I looked at my manual case and ran the two Mozmill tests. All passed, so r+.
r=jorgk (all lower case).

Comment 6

[:aceman]

(In reply to Jorg K (GMT+1) from comment #4)
> Oof, that's a mouthful of text ;-)

Yes, it may be longer, but the point is to only use a single function to format the address. In case we want to change the format in future.

Comment 7

[:aceman]

https://hg.mozilla.org/comm-central/rev/e52fd32c6b6c