231654 - make delete normally only apply to the threadpane - too easy to accidentally delete folder

Status: VERIFIED FIXED

(Thunderbird :: Mail Window Front End, defect, P1)

Description

[Asa Dotzler [:asa]]

This bug is a request for a change to delete functionality (via keboard or
toolbar) to make it only apply to the threadpane/messagepane. 

It's often the case that I intend to delete a message but the focus is on the
folder pane rather than the thread or message pane. I get a nice little dialog
asking me if I'm sure I want to delete the folder. I never want to delete the
folder. Ever. What I want to do is delete the message I'm reading. I suspect
that this is the common case and so the confirmation dialog certainly saves me
from a lot of pain. I'm convinced, however, that the occasions when the user
actually wanted to delete that folder are so rare, that we should find another
access point for folder deletion and eliminate the warning. 

Delete is already disabled for accounts and the primary folders (inbox, drafts,
sent, etc.) so it's not terribly inconsistent to disable it for the user-created
folders, too. Doing so, and changing the folder deletion access point to the
context menu or some other menuitem, might inconvenience a small number of users
who are all the time adding and deleting folders but it would increase
convenience and "just do the right thing" for the overwhelming majority of users
who get this dialog all the time when what they really want is to delete a message.

Comment 1

[Chris Johns]

I agree with this as the focus can vary and catch you out. Further to this the
position of the dialog box "Ok" button tends to vary and as a result I have
deleted 2 mail folders. For example the Ok for create folder (I have to do this
after deleting it the folder so know :-)) is on the left while the confirm
delete is on the right. Same goes for some of the other on the right mouse
button menu on a folder.

I have used Mozilla for years and had never done this before. Checking Mozilla I
see the Ok and Cancel have swapped. This would seem to be the source of my
problem as after years of use I am rather slow to change.

I also wonder if the the confirm delete folder dialog is too wordy compared to
Mozilla.

An option to make the delete button just delete mail would be welcome. I am
happy to use a menu or something to delete a mail folder.

Comment 2

[tom]

