Closed Bug 368415 Opened 14 years ago Closed 12 years ago
if you only download message headers, mails which are recognised as junk (spam) lose their "junk" status if you download their content
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.
I have a similar issue with TBird version 126.96.36.199 (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.
This could be the same problem as bug 424876.
I can reproduce with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090619 Shredder/3.0b3pre
Status: UNCONFIRMED → NEW
Component: General → Filters
Ever confirmed: true
Product: Thunderbird → MailNews Core
QA Contact: general → filters
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?
in my testing, it was manual "J"
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.
Assignee: nobody → kent
Status: NEW → ASSIGNED
(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)
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!
(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.
I think this is relatet to bug #289011.
OS: Windows XP → All
Hardware: x86 → All
Target Milestone: --- → Thunderbird 3.0rc1
Version: unspecified → Trunk
Attachment #401755 - Attachment description: First cut, no known issues but needs more checking → The fix, for both the click-on-url case, and the download-from-selection case.
Attachment #401755 - Flags: superreview?(bienvenu)
Attachment #401755 - Flags: review?(bienvenu)
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.
Whiteboard: [no l10n impact] [needs r/sr bienvenu]
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 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
Whiteboard: [no l10n impact] [needs r/sr bienvenu] → [no l10n impact] [needs updated patch]
Attachment #406559 - Flags: approval-thunderbird3?
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.
Whiteboard: [no l10n impact] [needs updated patch] → [no l10n impact] [needs a+]
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 on attachment 406559 [details] [diff] [review] use "orig" instead of "old" in naming. a=me
Attachment #406559 - Flags: approval-thunderbird3? → approval-thunderbird3+
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.
Whiteboard: [no l10n impact] [needs a+] → [no l10n impact] [needs landing]
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
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [no l10n impact] [needs landing] → [no l10n impact]
You need to log in before you can comment on or make changes to this bug.