Open
Bug 197439
Opened 22 years ago
Updated 4 years ago
too easy to accidentally delete folders
Categories
(SeaMonkey :: MailNews: Message Display, defect)
SeaMonkey
MailNews: Message Display
Tracking
(Not tracked)
NEW
People
(Reporter: per, Unassigned)
References
(Blocks 2 open bugs)
Details
(Keywords: dataloss)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
mnyromyr
:
review-
neil
:
superreview+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3) Gecko/20030312
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3) Gecko/20030312
I use the delete key to delete email messages.
It is easy to accidentally hit the delete key
when a flder is selected instead of a message.
This does pop up a confirmation, but the
default is to to confirm. So just typing
Delete followed by Enter is enough to permanently
delete a former and all its mail!
This is unacceptable UI. (It just happened to me.
Luckily it was only a mailbox for incoming messages
from the certain heavy-volume mailing lists.)
Reproducible: Always
Steps to Reproduce:
1.
2.
3.
Expected Results:
1. The delete key should not be bound to deleting
the folder when a folder is selected.
2. The default in the pop-up window should be Cancel.
Comment 1•22 years ago
|
||
+1
This happened to me twice now, enough that I'm annoyed and was planning on
opening a bug. :) It's also important to note that the edit menu also has
"undo delete message" available, but when I try to run it, it tells me the
command can not be completed:
The current command did not succeed. The mail server responded:
SELECT failed: Can't open mailbox mail/kde: no such mailbox.
I'm running a recent snap (2003040503) on MacOSX.
This has happened to me twice too. This is really bad as data lossage happens.
It would be good if any or beter all of the following were made true:
- it is not this EASY to delete a folder by accident (rebind DEL key?)
- it is not so easy to confirm (second confirm dialog with NO default answer,
i.e., when pressing RET or SPC during dialog, nothing happens)
- messages in folder would be moved into the trash folder
- it would be possible to UNDO the delete
Still another way to make it harder to accidently delete folders might be to
keep a list of undeletable folders or a undeletable flag per folder. A user
wishing to delete a folder would first have to remove the undeletable flag (or
remove the folder from this list).
A quick look into the IMAP RFC didn't reveal whether such a flag or list is
supported by IMAP itself or if a separate list / flag would have to be kept
internally by Mozilla.
As I'm thinking about it -- would it be possible to forbid deletion of a folder
which is not empty?
The bad thing about deleting a folder isn't the lossage of the folder per se
(except: see below) but the lossage of all the messages which were in the folder.
It might be good to insist on the user deleting all messages in a folder before
it's possible to delete the folder itself.
---
(* - Exception: some filter rules might be affected)
Comment 5•20 years ago
|
||
*** Bug 270608 has been marked as a duplicate of this bug. ***
Comment 6•20 years ago
|
||
xref bug 64440
I do not see any problem retrieving the deleted folder from Trash, so this
doesn't really count as a major bug.
Severity: major → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: PC → All
Comment 7•20 years ago
|
||
(In reply to comment #6)
> xref bug 64440
>
> I do not see any problem retrieving the deleted folder from Trash, so this
> doesn't really count as a major bug.
At least on thunderbird on Mac OS X, the folder does not go into the trash and is permanently deleted.
Comment 8•20 years ago
|
||
(In reply to comment #7)
> (In reply to comment #6)
> > xref bug 64440
> >
> > I do not see any problem retrieving the deleted folder from Trash, so this
> > doesn't really count as a major bug.
>
> At least on thunderbird on Mac OS X, the folder does not go into the trash
> and is permanently deleted.
Ron reminded me of the IMAP case, so yes, dataloss is an issue. Resetting
severity.
Severity: enhancement → major
Updated•20 years ago
|
Product: Browser → Seamonkey
Updated•20 years ago
|
Assignee: sspitzer → mail
I don't know whether it is reproducible on all systems, but I've been really
annoyed on my system ThunderBird doesn't even show a confirm box if you hit
shif-delete on a message (never tried it at folder level!), the message is gone
forever, atleast a confirm box with default to cancel button should show up
(this could be a setting option) in case a user tries to permanently delete any
item (could be a folder or a message).
I am pretty surprised that this bug has not been fixed till now as I agree with
the reporter that is unacceptable UI behaviour!
Comment 10•17 years ago
|
||
dataloss issue, so bumping severity.
The day will come when I will get burnt on this with a really big folder (it's almost happened) and then I will be really bummed. So to state to obvious this needs fixing. Would be nice to see this in version 3.
OTOH if bug 38227 were in force and worked on all platforms this might not a big deal. But that doesn't look likely.
related bugs: bug 276439, bug 125864, bug 197439 (this bug, but specific to imap), and, as mentioned by Mike, bug 64440. Bug 142291 might also be of some use in helping prevent "bad things".
Comment 11•16 years ago
|
||
related if not more
tb bug 308690
core bug 132121
Comment 12•16 years ago
|
||
fixed in thunderbird by bug 231654 and working rather nicely.
Assignee: mail → nobody
QA Contact: message-display
Comment 13•16 years ago
|
||
Port super delete - bug 231654 - to seamonkey.
Description, from bug 231654 comment 44:
- when no message is selected, delete key/button work like before
- when a message is selected, delete key/button always work on the message, no
matter if the focus is on the folder pane or not
- to facilitate keyboard access, in the 3-pane there is always a "Delete
Folder" menu item under File, disabled for nntp, account nodes, and folders
that can't be deleted
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #355828 -
Flags: superreview?(neil)
Attachment #355828 -
Flags: review?(mnyromyr)
Comment 14•16 years ago
|
||
Comment on attachment 355828 [details] [diff] [review]
proposed fix
>- MsgDeleteFolder();
>- break;
>+ // Even if the folder pane has focus, don't do a folder delete if
>+ // we have a selected message, but delete the message instead.
>+ if (GetNumSelectedMessages() == 0)
>+ MsgDeleteFolder();
>+ else
>+ DefaultController.doCommand(command);
>+ break;
The supportsCommand change should make this unnecessary, no?
>+ case "cmd_deleteFolder":
>+ var folders = GetSelectedMsgFolders();
>+ if (folders.length) {
>+ var folder = folders[0];
>+ if (folder.server.type == "nntp")
>+ return false; // Just disable the command for news.
>+ else
>+ return CanDeleteSelectedFolder();
>+ }
>+ return false;
This should say
if (folder.server.type != "nntp")
return CanDeleteSelectedFolder();
and just fall through to the return false; for news.
>\ No newline at end of file
Oops ;-)
Comment 15•16 years ago
|
||
(In reply to comment #14)
> (From update of attachment 355828 [details] [diff] [review])
> >- MsgDeleteFolder();
> >- break;
> >+ // Even if the folder pane has focus, don't do a folder delete if
> >+ // we have a selected message, but delete the message instead.
> >+ if (GetNumSelectedMessages() == 0)
> >+ MsgDeleteFolder();
> >+ else
> >+ DefaultController.doCommand(command);
> >+ break;
> The supportsCommand change should make this unnecessary, no?
Ah yes, we won't ever get there in unless it's a folder we should delete.
Comment 16•16 years ago
|
||
Attachment #355828 -
Attachment is obsolete: true
Attachment #356331 -
Flags: superreview?(neil)
Attachment #356331 -
Flags: review?(mnyromyr)
Attachment #355828 -
Flags: superreview?(neil)
Attachment #355828 -
Flags: review?(mnyromyr)
Comment 17•16 years ago
|
||
Comment on attachment 356331 [details] [diff] [review]
proposed fix, v2
> case "cmd_delete":
> case "cmd_shiftDelete":
> case "button_delete":
> case "button_shiftDelete":
>- //case "cmd_selectAll": the folder pane currently only handles single selection
>+ case "button_delete":
>+ // Even if the folder pane has focus, don't do a folder delete if
>+ // we have a selected message, but do a message delete instead.
>+ // Return false here for supportsCommand and let the command fall back
>+ // to the DefaultController.
>+ if (GetNumSelectedMessages() != 0)
>+ return false;
>+ // else fall through
>+ //case "cmd_selectAll": the folder pane currently only handles single selection
The "cmd_selectAll" reindent blinded me to the duplication of the "button_delete" case label :-( sr=me with that fixed.
Attachment #356331 -
Flags: superreview?(neil) → superreview+
Comment 18•16 years ago
|
||
(In reply to comment #13)
> Description, from bug 231654 comment 44:
> - when no message is selected, delete key/button work like before
So far so good.
> - when a message is selected, delete key/button always work on the message,
> no matter if the focus is on the folder pane or not
Sorry, but am I really the only one who finds this totally insane?
The keyboard focus is *meant* to symbolize where the keyboard action is going to happen! And now the focus is ignored and, very confusingly, something else is happening somewhere else - this is just hurting user expectations and any consistency rules of how UI should behave...
(I can see the justification coming "when the folder should be deleted anyway, it doesn't hurt deleting messages", but I think the damage done is far bigger than that.)
> - to facilitate keyboard access, in the 3-pane there is always a "Delete
> Folder" menu item under File, disabled for nntp, account nodes, and folders
> that can't be deleted
That doesn't help. Since we (still ;-) ) only have a Message menu but no Folder menu, the folder menu items are pretty unintuitive to find.
And, regardless, it doesn't heal the severe UI principles violation introduced by this patch.
Comment 19•16 years ago
|
||
The problem, of course, is that the focus isn't always where the user expects it to be => does follow user expectations, IMO.
This has been in thunderbird for a while now (granted, only beta/nightlies) and we haven't had any complaints AFAIK - but there were numerous complaints of accidental folder deletions.
Comment 20•16 years ago
|
||
(In reply to comment #19)
> The problem, of course, is that the focus isn't always where the user expects
> it to be
First, there's the problem that the focus *is* somewhere else - how did it get there?
But even more important is the question,
Why does the user not recognize where the focus really is?
Mostly that's because theming etc. don't give enough hints.
> => does follow user expectations, IMO.
Clearly not - YMMV, obviously.
"Do what I mean no matter of what I tell you to do" is very hard without reading minds, so we shouldn't try. (If it was easy, we could get along with just one big button "Do it!" ;-) )
> This has been in thunderbird for a while now (granted, only beta/nightlies)
> and we haven't had any complaints AFAIK
The usecase itself is not particularly common.
> but there were numerous complaints of accidental folder deletions.
Honestly, this "accidental folder deletion" requires quite an amount of work:
- a message needs to be selected (=> its folder is selected)
- the folder pane needs to have the focus
- focus theming for both folder and message must be non-obvious
- user needs to hit delete
- user needs to confirm the deletion dialog
Only _then_ the folder will be deleted.
The real bug to fix here is the dialog defaulting to "delete".
Comment 21•16 years ago
|
||
(In reply to comment #20)
> First, there's the problem that the focus *is* somewhere else - how did it get
> there?
There are probably bugs for some edge cases but the "normal way" I get it is just clicking different folders, resulting in the last selected message in that folder loading into view. (Changing focus programatically for this case doesn't give a nice behavior, it's quite "jumpy".)
> But even more important is the question,
>
> Why does the user not recognize where the focus really is?
I think the focus isn't very unclear, it's just a question of expectations. If you have are looking at a message, that's where your mental focus is. Folder delete is just soo uncommon compared to msg delete.
> The usecase itself is not particularly common.
If you mean getting folder delete instead of msg delete, at least for me it was common.
Updated•16 years ago
|
Attachment #356331 -
Flags: review?(mnyromyr) → review-
Comment 22•16 years ago
|
||
Comment on attachment 356331 [details] [diff] [review]
proposed fix, v2
r- and moa- for SM with regard to "delete message even though folder has focus".
The other changes look fine, modulo a few nits:
mail3PaneWindowCommands.js has mixed bracing style, so I prefer the align-braces-vertically style in all changed code.
>diff --git a/mailnews/base/resources/content/mail3PaneWindowCommands.js b/mailnews/base/resources/content/mail3PaneWindowCommands.js
> case "button_shiftDelete":
> if ( command == "cmd_delete" )
> goSetMenuValue(command, 'valueFolder');
>+ var folders = GetSelectedMsgFolders();
>+ if (folders.length) {
>+ var folder = folders[0];
"folder" is only used once and superfluous hence.
Also, please reformat the rest handful of lines in this function. ;-)
>+ case "cmd_deleteFolder":
>+ var folders = GetSelectedMsgFolders();
>+ if (folders.length) {
>+ var folder = folders[0];
>+ // Possibly enable for non-news.
>+ if (folder.server.type != "nntp")
>+ return CanDeleteSelectedFolder();
"folder" is only used once and superfluous hence.
>diff --git a/mailnews/base/resources/content/mailWindowOverlay.xul b/mailnews/base/resources/content/mailWindowOverlay.xul
>+ <menuitem id="menu_deleteFolder" label="&deleteFolder.label;"
>+ accesskey="&deleteFolder.accesskey;"
>+ command="cmd_deleteFolder"
>+ observes="mailHideMenus"/>
One attribute per line, please (despite the surroundings still violating this rule).
Comment 23•16 years ago
|
||
As a sidenote: both TB and SM still have "delete folder" as the default button on the confirmation dialog...
Comment 24•16 years ago
|
||
Oh well(In reply to comment #22)
> (From update of attachment 356331 [details] [diff] [review])
> r- and moa- for SM with regard to "delete message even though folder has
> focus".
Oh well, nothing much left to do here then. (And at least in thunderbird i think we've made it hard enough to accidentally get the dialog that we don't need to second guess the user that he/she really wanted to do it.)
->defaults
Assignee: mkmelin+mozilla → nobody
Status: ASSIGNED → NEW
You need to log in
before you can comment on or make changes to this bug.
Description
•