Closed Bug 353193 Opened 18 years ago Closed 18 years ago

RFE: Add customizable mail header display

Categories

(Thunderbird :: Mail Window Front End, enhancement)

x86
All
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bc, Assigned: Bienvenu)

References

(Blocks 1 open bug)

Details

(Keywords: fixed1.8.1.1)

Attachments

(2 files, 3 obsolete files)

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. ;-)
see https://bugzilla.mozilla.org/show_bug.cgi?id=351867#c11 for what code we'd need to change to implement this.
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/.
Attached patch add pref to allow extra headers (obsolete) (deleted) — Splinter Review
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
Attached patch proposed fix (obsolete) (deleted) — Splinter Review
What do you think, Scott? Is this simple approach worth it?
Attachment #242561 - Attachment is obsolete: true
Attachment #242565 - Flags: superreview?(mscott)
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.
(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.
Blocks: 61523, 224374
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 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...
> 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). ;-)
(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.
Attached patch patch addressing comments (deleted) — Splinter Review
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+
fixed on trunk and branch.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Keywords: fixed1.8.1.1
Resolution: --- → FIXED
This is something extensions might be interested in using.
Keywords: dev-doc-needed
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...?
>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.
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.
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...
Attached patch Patch for SeaMonkey (obsolete) (deleted) — Splinter Review
ported back attachment 248411 [details] [diff] [review] to SeaMonkey
Attachment #249050 - Flags: review?
Attachment #249050 - Flags: review? → review?(mnyromyr)
*** Bug 364807 has been marked as a duplicate of this bug. ***
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 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+
Matthias, you're going to drive this home?
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...
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?
(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.
Well, that bug already exists and is bug 364807 as you know. Sorry for bugspam.
Attached patch Patch adressing Neil's comments (deleted) — Splinter Review
Attachment #249050 - Attachment is obsolete: true
Attachment #267860 - Flags: superreview?(neil)
Attachment #267860 - Flags: review?(neil)
(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...
Attachment #267860 - Flags: superreview?(neil)
Attachment #267860 - Flags: superreview+
Attachment #267860 - Flags: review?(neil)
Attachment #267860 - Flags: review+
FYI I tweaked the code slightly before checking it in.
Very nice, else i had tweaked it today and rerequested r/sr ;)
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: