Last Comment Bug 368415 - if you only download message headers, mails which are recognised as junk (spam) lose their "junk" status if you download their content
: if you only download message headers, mails which are recognised as junk (spa...
Status: RESOLVED FIXED
[no l10n impact]
: fixed-seamonkey2.0.1
Product: MailNews Core
Classification: Components
Component: Filters (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 3.0rc1
Assigned To: Kent James (:rkent)
:
:
Mentors:
: 510690 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-27 07:07 PST by Snap
Modified: 2009-12-13 06:04 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
The fix, for both the click-on-url case, and the download-from-selection case. (5.44 KB, patch)
2009-09-20 15:34 PDT, Kent James (:rkent)
mozilla: review+
mozilla: superreview+
Details | Diff | Splinter Review
use "orig" instead of "old" in naming. (7.76 KB, patch)
2009-10-15 16:05 PDT, Kent James (:rkent)
mozilla: approval‑thunderbird3+
Details | Diff | Splinter Review
Rev d: add NS_ENSURE_ARG_POINTER tests (7.83 KB, patch)
2009-10-21 10:51 PDT, Kent James (:rkent)
no flags Details | Diff | Splinter Review

Description Snap 2007-01-27 07:07:19 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070118 Minefield/3.0a2pre
Build Identifier: Mozilla Thunderbird 2.0 beta 2 (20070116)

If you have downloaded message headers identified as junk, the junk status is set to "not junk" if you download the entire message.

Reproducible: Always

Steps to Reproduce:
1. Account settings: fetch headers only (and leave messages on server)
2. Download headers

=> Thunderbird identify junk mail

3. Click on a junk mail. You can not see its content because it is not downloaded. Click on the link to download it (pop://yyy:110/?uidl=xxx).

=> The status of the mail is no longer "junk". Remote images can be loaded.


Expected Results:  
The junk status should not change.
Comment 1 Robin Bankhead 2007-04-07 03:39:54 PDT
I have a similar issue with TBird version 1.5.0.10 (20070221) / Linux -- has been happening for all the 1.5.* versions.

I don't have any of my accounts set to "fetch headers only", though.  Spam is always filtered to the specified folder (I have it set to filter into 'Deleted' on each account's respective folderspace).  However, most of the time (at least 70% of mail retrievals, as an estimate, and I think it's getting more frequent) after filtering they lose their 'Junk' status.

It's interesting that this doesn't happen every time.  Also I should note that spam that gets past the filter, when manually marked (and automatically moved), keeps its mark.
Comment 2 Kent James (:rkent) 2008-03-30 21:16:56 PDT
This could be the same problem as bug 424876.
Comment 3 Wayne Mery (:wsmwk, NI for questions) 2009-06-22 11:26:40 PDT
I can reproduce with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090619 Shredder/3.0b3pre
Comment 4 Kent James (:rkent) 2009-06-22 11:34:53 PDT
I don't understand the step "=> Thunderbird identify junk mail" in the STR. Is that the user marking the message as junk? Or the bayes filter running and determining the message to be junk?
Comment 5 Wayne Mery (:wsmwk, NI for questions) 2009-06-22 11:44:54 PDT
in my testing, it was manual "J"
Comment 6 Kent James (:rkent) 2009-06-22 12:06:49 PDT
The filter plugin code does not filter for junk if junk status is already set. So this sounds like one of the class of bugs where the database information is being blown away.

Does the same thing happen to tags?

Anyway, I'll look at it.
Comment 7 Wayne Mery (:wsmwk, NI for questions) 2009-06-22 12:15:10 PDT
(In reply to comment #6)
> The filter plugin code does not filter for junk if junk status is already set.
> So this sounds like one of the class of bugs where the database information is
> being blown away.
> 
> Does the same thing happen to tags?

yea, tag is lost also. (like happened in bug 440366)
Comment 8 Adrian Zaugg 2009-07-22 20:42:38 PDT
The junk status gets also lost, when you mark a bunch of messages manually as junk and you have "move them to the account's junk folder" set, recollect those messages and move them back to the original folder (or any other). It doesn't matter if you reset the training data before.

I think in some cases do get the messages reevaluated and the manually set (or sometimes even unset!) junk state changes gets overriden. A manually set or unset junk states should never be changed automatically.

The story gets probably worse to sove when mentionning the situation where you use serveral Thunderbirds on different computers to the same IMAP account. 

This behavior has been in every Thunderbird so far. Thank you for correcting!
Comment 9 Snap 2009-07-23 10:08:41 PDT
(In reply to comment #4)
> I don't understand the step "=> Thunderbird identify junk mail" in the STR. Is
> that the user marking the message as junk? Or the bayes filter running and
> determining the message to be junk?

Hello Kent, as you may have noticed, I meant "the bayesian filter mark some mails as junk". I did not try to mark mails manually.
Comment 10 Adrian Zaugg 2009-07-27 12:35:38 PDT
I think this is relatet to bug #289011.
Comment 11 Kent James (:rkent) 2009-08-15 12:58:13 PDT
*** Bug 510690 has been marked as a duplicate of this bug. ***
Comment 12 Kent James (:rkent) 2009-09-20 15:34:34 PDT
Created attachment 401755 [details] [diff] [review]
The fix, for both the click-on-url case, and the download-from-selection case.
Comment 13 Kent James (:rkent) 2009-09-24 00:00:02 PDT
Comment on attachment 401755 [details] [diff] [review]
The fix, for both the click-on-url case, and the download-from-selection case.

I checked again, and it seems to be fine. This is part of my series of bugs to protect database properties.
Comment 14 Mark Banner (:standard8) 2009-09-30 03:23:07 PDT
Comment on attachment 401755 [details] [diff] [review]
The fix, for both the click-on-url case, and the download-from-selection case.

Cancelling approval request until this has review (so I can keep track of the queue more easily).

Either bienvenu can set when he reviews the patch or please re-request.

Please also include a statement of risk when requesting approvals.
Comment 15 David :Bienvenu 2009-10-15 11:59:47 PDT
Comment on attachment 401755 [details] [diff] [review]
The fix, for both the click-on-url case, and the download-from-selection case.

thx for the patch, makes sense.

I think instead of oldMessageUri, I'd prefer origMessageUri

This is also for a partially downloaded message as well, right?

+  /// message uri for header-only message version
Comment 16 Kent James (:rkent) 2009-10-15 16:05:00 PDT
Created attachment 406559 [details] [diff] [review]
use "orig" instead of "old" in naming.
Comment 17 Kent James (:rkent) 2009-10-15 16:14:48 PDT
I tried to understand the risks of this bug. I cannot say it is risk free. It relies on assumptions about pop3 urls, that the "uidl=" string will only be included in urls that refer to a single message. Bienvenu would be a better candidate to discuss risk of that.

I think the risk of new crashes is very low. The main risk, if any, is that I misunderstood the nature of pop3 URLs, and will try to update message headers with the message header from another message. But I really think Bienvenu needs to evaluate this. I mainly did this patch as part of my series to remove all occurrances where the message headers were being blown away as part of normal message processing.
Comment 18 David :Bienvenu 2009-10-16 11:05:01 PDT
afaik, the uidl is only set when we're trying to do a partial to a full download, so I think we're safe there.

+NS_IMETHODIMP nsMsgLocalMailFolder::UpdateNewMsgHdr(nsIMsgDBHdr* aOldHdr, nsIMsgDBHdr* aNewHdr)

could use NS_ENSURE_ARG_POINTERs on aOldHdr and aNewHdr, since it's an idl method.

I believe this is fairly safe.
Comment 19 David :Bienvenu 2009-10-16 11:06:33 PDT
Comment on attachment 406559 [details] [diff] [review]
use "orig" instead of "old" in naming.

a=me
Comment 20 Kent James (:rkent) 2009-10-18 20:53:38 PDT
As I am traveling and away from my development environment at the moment, I won't be able to land this until Wednesday, October 20. AFAIK that is OK, but if there are issues with that then please contact me.
Comment 21 Kent James (:rkent) 2009-10-21 10:51:42 PDT
Created attachment 407562 [details] [diff] [review]
Rev d: add NS_ENSURE_ARG_POINTER tests

This adds the tests requested in comment 18.
Comment 22 Kent James (:rkent) 2009-10-21 11:20:20 PDT
Comment on attachment 407562 [details] [diff] [review]
Rev d: add NS_ENSURE_ARG_POINTER tests

Pushed as http://hg.mozilla.org/comm-central/rev/e747fcd38477

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