RFE: Add customizable mail header display

RESOLVED FIXED

Status

Thunderbird
Mail Window Front End
--
enhancement
RESOLVED FIXED
11 years ago
7 years ago

People

(Reporter: bc, Assigned: Bienvenu)

Tracking

(Blocks: 1 bug, {fixed1.8.1.1})

x86
All
fixed1.8.1.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

11 years ago
Currently, Thunderbird allows two choices for displaying mail headers: normal and all. All is pretty much overkill for normal use. 

I would really like the ability to be able to add additional headers to the normal display such as X-Bugzilla-Product etc. I bet other people would like to include their favorite headers as well. ;-)
(Assignee)

Comment 1

11 years ago
see https://bugzilla.mozilla.org/show_bug.cgi?id=351867#c11 for what code we'd need to change to implement this.

Comment 2

11 years ago
From Bug 351867 comment 11:
>>On an unrelated topic, how hard would it be to add customized header views?
>I thought someone had done that, but I don't see it in the code. Scott, do you
>know if there's a patch out there? This would seem to be an oft-requested
>enhancement.

I started working on that in the course of bug 236954, but it somehow got neglected in the last couple of months. :|

Bob, in the meantime, you could try http:/mnenhy.mozdev.org/.
(Assignee)

Comment 3

11 years ago
Created attachment 242561 [details] [diff] [review]
add pref to allow extra headers

This does the basic work of adding a pref that allows the user to customize what additional headers get displayed in the normal header view. It can be simplified a bit, but at the cost of changing some existing code a little, in order to re-use createNewHeaderView.

This is not the full-on header service that Karsten proposes, and only displays headers as text (e.g., not as e-mail addresses, etc). But it works and will be helpful for some users/extensions.
Assignee: mscott → bienvenu
Status: NEW → ASSIGNED
(Assignee)

Comment 4

11 years ago
Created attachment 242565 [details] [diff] [review]
proposed fix

What do you think, Scott? Is this simple approach worth it?
Attachment #242561 - Attachment is obsolete: true
Attachment #242565 - Flags: superreview?(mscott)
(Assignee)

Comment 5

11 years ago
Eudora might be able to use something like this, if they have the option to display any headers that TB doesn't. Cc'ing Jeff and Geoffrey since they're the two Eudora bugzilla addresses I know...here's a link to the code that specifies the current headers we pass to the message display code that displays the message header part of message display.
(Assignee)

Comment 6

11 years ago
http://lxr.mozilla.org/seamonkey/source/mailnews/mime/emitters/src/nsMimeHtmlEmitter.cpp#185

and http://lxr.mozilla.org/seamonkey/source/mail/base/content/msgHdrViewOverlay.xul#134 

give you an idea...

Comment 7

11 years ago
(In reply to comment #5)
> Eudora might be able to use something like this, if they have the option to
> display any headers that TB doesn't. Cc'ing Jeff and Geoffrey since they're the
> two Eudora bugzilla addresses I know...here's a link to the code that specifies
> the current headers we pass to the message display code that displays the
> message header part of message display.

Eudora has two settings for this.  One is a list of headers to be *shown* when the message is being previewed below the mailbox, and another is a list of headers to be *hidden* when the message is in a separate window.  Both settings are comma-separated lists of header name prefixes.  The prefixes are so that you can easily add things like "X-" to the list of headers not to be shown.  If you want to only include a specific header to the list, then you just include the colon, as in "X-Foo:".

They are somewhat commonly used settings in Eudora, although not common enough to put them in the main Settings/Options UI, and so are implemented as hidden settings.

Updated

11 years ago
Blocks: 61523, 224374

Comment 8

11 years ago
Comment on attachment 242565 [details] [diff] [review]
proposed fix

This looks good David and is much simpler than the work I started and eventually gave up on :). My only question is, should we just go ahead and emit all of the headers in the message to the front end so an extension has access to all of the header data if it wants to get at it. This might help Karsten's  extension and enigmail. Both of which *I think* need to turn on view all headers to work right now (or at least they used to).
Attachment #242565 - Flags: superreview?(mscott) → superreview+

Comment 9

11 years ago
Comment on attachment 242565 [details] [diff] [review]
proposed fix

> This might help Karsten's extension and enigmail. Both of which *I think*
> need to turn on view all headers to work right now

Yes, both do turn on all headers and filter them down again.

>Index: mailnews/mailnews.js
>===================================================================
>+// comma-delimited list of extra headers to show in msg header display area.
>+pref("mailnews.headers.extraExpandedHeaders", "");

In theory, namely RFC 2822 section 3.6.8. Optional fields, header names can contain commas, so this is not a good idea (we already made that mistake for mail.compose.other.header which we should correct at some point also). You should use either a space or even better a colon as a delimiter...

Comment 10

11 years ago
> You should use either a space or even better a colon as a delimiter...

Looking at my old patch for bug 236954, I'd prefer a space as a delimiter here (but that's just me). ;-)

Comment 11

