Last Comment Bug 350868 - saved search folders show up for 'run now' in messages filters
: saved search folders show up for 'run now' in messages filters
Product: MailNews Core
Classification: Components
Component: Filters (show other bugs)
: Trunk
: All All
-- minor (vote)
: ---
Assigned To: :aceman
Depends on:
Blocks: TB2SM 350871
  Show dependency treegraph
Reported: 2006-08-31 09:10 PDT by JConley
Modified: 2015-09-25 05:50 PDT (History)
9 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

patch (1.42 KB, patch)
2012-08-27 11:34 PDT, :aceman
mconley: review+
bwinton: ui‑review+
rkent: feedback+
Details | Diff | Splinter Review
backout patch (1.36 KB, patch)
2012-09-12 00:12 PDT, :aceman
acelists: review+
Details | Diff | Splinter Review

Description User image JConley 2006-08-31 09:10:26 PDT
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.
Comment 1 User image Magnus Melin 2006-08-31 09:37:17 PDT
Confirmed on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1b2) Gecko/20060831 Thunderbird/2.0a1 ID:2006083103
Comment 2 User image JConley 2006-08-31 09:47:49 PDT
might consider with enhancements (bug 350871) and (bug 347190)
Comment 3 User image ovidiu 2008-08-28 13:07:21 PDT
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
Comment 4 User image :aceman 2012-08-27 07:54:43 PDT
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?
Comment 5 User image :aceman 2012-08-27 11:34:46 PDT
Created attachment 655680 [details] [diff] [review]

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.
Comment 6 User image Blake Winton (:bwinton) (:☕️) 2012-08-27 13:14:45 PDT
Comment on attachment 655680 [details] [diff] [review]

Yeah, this seems okay to me, and consistent with the menu item…
Comment 7 User image Kent James (:rkent) 2012-08-28 13:45:50 PDT
Comment on attachment 655680 [details] [diff] [review]

Seems reasonable to me.
Comment 8 User image :aceman 2012-08-28 13:49:46 PDT
Comment on attachment 655680 [details] [diff] [review]

Is this relevant for Seamonkey?
Comment 9 User image Jens Hatlak (:InvisibleSmiley) 2012-08-29 01:10:30 PDT
Comment on attachment 655680 [details] [diff] [review]

(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.
Comment 10 User image Mike Conley (:mconley) 2012-08-30 08:01:18 PDT
Comment on attachment 655680 [details] [diff] [review]

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

Good stuff! Thanks aceman!
Comment 11 User image Ryan VanderMeulen [:RyanVM] 2012-08-31 03:29:39 PDT
Comment 12 User image :aceman 2012-09-11 13:28:03 PDT
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.
Comment 13 User image :aceman 2012-09-11 13:39:16 PDT
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 User image Magnus Melin 2012-09-11 22:31:57 PDT
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.
Comment 15 User image :aceman 2012-09-12 00:11:40 PDT
Checkin rights will not help me, only faster reviews :)
Comment 16 User image :aceman 2012-09-12 00:12:38 PDT
Created attachment 660339 [details] [diff] [review]
backout patch

Backout patch for this feature, please land.
Comment 17 User image Ludovic Hirlimann [:Usul] 2012-09-12 00:19:01 PDT
Aceman that's where having test would help you and the reviewers ...
Comment 18 User image :aceman 2012-09-12 00:55:08 PDT
Comment 19 User image :aceman 2012-09-12 02:52:49 PDT
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? :)
Comment 20 User image Ryan VanderMeulen [:RyanVM] 2012-09-12 13:41:09 PDT

Leaving open for a better fix. Please close as WONTFIX if that's not the case.
Comment 21 User image :aceman 2013-02-20 00:44:58 PST
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.
Comment 22 User image Kent James (:rkent) 2013-02-21 14:05:06 PST
(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?
Comment 23 User image :aceman 2013-02-21 14:12:53 PST
Yes, or a GSoC project that I proposed on the wiki page.
Comment 24 User image Ludovic Hirlimann [:Usul] 2015-09-25 05:50:31 PDT
Removing myslef on all the bugs I'm cced on. Please NI me if you need something on MailNews Core bugs from me.

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