It is not only a Window Problem e.g. on Mac OS it is the same, so Hardware and OS should be "All".
<frustration ON>
I HATE that "feature" a long time now but today I was in hurry and I deleted a folder by pressing Enter 
instead of ESC in the warning dialog :(
So this is importand since stupid users may loose their data...
<frustration OFF>
When the folder is deleted there is NO UNDO possible it remains gone.

Comment 3

[blujay]

I concur.  I just deleted two virtual folders that I spent some time setting up
because when I clicked the delete toolbar button, the folder pane had focus, and
I didn't know it.

Comment 4

[Max Waterman]

fortunately, the deleted folders only go into the trash folder, so they can
easily be recovered (just thought I'd mention that for the benefit of any
readers); still very annoying, imo.

Comment 5

[Robert Walsh]

What they all said - this is a seriously annoying feature and has causes my
heart to skip a beat at least 3 times since yesterday, when I started using
thunderbird.

Comment 6

[Pete Riley]

For what it's worth, I've discovered that the delete toolbar button in the
"Buttons!" extension never tries to delete mailboxes or folders.  I've removed
Thunderbird's delete button and only use the one in the Buttons! extension now.

I would still like this bug to be addresses, though, and even with this
workaround the Delete KEY still tries to delete mailboxes and folders.

Comment 7

[Mike Cowperthwaite]

Compare to bug 218673, bug 315144.

Comment 8

[Mikel Ward]

I've been bitten by this as well.

I think the real problem is that after clicking on a folder, both a folder and a message appear to be selected.  Whenever I can see a message in the preview pane, I expect pressing Delete to delete that message.

Would it be better for Thunderbird to select the message (blue background) even if the user clicks on a folder?  As far as I can see, menu items such as Rename Folder and Compact Folder work the same whether the selection is a folder or a message in that folder.

Comment 9

[Magnus Melin [:mkmelin]]

I'd like this too.

Comment 10

[Wayne Mery (:wsmwk)]

Bryan, would you agree this describes a UI deficiency?  Perhaps there should be a new (non-enh) bug about it. If not, I think this bug deserves to be sev=major. (see also my rant in SM bug 197439)
 

(In reply to comment #7)
> Compare to bug 218673, bug 315144.

SM equivalent bug 197439

Comment 11

[Wayne Mery (:wsmwk)]

requesting for Thunderbird 3. I've had enough of this every other day "delete folder" prompt. At the very least, the default in the prompt should not be OK. sev>major since it seems incongruous to have dataloss keyword on a ENH bug.

Personally, I'd be happy with a hidden pref. But I don't care what the solution is as long as the risk associated with the delete key + folder goes away.

xref Bug 38227 – Undo/Redo -deleted folder can not be undone -- which might be an acceptable alternative solution

Comment 12

[David Ascher (:davida)]

Agree w/ comment #11.

Comment 13

[Wayne Mery (:wsmwk)]

just a slight back track - we need to be mindful of accessibility issues, which for some ppl are very important.

I won't drag everything from bug 197439 comment 10, but notable is
Bug 276439 – Delete folder ... should use Yes/No instead of OK/Cancel 

as a cautionary note, in case the dialog code is of the form noted in this bug (I don't think it is, but ...) -- Bug 345067 – Issues with prompt service's confirmEx - confirmEx always returns 1 when user closes dialog window using the X button in titlebar

Comment 14

[Magnus Melin [:mkmelin]]

I'll have a look at this.

Comment 15

[Magnus Melin [:mkmelin]]

Per driver discussion, not a blocker, but wanted+.

Comment 16

[David :Bienvenu]

leaving wanted, marking p1

Comment 17

[David :Bienvenu]

putting in b2, since it might require ui changes.

Comment 18

[Bryan Clark (DevTools PM) [:clarkbw]]

I'd use action text for the buttons:

+---------------------------------------+
| Delete folder '$FOLDER'?              |
|                                       |
|         ( Delete Folder )  ( Cancel ) |
+---------------------------------------+

Until there is exists folder delete undo I wouldn't make the Delete Folder the default button and have no default; requiring focus to be applied.

Comment 19

[Wayne Mery (:wsmwk)]

comment 18 sounds sensible

Comment 20

[Pete Riley]

I think that the request in comment 0 is the most important thing.  The wish is that the Delete button on the keyboard and toolbar only apply to messages in the threadpane -- never to the currently selected folder.  I don't think that changing the warning dialog fixes the core issue which is that the dialog is annoying and almost always unexpected.

The only way a user can create a subfolder is with the File menu or the context menu.  Thus it seems fairly safe to assume that the user also knows how to delete the subfolder with the File menu or the context menu in the infrequent times that this is necessary.  So I don't think the user misses anything by not being able to use the Delete buttons on the keyboard/toolbar to delete folders.  (I just noticed that the Delete Folder command isn't in the File menu but it probably should be).

If the bug is implemented in the way that Asa originally requested, please leave focus on the Delete Folder button of the warning dialog (not the Cancel button) otherwise you would just introduce a new annoyance to replace the existing one.  I think that this would be okay because it takes much more effort from the user if he has to select the Delete Folder command from the File menu or context menu than with the Delete buttons on the keyboard/toolbar.

Comment 21

[Bryan Clark (DevTools PM) [:clarkbw]]

Likely Asa does not remember (from 2004) exactly what it takes to repeat this problem but a point that should be investigated here is why the focus is unexpectedly on the folder pane.  It could have been bug 183394 or bug 378679 but fixing the unknown reason for focus on the folder pane is likely the root cause.  The dialog will be an improvement no matter what.

Comment 22

[David :Bienvenu]

My experience is that scrolling the folder pane grabs focus, and that's why the focus is unexpectedly in the folder pane.

Comment 23

[Chris Johns]

Maybe Asa does not remember but I do and I am pleased to see the bug may be resolved. Please consider the delete button to be a Delete message only. The user can always use a menu or drag the folder to the trash.

Comment 24

[Bryan Clark (DevTools PM) [:clarkbw]]

Chris, agreed however I do want to support a person clicking on a folder and pressing the delete key.  I believe that set of actions makes sense to people, or at least more sense than finding a right click / top menu item.  

If we can prevent focus from entering the folder pane unless a person tabs over to it (needed for accessibility) then I think we can have a system that works fairly well.

Comment 25

[Chris Johns]

I can only speak as a user and not a UI expert and I see the difficultly keeping accessibility easy and logical.

I currently have unread mail in 3 folders place there by filtering and I have to click the folder to select the unread mail folders so I end up with the focus in the folder pane. If I now hit delete I get the dialog and we are at the reason I adding to this bug report. I use to be able to hit Next to move folders and avoid the whole issue but this stopped working for me (could be my configuration). I hope this helps.

Comment 26

[Bryan Clark (DevTools PM) [:clarkbw]]

More good points.  Focus games, where we automatically shift focus from the folder pane to the message pane on certain types of clicks, are notorious ways to drive users and testers insane.  Not that this is being suggested, just that there be dragons there and I want to avoid the discussion. :)

I'm already getting beaten up from a number of different sides right now...  But our only clean solution is going to be moving most of the keyboard actions on most things not related to messages.  This doesn't mean wholesale removal of functionality, but a shift towards a global keyboard navigation interface that enables the most common functionality.  Many of the existing key combos that are moved will be changed into more focused inline context controls.

In reality, like many comments in this bug point out, folder operations (compared to message operations) are a small percentage of the overall operation of email users.  People move/delete/reply/forward/archive hundreds of messages per day, yet they don't create/delete/move/copy nearly that many folders.  Therefore it doesn't make sense that these, especially the destructive actions, have quick keyboard keys related to their actions.

Comment 27

[Wayne Mery (:wsmwk)]

Your last paragraph nails it.  Thinking about this last night though, will users be confused if, for example, we silently fail on delete key+folder?  If del key is removed for folders, does it make sense to give an informative message with the option to not show the message again?

Comment 28

[Bryan Clark (DevTools PM) [:clarkbw]]

Yes, a key point of any new design is going to be helping to transition user from what exists now to what we do next.  Making the new actions obvious and simple helps and like you point out it would be good to figure out some informative messages for people accustomed to using the older methods.

Comment 29

[Magnus Melin [:mkmelin]]

The situation it usually bites me is when I go between say the Inbox and my mozilla folder. If i've been in the folder before, the last loaded message gets reloaded into view - but focus remains on the folder pane (as it should). I agree we can't play focus games, I tried it and it's quite odd when the focus is shifted automatically.

Comment 30

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

So what I propose (and this patch implements) is that
 - when there's a message selected, the Delete key/button/menu act on the message
 - when no message is selected, they act on the folder

This takes care of the "normal" use cases. To cater for keyboard access to folder delete, the Edit menu includes (also) a "Delete Folder" menu item. I'm not too crazy about it, but I think it's needed.

Also changes the wording according to suggestions, make the buttons "Delete Folder" and "Cancel" + "Cancel" the default. When the imap mark as deleted model is used, folders aren't moved to trash at all, so that message is a bit longer and scarier like before.

Comment 31

[Wayne Mery (:wsmwk)]

(In reply to comment #30)
> Created an attachment (id=336519) [details]
> proposed fix
> 
> So what I propose (and this patch implements) is that
>  - when there's a message selected, the Delete key/button/menu act on the
> message

does patch mitigate (fix) the problem of bug 448146 - delete fails if focused in message header?   (hich might change when bryan's message IU checks in)

This patch will help, but I'm still keen on delete key not deleting a folder ever.

Comment 32

[Magnus Melin [:mkmelin]]

It doesn't affect that bug it seems.
Since we do send the folder to Trash most of the time, i don't think any more precautions are needed. Therefore I'm a bit unsure about defaulting to cancel, might be more normal OS behavior to default to "OK".

Comment 33

[Magnus Melin [:mkmelin]]

Attachment: proposed fix screenshot (confirmation dialog, other than mark-as-deleted imap case)

Comment 34

[Magnus Melin [:mkmelin]]

Attachment: proposed fix screenshot (confirmation dialog, mark-as-deleted imap case)

Comment 35

[Wayne Mery (:wsmwk)]

(In reply to comment #32)
> It doesn't affect that bug it seems.
> Since we do send the folder to Trash most of the time, i don't think any more
> precautions are needed. Therefore I'm a bit unsure about defaulting to cancel,
> might be more normal OS behavior to default to "OK".

I'm not sure what's normal, but since the action is not undoable I think it's smarter to be safer and have cancel be default.  Is this dialog for both click-delete and keyboard delete?  Regardless, for anyone who is clicking, cancel as default is surely not an inconvenience.

Comment 36

[Bryan Clark (DevTools PM) [:clarkbw]]

If someone has the "display last message selected" preference turned on, how would they select only a folder?

And I want to get Marco's opinion in here.

Marco: We're removing the keyboard (delete) action for folders in the folder pane when a message is in view because the more common action here is that the message in the message pane is what people are attempting to delete.  If no message is displayed in the message pane then the keyboard (delete) action will delete the folder.

Comment 37

[David :Bienvenu]

> If someone has the "display last message selected" preference turned on, how
>would they select only a folder?

Not sure I understand the context of the questions but the last selected message isn't persisted across sessions, currently (though I'd love to fix that) and it's also possible for it to get deleted w/o an other message getting selected, e.g., through a saved search.

Comment 38

[Bryan Clark (DevTools PM) [:clarkbw]]

I'm a bit confused.  I was wondering how you would select _only_ a folder with out a message being shown such that you could delete the folder.  If a message is being displayed then the message would be deleted and not the folder.  If there is no way to not select a message it makes it difficult to just select the folder.

Bug 452440 might help to solve this problem though.

Comment 39

[Magnus Melin [:mkmelin]]

Not sure I understand what you're asking either... With the patch you can still right-click delete a folder if that's what you mean. That makes it impossible in keyboard usage, which is why I propose the Edit menu show a separate Delete Folder item to make it possible at least through the menus.

Comment 40

[Wayne Mery (:wsmwk)]

if delete folder is only available from menus then it's cool that cancel NOT be the default.

Comment 41

[Magnus Melin [:mkmelin]]

I should probably put the "Delete Folder" under File instead, above "Rename Folder". If we want it...

Comment 42

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 336519 [details] [diff] [review]
proposed fix

> I should probably put the "Delete Folder" under File instead, above "Rename
> Folder". If we want it...

Ok, finally understand.  I think the File menu change makes sense.

Comment 43

[Mike Cowperthwaite]

(In reply to comment #38)
> I was wondering how you would select _only_ a folder with
> out a message being shown such that you could delete the folder.

I prefer running without a message automatically selected on folder select.


> If a message
> is being displayed then the message would be deleted and not the folder.  If
> there is no way to not select a message it makes it difficult to just select
> the folder.

It is possible to deselect the current message: with focus on the thread pane, type Ctrl+Space; or, Ctrl+LeftClick.  However, this doesn't blank the message pane (a bad thing, IMO).  Somewhere, there's a bug about that failure to blank, but I can't locate it right now.


> Bug 452440 might help to solve this problem though.

And drive me crazy in the process.

Comment 44

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v2

Carrying fwd ui-r=clarkbw. I made some modifications, think this is ready for review now. So, to recap

 - 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

In a change from the earlier patch, I made "Delete Folder" the default action in the dialog. Didn't get any feedback on it, and at least we have the Trash folder even if undo doesn't work.

Comment 46

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v3

Sorry, there was a problem with the name shown for local folders. This should do it.

Comment 47

[David :Bienvenu]

Comment on attachment 338732 [details] [diff] [review]
proposed fix, v3

In general, this looks good. I will apply it and run with it asap.

Comment 48

[David :Bienvenu]

Comment on attachment 338732 [details] [diff] [review]
proposed fix, v3

thx, Magnus, seems to work fine. 

Can you fix this before landing?

+## @loc None
+5108=&Delete Folder
\ No newline at end of file

Comment 49

[Magnus Melin [:mkmelin]]

No sm flag in this component, but we do touch seamonkey too.
(Someone please land this if it gets approved in time for the tb string freeze, i wont have time to.)

Comment 50

[Robert Kaiser]

Looks good, a=me for SM2a1. Does this patch actually fix the SeaMonkey case as well? I've run into this myself every now and then and would very much like to see this fix in SeaMonkey :)

Comment 51

[David :Bienvenu]

SM would need to make corresponding changes to hiddenWindow.js, mail3PaneWindowCommands.js, 
mailWindowOverlay.xul, and messageWindow.js. The affect on SM of this patch is to put the folder name in the confirmation dialog, basically.

Comment 52

[David :Bienvenu]

(I'm assuming SM has those same files :-) ) I guess I will land this, to beat the string freeze.

Comment 53

[David :Bienvenu]

checked in, changeset:   342:bff674025fdb

Comment 54

[Wayne Mery (:wsmwk)]

oh man!  this should be called "awesome delete".

WFM from special folders inbox, draft, template, trash, and regular folders, including after leaving a folder and coming back to it, picking up the last selected (remembered) message.

delete folder works as described, and greyed out for newsgroup

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b1pre) Gecko/20080917030604 Shredder/3.0b1pre

v. fixed on thunderbird

Comment 55

[[e-mail]]

Comment on attachment 338732 [details] [diff] [review]
proposed fix, v3

>       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.
>+        if (GetNumSelectedMessages() != 0)
>+          return DefaultController.isCommandEnabled(command);
This is best handled by making supportsCommand("cmd_delete") return false if any messages are selected; the command controller then falls back to the default controller automagically.

>+      case "cmd_deleteFolder":
>+        return FolderPaneController.isCommandEnabled(command);
And IMHO this is best handled by not handling cmd_deleteFolder in the folder pane controller but only in the default controller.

Comment 56

[Magnus Melin [:mkmelin]]

Attachment: proposed fix for neils nits

Address Neils nits.
Had to do some refactoring not to duplicate the CanDeleteFolder functionality (which also had a seemingly pointless try-catch).

Comment 57

[[e-mail]]

Comment on attachment 340569 [details] [diff] [review]
proposed fix for neils nits

>-          serverType = folder.server.type;
>-          if (serverType == "nntp") {
>-            if (command == "cmd_deleteFolder") {
>-              // Just disable the command for news.
>-              return false;
I think this got lost... put it in CanDeleteFolder?

Also as I don't build Thunderbird I'd prefer if you asked me for sr ;-)

Comment 58

[Magnus Melin [:mkmelin]]

Comment on attachment 340569 [details] [diff] [review]
proposed fix for neils nits

It's taken care of in the default controller. Since the folderpanecontroller doesn't support cmd_deleteFolder with this patch, we would never hit that if clause.
Switching r/sr:s.

Comment 59

[[e-mail]]

(In reply to comment #58)
> (From update of attachment 340569 [details] [diff] [review])
> It's taken care of in the default controller. Since the folderpanecontroller
> doesn't support cmd_deleteFolder with this patch, we would never hit that if
> clause.
But what about the case when focus is on a newsgroup and no messages are selected and delete is pressed?

Comment 60

[Magnus Melin [:mkmelin]]

That brings up the unsubscribe dialog, like before.

Comment 61

[Magnus Melin [:mkmelin]]

Comment on attachment 340569 [details] [diff] [review]
proposed fix for neils nits

Checked in; changeset:   511:a231ee033dda
http://hg.mozilla.org/comm-central/rev/a231ee033dda