364807 - API to get hidden mail headers

Status: NEW

(MailNews Core :: Backend, enhancement, P3)

Description

[Frederik Vanderstraeten]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; nl; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; nl; rv:1.8.0.9) Gecko/20061207 Thunderbird/1.5.0.9

Right now there isn't an API to get the hidden headers for the current e-mail. You can only get all headers with View > Headers set to All. The only work-around I know is really long: see the Enigmail and Mnenhy extensions.

Some extensions depend on one of these two for that reason: e.g. Country Lookup, Display mailing list header, Display mail route, Display Mail User Agent.

An API would be quite useful for extension developers.

Reproducible: Always

Steps to Reproduce:
1. Set View > Headers to Normal
2. Try to get a hidden header (like a Received header) from an extension
Actual Results:  
Hard to accomplish

Expected Results:  
Easy to accomplish

Comment 1

[David :Bienvenu]

there is a way to do this in 2.0 nightly builds - see bug 353193

*** This bug has been marked as a duplicate of 353193 ***

Comment 2

[Frederik Vanderstraeten]

Requesting reopen.

The fix of bug 353193 is insufficient because:
1. For headers which can occur multiple times (e.g. the Received header) it only shows one of them.
2. It's still a hack and doesn't play well with other extensions using it.

Comment 3

[Frederik Vanderstraeten]

I guess you don't read bugs marked DUPLICATE, so I'm going to reopen this bug myself. Please don't be angry if this is the wrong thing to do. Reasons are in my previous comment.

Comment 4

[Karsten Düsterloh]

Yes, the request is quite different from bug 353193.
Actually, addons should be able to get headers they want, preferrably when they want, without even having to meddle with the actual display.
Many MailNews related addons would like to use not (eg. Mnenhy, Enigmail, DispMua etc.).

Allowing extensions to call back into the backend to read arbitrary headers will slow down message display tremendously, though, so that may not be an option.

I can otoh imagine extensions registering an observer which gets passed headers by the backend in parallel to passing it top the display, eg. having several message sinks or a split-off "header sink"...


(Actually, I thought a bug with this bug's scope had already been filed a while ago, but I can't find it atm.)

Comment 5

[Frederik Vanderstraeten]

I think I can write this patch.

What I'd propose is one of these:
- In msgHdrViewOverlay.js, allow a third observer function in an object in the gMessageListeners array (now has two functions: onStartHeaders and onEndHeaders), maybe named onNewHeader, that gets called whenever a new header is available (with a check for existence to make sure we're not breaking an extension using this).
- Add a function to msgHdrViewOverlay.js (e.g. registerHeaderListener) that allows you to register an observer function that gets called when a new header is available. Maybe allow to specify what headers you want.
- Add a function to msgHdrViewOverlay.js that allows you to specify an object, the headers wanted, and will fill the object with the wanted headers. This function could be called in the onStartHeaders function, and the object will be full in the onEndHeaders function.

Please let me know what seems best, or if you have another proposition.

PS: What is the difference between /mail/base/content/msgHdrViewOverlay.js and /mailnews/base/resources/content/msgHdrViewOverlay.js? Which one should be used?

Comment 6

[Christian Weiske]

I would propose the following:
1.) Provide a method/observer which allows registering wanted headers per extension/key:
 someObserver->iWantThisHiddenHeader('myExtensionIdOrName', new Array('x-bug', 'received');

2.) Provide a second global header variable besides currentHeaderData that gets filled with the requested hidden headers

3.) Provide a second method that allows unregistering wanted headers per extension/key:
 someObject->iDontWantThisHeaders('myExtensionIdOrName', new Array('received'));

This would have several benefits:
- No callback which would make it only slow IMO
- Extensions don't influence each other
- Clean

Comment 7

[Frederik Vanderstraeten]

(In reply to comment #6)

That seems like a good idea to me.

Should we also allow regular expressions in the wanted headers array? It will only slow down loading the headers when it is actually used (I assume). And when someone needs it a little slowdown is better then not being able to do it, or having to find another, probably even slower, solution. Cases where this could be used are getting all headers, getting all headers starting with X-, or getting all headers not starting with X-.

Comment 8

[Christian Weiske]

I don't know if regex are a good idea - I would not add support for it. What I would like to see is having an asterisk "*" that matches all headers, but that should be the only "dynamic" value.

Comment 9

[Frederik Vanderstraeten]

OK, that would probably be as good as a regex.

Comment 10

[Karsten Düsterloh]

Any patch here probably needs to address two distinct areas:
- how and where to store what headers will be passed
- how to pass headers and let extensions hook up to that

We also need to keep in mind that the configurable headers need not only apply to the display but to forwarding and even printing°, too. Registering lists as in comment #6 is fine for maybe just the display, but to access these lists in the libmime for printing etc., this seems overly complicated.

Hence I'd propose using just plain simple pref lists for that, basically like we do for accounts:
- have a pref defining the names of the header lists
- use the header list name as part of the name of actual list pref
See bug 236954 for a proposal of such a header list which respects both printing needs and those of simple extensions (which could "get away" with just adding about one pref list line).

Re comment #5:
Currently, the backend loops over all headers¹, meanwhile building two distinct arrays for names and values, converts them to enumerators, passes both to the frontend function, which then loops over that again to create the display.
Does the price of doing two loops really pay? Would passing the headers one by one really be more expensive?
If we'd do this, we could indeed let extensions register a third "process header" callback very easily - but this would burden JS with doing the "wanted" checks which would not be wise, like Christian already mentioned.

Hence I'd propose letting the backend loop above do the distribution of headers between the registered "header sinks", i.e. extension register themselves (or observe) with the backend for specific header _lists_ and the loop passes/collects the respective values (see above; depending upon whether or not we can kill the second loop).

And as for regexp: see bug 19442 comment 40 about the problems. :|


° Printing adds another complication: printing order is usually not the native header order and we probably need to replace nonexisting headers with other values -> see libmime.
¹ backend header loop function: <http://lxr.mozilla.org/mozilla/source/mailnews/mime/emitters/src/nsMimeHtmlEmitter.cpp#155>

Comment 11

[Karsten Düsterloh]

> PS: What is the difference between /mail/base/content/msgHdrViewOverlay.js and
> /mailnews/base/resources/content/msgHdrViewOverlay.js? Which one should be
> used?

Both. ;-)
The /mailnews file is for SeaMonkey and that in /mail belongs to Thunderbird.

Comment 12

[Christian Weiske]

So the question is if passing all headers to the frontend is really too much.
I don't think so. Doing it would solve all problems, we didn't need an API and it could be used everywhere.

Comment 13

[Karsten Düsterloh]

It _is_ definitely a perf problem. Mnenhy, for example, running in All Headers mode (but usually hiding that from the user) does slow down header pane  generation visibly on my Athlon 500 box - but you're used to slow running programs on such machines, so nobody actually complained yet. ;-)

But always passing all headers is usually just passing *lots* of stuff we won't show, eg. the bugmail I got for comment #12 consisted of about 20 lines of content, but almost 60 lines of headers (after unfolding), of which *five* were shown in the UI...

Comment 14

[Frederik Vanderstraeten]

(In reply to comment #10)
> We also need to keep in mind that the configurable headers need not only apply
> to the display but to forwarding and even printing°, too. Registering lists as
> in comment #6 is fine for maybe just the display, but to access these lists in
> the libmime for printing etc., this seems overly complicated.

Could you explain that? Isn't this just a way for extensions to get headers? A header shouldn't be printed because an extension wants it, right? What am I missing?

> Hence I'd propose using just plain simple pref lists for that, basically like
> we do for accounts:
> - have a pref defining the names of the header lists
> - use the header list name as part of the name of actual list pref
> See bug 236954 for a proposal of such a header list which respects both
> printing needs and those of simple extensions (which could "get away" with just
> adding about one pref list line).

That would require an extension to edit the pref when the application loads, and to edit it again when the app unloads, otherwise an uninstall would leave an unnecessary header list behind. (Or is there some way to include an uninstall script? I would love to hear there is.) I think a lot of extension authors will forget this.

Comment 15

[David :Bienvenu]

Re Karsten's question:

>Does the price of doing two loops really pay? Would passing the headers one by
>one really be more expensive?

If I understand this question, yes, it would be a lot more expensive. Crossing the c++ js boundary is very expensive, and you don't want to do it in a loop if you can avoid it.

Re the drawbacks of the global pref approach, yes, it's going to be a real world problem if extensions aren't well-behaved, and it's in our interest to make it easier for extensions to be well-behaved. So in addition to a global pref, adding a method that allows an extension to register its interest in headers in a non-persistent way is a reasonable thing to do. I think this method should allow the caller to pass in multiple comma separated headers. Since this method would add the headers non-persistently, we should be able to get away without having a method to remove headers. And if we do that, then extensions can't collide, since two extensions adding the same header is harmless. Can anyone think of a reason an extension would dynamically want to remove a header?

I'd like to keep the pref-based approach so users can tweak the pref if they want, and TB itself can use the pref.

I'm less inclined to have multiple listeners, since I think that complicates the backend more than we need to, but feel free to try to convince me it's important :-)

Comment 16

[Frederik Vanderstraeten]

I'm not really following any more. Are the headers an extension requests supposed to be displayed or not?

Comment 17

[Frederik Vanderstraeten]

