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)
MailNews Core
Filters
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)
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Comment 1•24 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
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•24 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•24 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
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.
Comment 10•24 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•24 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•24 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•24 years ago
|
||
yes, I did.
Updated•24 years ago
|
Whiteboard: [nsbeta3+]
Comment 14•24 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•24 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-]
Keywords: relnote3,
relnoteRTM
Updated•24 years ago
|
Comment 17•24 years ago
|
||
reassigning my filter bugs to gayatrib..
Assignee: alecf → gayatrib
Status: ASSIGNED → NEW
Comment 19•24 years ago
|
||
marking nsbeta1-
Reporter | ||
Comment 20•24 years ago
|
||
*** Bug 66127 has been marked as a duplicate of this bug. ***
Updated•24 years ago
|
Keywords: mozilla1.0
Target Milestone: M18 → ---
Comment 21•24 years ago
|
||
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!
Comment 22•24 years ago
|
||
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?
Comment 23•24 years ago
|
||
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•24 years ago
|
||
Not OK. s/OK/Ignore/.
Comment 25•24 years ago
|
||
>Comments From Matthew Thomas (mpt) 2001-02-17 23:21
Sounds good!
Comment 26•24 years ago
|
||
> 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'.
Updated•24 years ago
|
Target Milestone: --- → Future
Reporter | ||
Comment 28•23 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•23 years ago
|
||
*** Bug 85717 has been marked as a duplicate of this bug. ***
Comment 30•23 years ago
|
||
*** Bug 61732 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 32•23 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•23 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•23 years ago
|
Priority: P3 → P2
Comment 34•23 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•23 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.
Comment 36•23 years ago
|
||
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•23 years ago
|
||
what happens if 3 filters reference one folder?
Keywords: nsenterprise
Comment 39•23 years ago
|
||
Navin: What will your fix include? There are many different aspects of this bug...
Assignee | ||
Comment 40•23 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•23 years ago
|
||
Assignee | ||
Comment 42•23 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•23 years ago
|
||
Cool. I'll test this patch some...
Comment 44•23 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•23 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•23 years ago
|
||
jatin, this it the alert I was talking about
All the filters having destination 'folderName' will be disabled.
Assignee | ||
Comment 47•23 years ago
|
||
There is a small change
All the filters having destination 'folderName' are disabled.
Comment 48•23 years ago
|
||
My suggested wording:
"Filters that are affected by deleting this folder are now disabled."
Assignee | ||
Updated•23 years ago
|
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Assignee | ||
Comment 49•23 years ago
|
||
fix checked in.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 50•23 years ago
|
||
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?)
Updated•23 years ago
|
Whiteboard: relnote-user → relnote-user; approved for 0.9.3
Reporter | ||
Comment 51•23 years ago
|
||
*** Bug 93831 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 52•23 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•23 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•23 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•23 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•23 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.
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
•