Closed Bug 193625 Opened 22 years ago Closed 21 years ago

Option to mark incoming junk mail as read

Categories

(MailNews Core :: Filters, enhancement)

enhancement
Not set
normal

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:
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
*** Bug 195436 has been marked as a duplicate of this bug. ***
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.
That's why the request is for an option.
mass re-assign.
Assignee: naving → sspitzer
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?
*** Bug 208481 has been marked as a duplicate of this bug. ***
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.

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.
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.
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.
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.
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 
*** Bug 232274 has been marked as a duplicate of this bug. ***
Attached patch proposed patch adding this functionality (obsolete) β€” β€” Splinter Review
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).
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.
 
> 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.
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.
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...
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 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 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.
(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 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-
Attached patch proposed patch v2 (obsolete) β€” β€” Splinter Review
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).
Attachment #140414 - Attachment is obsolete: true
Attachment #140825 - Flags: review?
Attached patch proposed patch v2 (obsolete) β€” β€” Splinter Review
sorry, wrong file...
Attachment #140825 - Attachment is obsolete: true
Attachment #140825 - Flags: review?
Attachment #140826 - Flags: review?
Comment on attachment 140826 [details] [diff] [review]
proposed patch v2

Setting specific reviewer.

Prog.
Attachment #140826 - Flags: review? → review?(bienvenu)
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.
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.
Attached patch proposed patch v3 (obsolete) β€” β€” Splinter Review
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.
Attachment #140826 - Attachment is obsolete: true
Attachment #140926 - Flags: review?(bienvenu)
Attachment #140826 - Flags: review?(bienvenu)
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-
Attached patch proposed patch v4 (obsolete) β€” β€” Splinter Review
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
Attachment #140969 - Flags: review?
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)
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 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 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.
(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.
Attached patch proposed patch v5 (obsolete) β€” β€” Splinter Review
Messeges manually marked as junk are now also marked read en-masse rather than
one-by-one.
Attachment #140969 - Attachment is obsolete: true
Attachment #140969 - Flags: review?(bienvenu)
Attachment #141410 - Flags: review?(bienvenu)
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+
Attachment #141410 - Flags: superreview?
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 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+
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?
In particular, I don't understand why (except one set is empty) the sets of
messages could be different.
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...
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?
(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.
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.
Attachment #141410 - Attachment is obsolete: true
Attachment #141410 - Flags: review?(neil.parkwaycc.co.uk)
Attached patch proposed patch v6 (obsolete) β€” β€” Splinter Review
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.
*** Bug 235523 has been marked as a duplicate of this bug. ***
Attached patch (hopefully) final patch (obsolete) β€” β€” Splinter Review
Attachment #145213 - Attachment is obsolete: true
Attachment #145458 - Flags: superreview?(bienvenu)
Attachment #145458 - Flags: review?(neil.parkwaycc.co.uk)
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+
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
Attached patch proposed patch v9 (obsolete) β€” β€” Splinter Review
Removed dump()'s and commented-out function, as per David's request.
Attached patch proposed patch v9 ( -uwbB version) (obsolete) β€” β€” Splinter Review
Attachment #145458 - Attachment is obsolete: true
Attachment #145509 - Flags: superreview?(bienvenu)
Attachment #145509 - Flags: review?(neil.parkwaycc.co.uk)
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+
Attachment #145509 - Flags: superreview?(bienvenu) → superreview+
Attachment #145458 - Flags: superreview?(bienvenu)
Fix checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
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.
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.
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.
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

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.
Attached patch diff against current cvs (obsolete) β€” β€” Splinter Review
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.
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...
Attached patch diff against current cvs (obsolete) β€” β€” Splinter Review
oops, the last one included a patch for a different bug...
Attachment #148775 - Attachment is obsolete: true
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)
Attachment #148776 - Attachment is obsolete: true
Attached patch proposed fix for batch start/end bug (obsolete) β€” β€” Splinter Review
Proposed patch with removal of redundancy in msMsgDBView.h, another short
comment in .h, and removed useless comment in .cpp .
Attachment #145509 - Attachment is obsolete: true
Attachment #145510 - Attachment is obsolete: true
Attachment #148774 - Attachment is obsolete: true
Attachment #148779 - Flags: superreview?(bienvenu)
Attachment #148779 - Flags: review?(neil.parkwaycc.co.uk)
If someone decides to check my fix in, please replace some tabs at the line:

if (mNumMessagesInBatch == 0)

and the following line. 

Thanks Andreas.
Attachment #148778 - Attachment is obsolete: true
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.
Attachment #148779 - Flags: superreview?(bienvenu)
Attachment #148779 - Flags: review?(neil.parkwaycc.co.uk)
Merged mNumMessagesInBatch and mCurrentIndexInBatch into a single
mNumMessagesLeftInBatch integer. Removed tabs. Fixed typo.
Attachment #148779 - Attachment is obsolete: true
Attachment #148956 - Flags: superreview?(bienvenu)
Attachment #148956 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #148956 - Flags: review?(neil.parkwaycc.co.uk) → review+
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)
> 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.
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.
> 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 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?
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 on attachment 148956 [details] [diff] [review]
fix for batch start/end bug v2

ok, great, thx.
Attachment #148956 - Flags: superreview?(sspitzer) → superreview+
I'll check it in later today, if Neil doesn't beat me to it.
checked in, thx, Eyal.
Bug 253234: This feature does not exist in Thunderbird.
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... :-(
no, it's just the front end for setting the option that needs to be ported...
Product: MailNews → Core
Blocks: 220933
No longer blocks: 220933
Blocks: 66425
> 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.
Product: Core → MailNews Core
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.

Attachment

General

Creator:
Created:
Updated:
Size: