An option to add certain header lines to *every* msg sent out via a certain identity would be nice. This is similar to bug 16925, but only that it doesn't require (allow?) edition during composition of an individual msg, but instead during setup of the identity. Possibly uses: - Secretary A secretary composes msgs in the name of his/her boss. The secretary sets up an identity with the From address sat to his boss, but with an additional header line "Sender: Bob Secretary <bob.secretary@example.com>" - No archive The user never wants his msgs to be archived. He adds a "X-No-Archive: ..." line to his identity. - Interal We might be able to use the code internally as well, for the headers we provide special fields for in the Prefs dialog, e.g. From, Reply-To etc..

s/secretary composes/secretary types/

A couple of other frequent uses of this would be including a PGP public key or an X-Face on every message.

future...

changing qa assign

*** Bug 122296 has been marked as a duplicate of this bug. ***

*** Bug 155742 has been marked as a duplicate of this bug. ***

I keep forgetting to add the X-No-Archive header to my mails... Damn spam... Jean-Francois: are you workin on this? If not I'll take it.

I am not working on this feature. Fell free to reassign this bug to yourself...

Taking, target -> 1.4a Could someone good with mail specifics answer the following. It would be great to be able to have multiple headers always specified. Is there any character that can not appear in a mail header that can be used as a delimiter? What does everyone want? 1) Headers specific to the current identity 2) Headers for all mail/news 3) Both of the above

> Is there any character that can not appear in a mail header that can be used > as a delimiter? See RFC 2822. > 1) Headers specific to the current identity That's where most prefs of this type are, yes.

> A field body may be composed of any US-ASCII characters, except for CR and LF Great :-( Suggestions anybody?

Blurrgghhhhh... It's Friday ignore me \r\n can be used as a delimiter

Patch to allow default headers to always be attached Headers ar specified per identity in the following pref mail.identity.<id>.misc_headers headers should be separated by \r\n so for example to always append the following headers to all mail from identity 1 - X-No-Archive: true James-Was-Here: maybe specify the pref in prefs.js as user_pref("mail.identity.id1.misc_headers", "X-No-Archive: true\r\nJames-Was-Here: maybe");

> headers should be separated by \r\n I don't think that this is a good idea. Why not mimic the method used with identities and the like: user_pref("mail.identity.<id>.headers", "header1,header2,header3"); user_pref("mail.identity.<id>.header.header1", "X-No-Archive: true"); user_pref("mail.identity.<id>.header.header2", "James-Was-Here: maybe"); user_pref("mail.identity.<id>.header.header3", "X-Taming-The-Lizard-Since: 1995");

Yes that would be better. New patch on its way

Changed format for specifying headers per karsten's comments. Headers are now specified as follows, user_pref("mail.identity.<id>.headers", "header1,header2,header3"); user_pref("mail.identity.<id>.header.header1", "X-No-Archive: true"); user_pref("mail.identity.<id>.header.header2", "James-Was-Here: maybe"); user_pref("mail.identity.<id>.header.header3", "X-Taming-The-Lizard-Since: 1995");

Adding CC to Jean-Francois and asking for review ;-)

Comment on attachment 116085 [details] [diff] [review] revised patch using suggestions from Karsten I'll let ssu doing the review. But would be nice if you completly remove those debugging "dump"

removed the commented out dump, but left in the latter because as we shouldn't get there it would be nice to know about it

*** Bug 188236 has been marked as a duplicate of this bug. ***

Previous patch also included patch for bug 195965 so have removed that code from the patch and mark as depends on 195965

Is this the best place for the patch, as the prefs are fetched on each mail send/send later. Would it be better if the prefs where held in the identity to avoid the overhead of pref fetching on everymail?

Comment on attachment 117078 [details] [diff] [review] updated patch. removed duplicate from bug 195965 this patch doesn't seem work for me. It inserts a 'null' here: ------------------------------------------- X-Accept-Language: en-us, ru MIME-Version: 1.0 To: ssu@netscape.com Subject: testing te null <<-------- Content-Type: text/html; charset=us-ascii Content-Transfer-Encoding: 7bit <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> <html> <head> <meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1"> <title></title> </head> -------------------------------------------

