541416 - Filter editor should not allow "Delete" and "Move" of same message

Status: RESOLVED FIXED

(MailNews Core :: Filters, defect)

Description

[Kent James (:rkent)]

In the referenced GS thread, I noted that the filter editor allows you to request that a message be both moved and deleted, but that is asking that the original message be deleted twice. This should be disabled somehow.

Comment 1

[:aceman]

Attachment: patch

I think it does what you wished. And also adds the same exclusion for Mark as read/Mark as unread (added in TB11, after this bug was filed).

Comment 2

[:aceman]

Wayne, I am not sure how you mark the getsatisfaction references so I CC you here.

Comment 3

[Wayne Mery (:wsmwk)]

(In reply to :aceman from comment #2)
> Wayne, I am not sure how you mark the getsatisfaction references so I CC you
> here.

no problem. details https://wiki.mozilla.org/Thunderbird/Support/Workflow#Bugzilla

http://getsatisfaction.com/mozilla_messaging/topics/filters_not_working-13gkn = https://getsatisfaction.com/mozilla_messaging/tags/bug_541416

Comment 4

[:aceman]

Just note that this bug probably won't fix the problems the user reported. It just prevents creating new filters with illogical actions.

Comment 5

[:aceman]

There is a question in Bug 742155 whether multiple copy action could be allowed. Would the backend handle it?

Comment 6

[David :Bienvenu]

(In reply to :aceman from comment #5)
> There is a question in Bug 742155 whether multiple copy action could be
> allowed. Would the backend handle it?

yes, the backend should allow multiple copies.

Comment 7

[:aceman]

Do we want to allow it in the UI? Currently the UI only allows one occurrence of each action within one filter, with the exception of 'Add Tag' and 'Forward' actions.
It is in the same spot as the patch is touching, I could add it.
I have tested it now in TB14 and it seems to work (multiple copies).

Comment 8

[Magnus Melin [:mkmelin]]

I'd say yes, if it works.

Comment 9

[David :Bienvenu]

(In reply to Magnus Melin from comment #8)
> I'd say yes, if it works.

Yes, I agree.

Comment 10

[:aceman]

Attachment: patch v2

Thanks, so here is the update.

The current state is:
Allow multiple actions of this kind:
- Add tag, Forward, Copy to
If either member from the following pairs exists, disallow the other one:
- Move to/Delete, Mark as read/Mark as Unread

Any other proposals?

(Also there are some actions that are completely hidden, depending on server type, but that is not the topic for this bug.)

Comment 11

[Kent James (:rkent)]

I've looked at this briefly, and don't see any issues with it. But I really need to compile and test it. Since my normal development is done on aurora, it will take me a day or so to get a trunk compile working.

Comment 12

[Kent James (:rkent)]

(In reply to :aceman from comment #10)
> Created attachment 614126 [details] [diff] [review]
> patch v2
> 
> Thanks, so here is the update.
> 
> The current state is:
> Allow multiple actions of this kind:
> - Add tag, Forward, Copy to
> If either member from the following pairs exists, disallow the other one:
> - Move to/Delete, Mark as read/Mark as Unread
> 
> Any other proposals?
>

I've now compiled and briefly tested with this patch (v2), and it seems to work as advertised. But I immediately started asking, are there other standard filter actions that have the same issue?

Looking briefly at the core code, and testing briefly, I guess that the other standard async actions (forward, reply) seem to have core workarounds to allow the combined actions.

It's unfortunate though that these are not generalized in some fashion. I do know that many of the more interesting filter actions that people request require async operations of one sort or another, and although these can be implemented in a custom filter action, they usually interact badly with other async operations such as move and delete. It would be so nice if the core code supported that better.

But I guess that is not this bug.

Comment 13

[:aceman]

Thanks. I think this already expanded beyond the original request, but only taking the easy options.

So if you can find out any new actions that should be blocked or allowed to occur multiple times, please make a new bug because it seems it will need more discussion and maybe core changes.

Comment 14

[WADA:World Anti-bad-Duping Agency]

(In reply to :aceman from comment #10)
> If either member from the following pairs exists, disallow the other one:
> - Move to/Delete

I think "Stop Filter Execution" is better prohibited when "Move to" or "Delete" is requested, because Move to/Delete implies "Stop Filter Execution" on the hit mail.
What do you think?

By the way, "Sorting of actions in msgFilterRules.dat based on priority of action" may be needed to avoid user's confusions, but it's different enhancement.

Comment 15

[:aceman]

That is true but it is not sure users would understand why "Stop Filter Execution" is disabled. They may think the option is not useful (when coupled with move/delete) and that filters will actually continue execution.

Comment 16

[WADA:World Anti-bad-Duping Agency]

I see.
"Delete + Stop Filter Execution" and "Move to + Stop Filter Execution" won't produce any confliction nor any problem, except bug around "Stop Filter Execution" if exists.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Kent, you aren't listed as a MailNews peer on the Modules wiki. I'm assuming you are OK to do reviews, but you should probably update the page.
https://wiki.mozilla.org/Modules/All

http://hg.mozilla.org/comm-central/rev/ad44a8e284a7

Comment 18

[David :Bienvenu]

I told aceman that rkent was good for this review and for reviewing search and filter code in general, even if he's reluctant to be an official peer.