Closed
Bug 193625
Opened 22 years ago
Closed 21 years ago
Option to mark incoming junk mail as read
Categories
(MailNews Core :: Filters, enhancement)
MailNews Core
Filters
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: trevor, Assigned: sspitzer)
References
Details
Attachments
(1 file, 15 obsolete files)
5.51 KB,
patch
|
neil
:
review+
Bienvenu
:
superreview+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3b) Gecko/20030211 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3b) Gecko/20030211 I tried out the junk mail control feature for the first time ever in Mozilla 1.3b, and MAN, this thing kicks a**! No spam at all for one whole week and counting! I've got it trained on about 800 spams and a couple thousand hams, and every time some spam comes into my inbox, Mozilla magically marks it as junk and moves it to the junk mail folder. There's only one problem. When a spam does come in, it's marked as "unread". That makes my junk mail folder switch to bold text, drawing my attention to it (as is the case with any Mozilla Mail folder containing unread messages). Trouble is, I don't *want* to pay attention to spam. I want to mark the spam as "read" and be done with it. There doesn't seem to be any option to do this in the Junk Mail Controls dialog box. As a workaround, I tried turning off the "Move incoming messages determined to be junk" option in Junk Mail Controls and instead set up a filter with the following rules: * Match all of the following: Junk Status - is - Junk * Perform this action: Move to folder - Junk * Perform this action: Mark the message as read This filter has no effect when enabled. Mozilla properly flags incoming spam as junk, but it doesn't move it and it doesn't mark it as read. I'm sure there are people out there who would agree with me that a "Mark incoming junk messages as read" option would be an excellent feature to have. (I've searched Bugzilla for previous RFEs on this, but could only find generic RFE bugs relating to the junk mail controls. I think this one's specific enough to warrant a separate bug number.) Reproducible: Always Steps to Reproduce:
Comment 1•22 years ago
|
||
I agree, junk messages being marked unread is distracting. Whenever I receive junk mail, the first thing I do is mark it as read. Marking NEW, CC'ing jglick.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 3•22 years ago
|
||
I personally like the existing behavior, since it allows me to quickly scan the newly filed mail in the Junk folder for false positives. Not many of those seem to pass through, but regardless, that is what I have found myself doing.
Comment 6•22 years ago
|
||
It would be interesting also to have a new condition under message filters: "Status - is - Junk". This would be a great improvement for whoever wants to perform complex actions on incoming messages, because it allows to label just the messages that are NOT spam too; not everybody moves spam in the junk folder, someone still keeps it in the inbox. Btw, is there anyone working on this? If not, is the "helpwanted" keyword appropriate?
I would love this too (The Bat! already has this). If this is turned on by the user, there shouldn't be a [sound|popup] notification either. It's rather annoying to get a "you got mail" sound, only to find that it was all spam.
Comment 9•22 years ago
|
||
This is a particular problem for IMAP mail users. The junk message is moved by Mozilla to the Junk folder, but not purged from the INBOX nor marked as "read" there. As a result, a mail monitor will still report "new messages" until the INBOX is purged. Perhaps this is a server-specific issue to a degree (reports deleted messages flagged new when asked for new message count), but it is definitely not ideal behaviour. Perhaps the best solution would be to purge deleted junk mail after it has been moved to the junk mailbox, but the option to mark it as read and 'to delete' is almost as good.
Comment 10•21 years ago
|
||
I agree with comment 3; leaving the messages marked Unread makes it easy to know that you should check your junk folder against false positives, before the mail gets automatically deleted. Preventing notification on junk (bug 189289, patch in progress) would remove most of the annoyance of new junk mail; I think if/when that is implemented, this bug's feature will be much less important. What I would like to see, however: mail that is *manually* marked as junk to be marked as read at the same time. There's a much lower chance of that mail being a false positive, and even if it were marked accidentally, the chance of losing the mail permanently is even less. Regarding Craig Ringer's comment 9: there are numerous bugs about IMAP and junk (e.g. bug 193199, bug 193229, bug 208950, bug 212176) and about IMAP and deletion; perhaps the complaint about messages being left in the inbox is better addressed by one of those. See also bug 210607.
Comment 11•21 years ago
|
||
I also would like to see this, as an option. I don't agree that bug 189289 makes this option less desirable. In fact, the bug 189289 feature together with the option to "mark junk mail as read" would really rock. I look through my junk folder once or twice a week, I don't need to be distracted everytime there's a new message there. See what I mean? Note, this doesn't have to be the default behaviour, but it would make my life happier if the possibility was there.
Comment 12•21 years ago
|
||
Currently, I have to open my junk mail folder every time I recieve junk, to mark it as read, and for some ungodly reason, either as part of Courier IMAP, or GkrellM's checking of it for new emails, any junk email gets my mail alarm stuck on 'new mail'. Admitidly it would be semi-useful to know that new junk mail had arrived, and might need checking at some point, but leaving it marked as unread leaves many other applications alerted and trying to poke the user to answer there email. It would be very very very nice to have the option to mark it as read, if only so I don't have to do it manually.
Comment 13•21 years ago
|
||
I agree. It would be nice to have this option, even if only in the prefs (without UI) for the beginning. However, I think that junk should be marked as read only when it is being moved to the Junk folder or deleted. Incoming junk which is left in Inbox should not be marked as read automatically. So in order to satisfy everyone we probably end up with two options: - mark incoming junk as read - mark junk as read when moved or deleted
Comment 15•21 years ago
|
||
This is the first patch I've ever written for Mozilla, and the first time I have actually dabbled in the source code, so caveat emptor. This adds a preference called 'markAsReadOnSpam' as part of the spam settings (similiar to 'moveOnSpam', for example), and the UI for setting it to the junk mail controls dialog. The setting is used on 2 occassions: when a message is classified by the filter to be spam, if 'markAsReadOnSpam' is set, the message is marked as read; when a message is manually marked as spam, if 'markReadAsSpam' is set, the message is marked as read. I have modified the code for the first occassion in both C++ and Javascript; as for the second occassion, I have not found a function which is run for manual marking of spam (maybe there isn't such a function), but have added an advisory comment to the function 'JunkSelectedMessages' in mozilla/mailnews/base/content/mailCommands.js I have tested the UI and the preference setting/unsetting, and the proper functioning of C++ code (i.e. got some junk mail and the automatic classification marked it as read when 'markAsReadOnSpam' was set, then took an unread non-spam message, manually flagged it as spam without reading it, and it was indeed marked read), but not the Javascript code since although I'm generally familiar with the Javascript syntax I have near-to-zero experience with writing JS scripts, for mozilla or otherwise. As per what exactly this patches against, I think it patches against the trunk (well, I used cvs diff -u etc etc blah blah , and what I had checked out seemed to be the trunk, at least in the sense that I didn't specify and branch), although I have also successfully applied it against the 1.6 source tarball (with the exception of mozilla/mailnews/mailnews.js which for some reason seems to be missing from my 1.6 source tarball - but this doesn't matter since this pertains only to the default pref value generation).
Comment 16•21 years ago
|
||
You know, come to think of it, the marking-as-read, which my patch solves, is only half the problem. I still feel unsatisfied... and I'll tell you why: on my system, there's a filter which moves messages from the e-mail account inbox to the local folders inbox. When spam arrives, the spam filter is not triggered; rather, all messages not filtered into other folders are moved into the local folders' inbox, including many messages that are spam, and only when I click my local folders' inbox is the spam filter triggered; it then marks the spam as spam, and now with my patch marks it also as read. But I still face an inbox with an invalid 'unread messages' number until I click it, as well as the 'messages received' taskbar notification. Perhaps I should file another bug asking that spam filtering be done on messages on arrival rather than on the first click on the folder.
Comment 17•21 years ago
|
||
> Perhaps I should file another bug asking that spam filtering be done on > messages on arrival rather than on the first click on the folder. There already is one: bug 200788. Fixing it is going to require a fairly extensive rewriting of the current junk controls; see also bug 223591, bug 202605. Also note that there is an RFE (bug 198961) that runs somewhat in the opposite direction, requesting that once a message is filtered, it be exempted from junk filtering, and another (bug 196964) to individually exempt folders from junk filtering.
Comment 18•21 years ago
|
||
Well, if anyone decides to move the junk filtering code to some other point in the life of an incoming message, if my patch is applied then they'll simply move the code with my own bit of code which also marks it as read (if necessary), and it'll work that way too.
Comment 19•21 years ago
|
||
Maybe I'm spamming this bug a little, but does anyone have any idea how I can get someone to review my patch? Nobody seems to be responding...
Comment 20•21 years ago
|
||
Eyal, to ask for review/superreview, edit the patch and set the flags. You can find more info here: http://www.mozilla.org/hacking/reviewers.html And if that doesn't help, the best place to ask would be #mozilla - irc://moznet/%23mozilla Prog.
Comment 21•21 years ago
|
||
Comment on attachment 140414 [details] [diff] [review] proposed patch adding this functionality personally i'd much rather be able to do it as in comment 6. is there anything preventing you from implementing it that way?
Attachment #140414 -
Flags: review?(bienvenu)
Comment 22•21 years ago
|
||
Comment 6's basic idea is: > It would be interesting also to have a new condition under message filters: > "Status - is - Junk". That's bug 196036. It is not currently feasible because junk status is not assigned to a message until JMC executes -- which happens after filter execution: bug 223591.
Comment 23•21 years ago
|
||
(In reply to comment #21) > personally i'd much rather be able to do it as in comment 6. > is there anything preventing you from implementing it that way? Yes, the fact that implementing this is a rather large-scale change of the mailnews backend, which, being a novice mozilla fixer-upper, I am not yet able to make. I of course agree that it would be a better solution, which would also solve the additional problem I described, as well as bug 196036 and others. Note that it would also render the 'moveOnSpam' option meaningless, since that too could be implemented as just another filter. But until this happens, my patch is more moderate fix.
Comment 24•21 years ago
|
||
Comment on attachment 140414 [details] [diff] [review] proposed patch adding this functionality this looks pretty good, except that it doesn't handle the imap case for new mail marked as junk. also, we don't use this as a method argument: nsCOMPtr <nsISpamSettings> spamSettings you just want to pass in an nsISpamSettings * here
Attachment #140414 -
Flags: review?(bienvenu) → review-
Comment 25•21 years ago
|
||
I haven't figured out whether I can actually overwrite a patch or not. So here's a second version of the patch. This removes a redundant marking-as-read I had and adds support for IMAP folders. I tested it with an IMAP folder, but only the mark-spam-as-read-when-manual-marking-as-spam functionality, not the mark-spam-as-read-when-bayesian-filters-marks-as-spam functionality. I have also not tested the Javascript code at all, and I have no idea whether it works (especially now that I'm doing more well-meaning-but-maybe-misguided trickery in there). Finally, I am not very well versed in the memory management instrumentation, so that may need to be looked over, and there's the issue of Biff, which I don't think my patch should address but it's not impossible that there's some linkage which you are welcome to make me aware of (actually I'd rather you didn't since that way I'll keep the belief that my work is done).
Updated•21 years ago
|
Attachment #140414 -
Attachment is obsolete: true
Updated•21 years ago
|
Attachment #140825 -
Flags: review?
Updated•21 years ago
|
Attachment #140825 -
Flags: review?
Updated•21 years ago
|
Attachment #140826 -
Flags: review?
Comment 27•21 years ago
|
||
Comment on attachment 140826 [details] [diff] [review] proposed patch v2 Setting specific reviewer. Prog.
Attachment #140826 -
Flags: review? → review?(bienvenu)
Comment 28•21 years ago
|
||
I have successfully tested my v2 patch with IMAP folders, for both types of functionality (i.e. now also mark-spam-as-read-when-the-bayesian-filter-marks-as-spam). The problem (not my problem) is that for servers with high delay it takes quite a long time before such changes become apparent.
Comment 29•21 years ago
|
||
Can you expand (in the bug) on this comment? What happens? It worries me a little. + // (for some reason the folder doesn't like the msgHdr + // we got from the URI) You can remove this comment: //this next line may not work for IMAP: //aMsgHdr.markRead(true); // so, instead, let's try this: In these two places, I'd remove the NS_ENSURE_SUCCESS(rv, rv), because that aborts the whole process, needlessly, by returning out of the function. + messageArray->AppendElement(messageISupports); + rv = srcFolder->MarkMessagesRead(messageArray, true); + NS_ENSURE_SUCCESS(rv,rv); + rv = mDatabase->MarkRead(msgKey, true, this); + NS_ENSURE_SUCCESS(rv, rv); + } Re biff, since we ignore junk mail for the purposes of biff, I don't think your patch should cause problems.
Comment 30•21 years ago
|
||
That's funny... I was sure the msgHdr had been causing the MarkMessagesRead to fail; I guess it must have been something else I had been doing wrong. Anyway, it seems that the way the code is now, the marking as read works just fine with the msgHdr obtained from the URI, in both local and IMAP folders. So I removed the extra header-retrieving code. Also replaced 2 NS_ENSURE_SUCCESS 's with if !NS_SUCCEEDED NS_WARNING("etc") 's, removed a useless comment, and fixed a bug where if you had manual marking disabled, the markAsReadOnSpam code would not be reached.
Updated•21 years ago
|
Attachment #140826 -
Attachment is obsolete: true
Updated•21 years ago
|
Attachment #140926 -
Flags: review?(bienvenu)
Updated•21 years ago
|
Attachment #140826 -
Flags: review?(bienvenu)
Comment 31•21 years ago
|
||
Comment on attachment 140926 [details] [diff] [review] proposed patch v3 > + if (!NS_SUCCEEDED(rv)) don't do !NS_SUCCEEDED use NS_FAILED. similarly, don't do !NS_FAILED, use NS_SUCCEEDED.
Attachment #140926 -
Flags: review?(bienvenu) → review-
Comment 32•21 years ago
|
||
timeless: done, but someone should say this in some FAQ, instead of having novice patchmakers re-submit due to such trivialities. I'll mark as ? without requestee.
Attachment #140926 -
Attachment is obsolete: true
Updated•21 years ago
|
Attachment #140969 -
Flags: review?
Comment 33•21 years ago
|
||
Comment on attachment 140969 [details] [diff] [review] proposed patch v4 oh, there's nothing wrong w/ asking bienvenu for the r+. I can only give r- :)
Attachment #140969 -
Flags: review? → review?(bienvenu)
Comment 34•21 years ago
|
||
I agree with comment 3 that junk should be automaticly mark as read incase the wrong messages get marked as junk. However what about marking all junk messages as read on exit? Because of the date not all messages get displayed in the right order therefor making it hard to distinguish which mails newly arrived and got marked as junk. If all junk messages get marked as read when you exit Mozilla mail you know which new messages got marked as junk the next time your receive new email.
Comment 35•21 years ago
|
||
comment 34 is a request for the feature of doing something at exit with messages received in this session. That's much more general in scope than this bug.
Comment 36•21 years ago
|
||
Comment on attachment 140969 [details] [diff] [review] proposed patch v4 If you want this to perform well on imap, you'll need to save off the messages to mark as read, and marking them all read at once when done, instead of marking them read one by one. So both mailCommands.js's saveJunkMsgForAction and nsMsgDBView::SaveJunkMsgForAction should be extended to build up an array of message(s) to be marked read, like it does for messages to be marked junk, and do it at the end.
Comment 37•21 years ago
|
||
(In reply to comment #34) Well, that seems like another feature to me. The reason I want the "mark junk mail as read" option is because I don't _want_ to know when new messages get marked as junk. Mozilla now gets it right over 99% of the time and I want the spam to silently enter the junk folder. About once a week I browse through the junk folder (no false positives so far) and empty it.
Comment 38•21 years ago
|
||
Messeges manually marked as junk are now also marked read en-masse rather than one-by-one.
Updated•21 years ago
|
Attachment #140969 -
Attachment is obsolete: true
Updated•21 years ago
|
Attachment #140969 -
Flags: review?(bienvenu)
Updated•21 years ago
|
Attachment #141410 -
Flags: review?(bienvenu)
Comment 39•21 years ago
|
||
Comment on attachment 141410 [details] [diff] [review] proposed patch v5 looks good - thx for doing this. And thx for changing the name of that method to Actions :-)
Attachment #141410 -
Flags: review?(bienvenu) → review+
Updated•21 years ago
|
Attachment #141410 -
Flags: superreview?
Comment 40•21 years ago
|
||
Although I've asked for superreview, I still would like someone with Javascript experience to help me test the JS changes I've made, before this patch is considered for checkin. Any volunteers - please e-mail me.
Comment 41•21 years ago
|
||
Comment on attachment 141410 [details] [diff] [review] proposed patch v5 I'll switch my review to a sr and ask Neil for a review.
Attachment #141410 -
Flags: superreview?
Attachment #141410 -
Flags: review?(neil.parkwaycc.co.uk)
Attachment #141410 -
Flags: review+
Comment 42•21 years ago
|
||
I don't understand why a) the JS version marks each message read individually b) the C++ version keeps track of two arrays, one for messages that are to be moved, and one for messages that are to be marked as read... surely you only need one array, and then decided afterwards whether to mark them as read and/or move them?
Comment 43•21 years ago
|
||
In particular, I don't understand why (except one set is empty) the sets of messages could be different.
Comment 44•21 years ago
|
||
if you're not moving junk mail to the junk folder, one will be empty - so I guess the code at the end needs to check what the action is as well...
Comment 45•21 years ago
|
||
Neil: It's true that currently the sets are either the same or one of them is empty. But when I wrote the patch I was assuming (falsely, for now) that we might be handling messages not all from the same folder. Otherwise the entire SaveJunkMsgForAction/s is useless, because either you save all of the messages or none of them. The code sort of lends itself to my assumption... do you want OnMessageClassified rewritten so as to be exactly suited for only handling bunches of messages all from the same folder?
Comment 46•21 years ago
|
||
(In reply to comment #45) >when I wrote the patch I was assuming (falsely, for now) that we might be >handling messages not all from the same folder. It doesn't matter. Just collect up all the messages, then at the end, decide whether to ApplyCommandToIndices( nsMsgViewCommandType::markMessagesRead, indices, numIndices ); and/or the existing move/delete code as appropriate.
Comment 47•21 years ago
|
||
Just in case anyone was wondering, the fix for bug 181631 seems to have no impact on this bug and on the proposed fix. The fix is also invalid, as I have taken Neil's criticism into account and have sent him a new proposed patch for the nsMsgDBView part of the fix, and am awaiting his reply.
Updated•21 years ago
|
Attachment #141410 -
Attachment is obsolete: true
Attachment #141410 -
Flags: review?(neil.parkwaycc.co.uk)
Comment 48•21 years ago
|
||
So as not to keep 'the public' out of the loop, here is a new patch with... shall we say... better quality changes in nsMsgDBView.* and mailCommands.js than the previous ones, and which is basically what I expect to be approved, and is also tested to a reasonable extent, but may not be what is eventually approved, so you might see a v7.
Updated•21 years ago
|
Attachment #145458 -
Flags: superreview?(bienvenu)
Attachment #145458 -
Flags: review?(neil.parkwaycc.co.uk)
Comment 51•21 years ago
|
||
Comment on attachment 145458 [details] [diff] [review] (hopefully) final patch This one has the same filename as the one I was sent that passed testing ;-)
Attachment #145458 -
Flags: review?(neil.parkwaycc.co.uk) → review+
Comment 52•21 years ago
|
||
can you do a -uw diff, to remove the changes that are just whitespace. It will make it easier for me to review. When it comes time to checkin, we can apply the full diff with the whitespace changes. please remove the dump statements like: + dump('markRead = ' + actionParams.markRead + ' ; junkTargetFolder = ' + actionParams + '\n'); and the commented out dump statements... and this commented out code. We have cvs history to recover stuff like this. +// this will need re-writing if it is to be used +//function analyzeSelectedMessagesForJunk() +//{ +// var messages = GetSelectedMessages(); +// analyzeMessagesForJunk(messages); +//} thx, - David
Updated•21 years ago
|
Attachment #145509 -
Flags: superreview?(bienvenu)
Attachment #145509 -
Flags: review?(neil.parkwaycc.co.uk)
Comment 55•21 years ago
|
||
Comment on attachment 145509 [details] [diff] [review] proposed patch v9 [You don't need rereview for comment/dump removal]
Attachment #145509 -
Flags: review?(neil.parkwaycc.co.uk) → review+
Updated•21 years ago
|
Attachment #145509 -
Flags: superreview?(bienvenu) → superreview+
Updated•21 years ago
|
Attachment #145458 -
Flags: superreview?(bienvenu)
Comment 56•21 years ago
|
||
Fix checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 57•21 years ago
|
||
this checkin had some bad line endings. see my follow up checkin to nsMsgDBView.cpp (1.176) It's not a big deal in .h / .cpp files, but I think it can cause problems in .xul files.
Comment 58•21 years ago
|
||
mscott: you have that backwards, it's the .cpp/.h files where line endings matter. .xml files don't care about whitespace in general, and as long as there's a line ending in a js file, js is happy.
Comment 59•21 years ago
|
||
Sorry about that, I didn't check the patch line endings before checking in, and when I did check in, and promptly conflicted with my other tree, I lost my build machine for 12 hours and forgot what the conflict was when it came back up.
Comment 60•21 years ago
|
||
I think this patch caused a pretty nasty bug with junk mail controls where stop writing out training changes because it introduces an unmatched call to junkPlugin->Batch without a matching EndBatch call. See Bug #243680
Comment 61•21 years ago
|
||
this is not a patch, just a diff between two files until I get my codebase up-to-date, rebuild, and am able to actually cvs diff something meaningful. The change should fix the batch start/end mismatch which is implicated in bug 243680.
Comment 62•21 years ago
|
||
I manually entered your changes and made a diff against current trunk. After verifying my changes are correct, you can probably mark your one as obsolete, I'll not do it just yet. Rebuilding.
Comment 63•21 years ago
|
||
Andreas, I appreciate the effort, but: - I haven't listed the requisite change of nsMsgDBView.h - your diff against the CVS has changes to nsMsgIncomingServer.cpp which are irrelevant...
Comment 64•21 years ago
|
||
oops, the last one included a patch for a different bug...
Attachment #148775 -
Attachment is obsolete: true
Comment 65•21 years ago
|
||
Ok, sorry for the confusion. I just rebuilt your proposal and it seems to work fine, write access to training.dat does happen. However, now that I'm already spamming all of you with this comment I figured I could also add the current patch including the nsMsgDBView.h changes without causing additional annoyance... (Not sure if you want to add more comments or kill some of the commented-out code, though)
Updated•21 years ago
|
Attachment #148776 -
Attachment is obsolete: true
Comment 66•21 years ago
|
||
Proposed patch with removal of redundancy in msMsgDBView.h, another short comment in .h, and removed useless comment in .cpp .
Updated•21 years ago
|
Attachment #145509 -
Attachment is obsolete: true
Attachment #145510 -
Attachment is obsolete: true
Attachment #148774 -
Attachment is obsolete: true
Updated•21 years ago
|
Attachment #148779 -
Flags: superreview?(bienvenu)
Attachment #148779 -
Flags: review?(neil.parkwaycc.co.uk)
Comment 67•21 years ago
|
||
If someone decides to check my fix in, please replace some tabs at the line: if (mNumMessagesInBatch == 0) and the following line. Thanks Andreas.
Updated•21 years ago
|
Attachment #148778 -
Attachment is obsolete: true
Comment 68•21 years ago
|
||
Comment on attachment 148779 [details] [diff] [review] proposed fix for batch start/end bug I don't like the way you reset mCurrentIndexInBatch at a different time to mNumMessagesInBatch. In fact I think using a single variable mNumMessagesRemainingInBatch would have made the logic clearer. Also I'm not sure you're allowed to put an NS_ASSERTION as the only statement of an if.
Updated•21 years ago
|
Attachment #148779 -
Flags: superreview?(bienvenu)
Attachment #148779 -
Flags: review?(neil.parkwaycc.co.uk)
Comment 69•21 years ago
|
||
Merged mNumMessagesInBatch and mCurrentIndexInBatch into a single mNumMessagesLeftInBatch integer. Removed tabs. Fixed typo.
Attachment #148779 -
Attachment is obsolete: true
Updated•21 years ago
|
Attachment #148956 -
Flags: superreview?(bienvenu)
Attachment #148956 -
Flags: review?(neil.parkwaycc.co.uk)
Updated•21 years ago
|
Attachment #148956 -
Flags: review?(neil.parkwaycc.co.uk) → review+
Comment 70•21 years ago
|
||
Comment on attachment 148956 [details] [diff] [review] fix for batch start/end bug v2 I think Seth had a reason for using the uri but I don't remember what the scenario was...requesting sr from him
Attachment #148956 -
Flags: superreview?(bienvenu) → superreview?(sspitzer)
Assignee | ||
Comment 71•21 years ago
|
||
> I think Seth had a reason for using the uri but I don't remember what the
> scenario was...requesting sr from him
sorry for the delay. let me look over the code and see if I can remember why I
did that.
Assignee | ||
Comment 72•21 years ago
|
||
actually, it was dmose who originally wrote that code: see http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsMsgDBView.cpp&branch=&root=/cvsroot&subdir=mozilla/mailnews/base/src&command=DIFF_FRAMESET&rev1=1.121&rev2=1.122 from what I can tell from his original checking on 10/24/02, it was to handle coalesing of junk batches. if that still works, then I think we are safe.
Comment 73•21 years ago
|
||
> from what I can tell from his original checking on 10/24/02, it was to handle
> coalesing of junk batches.
>
> if that still works, then I think we are safe.
Yes, it does work (I have replaced the use of the URI with the simpler use of
index counting which achieves the same result). BTW, As I mentioned in one of
the patch comments, I think junk batches should not be coalesced! The plugin
interface should be such that EndBatch is called after the last message is
dispatched for classification, not after it is classified; and the plugin should
have its own policy of when to write its data file to the HDD.
Comment 74•21 years ago
|
||
Comment on attachment 148956 [details] [diff] [review] fix for batch start/end bug v2 I wonder if the uri was used in case the user got two or more junk classifications going at the same time, e.g., select 10 messages, mark as junk, and while that's going on, select 10 more and repeat. What happens with your patch in that scenario?
Comment 75•21 years ago
|
||
Each time the user selects some messages and marks them as junk, ApplyCommandToIndices() is called with the new batch. Its pre-patch behavior was to replace the previous 'URI of last message to be classified' (in case the last batch had not yet been completed) with the last URI of the current batch - thus the batches were 'coalesced'. What we do now is achieve the same effect by adding the number of messages in the new batch to the 'remaining number of indices'. Each time a message is classified this number decreases by 1, and when it hits zero (rather than when the message with the 'URI of last message to be classified' is classified), we perform the necessary actions on the messages, and call the plugin's EndBatch().
Comment 76•21 years ago
|
||
Comment on attachment 148956 [details] [diff] [review] fix for batch start/end bug v2 ok, great, thx.
Attachment #148956 -
Flags: superreview?(sspitzer) → superreview+
Comment 80•20 years ago
|
||
What, are you kidding me? I thought Thunderbird and Seamonkey MailNews shared all the backend code! I need to port this now? Damn, I don't even know how to build Thunderbird... :-(
Comment 81•20 years ago
|
||
no, it's just the front end for setting the option that needs to be ported...
Updated•20 years ago
|
Product: MailNews → Core
Comment 82•18 years ago
|
||
> Well, that seems like another feature to me. The reason I want the "mark junk
> mail as read" option is because I don't _want_ to know when new messages get
> marked as junk. Mozilla now gets it right over 99% of the time and I want the
> spam to silently enter the junk folder. About once a week I browse through the
> junk folder (no false positives so far) and empty it.
That's exactly what I'd like to do. Come on, just make it work before final TB2.
Updated•16 years ago
|
Product: Core → MailNews Core
Comment 83•16 years ago
|
||
It seems in recent trunk builds junk gets moved to the junk folder without being marked read. The pref is also gone from the dialogs for the different accounts. Intentional? Accidental?
You need to log in
before you can comment on or make changes to this bug.
Description
•