Closed Bug 41720 Opened 24 years ago Closed 23 years ago

Filters: Need to handle rename or delete folder used in filter.

Categories

(MailNews Core :: Filters, defect, P2)

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.4

People

(Reporter: laurel, Assigned: naving)

References

Details

(Keywords: dataloss, Whiteboard: relnote-user; approved for 0.9.3)

Attachments

(1 file)

Using jun05 commercial build We need to implement handling for when a user renames or deletes a folder which is specified as a destination in a filter rule. I suppose this becomes more complicated in the multiple account world. Basic 4.x behavior (I'm using 4.7 win32 IMAP as my basis, I'm sure there are more branches in the behavior): 1. When user renamed a folder, alert dialog appeared asking user "Change rule to reflect new folder location? (OK/Cancel)". If user chose OK, the filter rule was changed to show the new/renamed folder as destination. If user chose Cancel, rule was changed to Inbox as the filter action destination. 2. When user deleted a folder, alert dialog appeared asking user "Disable filter rule for this folder? (OK/Cancel)". If user chose OK, filter rule was disabled and the rule was changed to Inbox as filter action destination. If user chose Cancel, folder was not deleted and rule was not changed at all. I remember from 4.5 days that there were varying opinions as to the best way to handle rename/delete folder in conjunction with filters. I think we need some kind of error handling. Whatever you decide, please spell out the rules.
QA Contact: lchiang → laurel
oh man. I forgot about this. Sure would be nice, but it's alot of work - for now I'll just make it M18...
Status: NEW → ASSIGNED
Target Milestone: --- → M18
It would be good to have a design for this in-place. cc: jennifer Or, have some way for users (as a workaround) to address this or at least a warning that their filter won't work and let them know that they have to manually change the filter.
At a minimum, this case should "do no harm." If the message is left in the Inbox then we've met that criteria and this becomes an enhancement request to do something better. I was tempted to just mark this enhancement, but I'd prefer one of you to make that call.
this is definately not an enhancement. If we don't fix the filter URIs, we run the risk that the folder may be recreated, or that it will loose the message entirely (i.e. dataloss!)
Keywords: correctness
mark nsbeta3 then.
Keywords: nsbeta3
If we lose the message - then I'd mark this nsbeta3+. If the filter fires and then recreates the renamed/deleted folder, then I'd mark this nsbeta3-/future. (It's annoying, but you didn't lose your mail.)
Current behavior (tried POP, assume IMAP same) is message lost. Move destination gets set to invalid/blank in the ui, rules.dat retains original path to the now deleted folder. Message is not found in destination folder (even if folder is still in trash) and is not returned to inbox either. Results align with bug #46876.
Keywords: relnote2
Keywords: mail4
+ per mail triage
Whiteboard: [nsbeta3+]
Adding myself to CC List.
ugh. This is looking really hard to fix. I think it would be best if we latered this. I thought it would be critical, but now that I look at it I don't think it's TERRIBLE.. as long as we don't crash and there is no dataloss. Can someone see what happens when we rename a folder and then try to filter into it?
Well, using aug15 commercial build, the message still gets lost. I haven't yet gone through testing for the fixes bienvenu made (#46876 and there's another one or two,I think) involving moving to an invalid destination. Anyway, after renaming a folder which is a destination for a filter move, when the filter fires you get an alert that the command failed because mailbox doesn't exist (I'm using IMAP) and the filtered message is in neither the INBOX or the destination/renamed folder. The message is lost.
david, did you just fix the above error - where the message appears to be lost? If so then this bug should be safe to nsbeta3-
yes, I did.
Whiteboard: [nsbeta3+]
renominating for nsbeta3- because the message should no longer be lost (and in fact the filter should now be disabled automatically)
- per mail triage. Laurel - pls confirm alec's items above. If message is still lost, we will have to reassess this. Thanks
Whiteboard: [nsbeta3-]
Keywords: relnote3, relnoteRTM
Sorry for the extra mail. Removing mail4 keyword.
Keywords: mail4
Keywords: relnote2, relnote3
Whiteboard: [nsbeta3-] → [nsbeta3-] relnote-user
reassigning my filter bugs to gayatrib..
Assignee: alecf → gayatrib
Status: ASSIGNED → NEW
nominating for consideration in next release
Keywords: nsbeta1
marking nsbeta1-
Keywords: nsbeta1, nsbeta3nsbeta1-
Whiteboard: [nsbeta3-] relnote-user → relnote-user
*** Bug 66127 has been marked as a duplicate of this bug. ***
Keywords: mozilla1.0
Target Milestone: M18 → ---
Wow, this really needs to be fixed. For me, this is one thing that prevents me from using Mozilla MailNews. But please, don't include that ridiculous 4.x dialog. To start with, it says "Cancel", when it will not cancel anything. To end with, I've never wanted to press "Cancel" in my life, so please nuke the dialog entirely and just auto-update the filters. Why would you not want to? If you wanted to move a bunch of messages out of the way and create another folder with the same name, you can just create another folder and move the messages!
The above refers to moving & renaming. Regarding deleting, I see possible problems. The following seem to be the two options to me: You could move with the folder, but mail that matches that filter will temporarily go to the trash. Or you could pop up a dialog and ask the user for a new folder (with the parent of the trashed folder being the default), but then you would have to fix the folder if you restored the folder from the trash. AFAICS it's difficult to fix this properly without a really advanced undo architecture, the sort of which I've never seen in an app before. If you choose the latter option above, then it would be nice to include a cancel action, but make it a real one please. mpt, comments on any of this bug's UI?
Re-adding `dataloss'. Scenario: * Have a `Comments' filter, which takes all messages from bugzilla-daemon@mozilla.org with `Additional Comments From' in the body, and places them in your `Bguzilla' folder. * Have a `Bugzilla Spam' filter, applying after the `Comments' filter, which takes any remaining messages from bugzilla-daemon@mozilla.org and deletes them. * Realize your mistake, delete your `Bguzilla' folder, and create a `Bugzilla' folder. * The `Comments' filter gets disabled, and *all* Bugzilla notifications you receive (even those with comments you wanted to read) begin to be deleted by the `Bugzilla spam' filter. Oops. Ok, on to the UI. For renaming a folder: There shouldn't be any UI at all. Filters (and other folder-specific things, such as where to keep a copy of sent messages) should not reference folders just by name. They should primarily reference folders by a unique id which is stored with the folder, and which does not change when you move/rename the folder. That way, when you rename the folder, even if it is an IMAP folder which you rename using a copy of Mozilla on someone else's computer, your Mozilla filters (and other settings) on your own computer will still work. If a folder with the appropriate id does not exist, only *then* should Mozilla look for another folder with the same name -- to cater for the case where a misbehaving folder was deleted and a new one with the same name (but a different id) was created in its place. For deleting a folder: use this alert. +-----------------------------------------------------+ |:::::::::::::::::::::::::::::::::::::::::::::::::::::| +-----------------------------------------------------+ | . The mail filter "Bugzilla spam" cannot move | \ large | /!\ messages to the "Bugzilla" folder, because the | > system | """ folder cannot be found. | / font | | | Until you delete or fix the filter, messages | \ small | which it matches will be left in your Inbox. | / system font | | | (Delete Filter) (Fix Filter ...) (( OK )) | +-----------------------------------------------------+ This alert should be presented as soon as a message is downloaded which would be processed by the filter, and no further messages should be downloaded until the alert is dismissed. That way, if the user wishes to fix the filter, they can do so and have the repaired filter immediately apply to the problem message and to any subsequent downloaded messages. Clicking `OK' should not disable the filter; it should change the filter action to `do nothing'. This prevents (or should prevent) dataloss from filters further down the food chain -- like the case I described above -- since those filters will still not activate on the messages which this filter used to move.
Keywords: dataloss
Not OK. s/OK/Ignore/.
>Comments From Matthew Thomas (mpt) 2001-02-17 23:21 Sounds good!
> Not OK. s/OK/Ignore/. Sorry, that was my mistake. I drew a `caution'-type alert (with a `!' icon), as in `you have to make a choice right now'. But it should actually be a `stop'-type alert (with a `X' icon), as in `something just went wrong'. The user does not have to make a choice right now, they can just click `OK'; the `Delete Filter' and `Fix Filter ...' buttons are just added bonuses, since the user can do both those things elsewhere in the UI later on. For a `caution'-type alert, Timeless is correct -- buttons should have descriptive names, to help the user make a quick decision (though `Disable' would perhaps be a better choice than `Ignore', since `Ignore' implies ignoring the alert itself). But for `note'- and `stop'-type alerts, the button text should always be `OK' for consistency -- even if the alert text is `Your house just burnt down'.
Depends on: 77232
No longer depends on: 77232
Blocks: 77248
Target Milestone: --- → Future
reassigning to naving
Assignee: gayatrib → naving
Update Comments for the triage team for dataloss keyword: The potential for dataloss has been greatly diminished since the fix for bug 66992. Last case for dataloss (mpt 2-17-01) is fixed by 66992 (verified 05-03-01). In simple tests may30 when the target folder is gone we do still retain message in Inbox, filter is disabled, and in the case of the recreated folder (66992) the filter continues to fly.
*** Bug 85717 has been marked as a duplicate of this bug. ***
*** Bug 61732 has been marked as a duplicate of this bug. ***
Taking
Assignee: naving → hwaara
Target Milestone: Future → mozilla0.9.3
hwaara, There are a lot of issues that will need to be taken care of in this bug. Are you planning to address all of them ?
This is my plan: First fix. (I will do this) Instead of disabling the filter if a target folder is not found, display the dialog mpt suggests with the buttons "Delete Filter" and "Ignore Filter". Second fix. (Hand over bug to naving) When renaming/deleting/moving a folder, check if any filters is watching this folder and pop up a dialog. This way we'll prevent dataloss in two cases! Does that sound good to you?
Priority: P3 → P2
Ignore my last comment and plan. Naving: yes, I can fix all cases - assuming you mean all of those that mpt describes. I think I can fix "Fix Filter...", "Delete Filter" and "Disable Filter" buttons. Naving, is this OK?
I think mpt suggestion for Delete will be a lot of work especially Fix Filters... A better and more easier way to deal with deleting of folders is to warn the user (popup alert) that he has deleted a filter folder and he should fix the filter otherwise messages will go to inbox. This alert should pop-up at the time of deletion of folder. hwaara, are you just talking about front end.
There are four parts to this bug, all of which should be fixed. (1) Add the alert which I drew, but with only the `OK' button. Clicking `OK' sets the filter's action to `do nothing' (which also needs to be reflected in the Edit Filter dialog). It does *not* disable the filter. (2) Add the `Delete Filter' button to the alert. Clicking it deletes the filter. (3) Add the `Fix Filter ...' button to the alert. Clicking it closes the alert, and opens the `Edit Filter' dialog (with `do nothing' selected), so the user can choose a new folder to move the messages to, or do whatever else they want to fix the filter. (4) Make the mail/news back-end use a primary key for folders which is not the name of the folder, such that I can rename an IMAP folder using Mozilla on computer A, and my filters in Mozilla on computer B will still work. Håkan could probably fix (1), and maybe even (2) and (3), but (4) will require some heavy lifting on the back-end side.
what happens if 3 filters reference one folder?
Keywords: nsenterprise
taking, I am working on a fix.
Assignee: hwaara → naving
Navin: What will your fix include? There are many different aspects of this bug...
The majority of the work is in the back-end area. I may do the FE part also otherwise i will give it back to you.
Attached patch proposed fix, v1 (deleted) — Splinter Review
I have added a boolean on nsIMsgFolder that helps us to keep track on what folder the filter is set. When the user does a rename, then we just change the filter destination to the renamed folder. Upon deleting a folder under trash i.e the folder will be no longer there, the user is alerted that the filter will be disabled. There is also some code that passes msgWindow in the mailparsing and filtering code. I have left it in there because msgWindow always helps us in telling user about the state of app. It could be used in future. Note, this implementation will bring the current behavior close to 4x behavior. We can spin off another bug if someone wants to do what mpt has suggested for deleted folders.
Cool. I'll test this patch some...
Overall, this looks good. However, I would rather not add another boolean to nsIMsgFolder - it already has way too many. I would prefer we did it the way 4.x did it, which was basically to ask the filter list if it had a filter that pointed to any of the folders being deleted before deleting the folders. This might also make it possible not to have to change FindSubFolder the way you have.
One problem with this is that if you have the "ask me before deleting a folder" pref on, and you delete a folder that a filter points to, you will have TWO dialog boxes popping up on you. Not very nice.
jatin, this it the alert I was talking about All the filters having destination 'folderName' will be disabled.
There is a small change All the filters having destination 'folderName' are disabled.
My suggested wording: "Filters that are affected by deleting this folder are now disabled."
Target Milestone: mozilla0.9.3 → mozilla0.9.4
fix checked in.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
a=dbaron for trunk checkin during 0.9.3 closure for checking in the file you missed yesterday. (Hmmm. Why are the review and super-review not noted on the bug?)
Whiteboard: relnote-user → relnote-user; approved for 0.9.3
*** Bug 93831 has been marked as a duplicate of this bug. ***
OK using aug29 commercial trunk builds: win98, mac OS 9.0, linux rh6.2 General scenarios tested, work ok for IMAP and POP. Any specific cases found to have a problem with the rename/delete of filter involved folders will be logged separately.
Status: RESOLVED → VERIFIED
I don't know if this is supposed to be a feature or if the fix is not yet complete, but using Build 2001082703 on Win98 the filter just gets fixed without informing the user at all. No dialog or anything is appearing. Although it is nice that the filter gets fixed this is *very* bad behaviour IMO.
I agree with the previous comment. If I rename or move a folder that's used by a filter, the filter automatically updates to use the renamed or moved folder. This is the desired behavior, but we should inform the user what's happening. "Filters that are affected by renaming this folder will be updated." "Filters that are affected by moving this folder will be updated."
No, we shouldn't inform the user - that is redundant. When the user makes a filter point to a folder, it doesn't really matter what the folder's name is. If the user chooses to rename "Work" to "Work email", this would have the exact same function and a alert to notify would be plain annoying.
FYI: I logged a bug about when the alert happens for deleting a folder -- see bug 93968. I think deleting a folder should alert the user when they del/move to trash. As for the general rename folder (not to trash) scenario I did not log any bug, so if someone feels strongly that we should have an alert on rename please log a separate bug. As you can see this bug report is essentially closed, so a better place to make sure the issue gets attention is in a new report.
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: