Closed
Bug 183614
Opened 22 years ago
Closed 21 years ago
Create Filter from Message doesn't handle no specified action (main filter ui does)
Categories
(MailNews Core :: Filters, defect)
MailNews Core
Filters
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.4alpha
People
(Reporter: laurel, Assigned: cavin)
References
Details
(Whiteboard: [adt3])
Attachments
(1 file, 1 obsolete file)
926 bytes,
patch
|
sspitzer
:
review+
sspitzer
:
superreview+
|
Details | Diff | Splinter Review |
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.
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•22 years ago
|
||
Mail triage team: nsbeta1+/adt3
Assignee | ||
Updated•22 years ago
|
Target Milestone: --- → mozilla1.4alpha
Assignee | ||
Comment 4•22 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.
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 | ||
Comment 6•21 years ago
|
||
Remove the default action after the dialog is initialized.
Assignee | ||
Updated•21 years ago
|
Attachment #116012 -
Flags: superreview?(sspitzer)
Comment 7•21 years ago
|
||
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");
Comment 8•21 years ago
|
||
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?
Comment 9•21 years ago
|
||
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•21 years ago
|
||
gFilter.clearActionList() does the same trick. Thanks to Seth for pointing me to this handy call.
Attachment #116012 -
Attachment is obsolete: true
Comment 11•21 years ago
|
||
Comment on attachment 116012 [details] [diff] [review] Proposed patch, v1. clearing review request
Attachment #116012 -
Flags: superreview?(sspitzer)
Comment 12•21 years ago
|
||
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•21 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 14•21 years ago
|
||
Using trunk build 30030314 on winxp, macosx and linux this is fixed. Verified.
Status: RESOLVED → VERIFIED
Updated•20 years ago
|
Product: MailNews → Core
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•