Apparently some people use the "Reply with Template" filter feature to create a manual vacation responder. I guess they leave Thunderbird running while they're on vacation? We've received complaints that this violates RFC 3834 and can leak the users personal email back to members of a list who otherwise may not know that personal address. > <http://tools.ietf.org/html/rfc3834> > > Personal and Group responses whose purpose is to notify the sender of > a message of a temporary absence of the recipient (e.g., "vacation" > and "out of the office" notices) SHOULD NOT be issued unless a valid > address for the recipient is explicitly included in a recipient > (e.g., To, Cc, Bcc, Resent-To, Resent-Cc, or Resent- Bcc) field of > the subject message. First, that's not "MUST NOT" so we're not violating that RFC, but it's a valid concern. On the other hand message filters are not intrinsically a vacation responder feature, and they're missing some important refinements to be one (such as the ability to only send one reply per address, and ease of user). One way to address this would be that if the user chooses the "Reply with Template" filter action we automatically add a rule of To or CC Contains <account address> This would not be a hidden implicit rule, it would be added to the visible list so that a user could adjust it or delete it if it's inappropriate.
Don't do auto-reply if we're not in To/Cc. (I don't see the point for Bcc, and the other headers suggested aren't easily available i think.) I'm not sure what should be done about filter logging for cases like these. Currently logging is done before action execution http://mxr.mozilla.org/comm-central/source/mailnews/base/search/src/nsMsgFilterService.cpp#465 On the other hand, the filter was executed, it just refused to actually do anything.
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #798314 - Flags: review?(kent)
I think the previous version wasn't what i tested...
Rebased to apply on top of bug 904458 + added/adjusted test. Kent, if you don't think you'll get to this anytime soon, can you redirect the review to irving perhaps?
Somehow I thought this review request went away, but it seems to be back again. I'll look at it.
Note that bug 904458 got backed out due to some mysterious crash. (This bug might also be better using some own test data, instead of reusing bugmail11, I've not yet verified changing the To doesn't break other tests using it.)
Comment on attachment 8408920 [details] [diff] [review] bug904478_reply_with_template_replies_to_mailing_lists.patch Review of attachment 8408920 [details] [diff] [review]: ----------------------------------------------------------------- I'm happy with the main code (with one minor issue) - but not with the tests. But I have not been able myself to come up with a version of the tests that will work. What I think we should do with this, and also with the predecessor bug 904458, is to land it without the tests, and add a separate bug to resolve the test issues. I don't think we should hold these bugs hostage to issues with the testing framework, which is what we seem to be dealing with. Though I am still investigating this, what I think i will do is r+ with two changes - fix the missing rv nit, and remove the tests. ::: mailnews/compose/src/nsMsgComposeService.cpp @@ +1086,5 @@ > // I think we're going to need to stream the template message to ourselves, > // and construct the body, and call setBody on the compFields. > + nsresult rv; > + nsCOMPtr <nsIMsgAccountManager> accountManager = > + do_GetService(NS_MSGACCOUNTMANAGER_CONTRACTID); You need do_GetService(NS_MSGACCOUNTMANAGER_CONTRACTID, &rv) otherwise rv is undefined.
Attachment #8408920 - Flags: review?(kent) → review+
So, how about we land the tests but skip running them on osx and linux debug since the others are fine? Having them landed makes it much easier too to keep track of it all, and for local testing. Probably skip-if = ( os == "mac") || (os == "linux" && debug)
I would be OK with that - but I think then you need to change the tests to use only a single framework (that is get rid of async_test_utils) and also use add_test instead of add_task. (Though I am not an expert on the newer framework, by understanding is that add_task expects to yield a promise). See some of the changes that I made in https://hg.mozilla.org/try-comm-central/rev/7010fcd736e0
Test now using just one async framework
Dependent bug's patch needed for the test_autoReply.js file
Depends on: 904458
Comment on attachment 8697086 [details] [diff] [review] bug904478_reply_with_template_replies_to_mailing_lists.patch Review of attachment 8697086 [details] [diff] [review]: ----------------------------------------------------------------- LGTM.
Attachment #8697086 - Flags: review?(rkent) → review+
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 45.0
You need to log in before you can comment on or make changes to this bug.