Closed
Bug 208197
Opened 21 years ago
Closed 15 years ago
If user marks a message as Not Junk, move message to inbox
Categories
(MailNews Core :: Backend, enhancement)
MailNews Core
Backend
Tracking
(Not tracked)
VERIFIED
FIXED
Thunderbird 3.0b2
People
(Reporter: ts, Assigned: Bienvenu)
References
(Blocks 2 open bugs)
Details
Attachments
(2 files, 7 obsolete files)
13.91 KB,
patch
|
neil
:
superreview+
|
Details | Diff | Splinter Review |
8.48 KB,
text/plain
|
Details |
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 2•21 years ago
|
||
*** This bug has been marked as a duplicate of 195006 ***
Status: UNCONFIRMED → RESOLVED
Closed: 21 years ago
Resolution: --- → DUPLICATE
Comment 3•21 years ago
|
||
Reopening to separate the two parts of bug 195006.
Status: RESOLVED → UNCONFIRMED
OS: Windows 2000 → All
Hardware: PC → All
Resolution: DUPLICATE → ---
Comment 4•21 years ago
|
||
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•21 years ago
|
||
*** Bug 220760 has been marked as a duplicate of this bug. ***
Comment 6•21 years ago
|
||
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 7•21 years ago
|
||
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•21 years ago
|
||
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.
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•20 years ago
|
||
*** Bug 232012 has been marked as a duplicate of this bug. ***
*** Bug 233537 has been marked as a duplicate of this bug. ***
Comment 12•20 years ago
|
||
*** Bug 233663 has been marked as a duplicate of this bug. ***
Comment 13•20 years ago
|
||
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•20 years ago
|
||
*** Bug 250994 has been marked as a duplicate of this bug. ***
Comment 15•20 years ago
|
||
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•20 years ago
|
||
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•20 years ago
|
||
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•20 years ago
|
||
*** Bug 265575 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Product: Browser → Seamonkey
Comment 19•20 years ago
|
||
I filed this bug under Thunderbird. I don't know how it got changed to the suite.
Comment 20•20 years ago
|
||
(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•20 years ago
|
||
*** Bug 227327 has been marked as a duplicate of this bug. ***
Comment 22•19 years ago
|
||
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.
Updated•19 years ago
|
Assignee: sspitzer → mail
Comment 23•18 years ago
|
||
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•18 years ago
|
||
*** Bug 255521 has been marked as a duplicate of this bug. ***
Updated•18 years ago
|
Assignee: mail → nobody
Component: MailNews: Main Mail Window → MailNews: Backend
Product: Mozilla Application Suite → Core
QA Contact: esther → backend
Comment 25•18 years ago
|
||
*** Bug 271712 has been marked as a duplicate of this bug. ***
No longer blocks: 271712
Comment 26•18 years ago
|
||
*** Bug 355908 has been marked as a duplicate of this bug. ***
Comment 27•17 years ago
|
||
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 29•17 years ago
|
||
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•17 years ago
|
||
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.
Assignee | ||
Comment 31•17 years ago
|
||
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 33•17 years ago
|
||
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•17 years ago
|
||
(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 37•16 years ago
|
||
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 39•16 years ago
|
||
(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•16 years ago
|
||
3 as well
Comment 41•16 years ago
|
||
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.
Updated•16 years ago
|
Product: Core → MailNews Core
Updated•16 years ago
|
Flags: wanted-thunderbird3?
Comment 42•16 years ago
|
||
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•16 years ago
|
||
(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•16 years ago
|
||
(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•16 years ago
|
||
(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•16 years ago
|
||
(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•16 years ago
|
||
Changes: - moves to the proper inbox - [FIXED] message is being unjunked to the inbox even if it is in the inbox
Attachment #345537 -
Attachment is obsolete: true
Attachment #345803 -
Flags: review?(bugzilla)
Attachment #345537 -
Flags: review?(bugzilla)
Comment 48•16 years ago
|
||
(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•16 years ago
|
||
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).
Attachment #345803 -
Flags: ui-review?(clarkbw)
Attachment #345803 -
Flags: review?(bugzilla)
Attachment #345803 -
Flags: review?(bienvenu)
Assignee | ||
Comment 50•16 years ago
|
||
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...
Attachment #345803 -
Flags: review?(bienvenu) → review-
Comment 51•16 years ago
|
||
> 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•16 years ago
|
||
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.
Assignee | ||
Comment 53•16 years ago
|
||
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•16 years ago
|
||
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•16 years ago
|
||
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•16 years ago
|
||
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.
Attachment #345803 -
Attachment is obsolete: true
Attachment #347103 -
Flags: ui-review?(clarkbw)
Attachment #347103 -
Flags: review?(bienvenu)
Attachment #345803 -
Flags: ui-review?(clarkbw)
Comment 57•16 years ago
|
||
(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•16 years ago
|
||
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
Assignee | ||
Comment 59•16 years ago
|
||
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•16 years ago
|
||
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•16 years ago
|
||
(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•16 years ago
|
||
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•16 years ago
|
||
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•16 years ago
|
||
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•16 years ago
|
||
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.
Updated•16 years ago
|
Attachment #347103 -
Flags: ui-review?(clarkbw)
Attachment #347103 -
Flags: review?(bienvenu)
Comment 66•16 years ago
|
||
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•16 years ago
|
||
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?
Summary: If user marks a message as Not Junk, move message back to intended folder → If user marks a message as Not Junk, move message to inbox
Comment 68•16 years ago
|
||
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•16 years ago
|
||
Filed bug 466041, for moving the message to its intended folder (not the inbox)
Comment 70•16 years ago
|
||
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
Attachment #347103 -
Attachment is obsolete: true
Attachment #349399 -
Flags: ui-review?(clarkbw)
Comment 71•16 years ago
|
||
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.
Attachment #349399 -
Flags: ui-review?(clarkbw) → ui-review+
Assignee | ||
Comment 72•16 years ago
|
||
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
Attachment #349399 -
Flags: review-
Comment 73•16 years ago
|
||
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•16 years ago
|
||
(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 ?
Assignee | ||
Comment 75•16 years ago
|
||
> > 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•16 years ago
|
||
updated to comments
Attachment #349399 -
Attachment is obsolete: true
Attachment #349641 -
Flags: review?(bienvenu)
Comment 77•16 years ago
|
||
(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•16 years ago
|
||
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•16 years ago
|
||
> 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•16 years ago
|
||
Yes.
Comment 81•16 years ago
|
||
updated to comments
Attachment #349641 -
Attachment is obsolete: true
Attachment #349646 -
Flags: review?(bienvenu)
Attachment #349641 -
Flags: review?(bienvenu)
Assignee | ||
Comment 82•16 years ago
|
||
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•16 years ago
|
||
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•16 years ago
|
||
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.
Assignee | ||
Comment 85•16 years ago
|
||
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.
Assignee: firefox → bienvenu
Attachment #349646 -
Attachment is obsolete: true
Attachment #352627 -
Flags: superreview?(neil)
Attachment #352627 -
Flags: review?(bugzilla)
Attachment #349646 -
Flags: review?(bienvenu)
Assignee | ||
Comment 86•16 years ago
|
||
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...
Attachment #352627 -
Attachment is obsolete: true
Attachment #352650 -
Flags: superreview?(neil)
Attachment #352650 -
Flags: review?(bugzilla)
Attachment #352627 -
Flags: superreview?(neil)
Attachment #352627 -
Flags: review?(bugzilla)
Comment 87•16 years ago
|
||
(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...
Updated•16 years ago
|
Attachment #352650 -
Flags: superreview?(neil) → superreview+
Comment 88•16 years ago
|
||
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•16 years ago
|
||
Yup, but only that and firstuse use the mailnews.ui.junk scheme.
Comment 90•16 years ago
|
||
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•15 years ago
|
||
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.
Assignee | ||
Comment 93•15 years ago
|
||
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•15 years ago
|
||
(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.
Assignee | ||
Comment 95•15 years ago
|
||
The first thing I notice is that we aren't creating the imap junk folder when I mark a message as junk.
Assignee | ||
Comment 96•15 years ago
|
||
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•15 years ago
|
||
(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•15 years ago
|
||
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.
Assignee | ||
Comment 99•15 years ago
|
||
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•15 years ago
|
||
(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?
Assignee | ||
Comment 101•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
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.
Attachment #352650 -
Flags: review?(bugzilla)
Assignee | ||
Comment 104•15 years ago
|
||
fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago → 15 years ago
Flags: wanted-thunderbird3? → wanted-thunderbird3+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b2
Comment 105•15 years ago
|
||
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
Status: RESOLVED → VERIFIED
Comment 106•15 years ago
|
||
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.
Depends on: 465794
Comment 107•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
Ok, thats true. Sorry for the confusion. Removing dependency.
No longer depends on: 465794
You need to log in
before you can comment on or make changes to this bug.
Description
•