Closed
Bug 505487
Opened 15 years ago
Closed 15 years ago
Message filter rule incorrectly matching/firing
Categories
(MailNews Core :: Filters, defect)
MailNews Core
Filters
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.0b4
People
(Reporter: mike, Assigned: Bienvenu)
References
Details
(Keywords: regression)
Attachments
(5 files, 1 obsolete file)
64.07 KB,
image/png
|
Details | |
162.20 KB,
image/png
|
Details | |
1.28 KB,
patch
|
standard8
:
review+
standard8
:
superreview+
|
Details | Diff | Splinter Review |
1.22 KB,
patch
|
standard8
:
review+
standard8
:
superreview+
|
Details | Diff | Splinter Review |
7.73 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1) Gecko/20090715 Firefox/3.5.1 (.NET CLR 3.5.30729) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.2pre) Gecko/20090721 Shredder/3.0b4pre I have a long-working message filter that started misbehaving with last night's nightly build (Jul 20/21). The filter contains two match patterns: "Match any of the following": Sender ends with slimdevices.com From contains slimdevices.com All messages since taking the nightly download started matching this rule (resulting in all my new messages being moved into another folder). When I ran the Message Filter manually on my Inbox, *all* of my Inbox items were moved (i.e. the rule incorrectly matched everything). Reproducible: Always Actual Results: Filter incorrectly matching anything. Expected Results: Filter should only match the given criteria.
Additional info. When I edit the rule, make no changes, but click OK to leave dialog, I get an error dialog: "This filter cannot be saved because some search terms are invalid in the current context."
Yet more info: it appears that Sender, while available as a choice, is no longer valid, and creating a rule with this criteria will cause the error dialog indicated in comment #1. So, there are two bugs here: 1) Choosing Sender in a message filter causes an error 2) Message Filter criteria which contain problematic rules result in a TRUE outcome, causing the message rule to fire. This should be changed such that problematic rules do not fire.
Updated•15 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
OS: Windows XP → All
Hardware: x86 → All
Version: unspecified → Trunk
Comment 3•15 years ago
|
||
I seem to be getting something very similar to this w/ my filter(s).
Assignee | ||
Comment 4•15 years ago
|
||
hmm, I hope the filter enumerator changes I made to search didn't cause this somehow...
Updated•15 years ago
|
Component: General → Filters
OS: All → Windows XP
Product: Thunderbird → MailNews Core
QA Contact: general → filters
Hardware: All → x86
Assignee | ||
Comment 5•15 years ago
|
||
ah, the issue is the begins with/ends with (contains, doesn't contain works fine with sender).
Updated•15 years ago
|
Comment 6•15 years ago
|
||
This popped up on me as well on MacOSX with last nights build. Screenshot show the "hidden?" SpamAssassinYes filter that is triggered from "Trust junk headers set by:" option in Account Settings. Second screenshot shows a filter that I use to auto-tag. Turned off "Trust junk headers" and edited my auto-tagging filter a few times (and made the filter screen bigger, which actually seemed to fix it for who knows what reason) so things are working fine for me right now.
Comment 7•15 years ago
|
||
Assignee | ||
Comment 8•15 years ago
|
||
I think the filter editor complaining about Sender is a regression from bug 495519, which is in b3, scarily. It looks to me like the FilterEditor code just reads off the end of the validity table for custom headers.
FYI: I created a new filter rule, w/o Sender (used From only), and the rule is firing incorrectly.
Reporter | ||
Comment 10•15 years ago
|
||
Scratch comment #9.
Comment 11•15 years ago
|
||
(In reply to comment #8) > I think the filter editor complaining about Sender is a regression from bug > 495519, which is in b3, scarily. It looks to me like the FilterEditor code just > reads off the end of the validity table for custom headers. The other alternative would be the filtered enumerator (as you suggested before) from bug 499806. We did change the nsMsgSearchAttrib::OtherHeader value so there could be something hard-coded somewhere.
Comment 12•15 years ago
|
||
I just did a local back out of bug 499806... there were various emails I got earlier that just wouldn't be marked as non-junk (they kept leaping back into junk folder regardless of what I did), backing out bug 499806 has resolved that, they stay in the inbox now.
Assignee | ||
Comment 13•15 years ago
|
||
I've just been looking at the filter editor issue because I'm trying to set up a filter like MrC did - that may very well be a separate issue, but it's the one I ran into first. My from filter works fine...
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → bienvenu
Assignee | ||
Comment 14•15 years ago
|
||
this fixes the filter editor regression, which, as I suspected, is separate from the filter execution error. I've reproduced that issue now, so I should be able to get a fix for that soon. The bug fixed in this patch is that the validity tables treat all custom headers as "Other", so we need to reduce "value" to "Other" in order to use the validity tables. Otherwise, we're just reading off the end of the table, and getting random results.
Attachment #389789 -
Flags: superreview?(bugzilla)
Attachment #389789 -
Flags: review?(bugzilla)
Assignee | ||
Comment 15•15 years ago
|
||
ok, this fixes the filter execution error for me - the issue was limited to begins and ends with, for custom headers, I believe. I'll go look at the test cases and see why this wasn't covered.
Attachment #389793 -
Flags: superreview?(bugzilla)
Attachment #389793 -
Flags: review?(bugzilla)
Assignee | ||
Comment 16•15 years ago
|
||
Comment on attachment 389793 [details] [diff] [review] proposed fix oh, the bufLength fix is unrelated, but it quiets an assertion I was seeing where we were calling SetLength(-1) on a CString when doing a body search.
Reporter | ||
Comment 17•15 years ago
|
||
Thanks for the fast work on this. I'll check out the results with the next build that contains the fix.
Assignee | ||
Comment 18•15 years ago
|
||
prev body handling part of patch was wrong so I'm just removing it...
Attachment #389793 -
Attachment is obsolete: true
Attachment #389811 -
Flags: superreview?(bugzilla)
Attachment #389811 -
Flags: review?(bugzilla)
Attachment #389793 -
Flags: superreview?(bugzilla)
Attachment #389793 -
Flags: review?(bugzilla)
Updated•15 years ago
|
Flags: in-testsuite?
Updated•15 years ago
|
Attachment #389789 -
Flags: superreview?(bugzilla)
Attachment #389789 -
Flags: superreview+
Attachment #389789 -
Flags: review?(bugzilla)
Attachment #389789 -
Flags: review+
Updated•15 years ago
|
Attachment #389811 -
Flags: superreview?(bugzilla)
Attachment #389811 -
Flags: superreview+
Attachment #389811 -
Flags: review?(bugzilla)
Attachment #389811 -
Flags: review+
Comment 19•15 years ago
|
||
I checked both of these patches in together: http://hg.mozilla.org/comm-central/rev/1e5e809179cb (in time for today's Thunderbird nightly). Forgot to say, would be good to extend/create a unit test to cover this bug.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b4
Comment 20•15 years ago
|
||
Had to build it myself since the MacOSX nightly is slow in coming, however this FIXED the problem for me. I will say that the "SpamAssassinYes" filter that is supposed to be hidden still shows up in my filter list, and when I go to Edit it, I still have the two blank "Subject" matches before the ***SPAM*** Subject match. I attached an image of what it looks like yesterday, still looks the same today. Looks VERY weird and confusing, but messages are not automatically going to spam folder any longer.
Reporter | ||
Comment 21•15 years ago
|
||
Filters seem to be working correctly now. Thanks!
Assignee | ||
Comment 22•15 years ago
|
||
this adds some basic search tests, including ones that would have caught this regression. There are a lot of tests we could add to this harness, but this is a start...
Attachment #390068 -
Flags: review?(bugzilla)
Comment 23•15 years ago
|
||
Comment on attachment 390068 [details] [diff] [review] some basic search tests Thanks, increasing coverage bit by bit is certainly the best way to do this.
Attachment #390068 -
Flags: review?(bugzilla) → review+
Assignee | ||
Comment 24•15 years ago
|
||
I think some of the view tests exercise a fair amount of the search code, but it's good to directly test the search criteria. This test adds some of the criteria that the view tests don't exercise.
Flags: in-testsuite? → in-testsuite+
Comment 25•15 years ago
|
||
Comment on attachment 390068 [details] [diff] [review] some basic search tests >+ { testString: "QAContact", >+ testAttribute: OtherHeader, >+ op: BeginsWith, >+ count: 1}, Fails everywhere: { Finished search does 1 equal 0? NEXT ERROR TEST-UNEXPECTED-FAIL | ../../mailnews/resources/searchTestUtils.js | 1 == 0 - See following stack: [...] JS frame :: ../../mailnews/resources/searchTestUtils.js :: anonymous :: line 66 } Nit: While there, the 'Finished search does ...' line seems superfluous (now).
Assignee | ||
Comment 26•15 years ago
|
||
pushed file I forgot...
Comment 27•11 years ago
|
||
Still unable to edit the filter, the response being "This filter cannot be saved because some search terms are invalid in the current context," been fixed? I am in 17.0.2 and still getting the error.
You need to log in
before you can comment on or make changes to this bug.
Description
•