Closed
Bug 353193
Opened 18 years ago
Closed 18 years ago
RFE: Add customizable mail header display
Categories
(Thunderbird :: Mail Window Front End, enhancement)
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)
(deleted),
patch
|
Bienvenu
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
neil
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
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•18 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•18 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•18 years ago
|
||
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•18 years ago
|
||
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•18 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•18 years ago
|
||
Comment 7•18 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•18 years ago
|
Comment 8•18 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•18 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•18 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•18 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•18 years ago
|
||
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•18 years ago
|
||
fixed on trunk and branch.
Assignee | ||
Comment 14•18 years ago
|
||
This is something extensions might be interested in using.
Keywords: dev-doc-needed
Comment 15•18 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•18 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•18 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•18 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•18 years ago
|
||
ported back attachment 248411 [details] [diff] [review] to SeaMonkey
Attachment #249050 -
Flags: review?
Assignee | ||
Updated•18 years ago
|
Attachment #249050 -
Flags: review? → review?(mnyromyr)
Assignee | ||
Comment 20•18 years ago
|
||
*** Bug 364807 has been marked as a duplicate of this bug. ***
Comment 21•18 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•18 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•18 years ago
|
||
Matthias, you're going to drive this home?
Comment 24•18 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•18 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•17 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•17 years ago
|
||
Well, that bug already exists and is bug 364807 as you know.
Sorry for bugspam.
Comment 28•17 years ago
|
||
Attachment #249050 -
Attachment is obsolete: true
Attachment #267860 -
Flags: superreview?(neil)
Attachment #267860 -
Flags: review?(neil)
Comment 29•17 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•17 years ago
|
Attachment #267860 -
Flags: superreview?(neil)
Attachment #267860 -
Flags: superreview+
Attachment #267860 -
Flags: review?(neil)
Attachment #267860 -
Flags: review+
Comment 30•17 years ago
|
||
FYI I tweaked the code slightly before checking it in.
Comment 31•17 years ago
|
||
Very nice, else i had tweaked it today and rerequested r/sr ;)
Comment 32•15 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
You need to log in
before you can comment on or make changes to this bug.
Description
•