For clarity: I assumed they would not be displayed, as that's what bug 353193 is for.
But in comment 10 Karsten seems to reference the display, and in comment 15 David talked about the user changing it, but I have no idea why the user would want to change a pref that has no visible effect.

Comment 18

[Karsten Düsterloh]

We have two modes of header access to cope with:
(a) asynchronous
    independent of a currently processed message, we want to get certain 
    header data, eg by passing a msgKey and a header name to a backend
    function
(b) synchronous 
    while processing a message for display, editing, forwarding or printing,
    we want to be able to filter the list of headers to pass on

Performance of (a) is a nightmare because of the XPCOM boundary (and even more if the requested data is inside a non-current message database file), so one does not want to do that while processing a message for display - even more so, because we do already look at each header before passing it on to the frontend!
OTOH, implementing (a) should be fairly easy - we probably just need to extend the  nsIMsgParseMailMsgState interface which already gives us _all_ headers as a kind of "string blob".

Implementing (b) needs some more thought:
1. Several extensions should be able to independently define which headers they want to get passed from the backend.
2. Do we want extensions to be able to _suppress_ headers eg. for display?
If we regard display as an extensions as per (1), this is a non-issue.
3. Should extensions be able to influence each other?
Better not, I'd say. Would probably be almost never used anyway.
4. Extensions should be able to hook into the actual display loop (this is different from (1), as this would be a frontend hook "after the fact" - the header data has already been passed to the frontend and extensions hook in then). It's very easy to implement, we'd just need a callback loop in msgHdrViewOverlay.js::processHeaders inside the while loop. This callback would then return whether to display the header at all and probably be able to manipulate what/how to display it.

