In the detailed header view we display the reply-to address even if it's the same as the from address. It seems like we should be doing a simple check to see if they are the same and hiding the reply-to address if so. ex: from: Bryan <clarkbw@example.com> subject: Extra line in detailed header view reply-to: clarkbw@example.com This would save an extra line of header space for what I don't think is useful information. I couldn't find an existing bug for this.

I bet there are more cases than just that where it makes sense to hide the reply-to. For example, in cases where we're only showing the display name (and not the email address) in the from line.

This patch hides the Reply-To: line if its value matches _either_ the From header (in which case it's completely useless), or the To header, which is the case for many mailing lists. (A later version of this patch could detect matches in substrings of the To field, but that seems like more complexity than needed at this stage). In my test data, the (few) messages that end up with a reply-to header showing are those which seem to have _some_ real value, e.g. "noreply@facebook", which is somewhat useful and hard to automate. Asking for ui-review first.

Comment on attachment 398530 [details] [diff] [review] patch v1 ui-r+ as the idea here is great. However this patch failed on me with errors of msgHeaderParser until I added the declaration like below. >diff --git a/mail/base/content/msgHdrViewOverlay.js b/mail/base/content/msgHdrViewOverlay.js >--- a/mail/base/content/msgHdrViewOverlay.js >+++ b/mail/base/content/msgHdrViewOverlay.js >@@ -494,6 +494,24 @@ > delete currentHeaderData.sender; > } > >+ if (("from" in currentHeaderData) && >+ ("to" in currentHeaderData) && >+ ("reply-to" in currentHeaderData)) { var msgHeaderParser = Components.classes["@mozilla.org/messenger/headerparser;1"] .getService(Components.interfaces.nsIMsgHeaderParser); >+ var replyToMailbox = kMailboxSeparator + >+ msgHeaderParser.extractHeaderAddressMailboxes( >+ currentHeaderData["reply-to"].headerValue) + kMailboxSeparator; >+ var fromMailboxes = kMailboxSeparator + >+ msgHeaderParser.extractHeaderAddressMailboxes( >+ currentHeaderData.from.headerValue) + kMailboxSeparator; >+ var toMailboxes = kMailboxSeparator + >+ msgHeaderParser.extractHeaderAddressMailboxes( >+ currentHeaderData.to.headerValue) + kMailboxSeparator; >+ >+ if (replyToMailbox == fromMailboxes || replyToMailbox == toMailboxes) { >+ delete currentHeaderData["reply-to"]; >+ } >+ } >+ > this.onEndHeaders(); > }, >

here's an updated patch. feels safe to me for b4, and best to have it in b4 if possible.

Comment on attachment 399749 [details] [diff] [review] updated patch >+ if (("from" in currentHeaderData) && >+ ("to" in currentHeaderData) && >+ ("reply-to" in currentHeaderData)) { >+ var replyToMailbox = kMailboxSeparator + >+ msgHeaderParser.extractHeaderAddressMailboxes( >+ currentHeaderData["reply-to"].headerValue) + kMailboxSeparator; >+ var fromMailboxes = kMailboxSeparator + >+ msgHeaderParser.extractHeaderAddressMailboxes( >+ currentHeaderData.from.headerValue) + kMailboxSeparator; >+ var toMailboxes = kMailboxSeparator + >+ msgHeaderParser.extractHeaderAddressMailboxes( >+ currentHeaderData.to.headerValue) + kMailboxSeparator; >+ >+ if (replyToMailbox == fromMailboxes || replyToMailbox == toMailboxes) { >+ delete currentHeaderData["reply-to"]; >+ } No brace for the one line if please. Hm, why are we adding kMailboxSeparator+foo+kMailboxSeparator to all of these? Since they are only compared to each other, doesn't it work to just compare the values? Also, please add a comment explaining what the code block does. A minor concern is that reply-to addresses tend to often lack the pretty name, so when you reply you get the correct address but users may wonder why the name isn't included too. But i guess we can live with that. r=mkmelin with the above fixed/explained.

(In reply to comment #5) > A minor concern is that reply-to addresses tend to often lack the pretty name, > so when you reply you get the correct address but users may wonder why the name > isn't included too. But i guess we can live with that. Good point. Do you think that's an easy fix? I can file a follow up bug for it so this doesn't get slowed down.

patch w/ review comments addressed (modulo followup bug). carrying forward r+ and ui-review+

Standards-wise i think it's working as designed, was just pointing out that it may be unexpected. If we want to "fix" it we could compare and hide reply-to only if the name is also set there. Not sure we want to though.

Or we could try to pull a display name from the address book for reply-to cases that offer no display name.

Doable but there's some wontfixed bug for doing that with replies... (for normal reply when addresses don't have names/have other names than we have in the ab).

Checked in: http://hg.mozilla.org/comm-central/rev/1e3c9a227b04

This is still not fixed. Reply-to field shows even though from is the same address.

(In reply to comment #12) > This is still not fixed. Reply-to field shows even though from is the same > address. Justin, can you file a new bug and attach the email that this didn't work for? Just add a comment here for the new bug when you have it filed. Otherwise these issues can get lost, thanks.