11 years ago
(In reply to comment #9)
> (From update of attachment 242565 [details] [diff] [review] [edit])
> > This might help Karsten's extension and enigmail. Both of which *I think*
> > need to turn on view all headers to work right now
> 
> Yes, both do turn on all headers and filter them down again.
> 
> >Index: mailnews/mailnews.js
> >===================================================================
> >+// comma-delimited list of extra headers to show in msg header display area.
> >+pref("mailnews.headers.extraExpandedHeaders", "");
> 
> In theory, namely RFC 2822 section 3.6.8. Optional fields, header names can
> contain commas, so this is not a good idea (we already made that mistake for
> mail.compose.other.header which we should correct at some point also). You
> should use either a space or even better a colon as a delimiter...
> 

great catch Karsten.
(Assignee)

Comment 12

11 years ago
Created attachment 248411 [details] [diff] [review]
patch addressing comments

Thx for the comments. This is what I intend to land, using a space delimited pref (this patch may not apply cleanly since I had to tweak it a bit). Carrying forward sr.
Attachment #242565 - Attachment is obsolete: true
Attachment #248411 - Flags: superreview+
(Assignee)

Comment 13

11 years ago
fixed on trunk and branch.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1.1
Resolution: --- → FIXED
(Assignee)

Comment 14

11 years ago
This is something extensions might be interested in using.
Keywords: dev-doc-needed

Comment 15

11 years ago
Notes to anyone trying this: you need to reopen the 3pane (via restart or some other technique) to see any added headers in the message pane; however, removing a header takes effect on the next message load.

I don't suppose there's any simple way to get the header name displayed in some reasonable Sentence-Caps format rather than all lowercase...?
(Assignee)

Comment 16

11 years ago
>I don't suppose there's any simple way to get the header name displayed in some
>reasonable Sentence-Caps format rather than all lowercase

We're displaying it as it is in the original message, right? Or as it appears in the mailnews.headers.extraExpandedHeaders pref? I've got X-Spam-Score set in the pref, and that's what gets displayed.

Comment 17

11 years ago
I'm seeing it as in the pref.  I thought I read something, somewhere, recently, about having to store the pref values in all-lowercase, but I guess that's another bug, not this bug?  

But with the header name entered in the pref as I want it displayed, it shows up just fine.
(Assignee)

Comment 18

11 years ago
yes, that's the bug for setting which headers should be fetched and stored in the .msf file - those need to be lower case, though, given more time, I could probably fix that...

Comment 19

11 years ago
Created attachment 249050 [details] [diff] [review]
Patch for SeaMonkey

ported back attachment 248411 [details] [diff] [review] to SeaMonkey
Attachment #249050 - Flags: review?
(Assignee)

Updated

11 years ago
Attachment #249050 - Flags: review? → review?(mnyromyr)
(Assignee)

Comment 20

11 years ago
*** Bug 364807 has been marked as a duplicate of this bug. ***

Comment 21

11 years ago
Comment on attachment 249050 [details] [diff] [review]
Patch for SeaMonkey

Nice. Just one nit: the attachment box (<listbox id="attachmentList">) needs to flex now to make use of the additional space below. 
r=me with that.
Attachment #249050 - Flags: superreview?(neil)
Attachment #249050 - Flags: review?(mnyromyr)
Attachment #249050 - Flags: review+

Comment 22

11 years ago
Comment on attachment 249050 [details] [diff] [review]
Patch for SeaMonkey

>+     	
Nit: spaces

>+  var extraHeaders = gExtraExpandedHeaders.split(' ');
This doesn't match (pun not intended) what the C++ code does, which is to tokenise the string. While gExtraExpandedHeaders.match(/[^ ]+/g) would be more accurate, watch out for a null result when there are no extra expanded headers.
Attachment #249050 - Flags: superreview?(neil) → superreview+

Comment 23

11 years ago
Matthias, you're going to drive this home?

Comment 24

11 years ago
Yes, I'll try to fix those nits at weekend. I had some busy weeks and didn't even remember I wrote a patch for this...

Comment 25

10 years ago
So here we have another way extensions need to hack around default thunderbird behavior to get extra headers. Bloating extension code again as they need to add another check for newer versions of thunderbird to get needed headers.

1)
Seeing the patch (attachment #248411 [details] [diff] [review]) I'm asking me why nobody had the idea to completely wipe out that hard coded list of headers that are transmitted by default and put all of them in the expandedheaders setting? That way you had even more control about thunderbird's message display.

2)
There is still no way for extensions to get headers without having them displayed. Using the new way, my extensions again need to hack the extraheaders setting and change it back before the message is displayed in the ui.
Why is there no "hiddenheaders" setting so that extensions can easily register required headers themselves easily?

Comment 26

10 years ago
(In reply to comment #25)
For sure, this patch doesn't make it easier for extension developers to get custom headers. But is it the purpose of this bug ?
I'd say that a new bug should be filled for the API stuff.

Comment 27

10 years ago
Well, that bug already exists and is bug 364807 as you know.
Sorry for bugspam.

Comment 28

10 years ago
Created attachment 267860 [details] [diff] [review]
Patch adressing Neil's comments
Attachment #249050 - Attachment is obsolete: true
Attachment #267860 - Flags: superreview?(neil)
Attachment #267860 - Flags: review?(neil)

Comment 29

10 years ago
(In reply to comment #22)
>While gExtraExpandedHeaders.match(/[^ ]+/g) would be more accurate,
>watch out for a null result when there are no extra expanded headers.
What I meant was that you should check the result, not the headers string...

Updated

10 years ago
Attachment #267860 - Flags: superreview?(neil)
Attachment #267860 - Flags: superreview+
Attachment #267860 - Flags: review?(neil)
Attachment #267860 - Flags: review+

Comment 30

10 years ago
FYI I tweaked the code slightly before checking it in.

Comment 31

10 years ago
Very nice, else i had tweaked it today and rerequested r/sr ;)

Comment 32

8 years ago
Removed "dev-doc-needed" keyword because I think the changes with message headers in 3.0 make this no longer applicable.
Keywords: dev-doc-needed

Updated

7 years ago
Duplicate of this bug: 61523
You need to log in before you can comment on or make changes to this bug.