Closed Bug 11039 Opened 25 years ago Closed 10 years ago

Filter outgoing/Sent messages (perhaps to use a different Sent/FCC folder)

Categories

(MailNews Core :: Filters, enhancement)

enhancement
Not set
normal

Tracking

(thunderbird38+ fixed)

RESOLVED FIXED
Thunderbird 37.0
Tracking Status
thunderbird38 + fixed

People

(Reporter: sspitzer, Assigned: neil)

References

(Blocks 4 open bugs, )

Details

(Keywords: feature, user-doc-needed, Whiteboard: [gs])

Attachments

(3 files, 8 obsolete files)

Related: FCC source folder
Whiteboard: HELP WANTED
Target Milestone: M15
Summary: Filter outgoing messages (perhaps to use a different Sent/FCC folder) → [HELP WANTED]Filter outgoing messages (perhaps to use a different Sent/FCC folder)
QA Contact: lchiang → laurel
*** Bug 5337 has been marked as a duplicate of this bug. ***
Blocks: 9417
Or delete the message if it's private. =) Bug #9417 is similar to this, in that it's about comprehensive filtering. Since this a subset of "comprehensive" filtering, I'll mark this depended on by that.
Bulk-resolving requests for enhancement as "later" to get them off the Seamonkey bug tracking radar. Even though these bugs are not "open" in bugzilla, we welcome fixes and improvements in these areas at any time. Mail/news RFEs continue to be tracked on http://www.mozilla.org/mailnews/jobs.html
Reopen mail/news HELP WANTED bugs and reassign to nobody@mozilla.org
*** Bug 19151 has been marked as a duplicate of this bug. ***
*** Bug 21584 has been marked as a duplicate of this bug. ***
Keywords: helpwanted
Summary: [HELP WANTED]Filter outgoing messages (perhaps to use a different Sent/FCC folder) → Filter outgoing messages (perhaps to use a different Sent/FCC folder)
Whiteboard: HELP WANTED
Target Milestone: M15
Does this bug also cover an ability to specify in the address book where the messages to this address should be FCC'ed (a-la-pine)?
The easy way for a user to achieve this is to BCC: themselves on all outgoing messages.
Except that if you do that you have to go in and read the message, or mark it unread. Otherwise you have bold mail folders because of the unread BCC which clutters up your ability to see if you REALLY have new mail and where it is.
why isn't bug 131887 a duplicate of this?
*** Bug 131887 has been marked as a duplicate of this bug. ***
Shouldn't the component be changed to "Filters" so that people can find this bug?
*** Bug 169772 has been marked as a duplicate of this bug. ***
Flags: blocking1.4b+
thank you for volunteering to fix this, please hurry. note: only drivers may set blocking1.4b+.
Assignee: nobody → kriskelvin
Flags: blocking1.4b+
bug 177040 should go in first, with this bug enabling the option of more advanced possibilities.
Depends on: 177040
Use the info gathered with bug 180683 and add a checkmark to save future replies to this folder.
Depends on: 180683
Blocks: 180683
No longer depends on: 180683
Need QA owner for this to write testcases and do the testing. Laurel is no longer working on this project.
QA Contact: laurel → nobody
Assignee: kriskelvin → sspitzer
QA Contact: nobody → esther
I'll just add that Pegasus Mail has this facility and it really helps reduce the clutter in the Sent folder.
Presumably this bug requires: 1) backend filtering on sent mail 2) a UI to indicate that the filter should be applied to outgoing mail (see bug 226422) 3) an action "FCC To" (which would only apply for the outgoing mail case). I really don't see how this bug depends on bug 177040 nor how it blocks 180683.
One thing I would like to have is the ability to add headers if the filter matches. For instance, if I send email to kernelnewbies@nl.linux.org, I want to add the header "Reply-to: kernelnewbies@nl.linux.org". This would force all replies to go to the mailing list rather than to me. This bug is really old and has 25 votes so far. I think it's time some actually started working on it.
(In reply to comment #20) > One thing I would like to have is the ability to add headers if the filter > matches. For instance, if I send email to kernelnewbies@nl.linux.org, I want to > add the header "Reply-to: kernelnewbies@nl.linux.org". This would force all > replies to go to the mailing list rather than to me. How do you want to handle conflicts if there is an existing header?
Adding headers is NOT THIS BUG. Open another bug for that, please.
This is also an essential feature of Outlook. At the moment I've do this manually by running my filters on the Sent folder two. But this really annoying. What about simply applying the whole list of filters not only on new mail in the inbox but also on every sent mail in the sent folder? Is that really so much work?
(In reply to comment #21) > How do you want to handle conflicts if there is an existing header? Well, in this particular case, I would prefer to be able to write a filter in a scripting language, like Perl, and that way my filter-script would check if the header already exists and take some predefined action of my choosing. Most likely, I would tell it to replace the existing header with the new one.
Product: MailNews → Core
What my users desperately want is that at send time of each email, they can manually choose which folder to put a copy of the sent email (this is a feature they all use presently in pegasus mail and the one reason why I cannot get them to switch to Mozilla). I have tried various extensions, but none have really worked satisfactorily. As you design the filters for outgoing emails, it would be great if this could be incorporated.
(In reply to comment #25) > What my users desperately want is that at send time of each email, they can > manually choose which folder to put a copy of the sent email This feature exists! In the compose window, Options | Send a Copy To (which really should be called "Save a Copy To" IMO).
(In reply to comment #26) > (In reply to comment #25) > > What my users desperately want is that at send time of each email, they can > > manually choose which folder to put a copy of the sent email > This feature exists! In the compose window, Options | Send a Copy To > (which really should be called "Save a Copy To" IMO). Unfortunately (to my knowledge), there is no way to default this - this means that i cannot somewhere tell thunderbird that each time a new email is to be composed, the save a copy to will pop up, so it is forgotten to be used till too late. On Pegasus, when I have finished the email and press send, a pop up comes up (if I have selected the option) which then asks which folder to put the email into.
In order to let the sending message process as quick and simple as possible, the solution I see is to let the user set the filters to be applied on the outgoing messages, not only on the incoming ones. This could be done without modifications on TB, saving the sent messages in the Sent folder, just adding the possibility (a checkbox ?) in the "New filter" window to let the user choose if the filter is to be applied on the Sent folder (or outgoing messages). For a first step to that direction, I think that it would be good if I could create filters to be applied on the Sent folder, without having to select certain named filters and the folder on which they have to be applied. The next step could be the automated "Filtering outgoing messages" feature...
Depends on: 301084
Another reason for filtering outgoing message is this scenario: As a project manager, I'm constantly bugging people for certain informations via e-mail. The problem is, that people don't necessarily like to get bugged and quite a few choose to forget a request, put it on the pile and do it "later", etc. So what I desperately need is an overview of what informations I requested, e. g. last week. I can now either keep a spreadsheet with who I pondered for what information, or I can do this more elegantly with Thunderbird. I would like to automatically mark outgoing mails with the label "follow-up", which I have assigned to Nr 5 on the keyboard. I would do this by putting the characters "[IR]" into the subject line of each outgoing mail that contain an "Information Request". By automatically marking all outgoing mails with "[IR] in the subject line with the label "Follow Up", I can easily check, what messages still lack answers. Hope this increases the motivation to work this feature into one of the next releases :-) Best, Bjoern
*** Bug 319954 has been marked as a duplicate of this bug. ***
I for one am astonished that this was first requested back in 2003. It is now, what? 2005...and its still not done. Seriously, how hard can it be? Also, the component and keyword are all screwed up. I search for similar, didn't find it and had to create #319954...only to find that it was indeed a dupe. I am a recent Eudora Pro 6 (Paid mode) refugee and I have just spent the better part of the day migrating over nine years of emails to TB 1.5RC3. While I was duplicating my filters, I realized that the TB filters only worked on incoming mails, not outgoing like in Eudora Pro. Thats highly restrictive. If filters would work on outgoing mails, then a filter could be made to add replies to the originating folder or wherever. e.g. In this shot from a EudoraPro filter, any message sent to or replied from that contains webmaster@3000ad.com, will be saved in the 'Webmaster' folder. http://www.3000ad.com/temp/tb301084.png Also, when To and From messages are in the same folder, the From messages are in italics. This helps to differentiate them from each other. So, I think that if the filters were extended to support From messages. Reproducible: Always Steps to Reproduce: 1. Just try to create a filter for outgoing messages. Actual Results: You cannot create a filter for outgoing messages. Expected Results: When an account is created, it has its own Inbox/Sent folders. However, as a game developer, I manage a lot of people and thus have various folders in Local Folders. So, when an email comes in from someone and goes into the inbox, it is then filtered into the appropriate sub-folder in Local Folders. However, when you reply, instead of it going back into the sub-folder that is used for messages with that person, it goes into the Sent box and has to be manually moved. e.g. # janedoe@anywhere.com\Inbox janedoe@anywhere.com\Sent # Local Folders\Sent Local Folders\John # When emails from John come in, they are filtered and sent (correctly) into Local Folders\John. However, when you reply to John, there is no way to filter your message so that the reply goes back into Local Folders\John, thereby preserving a conversation thread. Instead, the message goes into janedoe@anywhere.com\Sent and you have to manually move it to Local Folders\John
to #31: I downloaded a build of the latest trunk (1.6) and it supports fcc of replies in the same folder of the replied mail. You just need to add "mail.identity.<youraccountid>.fcc_reply_follows_parent = true" in your configuration. Unfortunately this will not be in TB 1.5 and this still doesn't have an UI to set this flag but *it works*. I know this is only a surrogate to outgoing filters, but it is anyway part of your problem.
Thanks!! I'll try that. Which config file does that setting need to go in? The user.js? I wonder why this isn't in 1.5; given that people have been clamouring for something like this for the past 2+ years.
to #33: Tools->Options...->Advanced->General->Config Editor (the same configuration editor as about:config in Firefox). Look at the bug #301084 to know more details. I think the patch as been submitted too late and TB is already in RC cycle (no new features): also, Scott is not happy with the UI submitted.
Thanks! That works fine now. It would have been nice if there was a setting to change the formatting (e.g. to italic) of the Fcc message. This way, like in other email programs (e.g. Eudora, PocoMail etc), you can see which messages were received and sent, without relying on the toolbar Sender heading. Oh well, can't have everything I suppose. Also, I have tracked this issue and it has been reported (in some form or another) for very long time. How did this end up not going into the TB beta? Now its at RC and feature locked it seems like its never going in. I don't think requiring a UI is that important. So perhaps they could implement the Config setting in 1.5 as they have in 1.6a1. My problem now is that most of my extensions don't work with 1.6a1. :( *sigh*
*** Bug 319976 has been marked as a duplicate of this bug. ***
Blocks: 66425
*** Bug 330441 has been marked as a duplicate of this bug. ***
*** Bug 335492 has been marked as a duplicate of this bug. ***
See Bug #Bug 294632, "Filters should be per-folder, not per-account"
*** Bug 257729 has been marked as a duplicate of this bug. ***
Assignee: sspitzer → nobody
Severity: normal → enhancement
Priority: P3 → --
QA Contact: esther → backend
I have a problem, with bcc to myself when sending mail. The blind copy is not delivered to my account :-( TB 2b2. Is there some solution to do this? THX A.
Isn't that a known issue with Gmail?
I do not know, but when I try to sent bcc to me from web interface (gmail) then the mail is delivered, only if I try to send bcc from TB then mail is not delivered. :-( A.
I bcc myself from TB frequently, without problem. There is a delay, of course, as the mail wends it way through cyberspace.
I try another email account a this work ok, it seems that there some problems with gmail account. :-( . So i think that TB is working properly. THX A.
Flags: wanted-thunderbird3?
I am very disappointed that there's been no progress on this. I put off migrating from The Bat! to Thunderbird by at least a year because TB did not have this feature. Today, I discovered that Evolution does - see the screenshot at http://farm4.static.flickr.com/3266/2556041004_263ae86087_o.gif . It baffles me that there's not even an add-on for this!
I write a extention for this, https://addons.mozilla.org/en-US/firefox/addon/5538, but I don't know if there are some dangers in my code. for hook the sending finished event, I do some thing be similar to hacking...
Any hope to have this fixed for v3.0?
Jeff, was this on the penelope agenda?
Yes, outgoing filtering is planned as part of Penelope. See bug 359236.
Jeff, you mean it won't be implemented in core Thunderbird?
No, it will be implemented in core TB. Lots of changes that we are making for Penelope/Eudora have to be done in core code. The goal of the Penelope project is to get Classic Eudora features in to Thunderbird. Sometimes those are easily done in the Penelope extension, but many times the feature requires enough tight integration that it has to be done in core code. Outgoing filtering will definitely require tight integration.
Nice to hear that! Will we see this in TB 3.0?
Yes, that is the plan.
Product: Core → MailNews Core
Any progress on this issue? The Target Milestone is still unknown while we're getting closer to v3.0 beta1...
I cannot believe that this really basic feature has been requested since 1999 and not implemented yet. The Bat! has had it for years, even Outlook could always do it since I knew the product. Because of this, I postponed migrating from Outlook @work and The Bat! @home for years. Somebody else said that before me, I guess. It is a shame, because otherwise Thunderbird is such a good product.
I am investigating improving the support for different email filter contexts in bug 198100, where my target is a context post-junk analysis. But the issues are very similar to those here, so if anyone (beckley?) starts to work on this, please talk to me.
does this filter will allow: * defer sending message (more like outlook than send later extension [1] * enforce sender address or some recipient based on some rules (like force sender address for mailing-list or add someone in cc if sent to someone else) * force a s/mime or opengpg signature thanks a lot can't wait for 3.0 [1] http://lifehacker.com/5168072/send-later-for-thunderbird-adds-custom-quick+delay-buttons http://www.lifehacker.com.au/2008/05/defer_email_delivery_in_microsoft_outlook-2/
It's good to know the FCC functionality is making it into the core. If it's in v3, do we need to enable anything? I've been trialling v3.0b3 (no B4 for en-UK) and am not able to locate outgoing filters option, certainly not under Tools | Message Filters. Yes, there is the "Send a Copy To ..." under the Options menu in a new message window. Sorry, that's brain dead! A mis-feature! The idea of using the FCC approach is that there is only ONE message stored locally. This copy approach simply places one message in the designated folder along with a second copy in the Sent folder, leaving an untidy Sent folder that requires constant attention and filing. I use Outlook at work and regularly use the equivalent option to "Save sent message to ..." an alternative folder. I don't end up with a copy in the default Sent folder as well. There used to be an extension called 'FCC on Compose' by Frank DiLecce that did fairly well at providing FCC functionality like Mutt and Pine, and Eudora for that matter. It had a button below the To field. Sadly, the extension no longer appears to be under development. The ultimate solution is: 'Save ONE copy to selected folder', button for folder selection below the To button, filters to handle saving the outgoing copy to different folders, and the cherry on the top: define the preferred folder for contacts in the address book.
$0.02 re: [comment 20] > One thing I would like to have is the ability to add headers if the filter I would like to add arbitrary headers and X-headers, too My own headers would enable other mail agents, as opposed to the composition client, to act on mail as well. re: [comment 21] > How do you want to handle conflicts if there is an existing header? A tickbox option "this rule has higher priority" or somesuch. Unchecked case: if already there leave it unmolested. Checked case: replace with this. For more fun allow the user to set a numeric weight. re-direct [comment 43] > a known issue with Gmail? gmail has pathetic, un-powerful [sic] filtering. It cannot act on user specified x-headers: if they exist or their respective content
I just started this work from today, maybe I can release a patch at the beginning of next year.
Attached patch a patch for outgoing filter (obsolete) — Splinter Review
It is a patch for outgoing filter, and base on the nightly changeset "4749:0e2fd3b00c96". It just implements the post sending filter.A filter calling before sending needs more works.
Attachment #423137 - Flags: review?(dmose)
Attachment #423137 - Flags: review?(bienvenu)
If you need a tester, I volunteer, but I have no build environment. Can you provide a build based on Thunderbird 3.0.1 (WinXP 32-bit), patched with your code?
Comment on attachment 423137 [details] [diff] [review] a patch for outgoing filter >diff -r 516bf521bc82 -r 493fdeceb191 mail/locales/en-US/chrome/messenger/FilterEditor.dtd >--- a/mail/locales/en-US/chrome/messenger/FilterEditor.dtd Fri Jan 22 12:59:47 2010 +0100 >+++ b/mail/locales/en-US/chrome/messenger/FilterEditor.dtd Sat Jan 23 23:20:15 2010 +0900 >@@ -8,22 +8,24 @@ > > <!ENTITY lowestPriorityCmd.label "Lowest"> > <!ENTITY lowPriorityCmd.label "Low"> > <!ENTITY normalPriorityCmd.label "Normal"> > <!ENTITY highPriorityCmd.label "High"> > <!ENTITY highestPriorityCmd.label "Highest"> > > <!ENTITY contextDesc.label "Apply filter when:"> >-<!ENTITY contextDesc.accesskey "w"> > <!ENTITY contextIncoming.label "Checking Mail"> > <!ENTITY contextManual.label "Manually Run"> >-<!ENTITY contextBoth.label "Checking Mail or Manually Run"> >-<!ENTITY contextPostPlugin.label "Checking Mail (after classification)"> >-<!ENTITY contextPostPluginBoth.label "Checking Mail (after classification) or Manually Run"> >+<!ENTITY contextBeforeCls.label "Before Classification"> >+<!ENTITY contextAfterCls.label "After Classification"> >+<!ENTITY contextOutgoing.label "Outgoing"> >+<!ENTITY contextBeforeSending.label "Before Sending"> >+<!ENTITY contextAfterSending.label "After Sending"> >+ > <!ENTITY filterActionDesc.label "Perform these actions:"> > <!ENTITY filterActionDesc.accesskey "P"> > > <!-- New Style Filter Rule Actions --> > <!ENTITY moveMessage.label "Move Message to"> > <!ENTITY copyMessage.label "Copy Message to"> > <!ENTITY forwardTo.label "Forward Message to"> > <!ENTITY replyWithTemplate.label "Reply with Template"> >diff -r 516bf521bc82 -r 493fdeceb191 mailnews/base/search/content/FilterEditor.js >--- a/mailnews/base/search/content/FilterEditor.js Fri Jan 22 12:59:47 2010 +0100 >+++ b/mailnews/base/search/content/FilterEditor.js Sat Jan 23 23:20:15 2010 +0900 >@@ -47,17 +47,17 @@ var gPromptService = Components.classes[ > > // The actual filter that we're editing if it is a _saved_ filter or prefill; > // void otherwise. > var gFilter; > // cache the key elements we need > var gFilterList; > // The filter name as it appears in the "Filter Name" field of dialog. > var gFilterNameElement; >-var gFilterContext; >+var gFilterTypeDelegator; > var gFilterBundle; > var gPreFillName; > var nsMsgSearchScope = Components.interfaces.nsMsgSearchScope; > var gPrefBranch; > var gMailSession = null; > var gFilterActionList; > var gCustomActions = null; > var gFilterType; >@@ -87,21 +87,20 @@ function filterEditorOnLoad() > var args = window.arguments[0]; > > if ("filterList" in args) > { > gFilterList = args.filterList; > // the postPlugin filters cannot be applied to servers that are > // deferred, (you must define them on the deferredTo server instead). > let server = gFilterList.folder.server; >- let postPluginDisabled = server.rootFolder != server.rootMsgFolder; >- document.getElementById("contextMenuListPostPlugin") >- .disabled = postPluginDisabled; >- document.getElementById("contextMenuListPostPluginBoth") >- .disabled = postPluginDisabled; >+ if(server.rootFolder != server.rootMsgFolder) >+ { >+ gFilterTypeDelegator.rdAfCl.style.display = "none"; >+ } > } > > if ("filter" in args) > { > // editing a filter > gFilter = window.arguments[0].filter; > initializeDialog(gFilter); > } >@@ -147,22 +146,23 @@ function filterEditorOnLoad() > onMore(null); > } > } > } > > if (!gFilter) > { > // This is a new filter. Set to both Incoming and Manual contexts. >- gFilterContext.selectedIndex = 2; >+ let nsMsgFilterType = Components.interfaces.nsMsgFilterType; >+ gFilterTypeDelegator.setType(nsMsgFilterType.Incoming | nsMsgFilterType.Manual); > } > > // in the case of a new filter, we may not have an action row yet. > ensureActionRow(); >- gFilterType = determineFilterType(); >+ gFilterType = gFilterTypeDelegator.getType(); > > gFilterNameElement.select(); > // This call is required on mac and linux. It has no effect under win32. See bug 94800. > gFilterNameElement.focus(); > } > > function filterEditorOnUnload() > { >@@ -239,46 +239,157 @@ function getScope(filter) > { > return getScopeFromFilterList(filter.filterList); > } > > function initializeFilterWidgets() > { > gFilterNameElement = document.getElementById("filterName"); > gFilterActionList = document.getElementById("filterActionList"); >- gFilterContext = document.getElementById("contextMenuList"); >+ initializeFilterTypeDelegator(); >+} >+ >+function initializeFilterTypeDelegator() >+{ >+ gFilterTypeDelegator = { >+ ckMan : document.getElementById("ckManual"), >+ ckIn : document.getElementById("ckIncoming"), >+ ckOut : document.getElementById("ckOutgoing"), >+ >+ rdgIn : document.getElementById("rdgIncoming"), >+ rdgOut : document.getElementById("rdgOutgoing"), >+ >+ rdBfCl : document.getElementById("rdBfClassify"), >+ rdAfCl : document.getElementById("rdAfClassify"), >+ rdBfSd : document.getElementById("rdBfSending"), >+ rdAfSd : document.getElementById("rdAfSending"), >+ >+ type : 0, >+ >+ init : function() >+ { >+ //It seems that there is only this way can handle the event >+ //of check status changing on both of mouse click and keyboard >+ //operation. Is there an another better way? >+ var ary = [ >+ ["checked", this.ckMan, this.ckIn, this.ckOut], >+ ["selectedItem", this.rdgIn, this.rdgOut] >+ ]; >+ for(var i=0;i<ary.length;i++) >+ { >+ for(var j=1;j<ary[i].length;j++){ >+ ary[i][j].watch(ary[i][0], function(p,o,n){ >+ setTimeout("gFilterTypeDelegator.click(null)",0); >+ return n; >+ }); >+ } >+ } >+ }, >+ >+ getType : function() >+ { >+ let nsMsgFilterType = Components.interfaces.nsMsgFilterType; >+ let type = nsMsgFilterType.None; >+ >+ if(this.ckMan.checked) >+ { >+ type = type | nsMsgFilterType.Manual; >+ } >+ >+ if(this.ckIn.checked) >+ { >+ type = type | (this.rdgIn.selectedItem === this.rdBfCl ? nsMsgFilterType.Incoming : nsMsgFilterType.PostPlugin); >+ } >+ >+ if(this.ckOut.checked) >+ { >+ type = type | (this.rdgOut.selectedItem === this.rdBfSd ? nsMsgFilterType.PreOutgoing : nsMsgFilterType.PostOutgoing); >+ } >+ >+ return type; >+ >+ }, >+ >+ setType : function(type) >+ { >+ let nsMsgFilterType = Components.interfaces.nsMsgFilterType; >+ if(type & nsMsgFilterType.Manual) >+ { >+ this.ckMan.checked = true; >+ } >+ else >+ { >+ this.ckMan.checked = false; >+ } >+ >+ if(type & (nsMsgFilterType.Incoming | nsMsgFilterType.PostPlugin)) >+ { >+ this.ckIn.checked = true; >+ this.rdgIn.disabled = false; >+ if(type & nsMsgFilterType.Incoming) >+ { >+ this.rdgIn.selectedItem = this.rdBfCl; >+ } >+ else // if(type & nsMsgFilterType.PostPlugin) >+ { >+ this.rdgIn.selectedItem = this.rdAfCl; >+ } >+ } >+ else >+ { >+ this.ckIn.checked = false; >+ this.rdgIn.disabled = true; >+ } >+ >+ if(type & (nsMsgFilterType.PreOutgoing | nsMsgFilterType.PostOutgoing)) >+ { >+ this.ckOut.checked = true; >+ this.rdgOut.disabled = false; >+ if(type & nsMsgFilterType.PreOutgoing) >+ { >+ this.rdgOut.selectedItem = this.rdBfSd; >+ } >+ else // if(type & nsMsgFilterType.PostOutgoing) >+ { >+ this.rdgOut.selectedItem = this.rdAfSd; >+ } >+ } >+ else >+ { >+ this.ckOut.checked = false; >+ this.rdgOut.disabled = true; >+ } >+ >+ this.click(); >+ }, >+ >+ click : function(event) >+ { >+ this.rdgIn.disabled = !this.ckIn.checked; >+ this.rdgOut.disabled = !this.ckOut.checked; >+ // Maybe we need some special logic about before sending type, >+ // but since we havn't implement it yet, just left it. >+ // As a temporary logic, we must set the after sending radio box checked. >+ if(!this.rdgOut.disabled) >+ { >+ this.rdgOut.selectedItem = this.rdAfSd; >+ } >+ >+ updateFilterType(); >+ } >+ }; >+ >+ gFilterTypeDelegator.init(); > } > > function initializeDialog(filter) > { > gFilterNameElement.value = filter.filterName; >- /* >- * contextIndex = 0: checking mail >- * = 1: manually run >- * = 2: checking mail or manually run >- * = 3: post analysis >- * = 4: post analysis or manually run >- */ > let filterType = filter.filterType; >- let nsMsgFilterType = Components.interfaces.nsMsgFilterType; >- let contextIndex; >- if (filterType & nsMsgFilterType.Manual) >- { >- if (filterType & nsMsgFilterType.Incoming) >- contextIndex = 2; >- else if (filterType & nsMsgFilterType.PostPlugin) >- contextIndex = 4; >- else >- contextIndex = 1; >- } >- else if (filterType & nsMsgFilterType.PostPlugin) >- contextIndex = 3; >- else >- contextIndex = 0; >- gFilterContext.selectedIndex = contextIndex; >+ gFilterTypeDelegator.setType(filter.filterType); > > var actionList = filter.actionList; > var numActions = actionList.Count(); > > for (var actionIndex=0; actionIndex < numActions; actionIndex++) > { > var filterAction = actionList.QueryElementAt(actionIndex, Components.interfaces.nsIMsgRuleAction); > >@@ -596,51 +707,25 @@ function getCustomActions() > while (customActionsEnum.hasMoreElements()) > gCustomActions.push(customActionsEnum.getNext().QueryInterface( > Components.interfaces.nsIMsgFilterCustomAction)); > } > } > > function updateFilterType() > { >- gFilterType = determineFilterType(); >+ gFilterType = gFilterTypeDelegator.getType(); > setFilterScope(gFilterType, gFilterList); > > // set valid actions > var ruleActions = gFilterActionList.getElementsByAttribute('class', 'ruleaction'); > for (var i = 0; i < ruleActions.length; i++) > ruleActions[i].mRuleActionType.hideInvalidActions(); > } > >-function determineFilterType() >-{ >- /* >- * contextIndex = 0: checking mail >- * = 1: manually run >- * = 2: checking mail or manually run >- * = 3: post analysis >- * = 4: post analysis or manually run >- */ >- let contextIndex = gFilterContext.selectedIndex; >- let filterType = 0; >- if (contextIndex == 1 || contextIndex == 2 || contextIndex == 4) // manual >- filterType |= Components.interfaces.nsMsgFilterType.Manual; >- if (contextIndex == 3 || contextIndex == 4) // post analysis >- filterType |= Components.interfaces.nsMsgFilterType.PostPlugin; >- if (contextIndex == 0 || contextIndex == 2) // checking mail >- { >- if (getScopeFromFilterList(gFilterList) == >- Components.interfaces.nsMsgSearchScope.newsFilter) >- filterType |= Components.interfaces.nsMsgFilterType.NewsRule; >- else >- filterType |= Components.interfaces.nsMsgFilterType.InboxRule; >- } >- return filterType; >-} >- > // Given a filter type, set the global search scope to the filter scope > function setFilterScope(aFilterType, aFilterList) > { > let filterScope = getFilterScope(getScopeFromFilterList(aFilterList), > aFilterType, aFilterList); > setSearchScope(filterScope); > } > >@@ -648,16 +733,19 @@ function setFilterScope(aFilterType, aFi > // Given the base filter scope for a server, and the filter > // type, return the scope used for filter. This assumes a > // hierarchy of contexts, with incoming the most restrictive, > // followed by manual and post-plugin. > function getFilterScope(aServerFilterScope, aFilterType, aFilterList) > { > let Ci = Components.interfaces; > >+ if (aFilterType & Ci.nsMsgFilterType.Outgoing) >+ return aServerFilterScope; >+ > if (aFilterType & Ci.nsMsgFilterType.Incoming) > return aServerFilterScope; > > // Manual or PostPlugin > // local mail allows body and junk types > if (aServerFilterScope == Ci.nsMsgSearchScope.offlineMailFilter) > return Ci.nsMsgSearchScope.offlineMail; > // IMAP and NEWS online don't allow body >diff -r 516bf521bc82 -r 493fdeceb191 mailnews/base/search/content/FilterEditor.xul >--- a/mailnews/base/search/content/FilterEditor.xul Fri Jan 22 12:59:47 2010 +0100 >+++ b/mailnews/base/search/content/FilterEditor.xul Sat Jan 23 23:20:15 2010 +0900 >@@ -74,38 +74,44 @@ > <label value="&filterName.label;" accesskey="&filterName.accesskey;" control="filterName"/> > <textbox flex="50%" id="filterName"/> > <spacer flex="50%"/> > </hbox> > > <separator/> > > <vbox flex="1"> >- <hbox align="center"> >- <label value="&contextDesc.label;" >- accesskey="&contextDesc.accesskey;" >- control="contextMenuList"/> >- <menulist id="contextMenuList" >- value="both" oncommand="updateFilterType();"> >- <menupopup> >- <menuitem label="&contextIncoming.label;" >- value="incoming"/> >- <menuitem label="&contextManual.label;" >- value="manual"/> >- <menuitem label="&contextBoth.label;" >- value="both"/> >- <menuitem label="&contextPostPlugin.label;" >- value="postPlugin" >- id="contextMenuListPostPlugin"/> >- <menuitem label="&contextPostPluginBoth.label;" >- value="postPluginBoth" >- id="contextMenuListPostPluginBoth"/> >- </menupopup> >- </menulist> >- </hbox> >+ <groupbox> >+ <caption label="&contextDesc.label;" /> >+ <grid> >+ <columns> >+ <column flex="1"/> >+ <column flex="2"/> >+ </columns> >+ <rows> >+ <row> >+ <checkbox id="ckManual" label="&contextManual.label;"/> >+ </row> >+ <row> >+ <checkbox id="ckIncoming" label="&contextIncoming.label;"/> >+ <radiogroup id="rdgIncoming" orient="horizontal" disabled="true"> >+ <radio id="rdBfClassify" label="&contextBeforeCls.label;"/> >+ <radio id="rdAfClassify" label="&contextAfterCls.label;"/> >+ </radiogroup> >+ </row> >+ <row> >+ <checkbox id="ckOutgoing" label="&contextOutgoing.label;"/> >+ <radiogroup id="rdgOutgoing" orient="horizontal" disabled="true"> >+ <radio id="rdBfSending" label="&contextBeforeSending.label;" style="display:none"/> <!-- not implemented --> >+ <radio id="rdAfSending" label="&contextAfterSending.label;"/> >+ </radiogroup> >+ </row> >+ </rows> >+ </grid> >+ </groupbox> > <hbox flex="1"> > <vbox id="searchTermListBox" flex="1"/> > </hbox> > </vbox> > > <separator/> > > <vbox> >diff -r 516bf521bc82 -r 493fdeceb191 mailnews/base/search/public/nsMsgFilterCore.idl >--- a/mailnews/base/search/public/nsMsgFilterCore.idl Fri Jan 22 12:59:47 2010 +0100 >+++ b/mailnews/base/search/public/nsMsgFilterCore.idl Sat Jan 23 23:20:15 2010 +0900 >@@ -49,16 +49,19 @@ interface nsMsgFilterType { > const long InboxJavaScript = 0x02; > const long Inbox = InboxRule | InboxJavaScript; > const long NewsRule = 0x04; > const long NewsJavaScript = 0x08; > const long News = NewsRule | NewsJavaScript; > const long Incoming = Inbox | News; > const long Manual = 0x10; > const long PostPlugin = 0x20; // After bayes filtering >+ const long PostOutgoing = 0x40; >+ const long PreOutgoing = 0x80; >+ const long Outgoing = PostOutgoing | PreOutgoing; > const long All = Incoming | Manual; > }; > > typedef long nsMsgFilterMotionValue; > > typedef long nsMsgFilterIndex; > > typedef long nsMsgRuleActionType; >diff -r 516bf521bc82 -r 493fdeceb191 mailnews/compose/src/Makefile.in >--- a/mailnews/compose/src/Makefile.in Fri Jan 22 12:59:47 2010 +0100 >+++ b/mailnews/compose/src/Makefile.in Sat Jan 23 23:20:15 2010 +0900 >@@ -114,16 +114,17 @@ CPPSRCS = \ > nsMsgComposeService.cpp \ > nsMsgComposeParams.cpp \ > nsMsgComposeProgressParams.cpp \ > nsMsgComposeContentHandler.cpp \ > nsMsgCompose.cpp \ > nsMsgQuote.cpp \ > nsURLFetcher.cpp \ > nsSmtpServer.cpp \ >+ nsMsgSendFilter.cpp \ > $(NULL) > > EXPORTS = \ > nsComposeStrings.h \ > $(NULL) > > ifneq (,$(filter cocoa mac, $(MOZ_WIDGET_TOOLKIT))) > CPPSRCS += nsMsgAppleDoubleEncode.cpp \ >diff -r 516bf521bc82 -r 493fdeceb191 mailnews/compose/src/nsMsgSend.cpp >--- a/mailnews/compose/src/nsMsgSend.cpp Fri Jan 22 12:59:47 2010 +0100 >+++ b/mailnews/compose/src/nsMsgSend.cpp Sat Jan 23 23:20:15 2010 +0900 >@@ -104,16 +104,17 @@ > #include "nsNativeCharsetUtils.h" > #include "nsIAbCard.h" > #include "nsIMsgProgress.h" > #include "nsIMsgMessageService.h" > #include "nsIMsgHdr.h" > #include "nsIMsgFolder.h" > #include "nsComposeStrings.h" > #include "nsString.h" >+#include "nsMsgSendFilter.h" > > static NS_DEFINE_CID(kRDFServiceCID, NS_RDFSERVICE_CID); > > #define PREF_MAIL_SEND_STRUCT "mail.send_struct" > #define PREF_MAIL_STRICTLY_MIME "mail.strictly_mime" > #define PREF_MAIL_MESSAGE_WARNING_SIZE "mailnews.message_warning_size" > #define PREF_MAIL_COLLECT_EMAIL_ADDRESS_OUTGOING "mail.collect_email_address_outgoing" > #define PREF_MAIL_DONT_ATTACH_SOURCE "mail.compose.dont_attach_source_of_local_network_links" >@@ -363,16 +364,21 @@ nsMsgComposeAndSend::~nsMsgComposeAndSen > for (i = 0; i < m_attachment_count; i++) > { > if (m_attachments [i].m_encoder_data) > MIME_EncoderDestroy(m_attachments[i].m_encoder_data, PR_TRUE); > } > > delete[] m_attachments; > } >+ if(m_sendFilter != nsnull) >+ { >+ m_sendFilter = nsnull; >+ } >+ > } > > NS_IMETHODIMP nsMsgComposeAndSend::GetDefaultPrompt(nsIPrompt ** aPrompt) > { > NS_ENSURE_ARG(aPrompt); > *aPrompt = nsnull; > > nsresult rv = NS_OK; >@@ -4068,16 +4074,36 @@ nsMsgComposeAndSend::NotifyListenerOnSta > > NS_IMETHODIMP > nsMsgComposeAndSend::NotifyListenerOnStopSending(const char *aMsgID, nsresult aStatus, const PRUnichar *aMsg, > nsIFile *returnFile) > { > if (mListener != nsnull) > mListener->OnStopSending(aMsgID, aStatus, aMsg, returnFile); > >+ //When we send the message now or background,we will create >+ //our own sendFilter to try to apply outgoing filters, >+ //otherwise, when mode is SendUnsent, the sendFilter >+ //will be created by the sendlater instance, so here we don't >+ //need create it. >+ if(m_sendFilter == nsnull >+ && (m_deliver_mode == nsIMsgSend::nsMsgDeliverNow >+ || m_deliver_mode == nsIMsgSend::nsMsgDeliverBackground) >+ ) >+ { >+ m_sendFilter = new nsMsgSendFilter(); >+ } >+ >+ if(m_sendFilter) >+ { >+ nsCString fcc; >+ mUserIdentity->GetFccFolder(fcc); >+ m_sendFilter->AddSentMessage(aMsgID, fcc.get()); >+ } >+ > return NS_OK; > } > > NS_IMETHODIMP > nsMsgComposeAndSend::NotifyListenerOnStartCopy() > { > nsCOMPtr<nsIMsgCopyServiceListener> copyListener; > >@@ -4202,16 +4228,21 @@ nsMsgComposeAndSend::NotifyListenerOnSto > // If we are here, its real cleanup time! > if (mListener) > { > copyListener = do_QueryInterface(mListener); > if (copyListener) > copyListener->OnStopCopy(aStatus); > } > >+ if(m_sendFilter) >+ { >+ m_sendFilter->ApplyFilters(); >+ } >+ > return aStatus; > } > > /* This is the main driving function of this module. It generates a > document of type message/rfc822, which contains the stuff provided. > The first few arguments are the standard header fields that the > generated document should have. > >diff -r 516bf521bc82 -r 493fdeceb191 mailnews/compose/src/nsMsgSend.h >--- a/mailnews/compose/src/nsMsgSend.h Fri Jan 22 12:59:47 2010 +0100 >+++ b/mailnews/compose/src/nsMsgSend.h Sat Jan 23 23:20:15 2010 +0900 >@@ -180,16 +180,17 @@ > > // > // Forward declarations... > // > class nsMsgSendPart; > class nsMsgCopy; > class nsIPrompt; > class nsIInterfaceRequestor; >+class nsMsgSendFilter; > > class nsMsgComposeAndSend : public nsIMsgSend > { > public: > // > // The exit method used when downloading attachments only. > // This still may be useful because of the fact that it is an > // internal callback only. This way, no thread boundry issues and >@@ -396,16 +397,18 @@ public: > // server to be opened, or would cause much seek()ing. > > PRBool mGUINotificationEnabled; // Should we throw up the GUI alerts on errors? > PRBool mLastErrorReported; // Last error reported to the user. > PRBool mAbortInProcess; // Used by Abort to avoid reentrance. > > nsCOMPtr<nsIMsgComposeSecure> m_crypto_closure; > >+ nsRefPtr<nsMsgSendFilter> m_sendFilter; >+ > protected: > nsCOMPtr<nsIStringBundle> mComposeBundle; > nsresult GetNotificationCallbacks(nsIInterfaceRequestor** aCallbacks); > private: > // will set m_attachment1_body & m_attachment1_body_length; > nsresult EnsureLineBreaks(const char *body, PRUint32 body_len); > > // generates a message id for our message, if necessary >diff -r 516bf521bc82 -r 493fdeceb191 mailnews/compose/src/nsMsgSendFilter.cpp >--- /dev/null Thu Jan 01 00:00:00 1970 +0000 >+++ b/mailnews/compose/src/nsMsgSendFilter.cpp Sat Jan 23 23:20:15 2010 +0900 >@@ -0,0 +1,344 @@ >+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ >+/* ***** BEGIN LICENSE BLOCK ***** >+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1 >+ * >+ * The contents of this file are subject to the Mozilla Public License Version >+ * 1.1 (the "License"); you may not use this file except in compliance with >+ * the License. You may obtain a copy of the License at >+ * http://www.mozilla.org/MPL/ >+ * >+ * Software distributed under the License is distributed on an "AS IS" basis, >+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License >+ * for the specific language governing rights and limitations under the >+ * License. >+ * >+ * The Original Code is mozilla.org code. >+ * >+ * The Initial Developer of the Original Code is >+ * Netscape Communications Corporation. >+ * Portions created by the Initial Developer are Copyright (C) 1998 >+ * the Initial Developer. All Rights Reserved. >+ * >+ * Contributor(s): >+ * xzer <xiaozhu@gmail.com> >+ * >+ * Alternatively, the contents of this file may be used under the terms of >+ * either of the GNU General Public License Version 2 or later (the "GPL"), >+ * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), >+ * in which case the provisions of the GPL or the LGPL are applicable instead >+ * of those above. If you wish to allow use of your version of this file only >+ * under the terms of either the GPL or the LGPL, and not to allow others to >+ * use your version of this file under the terms of the MPL, indicate your >+ * decision by deleting the provisions above and replace them with the notice >+ * and other provisions required by the GPL or the LGPL. If you do not delete >+ * the provisions above, a recipient may use your version of this file under >+ * the terms of any one of the MPL, the GPL or the LGPL. >+ * >+ * ***** END LICENSE BLOCK ***** */ >+#include "nsMsgSendFilter.h" >+#include "nsMsgBaseCID.h" >+#include "nsMsgCompCID.h" >+#include "nsIMutableArray.h" >+#include "nsIMsgFilterService.h" >+#include "nsIMsgFilterList.h" >+#include "nsIImapMailFolderSink.h" >+#include "nsXPCOMCIDInternal.h" >+#include "nsIProxyObjectManager.h" >+#include "nsThreadUtils.h" >+#include "nsIRDFService.h" >+#include "nspr.h" >+ >+bool operator==(nsMsgSendFilterMsgInfo const &m1 ,nsMsgSendFilterMsgInfo const & m2) >+{ >+ return (m1.messageId == m2.messageId) && (m1.fcc == m2.fcc); >+} >+ >+//////////////////////////////////////////////////////////////////////////////////// >+// This is the class for the send filter. We have to create this class >+// to handle message send completion and call the filters here >+//////////////////////////////////////////////////////////////////////////////////// >+NS_IMPL_ISUPPORTS1(nsMsgSendFilter, nsIMsgFolderListener) >+ >+#ifdef XZER_DEBUG >+ // I debug this on windows, so I have to include this for getting thread id, >+ // Is there a common way to get it? >+ #ifdef WIN32 >+ #include <windows.h> >+ #define CurThreadId GetCurrentThreadId() >+ #else >+ #define CurThreadId -1 >+ #endif >+ >+ static int sflidx = 0; >+ #define printLogPrefix printf("nsMsgSendFilter(idx=%d,thread=%d) : ", instIdx, CurThreadId) >+ #define XZERLOG(x) printLogPrefix;printf(x);printf("\n") >+ #define XZERLOG1(x,y) printLogPrefix;printf(x,y);printf("\n") >+ #define XZERLOG2(x,y,z) printLogPrefix;printf(x,y,z);printf("\n") >+#else >+ #define XZERLOG >+ #define XZERLOG1 >+ #define XZERLOG2 >+#endif >+ >+nsMsgSendFilter::nsMsgSendFilter() >+{ >+#ifdef XZER_DEBUG >+ instIdx = sflidx++; >+#endif >+ >+ m_msgArrayMonitor = PR_NewMonitor(); >+ >+ m_folderNotificationService = do_GetService(NS_MSGNOTIFICATIONSERVICE_CONTRACTID); >+ >+ m_folderNotificationRegistered = PR_FALSE; >+ >+ XZERLOG("OK, I am created!"); >+ >+} >+ >+nsMsgSendFilter::~nsMsgSendFilter(void) >+{ >+ if (m_msgArrayMonitor) >+ PR_DestroyMonitor(m_msgArrayMonitor); >+ >+ XZERLOG("OK, I am destroyed!\n"); >+} >+ >+NS_IMETHODIMP >+nsMsgSendFilter::ApplyFilters(void) >+{ >+ XZERLOG("ApplyFilters was called.\n"); >+ PR_EnterMonitor(m_msgArrayMonitor); >+ nsTArray<nsCString> reloadNeedFccAry; >+ PRUint32 len = m_msgArray.Length(); >+ nsMsgSendFilterMsgInfo msgInfo; >+ nsTArray<nsCString>::index_type idx; >+ for(int i=len-1;i>=0;i--){ >+ XZERLOG1("get element at %d", i); >+ msgInfo = m_msgArray.ElementAt(i); >+ idx = reloadNeedFccAry.IndexOf(msgInfo.fcc, 0); >+ if(idx == nsTArray<nsCString>::NoIndex) >+ { >+ reloadNeedFccAry.AppendElement(msgInfo.fcc); >+ if(msgInfo.fcc.Find("imap://") == 0) >+ { >+ m_msgArray.RemoveElementAt(i); >+ ReloadImapFolder(msgInfo.fcc); >+ } >+ else >+ { >+ XZERLOG("I got a local folder which outgoing filter was not fired."); >+ } >+ } >+ } >+ if(m_msgArray.Length() == 0) >+ { >+ if(m_folderNotificationRegistered){ >+ m_folderNotificationService->RemoveListener(this); >+ m_folderNotificationRegistered = PR_FALSE; >+ } >+ } >+ >+ PR_ExitMonitor(m_msgArrayMonitor); >+ return NS_OK; >+} >+ >+NS_IMETHODIMP >+nsMsgSendFilter::AddSentMessage(const char *aMsgId, const char *fcc) >+{ >+ >+ nsMsgSendFilterMsgInfo msgInfo; >+ msgInfo.messageId.Assign(aMsgId); >+ msgInfo.fcc.Assign(fcc); >+ >+ // The message id may be on a different format here >+ // So we strip the '<' and '>' >+ if(msgInfo.messageId.CharAt(0) == '<') >+ { >+ msgInfo.messageId.StripChars("<>"); >+ } >+ >+ PR_EnterMonitor(m_msgArrayMonitor); >+ >+ if(!m_folderNotificationRegistered){ >+ m_folderNotificationService->AddListener(this,nsIMsgFolderNotificationService::msgAdded); >+ m_folderNotificationRegistered = PR_TRUE; >+ } >+ m_msgArray.AppendElement(msgInfo); >+ PR_ExitMonitor(m_msgArrayMonitor); >+ XZERLOG2("A message Send! id = %s, fcc = %s\n", aMsgId, fcc); >+ return NS_OK; >+} >+ >+NS_IMETHODIMP >+nsMsgSendFilter::ReloadImapFolder(const nsCString &fcc) >+{ >+ nsresult rv; >+ nsCOMPtr<nsIRDFService> rdfService = do_GetService("@mozilla.org/rdf/rdf-service;1", &rv); >+ nsCOMPtr <nsIRDFResource> resource; >+ rv = rdfService->GetResource(fcc, getter_AddRefs(resource)); >+ nsCOMPtr <nsIMsgFolder> fccFolder = do_QueryInterface(resource, &rv); >+ >+ nsCOMPtr<nsIMsgFilterList> filterList; >+ rv = fccFolder->GetFilterList(nsnull, getter_AddRefs(filterList)); >+ if (NS_FAILED(rv)) return rv; >+ PRUint32 fcount; >+ filterList->GetFilterCount(&fcount); >+ nsIMsgFilter* curFilter; >+ nsMsgFilterTypeType type; >+ PRBool enable; >+ //For performance reason, if there is no outgoing filter on this >+ //folder, we can avoid the reloading which is maybe very slowly >+ for(PRUint32 i=0;i<fcount;i++){ >+ filterList->GetFilterAt(i, &curFilter); >+ curFilter->GetFilterType(&type); >+ curFilter->GetEnabled(&enable); >+ if(enable && type == nsMsgFilterType::PostOutgoing) >+ { >+ // Reload the imap folder after the current work was finished, >+ // then the filters will be fired by nsImapMailFolder >+ nsCOMPtr<nsIImapMailFolderSink> fccSink = do_QueryInterface(fccFolder); >+ nsCOMPtr<nsIProxyObjectManager> proxyManager(do_GetService(NS_XPCOMPROXY_CONTRACTID, &rv)); >+ if(proxyManager){ >+ nsCOMPtr<nsIImapMailFolderSink> outSink; >+ proxyManager->GetProxyForObject(NS_GetCurrentThread(), >+ NS_GET_IID(nsIImapMailFolderSink), >+ fccSink, >+ NS_PROXY_ASYNC, >+ getter_AddRefs(outSink)); >+ outSink->OnNewIdleMessages(); >+ } >+ break; >+ } >+ } >+ return NS_OK; >+} >+ >+NS_IMETHODIMP >+nsMsgSendFilter::ApplyFiltersOnLocalFolder(nsIMsgDBHdr *aMsg) >+{ >+ >+ XZERLOG("ApplyFiltersOnLocalFolder called."); >+ >+ nsCOMPtr<nsIMutableArray> array = do_CreateInstance(NS_ARRAY_CONTRACTID); >+ array->AppendElement(aMsg, PR_FALSE); >+ >+ nsIMsgFolder* fcc; >+ nsresult rv = aMsg->GetFolder(&fcc); >+ if (NS_FAILED(rv)) return rv; >+ >+ nsCOMPtr<nsIMsgFilterService> filterService(do_GetService(NS_MSGFILTERSERVICE_CONTRACTID)); >+ return filterService->ApplyFilters(nsMsgFilterType::PostOutgoing, array, fcc, nsnull /* nsIMsgWindow */); >+ >+#ifdef XZER_DEBUG >+ nsCString msgid; >+ aMsg->GetMessageId(getter_Copies(msgid)); >+ XZERLOG1("I called the ApplyFiltersOnLocalFolder on message %s", msgid); >+#endif >+} >+ >+ >+//nsIMsgFolderListener >+NS_IMETHODIMP >+nsMsgSendFilter::MsgAdded(nsIMsgDBHdr *aMsg) >+{ >+ nsMsgSendFilterMsgInfo msgInfo; >+ nsresult rv; >+ rv = aMsg->GetMessageId(getter_Copies(msgInfo.messageId)); >+ if (NS_FAILED(rv)) return rv; >+ >+ // The message id may be on a different format here >+ // So we strip the '<' and '>' >+ if(msgInfo.messageId.CharAt(0) == '<') >+ { >+ msgInfo.messageId.StripChars("<>"); >+ } >+ >+ nsIMsgFolder* fcc; >+ rv = aMsg->GetFolder(&fcc); >+ if (NS_FAILED(rv)) return rv; >+ >+ fcc->GetFolderURL(msgInfo.fcc); >+ >+ XZERLOG2("A message reloaded by folder notification service! id = %s, fcc = %s\n", msgInfo.messageId.get(), msgInfo.fcc.get()); >+ >+ PR_EnterMonitor(m_msgArrayMonitor); >+ >+ // On an imap folder, once the message is added, >+ // the filters must have been fired,but on a local folder, >+ // we fire the filters here >+ if(msgInfo.fcc.Find("imap://") == 0) >+ { >+ m_msgArray.RemoveElement(msgInfo); >+ } >+ else >+ { >+ nsCOMPtr<nsIRDFResource> rdfFcc = do_QueryInterface(fcc); >+ rdfFcc->GetValue(getter_Copies(msgInfo.fcc)); >+ XZERLOG1("get reload rdf uri %s", msgInfo.fcc.get()); >+ if(m_msgArray.RemoveElement(msgInfo)) >+ { >+ ApplyFiltersOnLocalFolder(aMsg); >+ } >+ if(m_folderNotificationRegistered >+ && m_msgArray.Length() == 0){ >+ m_folderNotificationService->RemoveListener(this); >+ m_folderNotificationRegistered = PR_FALSE; >+ } >+ } >+ >+ PR_ExitMonitor(m_msgArrayMonitor); >+ >+ >+ return NS_OK; >+} >+ >+NS_IMETHODIMP >+nsMsgSendFilter::MsgsClassified(nsIArray *aMsgs, PRBool aJunkProcessed, PRBool aTraitProcessed) >+{ >+ return NS_OK; >+} >+ >+NS_IMETHODIMP >+nsMsgSendFilter::MsgsDeleted(nsIArray *aMsgs) >+{ >+ return NS_OK; >+} >+ >+NS_IMETHODIMP >+nsMsgSendFilter::MsgsMoveCopyCompleted( >+ PRBool aMove, nsIArray *aSrcMsgs, nsIMsgFolder *aDestFolder, >+ nsIArray *aDestMsgs) >+{ >+ return NS_OK; >+} >+ >+NS_IMETHODIMP >+nsMsgSendFilter::FolderAdded(nsIMsgFolder *aFolder) >+{ >+ return NS_OK; >+} >+ >+NS_IMETHODIMP >+nsMsgSendFilter::FolderDeleted(nsIMsgFolder *aFolder) >+{ >+ return NS_OK; >+} >+ >+NS_IMETHODIMP >+nsMsgSendFilter::FolderMoveCopyCompleted(PRBool aMove, nsIMsgFolder *aSrcFolder, nsIMsgFolder *aDestFolder) >+{ >+ return NS_OK; >+} >+ >+NS_IMETHODIMP >+nsMsgSendFilter::FolderRenamed(nsIMsgFolder *aOrigFolder, nsIMsgFolder *aNewFolder) >+{ >+ return NS_OK; >+} >+ >+NS_IMETHODIMP >+nsMsgSendFilter::ItemEvent(nsISupports *aItem, const nsACString &aEvent, nsISupports *aData) >+{ >+ return NS_OK; >+} >diff -r 516bf521bc82 -r 493fdeceb191 mailnews/compose/src/nsMsgSendFilter.h >--- /dev/null Thu Jan 01 00:00:00 1970 +0000 >+++ b/mailnews/compose/src/nsMsgSendFilter.h Sat Jan 23 23:20:15 2010 +0900 >@@ -0,0 +1,88 @@ >+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ >+/* ***** BEGIN LICENSE BLOCK ***** >+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1 >+ * >+ * The contents of this file are subject to the Mozilla Public License Version >+ * 1.1 (the "License"); you may not use this file except in compliance with >+ * the License. You may obtain a copy of the License at >+ * http://www.mozilla.org/MPL/ >+ * >+ * Software distributed under the License is distributed on an "AS IS" basis, >+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License >+ * for the specific language governing rights and limitations under the >+ * License. >+ * >+ * The Original Code is mozilla.org code. >+ * >+ * The Initial Developer of the Original Code is >+ * Netscape Communications Corporation. >+ * Portions created by the Initial Developer are Copyright (C) 1998 >+ * the Initial Developer. All Rights Reserved. >+ * >+ * Contributor(s): >+ * xzer <xiaozhu@gmail.com> >+ * >+ * Alternatively, the contents of this file may be used under the terms of >+ * either of the GNU General Public License Version 2 or later (the "GPL"), >+ * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), >+ * in which case the provisions of the GPL or the LGPL are applicable instead >+ * of those above. If you wish to allow use of your version of this file only >+ * under the terms of either the GPL or the LGPL, and not to allow others to >+ * use your version of this file under the terms of the MPL, indicate your >+ * decision by deleting the provisions above and replace them with the notice >+ * and other provisions required by the GPL or the LGPL. If you do not delete >+ * the provisions above, a recipient may use your version of this file under >+ * the terms of any one of the MPL, the GPL or the LGPL. >+ * >+ * ***** END LICENSE BLOCK ***** */ >+ >+#ifndef _nsMsgSendFilter_H_ >+#define _nsMsgSendFilter_H_ >+ >+#include "nsCOMArray.h" >+#include "nsIMsgFolderListener.h" >+#include "nsIMsgHdr.h" >+#include "nsIMsgFolderNotificationService.h" >+#include "prmon.h" >+ >+ >+struct nsMsgSendFilterMsgInfo{ >+ nsCString messageId; >+ nsCString fcc; >+}; >+ >+//////////////////////////////////////////////////////////////////////////////////// >+// This is the class for the send filter. We have to create this class >+// to handle message send completion and call the filters here >+//////////////////////////////////////////////////////////////////////////////////// >+class nsMsgSendFilter : public nsIMsgFolderListener >+{ >+public: >+ nsMsgSendFilter(); >+ virtual ~nsMsgSendFilter(); >+ >+ NS_DECL_ISUPPORTS >+ NS_DECL_NSIMSGFOLDERLISTENER >+ >+ NS_IMETHODIMP AddSentMessage(const char *aMsgId, const char *fcc); >+ >+ NS_IMETHODIMP ApplyFilters(); >+ >+private: >+ >+ NS_IMETHODIMP ReloadImapFolder(const nsCString &fcc); >+ >+ NS_IMETHODIMP ApplyFiltersOnLocalFolder(nsIMsgDBHdr *aMsg); >+ >+ nsCOMPtr<nsIMsgFolderNotificationService> m_folderNotificationService; >+ PRBool m_folderNotificationRegistered; >+ nsTArray<nsMsgSendFilterMsgInfo> m_msgArray; >+ PRMonitor *m_msgArrayMonitor; >+ >+#ifdef XZER_DEBUG >+ int instIdx; >+#endif >+ >+}; >+ >+#endif /* _nsMsgSendFilter_H_ */ >diff -r 516bf521bc82 -r 493fdeceb191 mailnews/compose/src/nsMsgSendLater.cpp >--- a/mailnews/compose/src/nsMsgSendLater.cpp Fri Jan 22 12:59:47 2010 +0100 >+++ b/mailnews/compose/src/nsMsgSendLater.cpp Sat Jan 23 23:20:15 2010 +0900 >@@ -54,16 +54,17 @@ > #include "prlog.h" > #include "prmem.h" > #include "nsIMimeConverter.h" > #include "nsMsgMimeCID.h" > #include "nsComposeStrings.h" > #include "nsIMutableArray.h" > #include "nsArrayEnumerator.h" > #include "nsIObserverService.h" >+#include "nsMsgSendFilter.h" > > // Consts for checking and sending mail in milliseconds > > // 1 second from mail into the unsent messages folder to initially trying to > // send it. > const PRUint32 kInitialMessageSendTime = 1000; > > NS_IMPL_ISUPPORTS6(nsMsgSendLater, >@@ -108,16 +109,18 @@ nsMsgSendLater::~nsMsgSendLater() > PR_Free(m_fcc); > PR_Free(m_bcc); > PR_Free(m_newsgroups); > PR_Free(m_newshost); > PR_Free(m_headers); > PR_Free(mLeftoverBuffer); > PR_Free(mIdentityKey); > PR_Free(mAccountKey); >+ >+ mSendFilter = nsnull; > } > > nsresult > nsMsgSendLater::Init() > { > nsresult rv; > nsCOMPtr<nsIPrefBranch> prefs = do_GetService(NS_PREFSERVICE_CONTRACTID, &rv); > NS_ENSURE_SUCCESS(rv, rv); >@@ -409,23 +412,25 @@ nsMsgSendLater::OnStartRequest(nsIReques > > //////////////////////////////////////////////////////////////////////////////////// > // This is the listener class for the send operation. We have to create this class > // to listen for message send completion and eventually notify the caller > //////////////////////////////////////////////////////////////////////////////////// > NS_IMPL_ISUPPORTS2(SendOperationListener, nsIMsgSendListener, > nsIMsgCopyServiceListener) > >-SendOperationListener::SendOperationListener(nsMsgSendLater *aSendLater) >-: mSendLater(aSendLater) >+SendOperationListener::SendOperationListener(nsMsgSendLater *aSendLater, nsIMsgIdentity *aIdentity) >+: mSendLater(aSendLater), mIdentity(aIdentity) > { >+ NS_ADDREF(mIdentity); > } > > SendOperationListener::~SendOperationListener(void) > { >+ NS_RELEASE(mIdentity); > } > > NS_IMETHODIMP > SendOperationListener::OnGetDraftFolderURI(const char *aFolderURI) > { > return NS_OK; > } > >@@ -462,16 +467,23 @@ SendOperationListener::OnSendNotPerforme > { > return NS_OK; > } > > NS_IMETHODIMP > SendOperationListener::OnStopSending(const char *aMsgID, nsresult aStatus, const PRUnichar *aMsg, > nsIFile *returnFile) > { >+ >+ if(mSendLater->mSendFilter) >+ { >+ nsCString fcc; >+ mIdentity->GetFccFolder(fcc); >+ mSendLater->mSendFilter->AddSentMessage(aMsgID, fcc.get()); >+ } > if (mSendLater && !mSendLater->OnSendStepFinished(aStatus)) > NS_RELEASE(mSendLater); > > return NS_OK; > } > > // nsIMsgCopyServiceListener > >@@ -547,16 +559,19 @@ nsMsgSendLater::CompleteMailFileSend() > NS_ENSURE_SUCCESS(rv, rv); > > // Since we have already parsed all of the headers, we are simply going to > // set the composition fields and move on. > // > nsCString author; > mMessage->GetAuthor(getter_Copies(author)); > >+ nsCString msgId; >+ mMessage->GetMessageId(getter_Copies(msgId)); >+ > nsMsgCompFields * fields = (nsMsgCompFields *)compFields.get(); > > nsCString decodedString; > // decoded string is null if the input is not MIME encoded > mimeConverter->DecodeMimeHeaderToCharPtr(author.get(), nsnull, PR_FALSE, > PR_TRUE, > getter_Copies(decodedString)); > >@@ -581,24 +596,31 @@ nsMsgSendLater::CompleteMailFileSend() > mimeConverter->DecodeMimeHeaderToCharPtr(m_fcc, nsnull, PR_FALSE, PR_TRUE, > getter_Copies(decodedString)); > fields->SetFcc(decodedString.IsEmpty() ? m_fcc : decodedString.get()); > } > > if (m_newsgroups) > fields->SetNewsgroups(m_newsgroups); > >+ fields->SetMessageId(msgId.get()); >+ > #if 0 > // needs cleanup. SetNewspostUrl()? > if (m_newshost) > fields->SetNewshost(m_newshost); > #endif > >+ if(!mSendFilter) >+ { >+ mSendFilter = new nsMsgSendFilter(); >+ } >+ > // Create the listener for the send operation... >- SendOperationListener *sendListener = new SendOperationListener(this); >+ SendOperationListener *sendListener = new SendOperationListener(this, identity); > if (!sendListener) > return NS_ERROR_OUT_OF_MEMORY; > > NS_ADDREF(sendListener); > > NS_ADDREF(this); //TODO: We should remove this!!! > rv = pMsgSend->SendMessageFile(identity, > mAccountKey, >@@ -1331,16 +1353,21 @@ nsMsgSendLater::EndSendMessages(nsresult > > // or the enumerator, temp file or output stream > mEnumerator = nsnull; > mTempFile = nsnull; > mOutFile = nsnull; > > NOTIFY_LISTENERS(OnStopSending, (aStatus, aMsg, aTotalTried, aSuccessful)); > >+ if(mSendFilter) >+ { >+ mSendFilter->ApplyFilters(); >+ } >+ > // If we've got a shutdown listener, notify it that we've finished. > if (mShutdownListener) > { > mShutdownListener->OnStopRunningUrl(nsnull, NS_OK); > mShutdownListener = nsnull; > } > } > >diff -r 516bf521bc82 -r 493fdeceb191 mailnews/compose/src/nsMsgSendLater.h >--- a/mailnews/compose/src/nsMsgSendLater.h Fri Jan 22 12:59:47 2010 +0100 >+++ b/mailnews/compose/src/nsMsgSendLater.h Sat Jan 23 23:20:15 2010 +0900 >@@ -43,36 +43,39 @@ > #include "nsIMsgSendListener.h" > #include "nsIMsgSendLaterListener.h" > #include "nsIMsgSendLater.h" > #include "nsIMsgStatusFeedback.h" > #include "nsTObserverArray.h" > #include "nsIObserver.h" > #include "nsITimer.h" > #include "nsIMsgShutdown.h" >+#include "nsAutoPtr.h" > > //////////////////////////////////////////////////////////////////////////////////// > // This is the listener class for the send operation. We have to create this class > // to listen for message send completion and eventually notify the caller > //////////////////////////////////////////////////////////////////////////////////// > class nsMsgSendLater; >+class nsMsgSendFilter; > > class SendOperationListener : public nsIMsgSendListener, > public nsIMsgCopyServiceListener > { > public: >- SendOperationListener(nsMsgSendLater *aSendLater); >+ SendOperationListener(nsMsgSendLater *aSendLater, nsIMsgIdentity *aIdentity); > virtual ~SendOperationListener(); > > NS_DECL_ISUPPORTS > NS_DECL_NSIMSGSENDLISTENER > NS_DECL_NSIMSGCOPYSERVICELISTENER > > private: > nsMsgSendLater *mSendLater; >+ nsIMsgIdentity *mIdentity; > }; > > class nsMsgSendLater: public nsIMsgSendLater, > public nsIFolderListener, > public nsIObserver, > public nsIMsgShutdownTask > > { >@@ -121,16 +124,17 @@ public: > > // counters and things for enumeration > PRUint32 mTotalSentSuccessfully; > PRUint32 mTotalSendCount; > nsCOMArray<nsIMsgDBHdr> mMessagesToSend; > nsCOMPtr<nsISimpleEnumerator> mEnumerator; > nsCOMPtr<nsIMsgFolder> mMessageFolder; > nsCOMPtr<nsIMsgStatusFeedback> mFeedback; >+ nsRefPtr<nsMsgSendFilter> mSendFilter; > > // Private Information > private: > nsresult GetIdentityFromKey(const char *aKey, nsIMsgIdentity **aIdentity); > nsresult InternalSendMessages(PRBool aUserInitiated, > nsIMsgIdentity *aIdentity); > > nsTObserverArray<nsCOMPtr<nsIMsgSendLaterListener> > mListenerArray; >diff -r 516bf521bc82 -r 493fdeceb191 mailnews/imap/src/nsImapMailFolder.cpp >--- a/mailnews/imap/src/nsImapMailFolder.cpp Fri Jan 22 12:59:47 2010 +0100 >+++ b/mailnews/imap/src/nsImapMailFolder.cpp Sat Jan 23 23:20:15 2010 +0900 >@@ -684,23 +684,24 @@ NS_IMETHODIMP nsImapMailFolder::UpdateFo > // If this is the inbox, filters will be applied. Otherwise, we test the > // inherited folder property "applyIncomingFilters" (which defaults to empty). > // If this inherited property has the string value "true", we will apply > // filters even if this is not the inbox folder. > nsCString applyIncomingFilters; > GetInheritedStringProperty("applyIncomingFilters", applyIncomingFilters); > m_applyIncomingFilters = applyIncomingFilters.EqualsLiteral("true"); > >- if (mFlags & nsMsgFolderFlags::Inbox || m_applyIncomingFilters) >+ if (mFlags & (nsMsgFolderFlags::Inbox | nsMsgFolderFlags::SentMail) || m_applyIncomingFilters) > { > if (!m_filterList) > rv = GetFilterList(aMsgWindow, getter_AddRefs(m_filterList)); > // if there's no msg window, but someone is updating the inbox, we're > // doing something biff-like, and may download headers, so make biff notify. >- if (!aMsgWindow && mFlags & nsMsgFolderFlags::Inbox) >+ // (Since we will apply filter on sent folder too, so the same to sent box. By xzer.) >+ if (!aMsgWindow && mFlags & (nsMsgFolderFlags::Inbox | nsMsgFolderFlags::SentMail)) > SetPerformingBiff(PR_TRUE); > } > > if (m_filterList) > { > nsCOMPtr<nsIMsgIncomingServer> server; > rv = GetServer(getter_AddRefs(server)); > NS_ENSURE_SUCCESS(rv, rv); >@@ -731,17 +732,17 @@ NS_IMETHODIMP nsImapMailFolder::UpdateFo > ++index) > { > nsCOMPtr<nsIMsgFilter> filter; > m_filterList->GetFilterAt(index, getter_AddRefs(filter)); > if (!filter) > continue; > nsMsgFilterTypeType filterType; > filter->GetFilterType(&filterType); >- if (!(filterType & nsMsgFilterType::Incoming)) >+ if (!(filterType & (nsMsgFilterType::Incoming | nsMsgFilterType::PostOutgoing))) > continue; > PRBool enabled = PR_FALSE; > filter->GetEnabled(&enabled); > if (!enabled) > continue; > nsCOMPtr<nsISupportsArray> searchTerms; > PRUint32 numSearchTerms = 0; > filter->GetSearchTerms(getter_AddRefs(searchTerms)); >@@ -3054,27 +3055,29 @@ nsresult nsImapMailFolder::NormalEndHead > if (NS_SUCCEEDED(newMsgHdr->GetMessageSize(&messageSize))) > mFolderSize += messageSize; > m_msgMovedByFilter = PR_FALSE; > > // If this is the inbox, try to apply filters. Otherwise, test the inherited > // folder property "applyIncomingFilters" (which defaults to empty). If this > // inherited property has the string value "true", then apply filters even > // if this is not the Inbox folder. >- if (mFlags & nsMsgFolderFlags::Inbox || m_applyIncomingFilters) >+ // (Since we will apply filter on sent folder too, so the same to sent box. By xzer.) >+ if (mFlags & (nsMsgFolderFlags::Inbox | nsMsgFolderFlags::SentMail) || m_applyIncomingFilters) > { > PRUint32 msgFlags; > newMsgHdr->GetFlags(&msgFlags); >- if (!(msgFlags & (nsMsgMessageFlags::Read | nsMsgMessageFlags::IMAPDeleted))) // only fire on unread msgs that haven't been deleted >+ if ( (!(msgFlags & (nsMsgMessageFlags::Read | nsMsgMessageFlags::IMAPDeleted)) ) // only fire on unread msgs that haven't been deleted >+ || ((!(msgFlags & nsMsgMessageFlags::IMAPDeleted)) && mFlags & nsMsgFolderFlags::SentMail) ) // or on sent folder, we will always try to apply filters > { > PRInt32 duplicateAction = nsIMsgIncomingServer::keepDups; > if (server) > server->GetIncomingDuplicateAction(&duplicateAction); > if ((duplicateAction != nsIMsgIncomingServer::keepDups) && >- mFlags & nsMsgFolderFlags::Inbox) >+ mFlags & (nsMsgFolderFlags::Inbox | nsMsgFolderFlags::SentMail)) > { > PRBool isDup; > server->IsNewHdrDuplicate(newMsgHdr, &isDup); > if (isDup) > { > // we want to do something similar to applying filter hits. > // if a dup is marked read, it shouldn't trigger biff. > // Same for deleting it or moving it to trash. >@@ -3119,17 +3122,18 @@ nsresult nsImapMailFolder::NormalEndHead > rv = m_msgParser->GetAllHeaders(&headers, &headersSize); > > if (NS_SUCCEEDED(rv) && headers && !m_msgMovedByFilter && > !m_filterListRequiresBody) > { > if (m_filterList) > { > GetMoveCoalescer(); // not sure why we're doing this here. >- m_filterList->ApplyFiltersToHdr(nsMsgFilterType::InboxRule, newMsgHdr, this, mDatabase, >+ nsMsgFilterTypeType applyType = mFlags & nsMsgFolderFlags::SentMail ? nsMsgFilterType::PostOutgoing : nsMsgFilterType::InboxRule; >+ m_filterList->ApplyFiltersToHdr(applyType, newMsgHdr, this, mDatabase, > headers, headersSize, this, msgWindow, nsnull); > NotifyFolderEvent(mFiltersAppliedAtom); > } > } > } > } > // here we need to tweak flags from uid state.. > if (mDatabase && (!m_msgMovedByFilter || ShowDeletedMessages())) >@@ -4495,17 +4499,18 @@ nsImapMailFolder::NormalEndMsgWriteStrea > if (imapUrl) > { > nsresult rv; > nsCOMPtr<nsIMsgMailNewsUrl> msgUrl; > msgUrl = do_QueryInterface(imapUrl, &rv); > if (msgUrl && NS_SUCCEEDED(rv)) > msgUrl->GetMsgWindow(getter_AddRefs(msgWindow)); > } >- m_filterList->ApplyFiltersToHdr(nsMsgFilterType::InboxRule, newMsgHdr, >+ nsMsgFilterTypeType applyType = mFlags & nsMsgFolderFlags::SentMail ? nsMsgFilterType::PostOutgoing : nsMsgFilterType::InboxRule; >+ m_filterList->ApplyFiltersToHdr(applyType, newMsgHdr, > this, mDatabase, nsnull, nsnull, this, > msgWindow, nsnull); > NotifyFolderEvent(mFiltersAppliedAtom); > } > // Process filter plugins and other items normally done at the end of > // HeaderFetchCompleted. > PRBool pendingMoves = m_moveCoalescer && m_moveCoalescer->HasPendingMoves(); > PlaybackCoalescedOperations(); >@@ -8714,17 +8719,18 @@ nsImapMailFolder::OnMessageClassified(co > > NS_IMETHODIMP > nsImapMailFolder::GetShouldDownloadAllHeaders(PRBool *aResult) > { > NS_ENSURE_ARG_POINTER(aResult); > *aResult = PR_FALSE; > //for just the inbox, we check if the filter list has arbitary headers. > //for all folders, check if we have a spam plugin that requires all headers >- if (mFlags & nsMsgFolderFlags::Inbox) >+ //(Since we will apply filter on sent folder too, so the same to sent box. By xzer.) >+ if (mFlags & (nsMsgFolderFlags::Inbox | mFlags & nsMsgFolderFlags::SentMail)) > { > nsCOMPtr <nsIMsgFilterList> filterList; > nsresult rv = GetFilterList(nsnull, getter_AddRefs(filterList)); > NS_ENSURE_SUCCESS(rv,rv); > > rv = filterList->GetShouldDownloadAllHeaders(aResult); > if (*aResult) > return rv;
I am sorry, I made a mistake on the patch, when I try to modify it, I made a more mistake.... to Alexander: I only build a debug version on my computer, I need some time to learn how to build a release version or how can I supply a package for just test.
Attachment #423137 - Attachment is obsolete: true
Attachment #423144 - Flags: review?(dmose)
Attachment #423144 - Flags: review?(bienvenu)
Attachment #423137 - Flags: review?(dmose)
Attachment #423137 - Flags: review?(bienvenu)
Comment on attachment 423144 [details] [diff] [review] a fixed patch for outgoing filter I will be happy to look this over.
Attachment #423144 - Flags: review?(kent)
Thanks so much for working on this, xver! Because this is a relatively large patch, I am not able to give a full review in one pass. What I have done so far is to simply read over the code, and make some initial comments on things that I saw. I have not attempted to compile or test this. But here are those comments: > > <!ENTITY contextDesc.label "Apply filter when:"> >-<!ENTITY contextDesc.accesskey "w"> > <!ENTITY contextIncoming.label "Checking Mail"> > <!ENTITY contextManual.label "Manually Run"> >-<!ENTITY contextBoth.label "Checking Mail or Manually Run"> >-<!ENTITY contextPostPlugin.label "Checking Mail (after classification)"> >-<!ENTITY contextPostPluginBoth.label "Checking Mail (after classification) or Manually Run"> >+<!ENTITY contextBeforeCls.label "Before Classification"> >+<!ENTITY contextAfterCls.label "After Classification"> >+<!ENTITY contextOutgoing.label "Outgoing"> >+<!ENTITY contextBeforeSending.label "Before Sending"> >+<!ENTITY contextAfterSending.label "After Sending"> >+ The existing filter context menu is already unwieldy, and this will really make it completely incomprehensible to the average user. That issue did not start with this bug, but you (and I with my "after classification" context) have definitely contributed. Also, there is no easy way for the user to define the same filter as both incoming and outgoing as I understand it, while that would be a common request. I really think that we are going to have to modify this dropdown to work in a different manner. What is the difference between "Outgoing" and "Before/After Sending"? I have not read the rest of the code yet, but you should not expect that of your users either. ... OK I've read down in the code, and I see that Outgoing = both "Before" and "After" sending. You have the same issue here that I had with Incoming. So really we have three contexts, "Manual", "Incoming", and "Outgoing", with the latter two each having a variant. We probably need to reflect this structure in the context selection UI. >+ gFilterTypeDelegator.rdAfCl.style.display = "none"; I am guessing that "rdAfCl" is "read after close", but in general severely abbreviated identifiers like this are not a good idea. There are many other severly abbreviated identifier names in this code, I will not comment on each of them but the same issue applies everywhere. >diff -r 516bf521bc82 -r 493fdeceb191 mailnews/compose/src/nsMsgSendFilter.cpp >--- /dev/null Thu Jan 01 00:00:00 1970 +0000 >+++ b/mailnews/compose/src/nsMsgSendFilter.cpp Sat Jan 23 23:20:15 2010 +0900 ... >+ * The Initial Developer of the Original Code is >+ * Netscape Communications Corporation. >+ * Portions created by the Initial Developer are Copyright (C) 1998 You are using an old copy of the license header. Please locate the correct new boilerplate and use it. Set the Copyright as 2010, and give an "Initial Developer". This would typically be your company if this is sponsored by a company, or you could use your own name. Or you may use Mozilla or Mozilla Messaging here, but personally I would prefer to have the person or company who actually sponsored the work, which is you. This may not be a majority opinion by the way. >+#ifdef XZER_DEBUG I'm not sure of the current policy here, but I typically have not been seeing personal debug code like this in code for release. It is OK to leave it through the review process, but you will probably be asked to remove it before landing. >+ XZERLOG("OK, I am created!"); ditto here and elsewhere. >+ rv = fccFolder->GetFilterList(nsnull, getter_AddRefs(filterList)); >+ if (NS_FAILED(rv)) return rv; current practice is to use a call to NS_ENSURE_SUCCESS(rv, rv) instead, unless there is an expected error whose rv will be processed by the caller. >+class nsMsgSendFilter : public nsIMsgFolderListener >+{ >+public: >+ ... >+ NS_IMETHODIMP AddSentMessage(const char *aMsgId, const char *fcc); >+ ... >+private: >+ >+ NS_IMETHODIMP ReloadImapFolder(const nsCString &fcc); You seem to be using the NS_IMETHODIMP macro as a general term for defining a method. That is not its purpose, it is used as the definer for an XPCOM component implementation. I think all that you want here is: nsresult AddSentMessage(const char *aMsgId, const char *fcc); These declarations should also be changed on the class implementations. >+: mSendLater(aSendLater), mIdentity(aIdentity) > { >+ NS_ADDREF(mIdentity); > } > > SendOperationListener::~SendOperationListener(void) > { >+ NS_RELEASE(mIdentity); > } > You should define mIdentity as an nsCOMPtr and avoid the manual reference counting. > NS_IMETHODIMP > SendOperationListener::OnStopSending(const char *aMsgID, nsresult aStatus, const PRUnichar *aMsg, > nsIFile *returnFile) > { >+ >+ if(mSendLater->mSendFilter) >+ { >+ nsCString fcc; >+ mIdentity->GetFccFolder(fcc); >+ mSendLater->mSendFilter->AddSentMessage(aMsgID, fcc.get()); >+ } > if (mSendLater && !mSendLater->OnSendStepFinished(aStatus)) > NS_RELEASE(mSendLater); > I don't understand why you need this release. The definition of mSendLater was set as raw interface rather than a smart pointer in antiquity. If it was me, and I had to mess with its reference counting, I would just convert it into a smart pointer instead. Bienvenu may not agree with me though. ===== Dmose's current policy is that patches need automated testing, so this will certainly need some tests written before landing. This is not a trivial undertaking. The closest existing test to this is test_imapFilterActions.js, but unfortunately that test has an intermittent failure. You are encouraged to try to write a test for this. In general, what this starts to lead to is a development style where you use automated tests as the first step in your development. I am doing that now on my current project. If this is an insurmountable burden, I might be able to help with the XPCSHELL tests. I look forward to seeing this feature added in the mailnews code! rkent
Status: NEW → ASSIGNED
Component: Backend → Filters
QA Contact: backend → filters
Attached image the new UI image
to Kent >The existing filter context menu is already unwieldy ... >... I upload a image of how I modified the UI. Maybe the current design is not good enough, but at least I think it was acceptable. >I am guessing that "rdAfCl" is "read after close", but in general severely >... "rdAfCl" means "radiobox of after classification", maybe I should add a comment of these identifier abbreviations? I just want a shorter identifier, but if we think it's not good, I will modify it. > ... the other suggestions of source... Thank you for your suggestions, I am the first time of contributing source to thunderbird, you tell me a lot of my mistakes, anyway, thanks. >Dmose's current policy is that patches need automated testing, so this will >certainly need some tests written before landing. This is not a trivial >undertaking. The closest existing test to this is test_imapFilterActions.js, >but unfortunately that test has an intermittent failure. You are encouraged to >try to write a test for this. In general, what this starts to lead to is a >development style where you use automated tests as the first step in your >development. I am doing that now on my current project. >If this is an insurmountable burden, I might be able to help with the XPCSHELL >tests. I passed the test_imapFilterActions.js by my local build which is the nightly build of 20100123. I think it is a good starter of my own test. The problem is that where I should place my test? Perhaps a better place is under the compose, because we must ensure that it worked for both of imap folder and local folder and I think it as a function of sending message. I will try to write the test in the next week, if I get problems, Kent, please help me :-)
(In reply to comment #71) > Created an attachment (id=423202) [details] > the new UI image Dear xzer Is it please possible to give a self explanatory label rather than 'Before Classification"? I am reasonable ITerate and use something like 30 filters for each of 5 accounts but I can only guess that "Before Classification" might mean "Before giving a Spam score" or "Before marking as spam/not spam" Does the use of "Checking" rather than "Input" have any bearing on this and, if so, if that label was expanded would the options be made clearer?
cc'ing Bryan since he needs to bless UI decisions. In the dialog of attachment 423202 [details], it should be clear that "Checking mail" has the two options "Before classification" and "After classification", but there should not be check marks allowed on all three, unless somehow you intend to disable the "Post" and "Pre" choices when "Checking mail" is not set. Also, "Outgoing" needs to have the same parallel arrangment, with "Before Sending" and "After Sending" as the two choices.
(In reply to comment #74) > cc'ing Bryan since he needs to bless UI decisions. > > In the dialog of attachment 423202 [details], it should be clear that "Checking mail" has > the two options "Before classification" and "After classification", but there > should not be check marks allowed on all three, unless somehow you intend to > disable the "Post" and "Pre" choices when "Checking mail" is not set. Also, > "Outgoing" needs to have the same parallel arrangment, with "Before Sending" > and "After Sending" as the two choices. The default is "manually" and "checking mail (before classification)", "outgoing" is not checked, so it is the same to the current design. There is "Before Sending" option, but I set it unvisible, because I don't implement it yet. >+ <radio id="rdBfSending" label="&contextBeforeSending.label;" style="display:none"/> <!-- not implemented -->
(In reply to comment #73) > (In reply to comment #71) > > Created an attachment (id=423202) [details] [details] > > the new UI image > Dear xzer > > Is it please possible to give a self explanatory label rather than 'Before > Classification"? > > I am reasonable ITerate and use something like 30 filters for each of 5 > accounts but I can only guess that "Before Classification" might mean "Before > giving a Spam score" or "Before marking as spam/not spam" > > Does the use of "Checking" rather than "Input" have any bearing on this and, if > so, if that label was expanded would the options be made clearer? The words "Before Classification" is the current terms of Thunderbird 3.0, I just use it as the same.
(In reply to comment #76) > > The words "Before Classification" is the current terms of Thunderbird 3.0, I > just use it as the same. Dear xzer This Mozine thread is the only place I can find an explanation of the new terms in that dialogue. What's new with filters in Thunderbird/Shredder? • mozillaZine Forums http://forums.mozillazine.org/viewtopic.php?f=29&t=1442365 Possibly these are 'current terms' for developers but the UI is (naturally) at user level. Perhaps I have missed some essential 'Help' facility which would explain it to the end user but unless that is so I think users are left guessing how to correctly understand those terms and therefore the associated choice of settings. Surely we should not need, after hunting down the UI meaning, to find it only in a mozine bug topic? Is there please something you can do about that?
(In reply to comment #77) > Is there please something you can do about that? Yes, I can change the word in my patch, but I have no right to release it :) When I began this work, I asked this question to Kent, he said that: >One complication here that affected the wording is that the bayes filter can >now be used for other types of classifications, though junk is the only item >implemented in core. My extension TaQuilla was a first attempt at that - >though that extension is pretty much obsolete now and needs lots of updating. >So I did not want to use a word that included "junk" or "spam", though for >most users that would probably be clearer. So the wording difficulties are >real, not just a language issue. There was some discussion in the implementing >bug about the wording as I recall, and that is the best we could come up with. So, I agree with Kent, since we want to make a hint that there is not only junk judgment in bayes, before we find a better word, we had better not to change the current word.
(In reply to comment #78) > >One complication here that affected the wording is that the bayes filter can > >now be used for other types of classifications, though junk is the only item > >implemented in core. My extension TaQuilla was a first attempt at that - > >though that extension is pretty much obsolete now and needs lots of updating. >So I did not want to use a word that included "junk" or "spam", though for > >most users that would probably be clearer. So the wording difficulties are > >real, not just a language issue. There was some discussion in the implementing > >bug about the wording as I recall, and that is the best we could come up with. > > > So, I agree with Kent, since we want to make a hint that there is not only junk > judgment in bayes, before we find a better word, we had better not to change > the current word. Is the bayes filter now able to sort messages in other folders than "Junk"? And is bayes able to sort in different (more than one) folders? I haven't seen any UI mockups for this (classification) feature?
(In reply to comment #79) > > Is the bayes filter now able to sort messages in other folders than "Junk"? And > is bayes able to sort in different (more than one) folders? I haven't seen any > UI mockups for this (classification) feature? We're getting a little off topic here, which I hate to do in a bug with a large cc: list like this one. But briefly, bayes filters can now maintain multiple parallel categorizations. Check out "TaQuilla" at my blog http://mesquilla.com for a sample (and probably the only) implementation of additional processing categorizations.
(In reply to comment #78) > ..So, I agree with Kent, since we want to make a hint that there is not only junk > judgment in bayes, before we find a better word, we had better not to change > the current word. OK. I suppose things will look a lot clearer when Nidelven and Moz kb are updated to include the new 3.0 filter create/edit features. Thanks very much for explanation.
Comment on attachment 423144 [details] [diff] [review] a fixed patch for outgoing filter I'm going to defer the review to rkent. I suspect this also needs a UI review at some point.
Attachment #423144 - Flags: review?(bienvenu) → ui-review?(clarkbw)
(In reply to comment #74) > cc'ing Bryan since he needs to bless UI decisions. > > In the dialog of attachment 423202 [details], it should be clear that "Checking mail" has > the two options "Before classification" and "After classification", but there > should not be check marks allowed on all three, unless somehow you intend to > disable the "Post" and "Pre" choices when "Checking mail" is not set. Also, > "Outgoing" needs to have the same parallel arrangment, with "Before Sending" > and "After Sending" as the two choices. I think we (all parties involved) need to find some time to work something out here because I have to say I'm confused as to what is going on in the screenshot. Likely the discussion will go a bit out of scope of this bug and into the general filtering UI but I can't see making small changes to the current UI without some kind of overhaul. IRC plus maybe a call would be great and I'm free most days after Tuesday.
Status: ASSIGNED → NEW
Comment on attachment 423144 [details] [diff] [review] a fixed patch for outgoing filter see comment 83, we need some discussion about how we can change this. I'm available any day of the week and we can freely use the Mozilla conference call system or Skype.
Attachment #423144 - Flags: ui-review?(clarkbw) → ui-review-
xzer, if you have time for this discussion that would be great. I would be happy to participate, and Byran and I are both in US west coast time zone. I probably have the most flexible schedule of all.
Comment on attachment 423144 [details] [diff] [review] a fixed patch for outgoing filter Given that this has ui-review minus, I'm cancelling the standard review requests here as it sounds like this patch will need rework before it can be reviewed. If you're still willing to move forward with this patch, please contact Bryan direct and I'm sure he'll be happy to work through it with you.
Attachment #423144 - Flags: review?(kent)
Attachment #423144 - Flags: review?(dmose)
Note: it may be better to just land the back-end changes to enable being able to create filters for outgoing messages, and then experiment with the UI from an add-on.
Flags: wanted-thunderbird3?
It would be nice to add this functionality, long overdue for power users. This will solidify even more your place amongst serious mail agents. Cheers Marc
The is a "Send filter" extension for this (https://addons.mozilla.org/thunderbird/addon/send-filter/). But the author mentions in is only a stop-gap solution until this feature is implemented as standard in Thunderbird. Can the patch be updated according to comment 87?
pretty sure this is mentioned in >1 getsatisfaction topic
Keywords: helpwanted
Summary: Filter outgoing messages (perhaps to use a different Sent/FCC folder) → Filter outgoing/Sent messages (perhaps to use a different Sent/FCC folder)
Whiteboard: [gs]
bwinton, could you take a look at this bug, whether you can do something here. Maybe some of the patch is usable, I haven't looked at it myself. This is one of the practical annoyances I have (for new messages only, because replies go to same folder as original msg). Ideally, filters for incoming messages would equally - or reverse - apply to outgoing msgs, without me having to set up new filter rules.
any news about this feature?
Depends on: 775665
Blocks: 36482
If one of the allows "actions" for outgoing mails is "Set From: to foo@bar.com", we'd solve bug 36482 and the identity selection in a very generic way.
Attached patch reduced WIP patch from xzer (obsolete) — Splinter Review
The UI change into checkboxes will be done separately in bug 775665, so this reduces the patch to only adding the Outgoing filters on top of the patch there.
Attachment #423144 - Attachment is obsolete: true
What is happening with this bug? I see patches submitted and made obsolete, at one point there was a back end patch that was cancelled to allow for a UI review. Now the bug is simply unassigned. I have read, but I simply fail to understand
Some patches were made obsolete, but new patch is attached. The prerequisite bug 775665 has been done. Now rkent plans to finish this. The active patch seems to contain most of the needed work, it just needs a review.
Flags: needinfo?(kent)
Right aceman, the remaining work to move this forward is not excessive. I've also spent a couple of months hacking in the Send and Compose code recently, so I'm quite familiar with that. So let me go ahead and assign myself to this. With so little left to be done, it would be criminal to not get this done, so feel free to keep me legal by bugging me about this if it looks like we are pushing the TB 24 deadlines and this is still not done.
Assignee: nobody → kent
Status: NEW → ASSIGNED
Flags: needinfo?(kent)
(In reply to Kent James (:rkent) from comment #97) > So let me go ahead and assign myself to this. With so little left to be > done, it would be criminal to not get this done, so feel free to keep me > legal by bugging me about this if it looks like we are pushing the TB 24 > deadlines and this is still not done. Is it too late now for the TB24 branching?
I instead put my effort into two other filter improvements. One, bug 864187 "Run filters periodically" would have allowed an alternate way to achieve filtering of sent messages, by running a periodic filter on the Sent Items folder. (The other was filter on archive, bug 479823) Unfortunately after two months of trying I could not get either bug through review. One failed because I could not locate a suite reviewer who could look at it, and the other because the UI reviewer wanted a more complex UI than there was time to create. I can't be too critical though, as I have also been less available as a reviewer than I would like to be. The periodic filter may get added to the addon FiltaQuilla, archive is more difficult for an extension as it would need to be a binary extension.
Attached patch Possible patch (obsolete) — Splinter Review
This worked locally for me but I think I'm processing the filter at the wrong point on the process.
Comment on attachment 8489397 [details] [diff] [review] Possible patch The attachment 659679 [details] [diff] [review] already had some code in the Compose and Send c++ parts, they just need to be finished and reviewed. Also you seem to use the nsMsgFilterType::Inbox type instead of nsMsgFilterType::*Outgoing types that are added in that UI patch.
Attached patch Possible patch (obsolete) — Splinter Review
Attachment #8489464 - Flags: feedback?(kent)
Comment on attachment 8489464 [details] [diff] [review] Possible patch Review of attachment 8489464 [details] [diff] [review]: ----------------------------------------------------------------- I am assuming you are asking for feedback on the proof of concept. As aceman said, you need to add a filter type for outgoing along with the related UI and strings, rather than use the Inbox type. As you suggested in "I think I'm processing the filter at the wrong point on the process" there are some issues with this approach, namely that the full send and fcc gets done before you process the filter. That means that, for example, a filter that tried to prevent making copies of a message that was too large, or tried to strip attachments prior to saving to a server, would not work as expected, as all of the network traffic would already have taken place. Also, the filter body will not be available when the filter is processed, so any filter searches that worked on the body would not be available. Those are considerable disadvantages. Also, there are many use cases that hoped to operate on the message before it was sent (like adding a reply-to header for example, or using encryption). None of those would work in this case, since the filter is applied after it is sent. Another disadvantage of these filters is that there is no notification of successful completion. There are notifications of some kinds of failures in the filter themselves, but ideally we would have filter messages that are displayed as part of the composer alerts. It seems to me then that a better approach would be to detect if there are any outgoing message filters for a server, and if so force the message to go into a local outbox. Then apply the filter to the message in the outbox, and send it using SendLater functionality. I don't think that would be much more difficult than the approach here.
Attachment #8489464 - Flags: feedback?(kent) → feedback-
(In reply to aceman from comment #101) > The attachment 659679 [details] [diff] [review] already had some code in the > Compose and Send c++ parts, they just need to be finished and reviewed. Also > you seem to use the nsMsgFilterType::Inbox type instead of > nsMsgFilterType::*Outgoing types that are added in that UI patch. Sorry, I only just found this bug, and the patch looked incomplete (looking at the obsolete patches, you seem to have missed some pieces) and I'd started from scratch so I hadn't looked into the adjusting the UI to including outgoing options yet, I wanted to get feedback on my approach first. (In reply to Kent James from comment #103) > As you suggested in "I think I'm processing the filter at the wrong point on > the process" there are some issues with this approach, namely that the full Actually the wrong point was because I was trying to run the filter even when sent mail was disabled. > send and fcc gets done before you process the filter. That means that, for > example, a filter that tried to prevent making copies of a message that was > too large, or tried to strip attachments prior to saving to a server, would > not work as expected, as all of the network traffic would already have taken > place. Also, the filter body will not be available when the filter is > processed, so any filter searches that worked on the body would not be > available. Those are considerable disadvantages. Also, there are many use The compose process actually generates a .eml file so in theory it might be possible to run the filter directly on the .eml file, but I wanted to get a proof of concept working first. > cases that hoped to operate on the message before it was sent (like adding a > reply-to header for example, or using encryption). None of those would work > in this case, since the filter is applied after it is sent. As I mentioned above, I only just discovered the bug, I did not actually read it (except the summary) and I don't even know how you would categorise those as filters anyway. > Another disadvantage of these filters is that there is no notification of > successful completion. There are notifications of some kinds of failures in > the filter themselves, but ideally we would have filter messages that are > displayed as part of the composer alerts. Presumably I just have to provide a status feedback implementation and wait for StopMeteors to be called before proceeding with the next part of the operation? > It seems to me then that a better approach would be to detect if there are > any outgoing message filters for a server, and if so force the message to go > into a local outbox. Then apply the filter to the message in the outbox, and > send it using SendLater functionality. I don't think that would be much more > difficult than the approach here. (See the comment above about filtering the temporary .eml file directly.)
The UI should be ready in patch 659679, we just need the backend code to hook onto it. If you guys want I can remove the c++ code from the patch so that you can make it a separate patch. Also if you do not intend to support both preOutgoing and postOutgoing events, I can remove that too.
"The compose process actually generates a .eml file so in theory it might be possible to run the filter directly on the .eml file, but I wanted to get a proof of concept working first." Yes, but existing filter implementations all work on messages that exist in folders. You would have to devise all new filter code if you wanted to try to filter messages that only existed as a file. I don't think that is such a good approach. The infrastructure already exists to store the message in an outgoing folder and send it, so why not just use that? A message in a folder is just a file with the message, that has been parsed so that you have an nsIMsgDBHdr available (which you need for the filter anyway). "Presumably I just have to provide a status feedback implementation and wait for StopMeteors to be called before proceeding with the next part of the operation?" I'm not sure. The error reporting in filters is pretty bad in any case. Ideally you would be more aggressive about error reporting in Send filters than in automatic filters, as presumably Sends are being done one at a time with the user at the console, while Incoming filters can process large numbers of messages automatically (so you don't want error dialogs popping up or hanging the automatic processing). But we don't need to hold this bug hostage to unrelated limitations in the filter error processing. "Also if you do not intend to support both preOutgoing and postOutgoing events, I can remove that too." What is the use case for postOutgoing filters, if preOutgoing is available?
I actually don't know what preOutgoing would be. Does it filter the message even before it goes out? We'd not want the message to be moved (if even possible). We probably could restrict the actions possible on preOutgoing. But the main usecase is probably filtering only after message is sent out successfully.
preOutgoing would be e.g. to stop messages going to wrong people if some condition is met (cancel send) I guess
So that would probably need some new actions defined. We should get the basic functionality working here and then build on top of that in new bugs.
The most basic functionality for the filter is probably to store sent items in different folders (or apply tags) depending on search terms. That would work for either preOutgoing or postOutgoing. But other use cases people have defined will modify the message before it is sent. Those would be preOutgoing filters.
I am glad to see this bug still keeps breath. IMO, there are exactly two separated tasks: preOutgoing and postOutgoing. Since there has been a mostly workable prototype of postOutgoing, why we do not finish it at first? We will never move forward without actions although maybe I should say sorry for giving up this bug after applying the initial patch. Writing test cases is maybe a simple task for core developers but exactly difficult for free contributors, which is one of the reasons why I gave up 4 years ago(It took more time than understanding how the logic sources works and lacked of description). Thus, I suggest that someone who is similar with the test mechanism can write out all the necessary test cases at first, which can also be considered as a step for clarifying the actual specification of outgoing filter functionality.
Dear contributors, this bug has been open for 15 years. As much as I would like to have preOutgoing (content manipulation) as well as postOutgoing (sorting, tagging), I would much prefer to see a simple working solution in the next release to not having a perfect solution. Remember that agile principle? "What is the simplest thing that could possibly work and create customer value?" Thank you so much for engaging here and providing patches, I am looking forward to the released result. :-)
(In reply to Kent James from comment #106) > But we don't need to hold this bug > hostage to unrelated limitations in the filter error processing. Does that include all the ones you complained about in comment #103? I'm failing to work out what's left to do.
(In reply to neil@parkwaycc.co.uk from comment #113) > (In reply to Kent James from comment #106) > > But we don't need to hold this bug > > hostage to unrelated limitations in the filter error processing. > Does that include all the ones you complained about in comment #103? I'm > failing to work out what's left to do. I complained some of filter error processing in comment 103, and yes I do not think that you need to address issues of filter error processing to make progress on this bug, as there is no really viable filter error processing in existing filters. I'm confused by your "what's left to do". Here's what I see at the minimum. First, the user interface to select outgoing as a filter type in the editor needs to be implemented, including adding a new type under nsMsgFilterType. Second, we need to agree on a point to apply the filter. We've talked about preOutgoing and postOutgoing, and I suggested that preOutgoing might be more useful, and could be done by using an Outgoing folder. Your original proposal was a postOutgoing type, applied after the initial copy was done. Yes that works, but it has some significant disadvantages, mostly that the most common filter (storing in a different folder) results in an extra move. Could I accept the scheme in comment 102? I suppose yes since the alternative of trying to filter from a file is fairly difficult.
(In reply to Kent James (:rkent) from comment #114) > I'm confused by your "what's left to do". Here's what I see at the minimum. > > First, the user interface to select outgoing as a filter type in the editor > needs to be implemented, including adding a new type under nsMsgFilterType. It seems I must repeat the UI is already done in attachment 659679 [details] [diff] [review]...
Attached patch Possible patch (obsolete) — Splinter Review
Attachment #8489397 - Attachment is obsolete: true
Attachment #8489464 - Attachment is obsolete: true
Attachment #8526967 - Flags: feedback?(kent)
Assignee: kent → nobody
(In reply to :aceman from comment #115) > (In reply to Kent James (:rkent) from comment #114) > > I'm confused by your "what's left to do". Here's what I see at the minimum. > > > > First, the user interface to select outgoing as a filter type in the editor > > needs to be implemented, including adding a new type under nsMsgFilterType. > It seems I must repeat the UI is already done in attachment 659679 [details] [diff] [review] > [diff] [review]... I don't understand why you are pointing to this old patch, when you already added the key UI pieces in bug 775665. Neil just needed to add that to his patch, which he has now done.
Comment on attachment 8526967 [details] [diff] [review] Possible patch Review of attachment 8526967 [details] [diff] [review]: ----------------------------------------------------------------- This is giving feedback only, I have not looked at this carefully enough to give a review. Overall I think this is going to work out, so we could have a postSend filter working. My main concerns are with error handling. Your FilterSentMessage() silently fails on error. I think that what we need to do is to add a new process type in nsIMsgSendReport such as process_PostSendFilter, and add an error message that occurs if there is a filter failure. That message would need to point out that the message has been sent, but the filter of the sent message failed. I would need to look again at the filter service to see if there is any way that we could get feedback from the most typical error (which would be failure to move a message), but IIRC there is no way currently to get that information. Ideally that would be added, but probably not in this bug.
Attachment #8526967 - Flags: feedback?(kent) → feedback+
Status: ASSIGNED → NEW
(In reply to Kent James from comment #119) > Overall I think this is going to work out, so we could have a postSend > filter working. My main concerns are with error handling. Your > FilterSentMessage() silently fails on error. I think that what we need to do > is to add a new process type in nsIMsgSendReport such as > process_PostSendFilter, and add an error message that occurs if there is a > filter failure. That message would need to point out that the message has > been sent, but the filter of the sent message failed. The filter process is asynchronous so a completion callback would need to be added.
(In reply to Kent James (:rkent) from comment #118) > (In reply to :aceman from comment #115) > > (In reply to Kent James (:rkent) from comment #114) > > > I'm confused by your "what's left to do". Here's what I see at the minimum. > > > > > > First, the user interface to select outgoing as a filter type in the editor > > > needs to be implemented, including adding a new type under nsMsgFilterType. > > It seems I must repeat the UI is already done in attachment 659679 [details] [diff] [review] > > [diff] [review]... > > I don't understand why you are pointing to this old patch, when you already > added the key UI pieces in bug 775665. Neil just needed to add that to his > patch, which he has now done. Yes, he added it after this comment of mine. Which one I wrote because you requested the UI. So my comment is correctly placed in the timeline of the comments. And yes, everything is fine now ;)
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #659679 - Attachment is obsolete: true
Attached patch Proposed patch (obsolete) — Splinter Review
With error-checking, using a callback from the filter service.
Attachment #8526967 - Attachment is obsolete: true
Attachment #8529924 - Flags: review?(kent)
In reviewing the patch, I see that you tried to solve some of the same issues that were previously solved in previous patches of mine in bug 695671 and bug 479823. Those patches were largely complete, but then died for lack of a final review. Rather than try to work on issues in this current patch that were already solved in the previous patches, I really think that we should just finish the previous patches, and then update the current patch to reflect those changes. So I am reopening those previous bugs, updating the patches for review, and marking this bug dependent on them. In testing your patch, one issue that I had was that the filter silently fails, disabling the filter, in the common error case of where the target folder for a move no longer exists. We also need to discuss the async management of errors from the filterService, but that issue was also addressed in bug 479823, so let's resolve it first there. Removing the review request until we get the dependencies resolved.
Depends on: 695671, 479823
Attachment #8529924 - Flags: review?(kent)
Comment on attachment 8529924 [details] [diff] [review] Proposed patch Review of attachment 8529924 [details] [diff] [review]: ----------------------------------------------------------------- I'll make some very brief comments here, but mostly you need to revise the patch to work with the latest patch from bug 695671. Although bug 479823 was also delayed by bug 695671, let's just go ahead and do this bug first, then I'll revise bug 479823 to match the changes here. ::: mailnews/base/search/public/nsIMsgFilterService.idl @@ +18,5 @@ > +[scriptable, uuid(bdaef6ff-0909-435b-8fcd-76525dd2364c)] > +interface nsIMsgFilterComplete : nsISupports { > + /* Called when filtering after the fact has finished */ > + void onFilterComplete(in nsresult status); > +}; The patch for bug 479823 included an interface with the exact same purpose as this on, but rather than give it a name including Filter (and burying it in a filter idl, I gave it a generic name nsIMsgOperationListener and its own idl file. There is nothing specific to the use of Filters here for this interface. There have been other cases where we have needed a simple operation listener, and have sometimes used nsIUrlListener for this (ignoring the nsIURL parameter). I think we need a simple, generic listener, so I would prefer if you named this generically as was done in the patch for that bug.
Blocks: 479823
No longer depends on: 479823
(In reply to Kent James from comment #124) > I'll make some very brief comments here, but mostly you need to revise the > patch to work with the latest patch from bug 695671. Actually I'm not quite sure why bug 695671 needs any changes here, apart from updating the patch context slightly. > The patch for bug 479823 included an interface with the exact same purpose > as this one, but rather than give it a name including Filter (and burying it > in a filter idl, I gave it a generic name nsIMsgOperationListener and its > own idl file. > > There is nothing specific to the use of Filters here for this interface. > There have been other cases where we have needed a simple operation > listener, and have sometimes used nsIUrlListener for this (ignoring the > nsIURL parameter). I think we need a simple, generic listener, so I would > prefer if you named this generically as was done in the patch for that bug. (nsIUrlListener wouldn't have worked here since nsMsgSend already is one, which is why I made up a new one.)
Attached patch Updated patch (obsolete) — Splinter Review
Attachment #8529924 - Attachment is obsolete: true
Attachment #8537907 - Flags: review?(kent)
Comment on attachment 8537907 [details] [diff] [review] Updated patch Review of attachment 8537907 [details] [diff] [review]: ----------------------------------------------------------------- The patch did not apply cleanly for me after I applied the latest patch for bug 695671, but the changes themselves looked fine, so I just fixed that in my local version and continued with the review. I think that we are really close now. My main issues concern error handling. Overall, there is an assumption made here that filterAfterTheFact returns an error code when a filter fails. But that is not actually part of the current design of that function, in fact it is really hard to get the current OnEndExecution() to return anything but NS_OK. Rather than hold up this bug, I'd like to propose that we do a followup bug, that modifies OnEndExecution() to return an error if any filter function fails. I'll attach a patch here that does that for reference. Without that patch, the error handling in this bug is pretty useless as you never actually see any filtering errors. Issue that perhaps needs followup bugs: 1) I tested Forward, and that gets into a filter loop, as the forwarded message matches, and so it forwards THAT message as well, ad infinitum. ::: mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs.properties @@ +361,5 @@ > ## will be replaced with the name of the folder the message is being saved to. > errorSavingMsg=There was an error saving the message to %S. Retry? > > +errorFilteringMsg=Your message was saved, but there was an error while running message filters on it. Retry? > + Filtering is a multi-step process, and only one step might fail. It would be very rare for a retry to succeed, and common that a retry would double apply filters. So I do not think we should allow a retry. Also "Your message was sent and saved ..." would be better, as the user really wants to know if the message was sent or not. ::: mailnews/base/search/public/nsIMsgFilterService.idl @@ +36,5 @@ > * Apply filters to a specific list of messages in a folder. > * @param aFilterType The type of filter to match against > * @param aMsgHdrList The list of message headers (nsIMsgDBHdr objects) > * @param aFolder The folder the messages belong to > * @param aMsgWindow A UI window for attaching progress/dialogs Please add documentation for the callback. ::: mailnews/base/search/src/nsMsgFilterService.cpp @@ +252,5 @@ > // as well, and kick off the next filter when the action completes. > class nsMsgFilterAfterTheFact : public nsIUrlListener, public nsIMsgSearchNotify, public nsIMsgCopyServiceListener > { > public: > + nsMsgFilterAfterTheFact(nsIMsgWindow *aMsgWindow, nsIMsgFilterList *aFilterList, nsIArray *aFolderList, nsIMsgOperationListener *aCallback); This line has become really long. Please break into multiple lines. @@ +285,5 @@ > }; > > NS_IMPL_ISUPPORTS(nsMsgFilterAfterTheFact, nsIUrlListener, nsIMsgSearchNotify, nsIMsgCopyServiceListener) > > +nsMsgFilterAfterTheFact::nsMsgFilterAfterTheFact(nsIMsgWindow *aMsgWindow, nsIMsgFilterList *aFilterList, nsIArray *aFolderList, nsIMsgOperationListener *aCallback) This line has become really long. Please break into multiple lines. @@ +803,5 @@ > filterList->m_temporaryList = true; > return NS_OK; > } > > +NS_IMETHODIMP nsMsgFilterService::ApplyFiltersToFolders(nsIMsgFilterList *aFilterList, nsIArray *aFolders, nsIMsgWindow *aMsgWindow, nsIMsgOperationListener *aCallback) ditto (long line) @@ +808,5 @@ > { > NS_ENSURE_ARG_POINTER(aFilterList); > NS_ENSURE_ARG_POINTER(aFolders); > > + nsMsgFilterAfterTheFact *filterExecutor = new nsMsgFilterAfterTheFact(aMsgWindow, aFilterList, aFolders, aCallback); ditto (long line) @@ +887,5 @@ > // apply filters to a list of messages, rather than an entire folder > class nsMsgApplyFiltersToMessages : public nsMsgFilterAfterTheFact > { > public: > + nsMsgApplyFiltersToMessages(nsIMsgWindow *aMsgWindow, nsIMsgFilterList *aFilterList, nsIArray *aFolderList, nsIArray *aMsgHdrList, nsMsgFilterTypeType aFilterType, nsIMsgOperationListener *aCallback); ditto (long line) @@ +896,5 @@ > nsCOMArray<nsIMsgDBHdr> m_msgHdrList; > nsMsgFilterTypeType m_filterType; > }; > > +nsMsgApplyFiltersToMessages::nsMsgApplyFiltersToMessages(nsIMsgWindow *aMsgWindow, nsIMsgFilterList *aFilterList, nsIArray *aFolderList, nsIArray *aMsgHdrList, nsMsgFilterTypeType aFilterType, nsIMsgOperationListener *aCallback) ditto (long line) @@ +1010,5 @@ > folderList->AppendElement(aFolder, false); > > // Create our nsMsgApplyFiltersToMessages object which will be called when ApplyFiltersToHdr > // finds one or more filters that hit. > + nsMsgApplyFiltersToMessages * filterExecutor = new nsMsgApplyFiltersToMessages(aMsgWindow, filterList, folderList, aMsgHdrList, aFilterType, aCallback); ditto (long line) ::: mailnews/compose/src/nsMsgCompose.cpp @@ +1065,5 @@ > + { > + nsCString sender; > + MakeMimeAddress(NS_ConvertUTF16toUTF8(fullName), email, sender); > + m_compFields->SetFrom(sender.IsEmpty() ? email.get() : sender.get()); > + } Why is this change needed? I'm not objecting to it, but why do you need to add this? ::: mailnews/compose/src/nsMsgSend.cpp @@ +4044,5 @@ > + if (NS_FAILED(rv)) > + return rv; > + > + return filterSvc->ApplyFilters(nsMsgFilterType::PostOutgoing, msgArray, folder, nullptr, this); > +} These send filters are a context where the user is capable of responding to errors, so you need to pass a window to the filter. Without it, a deleted folder in a move filter silently disables the filter rather than alerting the user. This change will do the trick: - return filterSvc->ApplyFilters(nsMsgFilterType::PostOutgoing, msgArray, folder, nullptr, this); + nsCOMPtr<nsIMsgWindow> msgWindow; + if (mSendProgress) + mSendProgress->GetMsgWindow(getter_AddRefs(msgWindow)); + return filterSvc->ApplyFilters(nsMsgFilterType::PostOutgoing, msgArray, folder, msgWindow, this); @@ +4073,5 @@ > + rv = FilterSentMessage(); > + if (NS_FAILED(rv)) > + OnStopOperation(rv); > + return rv; > + } This is the point where I don't think that a retry makes sense, since applying filters is not a binary operation, but a multi-step process, so retrying after a partial failure gives undefined behavior with a strong risk of applying a filter twice. ::: suite/locales/en-US/chrome/mailnews/compose/composeMsgs.properties @@ +303,5 @@ > ## LOCALIZATION NOTE(errorSavingMsg): Do not translate the word %S. It > ## will be replaced with the name of the folder the message is being saved to. > errorSavingMsg=There was an error saving the message to %S. Retry? > + > +errorFilteringMsg=Your message was saved, but there was an error while running message filters on it. Retry? Make the same changes here as in mail.
Attachment #8537907 - Flags: review?(kent) → review-
Hey Neil, I was under some pressure from BenB to move forward with the review of this, which I did. The changes needed were pretty small. I would also like to see this finished, so that I can finish up my changes to filter-after-the-fact. Are you waiting for something more from me?
Flags: needinfo?(neil)
(In reply to Kent James from comment #129) > Hey Neil, I was under some pressure from BenB to move forward with the > review of this, which I did. The changes needed were pretty small. I would > also like to see this finished, so that I can finish up my changes to > filter-after-the-fact. > > Are you waiting for something more from me? Sorry, I was upgrading a server over last weekend and right now Christmas is still distracting me.
Flags: needinfo?(neil)
(In reply to Kent James from comment #127) > > + { > > + nsCString sender; > > + MakeMimeAddress(NS_ConvertUTF16toUTF8(fullName), email, sender); > > + m_compFields->SetFrom(sender.IsEmpty() ? email.get() : sender.get()); > > + } > do you need to add this? Whoops part of another patch sorry.
Attached patch Updated patchSplinter Review
Attachment #8537907 - Attachment is obsolete: true
Attachment #8542246 - Flags: review?(kent)
Comment on attachment 8542246 [details] [diff] [review] Updated patch OK, looks good. Let's ship it! I'll do a followup bug to add the error handling to nsMsgFilterAfterTheFact so that the errors will be seen.
Attachment #8542246 - Flags: review?(kent) → review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 37.0
Depends on: 1116982
(In reply to comment #131) > (In reply to Kent James from comment #127) > > > + { > > > + nsCString sender; > > > + MakeMimeAddress(NS_ConvertUTF16toUTF8(fullName), email, sender); > > > + m_compFields->SetFrom(sender.IsEmpty() ? email.get() : sender.get()); > > > + } > > do you need to add this? > Whoops part of another patch sorry. Another part of that other patch crept in here, but we both overlooked it. Oops.
Depends on: 1119529
This is a warning that this bug may get backed if the issues in regression bug 1119529 are not resolved soon. Waiting there for reviews and resolution of issues.
Blocks: 1135157
I think the age of this bug and the number of votes and CCs makes this a quite popular request and we could announce the fix in the TB38 features list (similarly to bug 479823). Thanks to Neil and rkent for solving this big task!
Keywords: feature
(In reply to :aceman from comment #137) > I think the age of this bug and the number of votes and CCs makes this a > quite popular request and we could announce the fix in the TB38 features > list (similarly to bug 479823). > > Thanks to Neil and rkent for solving this big task! +1 Thanks for all who helped make this happen! You are all wonderful! -- Marc
> Keywords: user-doc-needed Matt, might it be possible to add this to https://support.mozilla.org/en-US/kb/organize-your-messages-using-filters ? also setting flag for a test to be added to moztrap. https://moztrap.mozilla.org/manage/case/16262/ is an example.
Flags: needinfo?(unicorn.consulting)
Flags: in-moztrap?
That does appear appropriate Wayne.... I have started a discussion thread on the article to add both this and bug 479823. I have also started editing the article. On a factual note. The filter action here applies only to the mail involved in the current send operation, not the mail in the sent folder. Correct?
Flags: needinfo?(unicorn.consulting)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: