Open Bug 197439 Opened 22 years ago Updated 4 years ago

too easy to accidentally delete folders

Categories

(SeaMonkey :: MailNews: Message Display, defect)

defect
Not set
critical

Tracking

(Not tracked)

People

(Reporter: per, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: dataloss)

Attachments

(1 file, 1 obsolete file)

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.
+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)
*** Bug 270608 has been marked as a duplicate of this bug. ***
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
(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.
(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
Product: Browser → Seamonkey
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!
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".
Blocks: 194507
Severity: major → critical
Keywords: dataloss
QA Contact: esther
related if not more tb bug 308690 core bug 132121
Depends on: 231654
fixed in thunderbird by bug 231654 and working rather nicely.
Assignee: mail → nobody
QA Contact: message-display
Attached patch proposed fix (obsolete) (deleted) — Splinter Review
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 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 ;-)
(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.
Attached patch proposed fix, v2 (deleted) — Splinter Review
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 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+
(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.
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.
(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".
(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.
Attachment #356331 - Flags: review?(mnyromyr) → review-
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).
As a sidenote: both TB and SM still have "delete folder" as the default button on the confirmation dialog...
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.

Attachment

General

Created:
Updated:
Size: