Some users didn't set their real name when sending an email, so only their email address is displayed in the From: field, i.e. firstname.lastname@example.org instead of Mr Foo <email@example.com> When such emails arrive in the inbox, the notification which popup doesn't mention who sent the email. All we see is "... from " at the end of the message. If no name is set, then it should fallback to "... from firstname.lastname@example.org" so that we can at a glance see who sent the email and decide if it should be read immediately or can wait a bit while doing some other work.
Note to self: newMailNotification_messagetitle is located in chrome/en-US/locale/en-US/messenger/messenger.properties and is called by nsMessengerUnixIntegration::BuildNotificationBody() in mailnews/base/src/nsMessengerUnixIntegration.cpp line 347 only.
Created attachment 603152 [details] [diff] [review] Fix msg_parse_Header_addresses returns 0-length string, so we should also check the string.
Comment on attachment 603152 [details] [diff] [review] Fix I'm not quite sure I see why this is happening. test_nsIMsgHeaderParser2.js already has at least one test for this and we use the function in lots of places. Can you try to extend the test_nsIMsgHeaderParser2.js test to show this case please?
Created attachment 609667 [details] [diff] [review] Fix with a new test I do not prefer that a test checks several things (in this case this bug and others) because it's difficult to find cause if regression happens.
If the thing to test is very related, keeping it in a separate file isn't nice imho. E.g. it makes it very difficult to find when creating related tests. (And at the very least, the file name of the test should be descriptive.)
(In reply to Magnus Melin from comment #5) > If the thing to test is very related, keeping it in a separate file isn't > nice imho. E.g. it makes it very difficult to find when creating related > tests. (And at the very least, the file name of the test should be > descriptive.) I do not think it is difficult to create related tests. But I agree your comment in parentheses. Bug number file name is too bad. That was my neglect. Can you suggest better name for this test please?
test_nsIMsgHeaderParser-parseHeadersWithArray.js? BTW, the test file doesn't have a licence block (MPL2 or such).
Just to follow up on the separate test file - I'm happy with that, in theory it could be said the parseHeadersWithArray check shouldn't actually be in test_nsIMsgHeaderParser2.js, as it isn't quite related to the tests in the rest of the file.
Comment on attachment 609667 [details] [diff] [review] Fix with a new test >diff --git a/mailnews/mime/test/unit/test_bug684865.js b/mailnews/mime/test/unit/test_bug684865.js Please name this file test_parseHeadersWithArray.js and update the xpcshell.ini to match. r=me with that fixed.
Created attachment 623582 [details] [diff] [review] Change the test file name, carrying over review+.
Comment on attachment 623582 [details] [diff] [review] Change the test file name, Oops! Wrong patch
Created attachment 623589 [details] [diff] [review] This is correct!
So an alternative would be to change author.Assign(names ? names : emails); to author.Assign(names && *(names) ? names : emails); in http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMessengerUnixIntegration.cpp#337