Last Comment Bug 208197 - If user marks a message as Not Junk, move message to inbox
: If user marks a message as Not Junk, move message to inbox
Status: VERIFIED FIXED
:
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: Trunk
: All All
: -- enhancement with 46 votes (vote)
: Thunderbird 3.0b2
Assigned To: David :Bienvenu
:
Mentors:
: 220760 227327 232012 233537 233663 250994 255521 265575 271712 355908 381249 394314 411872 413320 420694 471392 (view as bug list)
Depends on: 359339 478840
Blocks: 11035 466041 227327 271948
  Show dependency treegraph
 
Reported: 2003-06-03 18:15 PDT by Tom Sommer
Modified: 2011-08-05 21:08 PDT (History)
53 users (show)
mozilla: wanted‑thunderbird3+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed fix v1 (2.04 KB, patch)
2008-10-30 12:13 PDT, Przemyslaw Bialik
no flags Details | Diff | Splinter Review
Proposed fix v2 (2.17 KB, patch)
2008-10-31 15:51 PDT, Przemyslaw Bialik
mozilla: review-
Details | Diff | Splinter Review
Proposed fix v3 (1.30 KB, patch)
2008-11-08 15:53 PST, Przemyslaw Bialik
no flags Details | Diff | Splinter Review
Proposed fix v4 (2.26 KB, patch)
2008-11-21 03:47 PST, Przemyslaw Bialik
mozilla: review-
clarkbw: ui‑review+
Details | Diff | Splinter Review
Fix v5 (2.25 KB, patch)
2008-11-23 06:48 PST, Przemyslaw Bialik
no flags Details | Diff | Splinter Review
Fix v6 (2.50 KB, patch)
2008-11-23 09:30 PST, Przemyslaw Bialik
no flags Details | Diff | Splinter Review
proposed fix (13.75 KB, patch)
2008-12-11 15:36 PST, David :Bienvenu
no flags Details | Diff | Splinter Review
undelete imap messages on non-junk with imap delete model (13.91 KB, patch)
2008-12-11 17:31 PST, David :Bienvenu
neil: superreview+
Details | Diff | Splinter Review
Log of marking as junk (8.48 KB, text/plain)
2009-01-05 07:23 PST, Mark Banner (:standard8) (afk until 26th July)
no flags Details

Description Tom Sommer 2003-06-03 18:15:10 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030603 Mozilla Firebird/0.6
Build Identifier: 

Once a message has automatically been marked as junk, and the user then clicks
"Not junk". The message should be then moved back to it's orginally intended
folder (be passed through filters)

Reproducible: Always

Steps to Reproduce:
Comment 1 Tom Sommer 2003-06-03 18:15:55 PDT
Maybe a blocker for bug 11035 ?
Comment 2 Mike Cowperthwaite 2003-07-10 15:04:39 PDT

*** This bug has been marked as a duplicate of 195006 ***
Comment 3 Vaclav Dvorak 2003-09-29 21:25:48 PDT
Reopening to separate the two parts of bug 195006.
Comment 4 Steve Wardell 2003-09-29 21:33:38 PDT
The original folder is probably not always known. How about moving it back to
the Inbox? This is how SpamBayes works I believe.
Comment 5 Gilles Durys 2003-09-30 02:20:05 PDT
*** Bug 220760 has been marked as a duplicate of this bug. ***
Comment 6 Vaclav Dvorak 2003-09-30 06:29:31 PDT
I see four possibilities:
1) when marking message as junk (automatically or manually), Mozilla should
remember where the message was at that moment; then, when the user un-junks the
message, move it back where it was
2) don't remember anything, but when the user un-junks a message, run the
filters on it to see if it gets moved somewhere; if not, just move it to inbox
3) always move to inbox when user un-junks a message
4) do nothing, WONTFIX this bug

There's a problem with options 2 and 3: a junk folder can be common for many
accounts, and without somehow marking the message, I suppose Mozilla doesn't
know which account the message came from, and so it doesn't know which inbox -
or which filters - are applicable.

So that actually leaves us with options 1 and 4, unless I fail to see something
else. Confirming the bug now.
Comment 7 Adam Hauner 2003-09-30 06:43:48 PDT
Vaclav: 2) it isn't solution also for e-mails moved between folders by user,
futhermore filters could be also modified etc.

So, I agree about 1) and 4). 
Comment 8 Steve Wardell 2003-09-30 10:14:51 PDT
Item 3 is a viable solution. Granted, some users could share a junk folder from
multiple incoming mail areas, is that the general case that needs to be
addressed? While 3 is less complex and doesn't necessarily meet 100% situations,
it should be considered worthwhile and valid. Personally, that would meet my
interest in this enhancement and is similar to how other spam products work (for
better or worse). Lastly, moving back to one of multiple folders (as opposed to
just to the Inbox) could cause user confusion in the multiple locations a
Ununked message could have moved.
Comment 9 tyl2 2004-01-17 11:17:06 PST
If the message is moved, should it then be marked as unread? Currently, I select
all in the Junk folder and uncheck the false alarms. I then delete these,
resulting in the one or two non-spams to be marked as read and displayed. I
suppose the mark-as-read-only-after-set-time feature recently added to TB (and
prolly Mail?) can be used here, but then I really wouldn't want that feature to
work on anything but the Junk folder.

This seems a lot of hassle, though I wouldn't mind if it's implemented properly.
Comment 10 R.K.Aa. 2004-01-23 18:43:00 PST
*** Bug 232012 has been marked as a duplicate of this bug. ***
Comment 11 Daniel Glazman (:glazou) 2004-02-09 11:05:12 PST
*** Bug 233537 has been marked as a duplicate of this bug. ***
Comment 12 R.K.Aa. 2004-02-10 10:29:43 PST
*** Bug 233663 has been marked as a duplicate of this bug. ***
Comment 13 WADA 2004-04-24 12:55:05 PDT
Vaclav: 1) can be easily implemented if new "X-Mozilla-FCC:" header like current
"FCC:" header for Drafts is written on mail move.
> FCC: mailbox://username@hostname/Sent

I think there is no need to track folder renaming in this function(Messaege
filter has).
If folder is renamed or deleted, or no "X-Mozilla-FCC:" header, message of
"Moving back folder not found" is sufficient.
Comment 14 Frank Wein [:mcsmurf] 2004-07-22 01:49:53 PDT
*** Bug 250994 has been marked as a duplicate of this bug. ***
Comment 15 Adam Zey 2004-07-22 10:31:29 PDT
Sorry to make a useless comment, but what's the status of this bug? Is it fixed
for Thunderbird 0.8 in CVS, or is this only for Mozilla Mail (In which case,
should a seperate bug be opened for Thunderbird?)
Comment 16 Andreas Kunz 2004-07-22 10:42:09 PDT
Adam, this bug i not marked as fixed, nor has anybody mentioned a checkin or
even attached a patch. So what makes you think this is fixed anywhere?

This bug, as it is filed, is for the Mozilla Suite (otherwise it would be listed
unter the "Thunderbird" product). I could imagine some of the fix would help
both, but there's surely a certain amount of work to be done separately for each
product (but having the fix for one would make easier porting it to the other).
Having a separate Thunderbird (with refernce to this one) would not hurt, but
I'm not sure if it is necessary...
Comment 17 WADA 2004-08-29 12:10:30 PDT
How about utilizing new "X-Account-Key" header if POP3 account?
(I could see "X-Account-Key" header in mails on 2004/5/22 and today but could
not see it in a mail on 2004/5/17.)  
Comment 18 Frank Wein [:mcsmurf] 2004-10-22 11:46:31 PDT
*** Bug 265575 has been marked as a duplicate of this bug. ***
Comment 19 Tom Edwards 2005-01-06 09:07:10 PST
I filed this bug under Thunderbird. I don't know how it got changed to the suite.
Comment 20 WADA 2005-01-06 09:24:26 PST
(In reply to comment #19)
> I filed this bug under Thunderbird. I don't know how it got changed to the suite.
See Bug 272179 Comment #1 for Product/Component change.
Comment 21 Doug Wright 2005-01-09 13:24:12 PST
*** Bug 227327 has been marked as a duplicate of this bug. ***
Comment 22 Alan Batie 2005-03-26 11:39:32 PST
It seems to me there's an order issue here: I go to a folder to read messages
that have been automatically filed, and the junk mail filter runs *then* and
moves some of them to the junk folder.  99.99999% of these are not junk, mostly
mailing list postings.  Or in the case where spamassassin tags spam and I have
it directed to a Spam folder, when I go to check for false positives, it all
gets moved to the junk folder and I have to move it all back (I keep the
spamassassin tagged spam separate from what leaks through and tagged by mozilla
just for curiosity and comparison).  I guess what I would like to see is a check
box on the filter rules that says whether or not the filter rule should override
junk mail controls, and have them all only work on INBOX.  Once a message is
filed, it stays put unless you specifically run or apply the rules to another
folder.
Comment 23 jwq 2006-04-18 21:18:22 PDT
 A couple of Core bugs have be duped against this one: Bug 232012 and Bug 233663, and blocks Thunderbird Bug 271712 for reasons set out in Bug 271712 comment #1 and Bug 271712 comment #2.

 Should the Product of this bug now be changed to Core?
Comment 24 Florian Effenberger 2006-04-19 00:58:09 PDT
*** Bug 255521 has been marked as a duplicate of this bug. ***
Comment 25 Magnus Melin 2006-10-08 08:10:16 PDT
*** Bug 271712 has been marked as a duplicate of this bug. ***
Comment 26 Magnus Melin 2006-10-08 08:13:09 PDT
*** Bug 355908 has been marked as a duplicate of this bug. ***
Comment 27 Trevor Morgan 2007-04-24 05:50:19 PDT
This would be very useful.
An option to move the message to the corresponding inbox would probably be sufficient for most users.  From there they can be re-filtered.
For advanced users, a filter on the junk folder could check the junk status and move the message.  I think this would need an addition to filters.
Comment 28 Jo Hermans 2007-05-19 03:55:57 PDT
*** Bug 381249 has been marked as a duplicate of this bug. ***
Comment 29 Henrik Skupin (:whimboo) 2007-05-19 04:06:38 PDT
Are there any rules how to build a new x-header?

Scott and David, what is your suggestion to get this RFE implemented?
Comment 30 Markus Majer 2007-05-19 04:33:15 PDT
That sounds good - I would vote for the idea from comment 8 + 9.

Option 3, move to inbox - and mark them as unread.

That wouldnt be the perfect option, but a good consense between "comfortable"
and "easy to fix".

I and my colleagues would be happy if it would be just this easy solution.

Perhaps some of this things could be made as option fields in the junk options
(like "mark the sent-back to inbox false positives as unread" in better english
 of course *g*) 



(In reply to comment #9)
> If the message is moved, should it then be marked as unread? Currently, I
> select
> all in the Junk folder and uncheck the false alarms. I then delete these,
> resulting in the one or two non-spams to be marked as read and displayed. I
> suppose the mark-as-read-only-after-set-time feature recently added to TB (and
> prolly Mail?) can be used here, but then I really wouldn't want that feature to
> work on anything but the Junk folder.
> 
> This seems a lot of hassle, though I wouldn't mind if it's implemented
> properly.
Comment 31 David :Bienvenu 2007-05-19 09:37:40 PDT
Henrik, you can't add a new x-header to a message you've already downloaded (well, not easily). You could easily set a string property on the msg hdr that gets stored in the .msf file that could store the uri of the folder you want the message to get moved to on "un junk".
Comment 32 Jon B 2007-08-30 08:13:42 PDT
*** Bug 394314 has been marked as a duplicate of this bug. ***
Comment 33 JG Berson 2007-11-10 10:04:11 PST
If there are multiple places to return a "not junk" item, then simply ask!
I don't really see the problem here.  

But currently, if the incoming item is NOT junk and TB determines it to be NOT junk, it knows which folder to send it.  So it should be simple to send the item back to the incoming stream, and determine which folder it should go to knowing it is now declared "not junk".  ie: don't junk filter it, just move it.
Comment 34 Jon B 2007-11-10 10:22:08 PST
(In reply to comment #13)
> If folder is renamed or deleted, or no "X-Mozilla-FCC:" header, message of
> "Moving back folder not found" is sufficient.

Please don't make dead end error messages like this.  You could at least ask where the user wants it to go. 

Comment 35 Magnus Melin 2008-01-11 10:37:57 PST
*** Bug 411872 has been marked as a duplicate of this bug. ***
Comment 36 Magnus Melin 2008-01-21 09:15:25 PST
*** Bug 413320 has been marked as a duplicate of this bug. ***
Comment 37 Michael Mess 2008-01-21 09:57:30 PST
There should be a Junk Master mode which can be activated in the configuration:
When multiple users share an imap account and this is activated, a mail which has been marked as Junk or un-junked by another client should also be moved to the appropriate folder and the junk filter should learn from it.

For details, see https://bugzilla.mozilla.org/show_bug.cgi?id=413320

Thus there would be only one Junk Master and for the other clients Junk mail control should be turned off, they (=their user) will only manually mark and unmark mails as Junk and let the moving be done by the Junk Master.
The goal is that there is only one client which is responsible for sorting out Junk and only one training data file which needs to be trained.

When someone marks a mail as junk by accident, this could be undone very easy by unjunking the mail in the Junk folder and only one client (the Junk Master) needs to learn from it and move it back.
Comment 38 Magnus Melin 2008-03-03 10:55:32 PST
*** Bug 420694 has been marked as a duplicate of this bug. ***
Comment 39 LuDean Marvin 2008-06-27 20:56:01 PDT
(In reply to comment #6)
> I see four possibilities:
> 1) when marking message as junk (automatically or manually), Mozilla should
> remember where the message was at that moment; then, when the user un-junks the
> message, move it back where it was
> 2) don't remember anything, but when the user un-junks a message, run the
> filters on it to see if it gets moved somewhere; if not, just move it to inbox
> 3) always move to inbox when user un-junks a message
> 4) do nothing, WONTFIX this bug
> 
> There's a problem with options 2 and 3: a junk folder can be common for many
> accounts, and without somehow marking the message, I suppose Mozilla doesn't
> know which account the message came from, and so it doesn't know which inbox -
> or which filters - are applicable.
> 
> So that actually leaves us with options 1 and 4, unless I fail to see something
> else. Confirming the bug now.

