Closed
Bug 541416
Opened 14 years ago
Closed 12 years ago
Filter editor should not allow "Delete" and "Move" of same message
Categories
(MailNews Core :: Filters, defect)
MailNews Core
Filters
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 14.0
People
(Reporter: rkent, Assigned: aceman)
References
(Blocks 1 open bug, )
Details
(Keywords: polish, ux-error-prevention, Whiteboard: [gs])
Attachments
(1 file, 1 obsolete file)
2.28 KB,
patch
|
rkent
:
review+
|
Details | Diff | Splinter Review |
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.
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).
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #613795 -
Flags: review?(kent)
Attachment #613795 -
Flags: feedback?(dbienvenu)
Wayne, I am not sure how you mark the getsatisfaction references so I CC you here.
Comment 3•12 years ago
|
||
(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
Just note that this bug probably won't fix the problems the user reported. It just prevents creating new filters with illogical actions.
There is a question in Bug 742155 whether multiple copy action could be allowed. Would the backend handle it?
Comment 6•12 years ago
|
||
(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.
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).
Keywords: ux-error-prevention
Comment 8•12 years ago
|
||
I'd say yes, if it works.
Comment 9•12 years ago
|
||
(In reply to Magnus Melin from comment #8) > I'd say yes, if it works. Yes, I agree.
![]() |
Assignee | |
Comment 10•12 years ago
|
||
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.)
Attachment #613795 -
Attachment is obsolete: true
Attachment #614126 -
Flags: review?(kent)
Attachment #613795 -
Flags: review?(kent)
Attachment #613795 -
Flags: feedback?(dbienvenu)
Reporter | ||
Comment 11•12 years ago
|
||
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.
Reporter | ||
Comment 12•12 years ago
|
||
(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.
Reporter | ||
Updated•12 years ago
|
Attachment #614126 -
Flags: review?(kent) → review+
![]() |
Assignee | |
Comment 13•12 years ago
|
||
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.
Keywords: checkin-needed
Comment 14•12 years ago
|
||
(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.
![]() |
Assignee | |
Comment 15•12 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 14.0
Comment 18•12 years ago
|
||
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•