If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

VERIFIED FIXED in mozilla1.4alpha

Status

MailNews Core
Filters
VERIFIED FIXED
15 years ago
9 years ago

People

(Reporter: laurel, Assigned: Cavin Song)

Tracking

Trunk
mozilla1.4alpha

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt3])

Attachments

(1 attachment, 1 obsolete attachment)

926 bytes, patch
(not reading, please use seth@sspitzer.org instead)
: review+
(not reading, please use seth@sspitzer.org instead)
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

15 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.

Steps:
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.
(Reporter)

Updated

15 years ago
Keywords: nsbeta1

Comment 1

15 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

15 years ago
Mail triage team: nsbeta1+/adt3
Keywords: nsbeta1 → nsbeta1+
Whiteboard: [adt3]
(Assignee)

Comment 3

15 years ago
Reassigning.
Assignee: naving → cavin
(Assignee)

Updated

15 years ago
Target Milestone: --- → mozilla1.4alpha
(Assignee)

Comment 4

15 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.

Updated

15 years ago
QA Contact: laurel → esther

Comment 5

15 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.

(Assignee)

Updated

15 years ago
Blocks: 185642
(Assignee)

Comment 6

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

Remove the default action after the dialog is initialized.
(Assignee)

Updated

15 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");
      window.alert(str);
      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 
initialized.
+            gFilter.clearActionList();

will fix it.
Status: NEW → ASSIGNED
(Assignee)

Comment 10

15 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

r/sr=sspitzer

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

Comment 13

15 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 14

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