SSU: I can't reproduce this with any settings send HTML/plain/both with/without headers What platform are you running on/SMTP server are you using? What do you mean by a 'null'? do you mean '\0' "null" or ""? Is it also adding "\r\n" to the end?? I'm also confused as you have an extra line after the subject and before the null. What values of headers where you trying to add? can you check in about:config and post a copy of what it thinks about mail.identity.<mailid>.headers and related prefs, and the prefs you have changed? It looks something like the following has been entered into prefs. user_pref("mail.identity.1.headers", "header1,header2"); user_pref("mail.identity.1.header.header1", ""); note that header1 is blank (hence extra line) and header2 is missing (or misspelt)...

James Nord, you should do some sanity checking on the prefs, so that you don't send invalid messages even with invalid prefs. I wouldn't be very paranoid (backend prefs *are* a sharp knife), to save code and processing time, but such easy mistakes should be protected against IMHO.

Added some sanity checks to the patch. We check now that the headers have a somewhat reasonable value (ie contain at least one ':')

Note: I'm working on a better patch for nsMsgSend.cpp, but I'm trying to get my head around String handling first, so won't be in time for 1.4b. prefs would be the same so would be easy to migrate to the new patch if the above one makes it into 1.4 (hint!).

Patch to nsMsgSend.cpp. Needs some tidying-up but otherwise feedback anyone?

Now MIME encode headers. (For some reason this doesn't work on the first message you send but does for subsequent messages)

Updated patch, * now fixes not encoding the headers on the first send only. * cleaned up commented out code

Updated patch so it mime encodes headers proplerly.

Comment on attachment 121958 [details] [diff] [review] patch to nsMsgSend.cpp This is great functionality, I can't wait to land this. There's still a few kinks we need to work out, I think. In particular... a) Please try and make sure that all lines that you touch or add wrap at column 78. This makes things like patches more readable in regular terminal windows. b) The C++ function being extended is already way way too long to be easily readable. Instead of making the problem worse, can you put this in its own function and have ::InitCompositionFields call that? + // Bug 61520 c) This comment isn't terribly useful on it's own. Can you write a sentence or two describing what the code you've got does. + nsXPIDLCString headersList; + //mail.identity.<id>.headers + nsresult headerRv = mUserIdentity->GetCharAttribute("headers", getter_Copies(headersList)); d) Since this will be off in its own function, you can just use the standard "rv" variable name. + if (NS_SUCCEEDED(headerRv) && !headersList.IsEmpty()) { + PRInt32 start = 0; + PRInt32 end = 0; + PRInt32 len = 0; + nsCAutoString newHeaderVal(mCompFields->GetOtherRandomHeaders()); e) This can't work: GetOtherRandomHeaders returns an nsresult, and requires an out parameter to store the headers in. Whatever testing you've got now didn't catch this, please do some testing that would catch this before and after you fix this to ensure that the problem really goes away. Note that most compilers will at least warn you about the type mismatches here. Please verify that any changes you make do not add any warnings or errors to the build. + + while (end != -1) { + end = headersList.FindChar(',',start); + if (end == -1) { + len = headersList.Length() - start; + } else { + len = end - start; + } + nsCAutoString headerName(Substring(headersList, start, len)); + 1+1; f) OK, while 1+1 may be a valid C expression, it's not terribly useful here. :-) + start = end + 1; + + nsXPIDLCString headerVal; + headerName.Insert("header.",0); g) I think this and the declaration could be condensed to nsCAutoString headerName = NS_LITERAL_CSTRING("header.") + Substring(...); + headerRv = mUserIdentity->GetCharAttribute(headerName.get(), + getter_Copies(headerVal)); + if (NS_SUCCEEDED(headerRv)) { + // check that the header is *most likely* valid. + if (headerVal.FindChar(':') != -1) { + char * convHeader = nsMsgI18NEncodeMimePartIIStr(headerVal.get() + headerVal.FindChar(':') + 1, + PR_FALSE, + mCompFields->GetCharacterSet(), + headerVal.FindChar(':') + 1, + PR_TRUE); h) I suspect that most optimizers are not going to be smart enough to cache the result of headerVal.FindChar(':') on their own, so how about just calling this function once, and reusing the result? i) Also, GetCharacterSet is an XPCOM getter like GetOtherRandomHeaders, meaning that the compiler should be complaining that it returns an nsresult that needs to be checked and it also needs to be provided with an out param for the result. Same testing caveat as earlier. + if (convHeader) { + newHeaderVal.Append(Substring(headerVal, 0, headerVal.FindChar(':') + 1)); + newHeaderVal.Append(convHeader); + PR_FREEIF(convHeader); + // we must terminate the header with CRLF here + // as nsMsgCompUtils.cpp just calls PUSH_STRING + newHeaderVal.Append("\r\n"); j) You may be able to save allocations here by using operator+ instead of .Append directly. That is, something like: newHeaderVal += Substring(...) + convHeader + NS_LITERAL_CSTRING("\r\n"); Thanks for putting this patch together!