So, how should (1) work?
I. Extensions register with a callback function (would it be worth to let them register a messageSink?) and a space(!)-delimited list of headers they want to get. This list may be stored by the extension in a pref (we won't care), but we would keep our known prefs for that.
II. When selecting headers for display/editing/forwarding/printing, the backend builds up independent arrays of header data, according to these header lists.
(This would mean touching "real" libmime code, not just nsMimeHtmlEmitter.cpp!)
III. After looping over all the headers, call the registered callbacks with their respective header data array (currently we just call processHeaders here).

Comment 19

[Frederik Vanderstraeten]

(In reply to comment #18)
> (a) asynchronous

Do you think I should implement it here? I didn't think of this at first but I'm willing to do it if people want me to.

> (b) synchronous

I agree with point 1.

> 2. Do we want extensions to be able to _suppress_ headers eg. for display?
> 3. Should extensions be able to influence each other?

Both depend on the same thing. 3 isn't really necessary, but if it's harder to prevent than to allow, it won't do harm I think. You never know an extension might extend another extension. It doesn't happen now AFAIK but maybe it will be common in the future.

> 4. Extensions should be able to hook into the actual display loop

Just like asynchronous mode, this is not what I intended when filing this bug, but it might be a good idea. I'll wait to know what David Bienvenu thinks of this.

> So, how should (1) work?

What would the callback register function look like?
I'd do this (feel free to suggest something else):
registerCallback("header1 header2 header3", callback, "id");
The id argument could later be used to change or cancel a header listener. This would make your (2) and (3) easy. Just use the same id. We could probably hard code the display/editing/... id's, which wouldn't have to register themselves. For their standard headers, we could maybe use a preference.

What would the callback be? Would it be just a function, or should we make it an object with an interface?

Comment 20

[Karsten Düsterloh]

> > (a) asynchronous
> 
> Do you think I should implement it here? I didn't think of this at first but
> I'm willing to do it if people want me to.

Not necessarily; most extensions would probably be content with touching the synchronous case. In either case, it should probably better be a separate bug.

> > 2. Do we want extensions to be able to _suppress_ headers eg. for display?
> > 3. Should extensions be able to influence each other?
> 
> Both depend on the same thing. 3 isn't really necessary, but if it's harder to
> prevent than to allow, it won't do harm I think. You never know an extension
> might extend another extension. It doesn't happen now AFAIK but maybe it will
> be common in the future.

I was more concerned with "If extension #1 says 'show this header' and extension #2 says 'do not show this header', what shall we do?"
I "solved" that in comment #18 in the backend by using separate arrays for each extension, but it's still an open issue for the frontend case (4).

> > 4. Extensions should be able to hook into the actual display loop
> 
> Just like asynchronous mode, this is not what I intended when filing this bug,
> but it might be a good idea. I'll wait to know what David Bienvenu thinks of
> this.

Actually, my comment #18 proposal kind of requires these hook(s) for extensions that wish to alter the set of headers to be *displayed*. Currently, they have to override the entire processHeaders function...

> > So, how should (1) work?
> 
> What would the callback register function look like?
> I'd do this (feel free to suggest something else):
> registerCallback("header1 header2 header3", callback, "id");

I'd say registerCallback("id", "header1 header2 header3", callback), but that's just a matter of taste. ;-)
But the id is a good idea, especially if we allow the default processing to be overwritten/replaced!

> What would the callback be? Would it be just a function, or should we make it
> an object with an interface?

We probably could just use nsIMsgHeaderSink.

> We could probably hard code the display/editing/... id's, which wouldn't
> have to register themselves.

... but should be overidable.

> For their standard headers, we could maybe use a preference.

Yes (although the "if header #1 does not exist, use header #2 or #3 instead" mechanism in libmime is no easy prey).

Comment 21

[Frederik Vanderstraeten]

(In reply to comment #20)
> > > 4. Extensions should be able to hook into the actual display loop
> 
> Actually, my comment #18 proposal kind of requires these hook(s) for extensions
> that wish to alter the set of headers to be *displayed*. Currently, they have
> to override the entire processHeaders function...

OK. I would say we just call the callbacks in the order they register. A user will probably understand that two extensions trying to mess with the same header will not work together too well.
Should we also make the callback register function accept a list of headers, or should we just ask the callback for every header?
About the callback function, would these return values be ok?
false - don't show the header
true - show the header
a string - change the header value to this string
If the return value is true or a string, the next callback should be called. Not sure about changing how to display the header (e.g. linkifying some parts). Probably by returning false and display it yourself.

> We probably could just use nsIMsgHeaderSink.

I don't know, it has some functions related to the message body and attachments that an extension wouldn't need I think, but it would have to implement them so it conforms to the interface (right?).

Comment 22

[Karsten Düsterloh]

> > > > 4. Extensions should be able to hook into the actual display loop
[...]
> OK. I would say we just call the callbacks in the order they register.

Part of the infrastructure is already there, waiting to be extended (namely as gMessageListeners in msgHdrViewOverlay.js). ;-)

> Should we also make the callback register function accept a list of headers, 
> or should we just ask the callback for every header?

The frontend gets the headers from an enumerator (headerNameEnumerator) in a loop (filling currentHeaderData), doing another loop inside it (over the registered handlers and probably over their internal header lists) isn't very efficient. But passing only currentHeaderData afterwards is not what we want, because that doesn't contain the original data.

So there's not much choice, I think, just call back for each header.

Passing each header into the very same function, otoh, which then needs to do some comparisons in order to decide what to do, is rather complx, though.
It'd be more efficient to have an object with members named like the header they shall handle and attach a function to that:

var reghandle = {"x-bla" : function() {...}, "from" : function() {...},  ...};
...
if (header in reghandle) 
  reghandle[header]();

> About the callback function, would these return values be ok?
> false - don't show the header
> true - show the header
> a string - change the header value to this string
> If the return value is true or a string, the next callback should be called.

This sounds very messy, if you want to pass this as just one "variant" result.
You can, though, just return an object {a: b, c: d, ...} with the values you need.

> Not sure about changing how to display the header (e.g. linkifying
> some parts).

You don't have control over the "how", just over the "what" to display here.

> Probably by returning false and display it yourself.

Yeah.

> > We probably could just use nsIMsgHeaderSink.
> 
> I don't know, it has some functions related to the message body and 
> attachments that an extension wouldn't need I think, but it would have to 
> implement them so it conforms to the interface (right?).

Yes and no. You only have to implement those functions which will be called - but this are probably all. But you just have to have "stubs", probably empty functions - and you'd have the chance to allow for more actions in future.

Comment 23

[Frederik Vanderstraeten]

Hi,

I didn't have much time lately. But I have some time now.

I decided to work on getting headers from the backend first. What does real libmime code mean? Where is this code?

Also, would I need to define a new interface with the functions to register headers? Where should I put this in the source tree?
What should this be called? nsIHeaderObserverService?

Comment 24

[Peter McCurdy]

(In reply to comment #18)
> We have two modes of header access to cope with:
> (a) asynchronous
>     independent of a currently processed message, we want to get certain 
>     header data, eg by passing a msgKey and a header name to a backend
>     function

Note that, as the author of the <a href="http://alumnit.ca/wiki/index.php?page=ReplyToListThunderbirdExtension">Reply To List</a> extension, this API is the one I'm desperately missing.  I don't care about displaying the List-Post: header info, and I want to toggle the Reply To List button even when the preview pane is disabled, if you right-click on a random message in a mailbox, etc.  In fact, I was amazed that this API didn't already exist.

Comment 25

[Magnus Melin [:mkmelin]]

I think this would be very useful both to get some core features in as well as for extensions.

Comment 26

[David :Bienvenu]

this becomes easier if we have everything offline; then we can look in the offline store.

Comment 27

[Magnus Melin [:mkmelin]]

Leaving as wanted3+. This needs a ower...