Create Filter from Message doesn't handle no specified action (main filter ui does)

VERIFIED FIXED in mozilla1.4alpha


16 years ago
11 years ago


(Reporter: laurel, Assigned: cavin)



Firefox Tracking Flags

(Not tracked)


(Whiteboard: [adt3])


(1 attachment, 1 obsolete attachment)



16 years ago
Using dec4 commercial trunk

When you access the filter rules ui via Create Filter from Message (prefilled)
we set the Move To Folder action as enabled by default.  If you disable that
action then forget to enable at least one action and OK the rules dialog, we
don't warn the user to set an action.  This differs from the current handling if
accessing filter rules through the main filter list ui; there we throw an alert
dialog for user to select an action.

1.  Select a mail message (not in Local Folders), use Create Filter from Message
from either Message menu or message header popup.
2.  In the rules dialog notice the default action is enabled -- Move to Folder.
 Uncheck/disable this default action. Don't enable any (other) actions.
3.  OK/confirm the rules dialog.  

Result:  No alert, lets you close the dialog.  Examining the stored filter, it
shows it left the default action enabled, but it is then left at the default
destination -- the relative account, i.e. no destination folder is set.

Expected:  An alert telling you to set at least one action.


16 years ago
Keywords: nsbeta1

Comment 1

16 years ago
User should get the same alert they get when creating a filter through the 
main filter ui; "You must select at least one filter action."

Comment 2

16 years ago
Mail triage team: nsbeta1+/adt3
Keywords: nsbeta1 → nsbeta1+
Whiteboard: [adt3]

Comment 3

16 years ago
Assignee: naving → cavin


16 years ago
Target Milestone: --- → mozilla1.4alpha

Comment 4

16 years ago
It's caused by the default action entry (MoveToFolder) that's added under a 
'prefill' condition which is the scenario for this bug. So the action count is 
at least one in this case even if you disable the (default) MoveToFolder action, 
and the code only alerts you if the action count is 0 or less (which is never 
true in this case). Working on a solution now.


16 years ago
QA Contact: laurel → esther

Comment 5

16 years ago
Using build 20030228 on winxp, linux and mac osx this is still a problem.  
I don't get an error when disabling the move to option.
The filter is listed and selected when I view my filters.
The filter is showing the move to is enabled and the default location is at the
 server level even though I had changed my location to local folders before I
disabled the option to move.  
Running the filter does nothing because the location is at the server level.



16 years ago
Blocks: 185642

Comment 6

16 years ago
Created attachment 116012 [details] [diff] [review]
Proposed patch, v1.

Remove the default action after the dialog is initialized.


16 years ago
Attachment #116012 - Flags: superreview?(sspitzer)
this doesn't seem like the right fix.

what's going on in saveFilter()?

why isn't this code doing the right thing?

 if (gMoveToFolderCheckbox.checked)
    if (gActionTargetElement)
      targetUri = gActionTargetElement.getAttribute("uri");
    if (!targetUri || targetUri == "") 
      str = gFilterBundle.getString("mustSelectFolder");
      return false;

hmm, note that targetUri is not declared locally.  we should do that.

var targetUri;
if (gActionTargetElement)
      targetUri = gActionTargetElement.getAttribute("uri");
have you noticed that if you launch the 3 pane, and immediately try to create a 
filter from the message, the "move" checkbox is not checked, and you can't 
check it?

the second time, it works fine.

cavin / shuehan, if you are seeing this? 
cavin explained what's going on over aim.

he's trying to see if 

+            // Remove the default action added above now that the dialog is 
+            gFilter.clearActionList();

will fix it.

Comment 10

16 years ago
Created attachment 116158 [details] [diff] [review]
Proposed patch, v2

gFilter.clearActionList() does the same trick. Thanks to Seth for pointing me
to this handy call.
Attachment #116012 - Attachment is obsolete: true
Comment on attachment 116012 [details] [diff] [review]
Proposed patch, v1.

clearing review request
Attachment #116012 - Flags: superreview?(sspitzer)
Comment on attachment 116158 [details] [diff] [review]
Proposed patch, v2


nice work, cavin.
Attachment #116158 - Flags: superreview+
Attachment #116158 - Flags: review+

Comment 13

16 years ago
Fix checked in.
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 14

16 years ago
Using trunk build 30030314 on winxp, macosx and linux this is fixed. Verified. 
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.