I cast my vote for option 3 
Comment 40 Andreas Goetz 2008-06-28 01:29:14 PDT
3 as well
Comment 41 Markus Majer 2008-06-28 02:51:00 PDT
3 from me too, but perhaps with a enhancement:

Give the "recognized as junk but later un-junked" mails a tag with this thunderbird tag feature - I do not know how it is called "Schlagwörter" in the german localisation.
Comment 42 Przemyslaw Bialik 2008-10-30 12:13:26 PDT
Created attachment 345537 [details] [diff] [review]
Proposed fix v1

Always unjunks messages to the Inbox folder.

Notes:
- I've used GetInboxFolder(defaultServer) temporary because I had problems getting inbox folder of the current account, can you please advice ?
- message is being unjunked to the inbox even if it is in the inbox, is it worth to the if-then check in this case ?
- enhancements / unjunking intelligence can be fixed in follow-up bugs
Comment 43 u88484 2008-10-30 12:36:19 PDT
(In reply to comment #42)
> Created an attachment (id=345537) [details]
> Proposed fix v1
> Always unjunks messages to the Inbox folder.

If it unjunks the message to the inbox folder, will it then be filtered into the correct folder?
Comment 44 Przemyslaw Bialik 2008-10-30 12:57:45 PDT
(In reply to comment #43)
> If it unjunks the message to the inbox folder, will it then be filtered into
> the correct folder?
Currently this is a pure move to the inbox without extras but as said it can be improved later on
Comment 45 Magnus Melin 2008-10-31 11:39:01 PDT
(In reply to comment #42)
> - I've used GetInboxFolder(defaultServer) temporary because I had problems
> getting inbox folder of the current account, can you please advice ?

Find the msgHdr, and then GetInboxFolder(msgHdr.folder.server) perhaps?

> - message is being unjunked to the inbox even if it is in the inbox, is it
> worth to the if-then check in this case ?

Since you'd be getting the inbox folder anyway, might as well check that you're not alreday there...
Comment 46 Przemyslaw Bialik 2008-10-31 15:47:35 PDT
(In reply to comment #45)
> Find the msgHdr, and then GetInboxFolder(msgHdr.folder.server) perhaps?
thanks! , patch coming (if you want feel free to do the review but junkCommands.js is under mailnews so second review of mailnews peer would be needed anyway)
Comment 47 Przemyslaw Bialik 2008-10-31 15:51:57 PDT
Created attachment 345803 [details] [diff] [review]
Proposed fix v2

Changes:
- moves to the proper inbox
- [FIXED] message is being unjunked to the inbox even if it is in the inbox
Comment 48 Przemyslaw Bialik 2008-11-02 04:58:59 PST
(In reply to comment #43)
> If it unjunks the message to the inbox folder, will it then be filtered into
> the correct folder?
Is your use case as follows ? :
New message is catched by the message filter AND by the junk filter, junk filter wins in such cases and the message is moved to the junk folder and not the folder you did specify in the message filters dialog.
Does it happen often ?
Comment 49 Mark Banner (:standard8) (afk until 26th July) 2008-11-03 05:15:39 PST
Comment on attachment 345803 [details] [diff] [review]
Proposed fix v2

I'm going to pass this over to David, as I think he knows this area of code better than I do, and I've got a lot of big patches in my queue at the moment.

Also, adding Bryan our user experience guy onto the review list to check the behaviour is acceptable (which I think it probably is, just checking).
Comment 50 David :Bienvenu 2008-11-03 07:15:15 PST
Comment on attachment 345803 [details] [diff] [review]
Proposed fix v2

thx for working on this. I've always wanted this feature. There are a few problems, though.

With your patch, we won't reload messages anymore when the junk status changes, even if we're not moving the message back. And I don't think your logic is quite right - we would only want to move the message back if (!isJunk && moveJunkMail) - what you've done, I think, is if (!isJunk || !moveJunkMail)

Also,I don't think you're clearing the junk flag anymore, in the case of marking a message as non-junk; you're just moving it back to the inbox.

I'm not clear on why you're doing what you're doing in the HandleJunkStatusChanged part either - the command handler should do the move as well as clearing the junk status, not the code that's called when the command has finished.

Finally, there are cases where this isn't quite the right thing to do. The message might be in a folder other than the inbox or the junk folder, e.g., moved there by a mail filter or the user. Or the user might be using a single junk folder for multiple accounts, in which case you don't know which inbox to move the message to. At the very least, you should be checking that the message is in a junk folder, not that it's not in the inbox...
Comment 51 Przemyslaw Bialik 2008-11-03 14:14:01 PST
> thx for working on this. I've always wanted this feature.
me too, good to hear that it is also wanted by the TB team
 
> With your patch, we won't reload messages anymore when the junk status changes,
> even if we're not moving the message back. And I don't think your logic is
> quite right - we would only want to move the message back if (!isJunk &&
> moveJunkMail) - what you've done, I think, is if (!isJunk || !moveJunkMail)
I need to add second "if" instead of only modifying the existing one...

> Also,I don't think you're clearing the junk flag anymore, in the case of
> marking a message as non-junk; you're just moving it back to the inbox.
+    gDBView.doCommand(nsMsgViewCommandType.unjunk);
seems to do fine

> I'm not clear on why you're doing what you're doing in the
> HandleJunkStatusChanged part either - the command handler should do the move as
> well as clearing the junk status, not the code that's called when the command
> has finished.
if you have on mind not using JunkSelectedMessages(false); then I would have to duplicate code added to the JunkSelectedMessages function or I'm missing sth ?
it was added so clicking on the junk status column would unjunk using new code - not only refresh
> // we used to only reload the message if we were toggling the message to NOT
JUNK from junk 

> Finally, there are cases where this isn't quite the right thing to do. The
> message might be in a folder other than the inbox or the junk folder, e.g.,
> moved there by a mail filter or the user. Or the user might be using a single
> junk folder for multiple accounts, in which case you don't know which inbox to
> move the message to. At the very least, you should be checking that the message
> is in a junk folder, not that it's not in the inbox...
yes, I've thought about checking for junk folder after attaching the patch
It's hard to cover all cases but I think we can assume (at least in this bug) that if the junk message is not in the junk folder then it has been either moved there manually or by message filter -> user knows it's the proper place and definitely not inbox

thank you for the insightful comments
Comment 52 Bryan Clark (DevTools PM) [@clarkbw] 2008-11-06 10:14:32 PST
This behavior sounds on track to me, as outlined in David's Comment #50.  I'm wondering if there exists (or exists a way to add) some meta-data about what folder a junk mail came from.  Otherwise I feel like our option becomes asking the user where to put the non-junk mail back to; not something I like.
Comment 53 David :Bienvenu 2008-11-06 11:13:18 PST
Right, that's exactly what we would have to do, and we do have the technology to do it, though I think I would only handle the single machine case, not the general imap case...i.e., I would annotate locally.
Comment 54 Carlo Alberto Ferraris 2008-11-06 12:12:35 PST
It may sound biased but I think that not dealing with the IMAP case would quickly become a pain, especially since it is gaining wider adoption. If you think that it won't make in time for the tb3 timeframe but will be considered for the next minor/major release, then it's a pity but that's OK. OTH if you're going to scrap it completely then it looks to me some kind of lack of foresight (that is, considering the users' growing attitude to connect from a moltitude of different devices - be them mobile or not).
Comment 55 Bryan Clark (DevTools PM) [@clarkbw] 2008-11-08 09:40:03 PST
I think local only is going to be fine.  This would only effect the case where Thunderbird is being used from multiple machines.  I don't think mobile or other applications would be effected as they would need to understand the "came-from" meta-data applied to the message and offer the option to restore it.

Perhaps (and I love to keep insisting we'll have this) when we finalize weave integration this meta-data will be kept with weave such that multiple instances of Thunderbird can make use of it.
Comment 56 Przemyslaw Bialik 2008-11-08 15:53:34 PST
Created attachment 347103 [details] [diff] [review]
Proposed fix v3

Changes:
- unjunkes to the inbox only from the junk folder
- restored changes made to the HandleJunkStatusChanged

Is there a need (and in this bug) to unjunk messages to the inbox by clicking on the junkcol icon ? (currently it only changes it junk status)
if yes then please give some hints because I had problems doing it correctly and the way it would also apply to the not currently selected messages.

Regarding moving back to the folder from which junk mail came from:
I think current patch covers most of the users (bug voters, cc users ?).
Using current available functions I can't get info from which folder junk mail came from, right ?
If yes, then I think it should be fixed in other / follow-up bug.
Comment 57 Przemyslaw Bialik 2008-11-08 16:27:43 PST
(In reply to comment #56)
> Is there a need (and in this bug) to unjunk messages to the inbox by clicking
> on the junkcol icon ? (currently it only changes it junk status)
> if yes then please give some hints because I had problems doing it correctly
> and the way it would also apply to the not currently selected messages.
probably in nsMsgDBView.cpp:
>  case 'j': // junkStatus column 
would have to be updated in a similar way JunkSelectedMessages(setAsJunk) was but unfortunately I don't have build environment ready ATM to test the changes and Bug 431375 is still not fixed :(
Comment 58 Henrik Skupin (:whimboo) 2008-11-09 02:53:51 PST
Thanks Przemyslawv for your work here. Just a small hint. Please keep the Mozilla coding style: https://developer.mozilla.org/En/Mozilla_Coding_Style_Guide
Comment 59 David :Bienvenu 2008-11-17 09:41:16 PST
Comment on attachment 347103 [details] [diff] [review]
Proposed fix v3

I'll let Bryan chime in about whether we should do this without knowing the real original folder.
Comment 60 Tuukka Tolvanen (sp3000) 2008-11-18 12:34:46 PST
How do I know where the message went? One annoying case (workaroundable, yes, but working around it implies you'd have already learned what's going to happen):
 1. scan through junk, land on a somewhat aged false positive
 2. decide not to read it now, just unjunk
 -> where'd it go? If the inbox is not of the empty kind, the message would tend to be somewhere not with the very recent messages, and marked read. OK, so we'll have better search, but...

IMO the unjunked message should appear unread (and probably new), iow despite not taking te "re-deliver" approach, follow it in the msg status respect, so that the message is findable and there's some (if a bit subtle) immediate feedback on where it appears.
Comment 61 Przemyslaw Bialik 2008-11-18 13:10:33 PST
(In reply to comment #60)
> IMO the unjunked message should appear unread (and probably new), iow despite
> not taking te "re-deliver" approach, follow it in the msg status respect, so
> that the message is findable and there's some (if a bit subtle) immediate
> feedback on where it appears.
if there is a need to mark message unread when unjunking then it should be a hidden pref (if you decide sth is not junk you already read it at least partially, right ?)

it is not yet decided if unjunking to inbox would be approved (if yes then probably it will be mentioned in the relnotes or sth), though I myself would like to split the enhancements into follow-up bugs especially because I'm not sure I would have currently the skills to take care of all of them

I can add that I've built successfully shredder from the sources so I can now verify / work on complete version of previous patch (nsMsgDBView.cpp changes).
Comment 62 Bryan Clark (DevTools PM) [@clarkbw] 2008-11-18 14:34:34 PST
Hmm, this seems more like a surface level issue to me.

I feel if we use a button like ( not junk ) then we cannot push a message back to the inbox.  However if, at least for now, we changed that button to be something like ( restore to inbox ) then I'd feel more comfortable with the change.  

The ( not junk ) button only indicates that it's going to change the status of the message, not move it.  So if we were to move a messages using 'not junk' we would really have to return it to it's original location.  While a button like ( restore to inbox ), and that text isn't perfect, at least lets a person know exactly what is going to happen with the message.

Also I particularly like the suggestion of the unread status change when moving a message back.  I believe the intention is to read the message after it has been restored from the junk folder.
Comment 63 JG Berson 2008-11-19 05:38:00 PST
This bug has been danced around for 4 1/2 years already.  

You guys are overthinking a simple problem.  

A year ago I said "if the incoming item is NOT junk and TB determines it to be NOT junk, it knows which folder to send it.  So it should be simple to send the
item back to the incoming stream, and determine which folder it should go to
knowing it is now declared "not junk".  ie: don't junk filter it, just move it."

The original #1 post on this subject said pretty much the same thing.

Let me clarify the solution further.
You get an email that TB declares is junk, be it heuristically or by junk filter criteria, and that is moved into the Junk Folder.  But TB must annotate the email of what function that declared it as junk.
The user reviews that email and clicks the "NOT JUNK" button.
TB removes the "junk" classification and places the email back into the incoming mail stream, with one exception, the function that originally declared it junk is BYPASSED this time only.
The email either is junked by some subsequent filter or, most likely, is sent to whatever folder the non-junk email would normally goto, usually the inbox.
TB can mark it as unread or whatever, it really doesn't matter that much.  The user has read it and is aware of it.  If the destination is some other user account sharing the same incoming mail stream, then, yeah, mark it as "unread".

End of story.

If the user is made aware of what filter originally junked it, then the user can modify the filter so as to not junk it in the future.

To the TB developers; Come on, you guys have solved far more complex problems than this one.  Four and a half years????  GOYA.

jgb
Comment 64 Przemyslaw Bialik 2008-11-19 11:20:13 PST
Bryan: what do you think about this:
- pressing "not junk" would mark message as not junk and would move it to the inbox folder (if unjunking from the junk folder) and it would mark it as unread (I will add a hidden pref to not mark as unread).

I'm seeing this bug as enhancement of marking message as "not junk" - it does the same but a little more (it's a new feature but mostly expected) - message was falsely classified so it is moved back to the inbox (we don't know from where it come from but if the mail was message filtered then it is small chance that it was marked as junk and user later unjunks it) - I would like to move it to it's original location but I don't know how to do it / if we have such knowledge.
As this is enhancement of unjunking function it should be listed in what's new for TB 3.0
IOW I don't think it is a good idea to change the name to "restore to inbox" - restore to inbox might not mean to the user marking as not junk ?
also a few buttons and menus would need to have their name changed.
So do you still want to change the name while fixing this bug ?

[edit] info regarding changes can be put also in the junkMailInfo.dtd (for first time users of junk filtering)
Comment 65 Przemyslaw Bialik 2008-11-19 12:13:47 PST
OK, let's not waste time here, please report Bryan if you would change your mind regarding my previous comment.
I prefer better / fully approved patches so in the meantime...
based on your comment:
> So if we were to move a messages using 'not junk' we
> would really have to return it to it's original location.
I will file bug for:
> I'm wondering if there exists (or exists a way to add) some meta-data about 
> what folder a junk mail came from.

> Right, that's exactly what we would have to do, and we do have the technology
> to do it

and add dependency, when it would be fixed I will go back to this bug.
Comment 66 Bryan Clark (DevTools PM) [@clarkbw] 2008-11-20 12:06:46 PST
I think with marking the message unread and moving it back to the Inbox we're making good progress, lets get that in and worry about better solutions later.
Comment 67 u88484 2008-11-20 13:27:33 PST
Changing summary since this bug was hijacked from its original purpose of moving the message to its intended folder.  So now we have to file another bug for what this bug, comments and dupes were originaly intended for.  Why not just do it right the first time?
Comment 68 Przemyslaw Bialik 2008-11-20 13:40:55 PST
Because bug 465794 is blocking it and it was decided that moving to inbox is better than waiting nobody knows how long for mentioned bug to be fixed.
Initial patch (v4) might be checked in and bug be left open for the second patch (for example) - hijacking was not planned.
Comment 69 u88484 2008-11-20 14:03:49 PST
Filed bug 466041, for moving the message to its intended folder (not the inbox)
Comment 70 Przemyslaw Bialik 2008-11-21 03:47:58 PST
Created attachment 349399 [details] [diff] [review]
Proposed fix v4

Changes since last patch:
- adds hidden pref mailnews.junk.MarkAsNotJunkMarksUnRead (defaults to true), if true message goes to inbox unread, if false read status is not changed
Comment 71 Bryan Clark (DevTools PM) [@clarkbw] 2008-11-21 18:32:05 PST
Comment on attachment 349399 [details] [diff] [review]
Proposed fix v4

I'm not really a fan of hidden prefs in general but lets see how this goes.
Comment 72 David :Bienvenu 2008-11-21 19:24:35 PST
Comment on attachment 349399 [details] [diff] [review]
Proposed fix v4

thx for working on this.

I think you should mark the message unread first, before moving the message.
Also, the gDBView.command(nsMsgViewCommandType.junk) should be on its own line.

also, might be a bit cleaner to do this:

let folder = gDBView.hdrForFirstSelectedMessage.folder;

then use folder and folder.server below...

and you should add mailnews.junk.MarkAsNotJunkMarksUnRead to mailnews.js, or this code will throw an exception
Comment 73 Magnus Melin 2008-11-22 02:13:04 PST
While you're at it, can we please make the pref name lower_case or camelCase like most other prefs. (And I don't know why the ".junk" part is there.)
Comment 74 Przemyslaw Bialik 2008-11-22 14:47:32 PST
(In reply to comment #71)
> I'm not really a fan of hidden prefs in general but lets see how this goes.
I know that TB limits prefs configurable by UI and my believe is to have at least a hidden pref than no pref at all (I think such is wanted here)
if you had on idea to add also UI for this than I can add it (here ? or in follow-up bug)

(In reply to comment #72)
> I think you should mark the message unread first, before moving the message.
I did it to not show message being marked as unread for a moment and disappearing but I can change that - users will knew that msg will me marked as unread though they would in the end anyway

> also, might be a bit cleaner to do this:
> let folder = gDBView.hdrForFirstSelectedMessage.folder;
might be, ok

> and you should add mailnews.junk.MarkAsNotJunkMarksUnRead to mailnews.js, or
> this code will throw an exception
I thought I did ;)

(In reply to comment #73)
> While you're at it, can we please make the pref name lower_case or camelCase
> like most other prefs. 
> (And I don't know why the ".junk" part is there.)
mailnews.markAsNotJunkMarksUnRead ?
Comment 75 David :Bienvenu 2008-11-22 15:48:15 PST
> > and you should add mailnews.junk.MarkAsNotJunkMarksUnRead to mailnews.js, or
> > this code will throw an exception
> I thought I did ;)
Oh yes, so you did, sorry about that.

I just worry if you move a message before you mark it unread, it may not be unread in the target folder.
Comment 76 Przemyslaw Bialik 2008-11-23 06:48:27 PST
Created attachment 349641 [details] [diff] [review]
Fix v5

updated to comments
Comment 77 Magnus Melin 2008-11-23 07:17:00 PST
(In reply to comment #74)
> > (And I don't know why the ".junk" part is there.)
> mailnews.markAsNotJunkMarksUnRead ?

The other spam related settings hare named mail.spam.xxx (sorry for being unclear!)
Comment 78 Magnus Melin 2008-11-23 07:21:25 PST
Comment on attachment 349641 [details] [diff] [review]
Fix v5

>+  if (setAsJunk)
>+    gDBView.doCommand(nsMsgViewCommandType.junk)

The style rule is: if one of the if/else use braces, all should.

>+  else
>+  {
>+    gDBView.doCommand(nsMsgViewCommandType.unjunk);
>+    let folder = gDBView.hdrForFirstSelectedMessage.folder;
>+    if (IsSpecialFolder(folder, MSG_FOLDER_FLAG_JUNK, true))
>+      {

Brace should be aligned with if
Comment 79 Przemyslaw Bialik 2008-11-23 07:39:27 PST
> The other spam related settings hare named mail.spam.xxx (sorry for being
> unclear!)
I don't want to enforce anything
mail.spam.markAsNotJunkMarksUnRead and placed after mail.spam.manualMarkMode ?

> The style rule is: if one of the if/else use braces, all should.
so even one-liners:
  if (setAsJunk)
  {
    gDBView.doCommand(nsMsgViewCommandType.junk);
  }
?
Comment 80 Magnus Melin 2008-11-23 08:57:20 PST
Yes.
Comment 81 Przemyslaw Bialik 2008-11-23 09:30:51 PST
Created attachment 349646 [details] [diff] [review]
Fix v6

updated to comments
Comment 82 David :Bienvenu 2008-11-26 07:46:09 PST
I applied this patch and was playing around with it - I had some problems with assertions thrown getting prefs. Maybe it was a build problem - I'll try to investigate, but I think this will have to wait for b2.
Comment 83 Przemyslaw Bialik 2008-11-27 11:48:31 PST
WFM even on compiled build so it's probably a problem on your side, try to update the sources and do the fresh patched build.

Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9.1b3pre) Gecko/20081127 Shredder/3.0b1pre
Comment 84 Przemyslaw Bialik 2008-12-01 10:50:16 PST
David, Magnus - you are way more familiar with mozilla c++ code than I am, can I ask one of you to provide the patch for the "case 'j': // junkStatus column" in nsMsgDBView.cpp to update it with this bug changes ? (I assume this is wanted ?)
I can offer to compile the build and do the tests.
Comment 85 David :Bienvenu 2008-12-11 15:36:15 PST
Created attachment 352627 [details] [diff] [review]
proposed fix

turns out that in order to do the junk column, we don't need the front end changes, and both the junk button and junk column can go through the c++ code.

I generalized the c++ methods so that they handle both the junk and unjunk cases.

firefox@poczta.neostrada.pl, it would be great if you want to try out the c++ changes.
Comment 86 David :Bienvenu 2008-12-11 17:31:57 PST
Created attachment 352650 [details] [diff] [review]
undelete imap messages on non-junk with imap delete model

Neil requested this.

In my tree, manually marking a message as junk with the imap delete model isn't imap deleting the message, but maybe the incoming junk filtering does that...
Comment 87 neil@parkwaycc.co.uk 2008-12-12 04:47:10 PST
(In reply to comment #86)
> In my tree, manually marking a message as junk with the imap delete model isn't
> imap deleting the message, but maybe the incoming junk filtering does that...
Well, it works in the old build that I use to read bugmail...
Comment 88 neil@parkwaycc.co.uk 2008-12-12 06:35:48 PST
Comment on attachment 352650 [details] [diff] [review]
undelete imap messages on non-junk with imap delete model

warning: 12 lines add whitespace errors.

>+                "the classification of a manually-marked junk message has\
>+                been classified as junk, yet there seem to be no such outstanding messages");
Nit: don't use \, use
"some text here "
"some more text here"

>+  nsresult rv = DetermineActionsForJunkChange(msgsAreJunk, moveMessages, 
>+                                            changeReadState, 
>+                                            getter_AddRefs(targetFolder));
Nit: indentation

>+nsMsgDBView::DetermineActionsForJunkChange(PRBool msgsAreJunk, 
>+                                         PRBool &moveMessages, 
>+                                         PRBool &changeReadState, 
>+                                         nsIMsgFolder** targetFolder)
Nit: indentation

>+  // handle the easy case of marking a junk message as good first...
>+  // move back to inbox, if any, and 
and what? ;-)

>+  if (!msgsAreJunk)
>+  {
>+    if (folderFlags & nsMsgFolderFlags::Junk)
>+    {
>+      prefBranch->GetBoolPref("mail.spam.markAsNotJunkMarksUnRead", 
>+                              &changeReadState);
I think the mark unread shouldn't depend on which folder the messages are in.

>+  nsresult DetermineActionsForJunkChange(PRBool msgsAreJunk,
>+                                       PRBool &moveMessages, 
>+                                       PRBool &changeReadState, 
>+                                       nsIMsgFolder** targetFolder);
Nit: indentation

>+pref("mail.spam.markAsNotJunkMarksUnRead", true);
I guess the prefs are somewhat inconsistent, as the mark as read pref is mailnews.ui.junk.manualMarkAsJunkMarksRead
Comment 89 Magnus Melin 2008-12-14 09:14:55 PST
Yup, but only that and firstuse use the mailnews.ui.junk scheme.
Comment 90 Przemyslaw Bialik 2008-12-15 14:10:54 PST
Thanks for taking, I can confirm that the patch works as expected.

patched Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9.1b3pre) Gecko/20081215 Shredder/3.0b2pre
Comment 91 Mark Banner (:standard8) (afk until 26th July) 2008-12-29 01:58:42 PST
Comment on attachment 352650 [details] [diff] [review]
undelete imap messages on non-junk with imap delete model

So I'm getting into a weird loop here:

1) In IMAP inbox, mark message as junk.
-> For some reason this isn't moving it to junk straight away.

2) Move message to junk
-> Message is in junk, but has lost junk indication (bug 359339).

3) Mark as Junk then clear junk status
-> Message removed from junk folder, Inbox says it has one unread message

4) Select Inbox folder
-> Message not in inbox, seems to have been moved back to junk as that is now showing one new message.

5) Go to Junk folder, select message (which does now have junk status), select not junk.
-> Inbox shows 1 new message

6) Repeat 4 and 5

7) Select inbox. Previously junked message now shown in inbox with unread status.


This may be being made worse by bug 359339, but its still a strange loop to be getting into.
Comment 92 Phil Ringnalda (:philor, back in August) 2008-12-29 02:34:01 PST
*** Bug 471392 has been marked as a duplicate of this bug. ***
Comment 93 David :Bienvenu 2008-12-29 08:14:05 PST
very odd - does #1 happen w/o this patch? I need to pick up this patch and address Neil's comments soon. What kind of IMAP server is this? Does it support user-defined keywords?
Comment 94 Mark Banner (:standard8) (afk until 26th July) 2008-12-29 08:29:05 PST
(In reply to comment #93)
> very odd - does #1 happen w/o this patch? I need to pick up this patch and
> address Neil's comments soon. What kind of IMAP server is this? Does it support
> user-defined keywords?

#1 happens on this profile without this patch (sorry I forgot to check earlier). My trunk nightlies (same server, different account) work fine. I played around with account settings and still couldn't get it to work.

The server is my standard Courier-IMAP one.
Comment 95 David :Bienvenu 2008-12-30 21:06:05 PST
The first thing I notice is that we aren't creating the imap junk folder when I mark a message as junk.
Comment 96 David :Bienvenu 2008-12-31 15:46:11 PST
I didn't have my imap account configured to move junk messages to the junk folder, so that explains why I wasn't getting a junk folder.

Standard8, can you attach a protocol log of a session showing the problem you're having? It's all working fine here.
Comment 97 Mark Banner (:standard8) (afk until 26th July) 2009-01-05 07:12:25 PST
(In reply to comment #96)
> I didn't have my imap account configured to move junk messages to the junk
> folder, so that explains why I wasn't getting a junk folder.

Argh, I had my account configured to move junk messages to the junk folder, but not the general preferences.

> Standard8, can you attach a protocol log of a session showing the problem
> you're having? It's all working fine here.

It is still not marking/keeping it as junk on move into the folder, so I'll attach a log in a few minutes.
Comment 98 Mark Banner (:standard8) (afk until 26th July) 2009-01-05 07:23:12 PST
Created attachment 355402 [details]
Log of marking as junk

Hmm, not sure if you want this here or not, this is an extract of the log from just marking a message as junk (with move to junk folder switched on). It starts with the message already selected in the inbox, then clicking on the button, then gointo into the junk folder and seeing it is not marked as junk.
Comment 99 David :Bienvenu 2009-01-05 08:51:28 PST
I can't tell from that log if that particular server supports user defined keywords - I'd need to see the response from the server when we issue a select on the inbox for that server.

What I can tell from that log is that we're not issuing protocol to set the junk flag on the message, though we're running a url to do so. In theory, if the server didn't support custom keywords, we'd remember that the next time we downloaded headers from the target folder that the moved msg should have the junk flag set.
Comment 100 Mark Banner (:standard8) (afk until 26th July) 2009-01-06 02:02:46 PST
(In reply to comment #99)
> I can't tell from that log if that particular server supports user defined
> keywords - I'd need to see the response from the server when we issue a select
> on the inbox for that server.

Ok, so my issue with not retaining the marking as junk is bug 359339. I'll attach a protocol log there in a while.

I've just tried this with our zimbra test server, and steps 3 - 7 are still happening in a loop. Could part of the problem be that the message is getting into the inbox as a new mail, rather than a read mail?
Comment 101 David :Bienvenu 2009-01-06 08:29:40 PST
Ah, yes, Zimbra. Normally this wouldn't be an issue because even though the message looked new, we would have already classified it as junk/non junk, and wouldn't run it through the spam filter. But Zimbra strips off the junk flags when moving messages between folders, without telling us, and we really have no way of knowing that it's doing this because it claims to support user-defined keywords fully. 

The easiest way I can think of dealing with this is to set the pending attributes on the hdr moved back to the inbox even though the server claims to support user-defined keywords. I don't know if I can do that for just Zimbra servers or not...
Comment 102 Jose Sa 2009-01-12 09:06:28 PST
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-GB; rv:1.9.1b3pre) Gecko/20081204 Thunderbird/3.0b1

Moving mail marked as junk by spamassassin from junk folder to Inbox makes the message to return to junk folder.

I believe this is related to this bug as it necessary to guarantee that the message stays where it is intended after unmarking it as junk.
Comment 103 Mark Banner (:standard8) (afk until 26th July) 2009-02-05 13:51:59 PST
Comment on attachment 352650 [details] [diff] [review]
undelete imap messages on non-junk with imap delete model

This patch got included with the on on bug 359339 I believe.
Comment 104 David :Bienvenu 2009-02-05 14:37:40 PST
fix checked in.
Comment 105 Henrik Skupin (:whimboo) 2009-02-08 10:18:01 PST
Verified fixed with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090208 Shredder/3.0b2pre

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090208 Shredder/3.0b2pre
Comment 106 Henrik Skupin (:whimboo) 2009-02-08 11:41:57 PST
Please don't remove bug 465794 from the dependency list. It's still valid. If you initially move junk mail to another account and undo this action, the message is not moved back to the origin account. Instead it lands in the inbox of same account.
Comment 107 Valery Kondakoff 2009-02-09 05:28:39 PST
I can confirm the comment 106.

If TB is set up to move junk mail to a single Junk folder, located, say, in Local folders, marking mail as 'Not Junk' will move all the messages to the Local folders Inbox instead of the origin accounts Inboxes.

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090207 Shredder/3.0b2pre
Comment 108 Przemyslaw Bialik 2009-02-09 09:33:46 PST
Re: 106, 107
While this is true please note that bug 466041 was filed to enhance the current behavior and so bug 465794 is not directly related to this closed bug but to the mentioned one.
Comment 109 Henrik Skupin (:whimboo) 2009-02-09 12:09:00 PST
Ok, thats true. Sorry for the confusion. Removing dependency.

Note You need to log in before you can comment on or make changes to this bug.