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

VERIFIED FIXED in mozilla0.9.4

Status

P2
normal
VERIFIED FIXED
19 years ago
11 years ago

People

(Reporter: laurel, Assigned: naving)

Tracking

({dataloss})

Trunk
mozilla0.9.4
dataloss

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: relnote-user; approved for 0.9.3)

Attachments

(1 attachment)

(Reporter)

Description

19 years ago
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.
(Reporter)

Updated

19 years ago
QA Contact: lchiang → laurel

Comment 1

19 years ago
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

Comment 2

19 years ago
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.

Comment 3

19 years ago
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.

Comment 4

19 years ago
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

Comment 5

19 years ago
mark nsbeta3 then.
Keywords: nsbeta3

Comment 6

19 years ago
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.)
(Reporter)

Comment 7

19 years ago
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.
(Reporter)

Updated

19 years ago
Keywords: relnote2

Updated

19 years ago
Keywords: mail4

Comment 8

19 years ago
+ per mail triage
Whiteboard: [nsbeta3+]

Comment 9

19 years ago
Adding myself to CC List.

Comment 10

19 years ago
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?
(Reporter)

Comment 11

19 years ago
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.

Comment 12

19 years ago
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-

Comment 13

19 years ago
yes, I did.

Updated

19 years ago
Whiteboard: [nsbeta3+]

Comment 14

19 years ago
renominating for nsbeta3- because the message should no longer be lost (and in 
fact the filter should now be disabled automatically)

Comment 15

19 years ago
- per mail triage.  Laurel - pls confirm alec's items above.  If message is 
still lost, we will have to reassess this.

Thanks
Whiteboard: [nsbeta3-]
(Reporter)

Updated

19 years ago
Keywords: relnote3, relnoteRTM

Comment 16

19 years ago
Sorry for the extra mail. Removing mail4 keyword.
Keywords: mail4
Keywords: relnote2, relnote3
Whiteboard: [nsbeta3-] → [nsbeta3-] relnote-user

Comment 17

19 years ago
reassigning my filter bugs to gayatrib..
Assignee: alecf → gayatrib
Status: ASSIGNED → NEW
(Reporter)

Comment 18

19 years ago
nominating for consideration in next release
Keywords: nsbeta1

Comment 19

19 years ago
marking nsbeta1-
Keywords: nsbeta1, nsbeta3 → nsbeta1-
Whiteboard: [nsbeta3-] relnote-user → relnote-user
(Reporter)

Comment 20

18 years ago
*** 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

Comment 24

18 years ago
Not OK. s/OK/Ignore/.

Comment 25

18 years ago
>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'.
(Reporter)

Updated

18 years ago
Depends on: 77232
(Reporter)

Updated

18 years ago
No longer depends on: 77232
(Reporter)

Updated

18 years ago
Blocks: 77248

Updated

18 years ago
Target Milestone: --- → Future

Comment 27

18 years ago
reassigning to naving
Assignee: gayatrib → naving
(Reporter)

Comment 28

18 years ago
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.

Comment 29

18 years ago
*** Bug 85717 has been marked as a duplicate of this bug. ***

Comment 30

18 years ago
*** Bug 61732 has been marked as a duplicate of this bug. ***

Comment 31

18 years ago
Taking
Assignee: naving → hwaara
Target Milestone: Future → mozilla0.9.3
(Assignee)

Comment 32

18 years ago
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 ?

Comment 33

18 years ago
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?

Updated

18 years ago
Priority: P3 → P2

Comment 34

18 years ago
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?
(Assignee)

Comment 35

18 years ago
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.

Comment 37

18 years ago
what happens if 3 filters reference one folder?

Updated

18 years ago
Keywords: nsenterprise
(Assignee)

Comment 38

18 years ago
taking, I am working on a fix. 
Assignee: hwaara → naving

Comment 39

18 years ago
Navin: What will your fix include?  There are many different aspects of this bug...
(Assignee)

Comment 40

18 years ago
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. 
(Assignee)

Comment 41

18 years ago
(Assignee)

Comment 42

18 years ago
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. 

Comment 43

18 years ago
Cool. I'll test this patch some...

Comment 44

18 years ago
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.

Comment 45

18 years ago
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.
(Assignee)

Comment 46

18 years ago
jatin, this it the alert I was talking about 

All the filters having destination 'folderName' will be disabled.
(Assignee)

Comment 47

18 years ago
There is a small change

All the filters having destination 'folderName' are disabled.

Comment 48

18 years ago
My suggested wording:
"Filters that are affected by deleting this folder are now disabled."

(Assignee)

Updated

18 years ago
Target Milestone: mozilla0.9.3 → mozilla0.9.4
(Assignee)

Comment 49

18 years ago
fix checked in. 
Status: NEW → RESOLVED
Last Resolved: 18 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
(Reporter)

Comment 51

18 years ago
*** Bug 93831 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 52

18 years ago
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

Comment 53

18 years ago
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.

Comment 54

18 years ago
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."

Comment 55

18 years ago
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.
(Reporter)

Comment 56

18 years ago
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.