>e) This can't work: GetOtherRandomHeaders returns an nsresult, and >requires an out parameter to store the headers in. http://lxr.mozilla.org/seamonkey/source/mailnews/compose/src/nsMsgCompFields.h#143 >i) Also, GetCharacterSet is an XPCOM getter like >GetOtherRandomHeaders, meaning that the compiler should be complaining >that it returns an nsresult that needs to be checked and it also needs >to be provided with an out param for the result. Same testing caveat >as earlier. http://lxr.mozilla.org/seamonkey/source/mailnews/compose/src/nsMsgCompFields.h#149 other comments taken into account and new patch will be coming... BTW, this has been tested, I'm currently using this in my own mail with OTHER_RANDOM_HEADERS also beeing added from the compose window, its also been completly stepped through in the debugger.

Updated patch, * wrap at 78 chars * split off into separate fn * better(?) comments * condensed string declarations where possible * slight optimizations

Comment on attachment 122391 [details] [diff] [review] patch to nsMsgSend You're right as far as those two methods; I was looking at the ones declared in the .idl file, not the ones in the .h file. Man, I hate languages that support overloading. :-) Almost there; just a few minor nits remain: >Index: mailnews/compose/src/nsMsgSend.cpp >=================================================================== >RCS file: /cvsroot/mozilla/mailnews/compose/src/nsMsgSend.cpp,v >retrieving revision 1.334 >diff -u -r1.334 nsMsgSend.cpp >--- mailnews/compose/src/nsMsgSend.cpp 9 Apr 2003 00:47:43 -0000 1.334 >+++ mailnews/compose/src/nsMsgSend.cpp 3 May 2003 13:38:30 -0000 >@@ -2953,7 +2953,9 @@ > pStr = fields->GetOtherRandomHeaders(); > if (pStr) > mCompFields->SetOtherRandomHeaders((char *) pStr); >- >+ >+ AddDefaultCustomHeaders(); >+ A) Since the method returns an nsresult, you need to check it here. >@@ -2991,6 +2993,67 @@ > > return rv; > } >+ >+// Add default headers to outgoing messages >+// see Bug #61520 >+// mail.identity.<id#>.headers pref is a comma separated value of pref names >+// containging headers to add >+// headers are stored in mail.identity.<id#>.header.<header name> >+// grab all the headers, mime encode them and add them to the other custom >+// headers. B) Can you cause the above paragraph to be word-wrapped in a readable way? (eg M-q would do the trick in emacs). >+nsresult >+nsMsgComposeAndSend::AddDefaultCustomHeaders() { >+ nsXPIDLCString headersList; >+ // get names of prefs containing headers to add >+ nsresult rv = mUserIdentity->GetCharAttribute("headers", >+ getter_Copies(headersList)); >+ if (NS_SUCCEEDED(rv) && !headersList.IsEmpty()) { >+ PRInt32 start = 0; >+ PRInt32 end = 0; >+ PRInt32 len = 0; >+ // preserve any custom headers that have been added through the UI >+ nsCAutoString newHeaderVal(mCompFields->GetOtherRandomHeaders()); >+ >+ while (end != -1) { >+ end = headersList.FindChar(',', start); >+ if (end == -1) { >+ len = headersList.Length() - start; >+ } else { >+ len = end - start; >+ } >+ // grab the name of the current header pref >+ nsCAutoString headerName(NS_LITERAL_CSTRING("header.") + >+ Substring(headersList, start, len)); >+ start = end + 1; >+ >+ nsXPIDLCString headerVal; >+ rv = mUserIdentity->GetCharAttribute(headerName.get(), >+ getter_Copies(headerVal)); >+ if (NS_SUCCEEDED(rv)) { >+ PRInt32 idx = headerVal.FindChar(':') + 1; C) Instead of naming this variable idx, how about colonIdx in order to improve code readability? >+ if (idx != 0) { // check that the header is *most likely* valid. >+ char * convHeader = >+ nsMsgI18NEncodeMimePartIIStr(headerVal.get() + idx, >+ PR_FALSE, >+ mCompFields->GetCharacterSet(), >+ idx, >+ PR_TRUE); >+ if (convHeader) { >+ newHeaderVal.Append(Substring(headerVal, 0, idx)); >+ newHeaderVal.Append(convHeader); >+ // we must terminate the header with CRLF here >+ // as nsMsgCompUtils.cpp just calls PUSH_STRING >+ newHeaderVal.Append("\r\n"); >+ PR_Free(convHeader); >+ } >+ } >+ } >+ } >+ mCompFields->SetOtherRandomHeaders(newHeaderVal.get()); >+ } >+ return rv; If rv is set to a failure, that could mean that the preference doesn't exist (which is not really an error) or that there was an error getting the preference (which should be a real error). So what you return needs to depend on the specific value of rv.

