19403 - Edit filters for multiple newsgroups simultaneously, a.k.a "server wide news filters"

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, enhancement, P3)

Description

[revenant]

I have Communicator 4.6, so please forgive me if 4.7 has already addressed this.

At the moment, it appears that if you want to apply a specific filter (e.g.
CONTAINS "Netscape") to multiple newsgroups that you have to set up
the filter individually in each and every newsgroup you want to apply it to.

It would be very handy to be able to edit the filters for multiple newsgroups
simultaneously.

Thank you.

Comment 1

[Phil Peterson]

Add to [help wanted] list

Comment 2

[Stewart Gordon]

I entirely agree.  There should at least be an easy way to have a filter for a
whole news account and even for all newsgroups on all accounts.

Comment 3

[punkadiddle]

That could be an entry like mynewsserver.org > (all)

I think that's really neccessary... I'm not willing to add the same filter that
marks messages from me for twelve groups.

Comment 4

[Juliusz Gonera]

OMG this is so old. Why anybody hasn't done this yet? I totally agree, this
feature would be very useful.

Comment 5

[Scott MacGregor]

working on a patch...

Comment 6

[Scott MacGregor]

Attachment: initial patch, still needs work

Comment 7

[Scott MacGregor]

Attachment: improved patch, still working on it and testing it.

Comment 8

[Scott MacGregor]

Attachment: updated patch, it works!

Comment 9

[Scott MacGregor]

things to figure out:

a) the order, server or per group filter first, see my "// XXX ask david b,
should m_filterList or m_serverFilterList be first?" comment
b) I started out doing the temporary filter thing, but I'm not sure it's the way
to go.  I don't want to mark the filters from the filterlist for the group as
temporary, as changes I make to them I want changes to them to persist on disk.
 (There is UI for them, unlike the MDN / temp spam stuff for mail).  I also have
the luxury for news filters we never move message, so I can always process
filters, so I could just get two filter lists and apply them.  Also, I need the
filter log for server wide actions to be off the server, if that makes sense.

NS_IMETHODIMP nsNNTPNewsgroupList::ApplyFilterHit(nsIMsgFilter *aFilter,
nsIMsgWindow *aMsgWindow, PRBool *aApplyMore)
{
 NS_ENSURE_ARG_POINTER(aFilter);
 NS_ENSURE_ARG_POINTER(aApplyMore);
 NS_ENSURE_TRUE(m_newMsgHdr, NS_ERROR_UNEXPECTED);
 NS_ENSURE_TRUE(m_newsDB, NS_ERROR_UNEXPECTED);
 
 // you can't move news messages, so applyMore is always true
 *aApplyMore = PR_TRUE;

c)  I also have to figure out a better fix for the issue FilterListDialog.js

function onFilterServerClick(selection)
{
   var itemURI = selection.getAttribute('id');

   // Return if same server is selected.
   // XXX this "!itemURI" thing is a hack
   // we come into onFilterServerClick twice
   // and when you choose a news server, and the second time it is ""
   if (!itemURI || itemURI == gCurrentServerURI)
     return; 

Comment 10

[David :Bienvenu]

-  if (type.Equals("nntp")) {
+  if (type.Equals("nntp") && !isServer) {

can you fix the braces here? A few K&R'isms have crept into this file, and I'd
like to nip that in the bud...thx for fixing the other cases :-)
if (a)
{
}

I believe the per-newsgroup filters should probably go first. It doesn't matter
right now since nothing stops filter execution for newsgroups, but if something
does, like adding a "stop execution" action, then users should be able to
override the global filters in the per-newsgroup filters.

I believe you can just use the member variable, mIsServer, instead of calling
GetIsServer. It's just code bloat to call that method and check the error
return, when we have a member var already...I can't imagine a scenario where
you're calling GetIsServer that we haven't set mIsServer already...

other than that, looks fine to me.

Comment 11

[Scott MacGregor]

Attachment: updated patch, includes fixes for bugs #227224 and bug #240879

Comment 12

[Scott MacGregor]

Attachment: patch with code cleanup parts removed (I already checked them in)

Comment 13

[Stewart Gordon]

The recent addition to the summary line discards the possibility of setting a
filter across all servers, which would be useful just as well.

Comment 14

[Scott MacGregor]

Attachment: patch, I decided to execute the server filters first, then group specific.

Comment 15

[Scott MacGregor]

I still need to make sure that with the MarkThreadWatched / MarkThreadIgnored
changes (for bug #227224) works.

I have a feeling that I'll need to move that code to after the point where we
call AddNewHdrToDB().

Comment 16

[Scott MacGregor]

Attachment: really get watch/ignore to work, after we add the msg hdr, get the thread and mark it as watched (or ignored)

Comment 17

[Scott MacGregor]

Attachment: patch, with the patch for bug #196441 (make "label as none" be a possible filter action)

Comment 18

[David :Bienvenu]

it should work to set the flag on the hdr and then add it to the db...let me see
why that doesn't work.

Comment 19

[David :Bienvenu]

the mark thread watched/ignored filter actions work in the db, but the view
isn't getting notified, so you don't see the watch/ignore icon until you go to
another newsgroup and then back to the original one. I could just fix the thread
code to call back into the db to have the correct notifications sent around, and
save you adding state to nsNNTPNewsgroupList...

Comment 20

[Scott MacGregor]

> The recent addition to the summary line discards the possibility of setting a
> filter across all servers, which would be useful just as well.

we'll have to spin that off into a new bug.  next up will be global mail filters.

Comment 21

[Scott MacGregor]

Attachment: patch, includes changes based on feedback from david

Comment 22

[Scott MacGregor]

Attachment: patch, does not include a fix for bug #227224

Comment 23

[Scott MacGregor]

fixed.

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+1]]

We have a chance to get this fix into the 1.7 branch or is it already done?

verified.

Comment 25

[Scott MacGregor]

Attachment: actual patch

Comment 26

[Scott MacGregor]

fixed on the 0.6 tbird branch