saved search folders show up for 'run now' in messages filters

REOPENED
Assigned to

Status

MailNews Core
Filters
--
minor
REOPENED
11 years ago
2 years ago

People

(Reporter: JConley, Assigned: aceman)

Tracking

(Blocks: 1 bug)

Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060831 BonEcho/2.0b2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060831 BonEcho/2.0b2

saved search folders show up as potential targets of 'run now' for message filters, however message filters will not work on 'saved search' folders

this is already done in another area: saved search folders are removed in the saved search builder (since saved searches cannot be based on saved searches)


Reproducible: Always

Steps to Reproduce:
1. create a saved search and a message filter
2. attempt to run that message filter manually on a saved search (nothing happens)




Expected Results:  
I wouldn't expect that message filters would work on saved searches, so I would expect that saved searches not show up as an option for manually running a messages filter via the 'run now' button.
(Reporter)

Updated

11 years ago
Version: unspecified → 2.0

Comment 1

11 years ago
Confirmed on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1b2) Gecko/20060831 Thunderbird/2.0a1 ID:2006083103
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
(Reporter)

Comment 2

11 years ago
might consider with enhancements (bug 350871) and (bug 347190)
Blocks: 350871
Severity: normal → minor
Hardware: PC → All

Updated

9 years ago
Assignee: mscott → nobody

Comment 3

9 years ago
version 3.0a2 (2008072418) vista

-if select a virtual folder, tools/run filters.. is grayed, as expected ..
-in the filter manager in the folder dropdown it shows saved searches. with no
effect. wrong

so I guess it's still here
Version: 2.0 → Trunk
Component: General → Filters
Product: Thunderbird → MailNews Core
QA Contact: general → filters
(Assignee)

Comment 4

5 years ago
Adding mode=filing to the folder picker seems to filter out virtual folders. But it also hides Outbox folder.
Is this the correct mode to set or do we have to define some new one?
Assignee: nobody → acelists
(Assignee)

Comment 5

5 years ago
Created attachment 655680 [details] [diff] [review]
patch

I am not sure if Seamonkey needs this too. It has a different picker I think.

The menu item Run filters on folder is also disabled when Outbox is selected so the patch behaves consistently with that.
Attachment #655680 - Flags: ui-review?(bwinton)
Attachment #655680 - Flags: feedback?(kent)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
Comment on attachment 655680 [details] [diff] [review]
patch

Yeah, this seems okay to me, and consistent with the menu item…
Attachment #655680 - Flags: ui-review?(bwinton) → ui-review+

Comment 7

5 years ago
Comment on attachment 655680 [details] [diff] [review]
patch

Seems reasonable to me.
Attachment #655680 - Flags: feedback?(kent) → feedback+
(Assignee)

Comment 8

5 years ago
Comment on attachment 655680 [details] [diff] [review]
patch

Is this relevant for Seamonkey?
Attachment #655680 - Flags: review?(mconley)
Attachment #655680 - Flags: feedback?(jh)
Comment on attachment 655680 [details] [diff] [review]
patch

(In reply to :aceman from comment #8)
> Is this relevant for Seamonkey?

The file you are changing is TB-only (all files under mail/ are) so your patch is not affecting SM. The copy of our code doesn't include the line you are changing, so I guess it's best to fix this on our end in a follow-up bug since we need a different strategy of unknown complexity. For now I'll just mark this one TODO by adding it to our to-port meta bug.
Attachment #655680 - Flags: feedback?(jh)
Blocks: 360488
Comment on attachment 655680 [details] [diff] [review]
patch

Review of attachment 655680 [details] [diff] [review]:
-----------------------------------------------------------------

Good stuff! Thanks aceman!
Attachment #655680 - Flags: review?(mconley) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/8502d1fd3ccc
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 18.0
(Assignee)

Comment 12

5 years ago
Suddenly in TB18 I get the menu item "Run filters on folder" enabled on a Saved search folder. Can anybody see it too?
I have also tested a filter on such a folder and it worked.
(Assignee)

Comment 13

5 years ago
Also, this causes the folder list dropdown in the filter list to be empty for news accounts and throwing an exception. Running filters after the fact seems to work on news accounts/folders so the folder picker should show them.

How do I request backout of this patch?

Comment 14

5 years ago
I suppose you can attach a reverse of the patch, reopen and mark it checkin-needed.

You seriously need to get checkin rights yourself though. Please file a bug about that, cc me and a few more people who's done reviews for your patches so we can vouch for you.
(Assignee)

Comment 15

5 years ago
Checkin rights will not help me, only faster reviews :)
Status: RESOLVED → REOPENED
Keywords: checkin-needed
Resolution: FIXED → ---
(Assignee)

Comment 16

5 years ago
Created attachment 660339 [details] [diff] [review]
backout patch

Backout patch for this feature, please land.
Attachment #660339 - Flags: review+
Aceman that's where having test would help you and the reviewers ...
(Assignee)

Comment 18

5 years ago
Test?
(Assignee)

Comment 19

5 years ago
There should have been a test in existence before we started to work on this. So that we could know what is the correct behaviour. We still do not know it now so I can't create one now. E.g. a comment in the code says filters after the fact (manual) do not work on news so it tries to disable the folder picker (but fails to do so). However, I talked to rkent and jcranmer on IRC and we found those filters do work on news. I even found that they work on a Saved search folder and the menu item in main menu is suddenly enabled on such a folder. So I am not sure what the proper behaviour here is. Maybe if we find it out we can make a test.

Can't mozilla arrange for a person creating tests full time? :)
https://hg.mozilla.org/comm-central/rev/b5b960f438ed

Leaving open for a better fix. Please close as WONTFIX if that's not the case.
Keywords: checkin-needed
Target Milestone: Thunderbird 18.0 → ---
(Assignee)

Comment 21

5 years ago
Can we add a new attribute on nsIMsgFolder for this? So that we can easily disable filters to run on types of folders if needed in the future.
Flags: needinfo?(kent)

Comment 22

5 years ago
(In reply to :aceman from comment #21)
> Can we add a new attribute on nsIMsgFolder for this? So that we can easily
> disable filters to run on types of folders if needed in the future.

In bug 272490 comment 10 I asked whether we should try to avoid creating additional new attributes to nsIMsgFolder and nsIMsgIncomingServer, and instead use existing methods with a text identifier.

A few years back I added the inheritable string properties to the folder. Internally, it is currently only used to represent booleans (for applying junk processing, and applying incoming filters). Personally I would prefer if new attributes are added in that way rather than expanding the definitions of nsIMsgFolder and nsIMsgIncomingServer, but in the past I have been the only proponent of that position. Perhaps it would be a good tb-planning topic?
Flags: needinfo?(kent)
(Assignee)

Comment 23

5 years ago
Yes, or a GSoC project that I proposed on the wiki page.
Removing myslef on all the bugs I'm cced on. Please NI me if you need something on MailNews Core bugs from me.
You need to log in before you can comment on or make changes to this bug.