* changed idx to colonIdx * don't throw away rv from AddDefaultCustomHeaders() don't check rv from + nsresult rv = mUserIdentity->GetCharAttribute("headers", + getter_Copies(headersList)); as the header exists, so a failure is a failure

check rv from AddDefaultCustomHeaders()

Comment on attachment 123843 [details] [diff] [review] patch to nsMsgSend.cpp r=dmose@mozilla.org

*** Bug 165069 has been marked as a duplicate of this bug. ***

This looks good and it shouldn't have any impact on the current behavior unless you set some of these prefs. One minor question, shouldn't we be ignoring the rv value that comes back from AddDefaultCustomHeaders? + rv = AddDefaultCustomHeaders(); + if (NS_FAILED(rv)) { + return rv; + } Whether or not your routine succeeded, the message should still be sent. I'd consider any errors that happened while trying to add these new headers to not be important enough to prevent the message from actually going out. Just my two cents. Also in nsMsgSend.h, no need to quote a bug number before declaring the new method. You already point out the bug number in the implementation of that method.

>One minor question, shouldn't we be ignoring the rv >value that comes back from AddDefaultCustomHeaders? >+ rv = AddDefaultCustomHeaders(); >+ if (NS_FAILED(rv)) { >+ return rv; >+ } This was at the request of Dan Mosedale for the review (see comment #35 and also as result of conversation on IRC). It could be removed, but isn't it better to flag an error. >Whether or not your routine succeeded, the message should still be sent. Actually I tested (quickly) and returned an error (can't recall which one)and the message sends anyway (something is ignoring rv), but you miss the mime sanity checks > I'd consider any errors that happened while trying to add these new headers > to not be important enough to prevent the message from actually going out. Well there seems to be no other way of saying we had an error, this is definatly the wrong place for any alert. Anyway if You & Dan can come to an agreement I'll code it up ;-)

removed bug# from .h file. Still with rv check

removed bug# from .h file, and removed rv check

Comment on attachment 127171 [details] [diff] [review] patch without rv check sr=mscott on the no rv patch. It would be great if we could ping David Tenser and add information about this preference to his Tips & Tricks section of the thunderbird help site.

Thanks. I've got emails ready to go to Tenser and will write one to the habeas folks, apropos http://www.habeas.com/support/mozilla.htm, as soon as this bug is marked fixed, or perhaps I'll wait 'till the next branch/release to send 'em.

WFM did somebody try to set a default value to Xtra headers ? Is there a way to ?

Comment on attachment 127171 [details] [diff] [review] patch without rv check seeking 1.5b approval

Comment on attachment 127171 [details] [diff] [review] patch without rv check seeking 1.5b approval Patch addresses a requested feature for moz mail. It would be good to get widespread testing of this feature too! Risks are minimal, code is not entered unless the new feature is used (by setting a hidden preference)

Comment on attachment 127171 [details] [diff] [review] patch without rv check Reaky, how did it get _two_ approval1.5b lines? In any case, approved for 1.5b by rjesup and asa

Fix checked in.

Comment on attachment 123843 [details] [diff] [review] patch to nsMsgSend.cpp obsolete review request

FYI, this is in mozilla1.5rc1, now available. It hadn't made it into 1.5b.

I wrote to the Habeas folks, and Tenser, FYI. Here's what I use: (I have yet to dig up my old X-face from old mail archives.) user_pref("mail.identity.id1.headers", "habeas1,habeas2,habeas3,habeas4,habeas5,habeas6,habeas7,habeas8,habeas9"); user_pref("mail.identity.id1.header.habeas1", "X-Habeas-SWE-1: winter into spring"); user_pref("mail.identity.id1.header.habeas2", "X-Habeas-SWE-2: brightly anticipated"); user_pref("mail.identity.id1.header.habeas3", "X-Habeas-SWE-3: like Habeas SWE (tm)"); user_pref("mail.identity.id1.header.habeas4", "X-Habeas-SWE-4: Copyright 2002 Habeas (tm)"); user_pref("mail.identity.id1.header.habeas5", "X-Habeas-SWE-5: Sender Warranted Email (SWE) (tm). The sender of this"); user_pref("mail.identity.id1.header.habeas6", "X-Habeas-SWE-6: email in exchange for a license for this Habeas"); user_pref("mail.identity.id1.header.habeas7", "X-Habeas-SWE-7: warrant mark warrants that this is a Habeas Compliant"); user_pref("mail.identity.id1.header.habeas8", "X-Habeas-SWE-8: Message (HCM) and not spam. Please report use of this"); user_pref("mail.identity.id1.header.habeas9", "X-Habeas-SWE-9: mark in spam to <http://www.habeas.com/report/>.");

i want to bcc myself on every message sent out, because i have my e-mail profile setup on two computers. there is a setting to bcc an address on every message, that i have been using. this is very annoying for several reasons. first of all, i always have this extra address at the top of the message. many times i find myself accidently having an address set to bcc for some reaon because i hit enter or something at the first e-mail address, because i backspaced all the other e-mail addresses, and wound up on the first line, which is the bcc address. when i hit enter, the next address is bcc. there are other goofy things that happen. also, it is just confusing to have to look at this other e-mail address, and deal with it all the time. anyway, i tried using this for putting the bcc in that i want. it works, but the header remains in the e-mail, and comes up on the recipeint in with the addresses. i am not exactly sure how bcc works with e-mail servers, if the bcc is actually in the e-mail when it is sent, and the server removes it, or if the bcc is just part of the command that is sent when the e-mail is sent. if the e-mail server does remove it, then i was wondering if there is any way that we could have a way to move the header to a different location besides the end of all the headers, so that the bcc field could come up towards the top of the e-mail as it reads in my sent folder when i do a manual bcc. if the bcc works as part of the command when the e-mail is sent, i guess i will have to file a new bug to have a hidden bcc. anyway, it is not really that big of a deal for me if i have bcc coming up for every message i send out, because i am not particularly having to hide this in this case, but it just doesn;t look right, and i was thinking that there would be other uses for people having an automatic, hidden, bcc, which is only modifiable in the prefs.js file-- for example, a parent who wants to monitor his 10 year old's e-mail usage.

andy, you can't thats just the way SMTP works. you can put whatever you want in a To or Cc header, all thats important is what you tell the SMTP server with "RCPT TO"

totally ignore my previous comment. i went back, and rechecked it, and it did not send the e-mail to the address in which i put in the bcc header that was attatched to every outgoing e-mail. i don;t know what i was thinking. sorry.

Is this feature in Thunderbird yet?

(In reply to comment #57) > Is this feature in Thunderbird yet? Yes, in the prefs.js or user.js file but not in the UI