49323 - Make shift-delete work with toolbar button

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, enhancement)

Description

[Phil Peterson]

Using 2000-08-16-08 commercial build on NT

1. Select a mail message in the thread pane
2. Hold down the Shift key and press the Delete toolbar button

Expected: message to be deleted without copying to Trash
Actual: message is deleted, but also copied to Trash

Comment 1

[Phil Peterson]

Add keywords 4xp, regression, perf, nsbeta3. I love keywords!

Comment 2

[scottputterman]

I don't think this is a regression.  Using shift-delete on the keyboard seems 
to work for me.  I never made it work for the shift key and the delete toolbar 
button. In 4.x hitting shift and the toolbar button delete also seems to copy to 
the trash. 

Comment 3

[Phil Peterson]

I'm stunned, but you're right. I think that's a bug in 4.x. Adjusting keywords

Comment 4

[scottputterman]

moving to future milestone.

Comment 5

[Håkan Waara]

The "shift" doesn't seem to have any impact on this, it moves to the thrash
anyway. Build 2001010504, Windows 98.

Comment 6

[Håkan Waara]

By the way, shift-delete is a feature; not a bug as far as I know. It's a fast
way to delete a message without being forced to first move it to trash the empty
trash.

I am suggesting this as a wontfix.

Comment 7

[Phil Peterson]

Håkan, there are plenty of features filed in bugzilla -- I don't think this
means it should be closed. Having the bug marked MFuture means Scott isn't going
to get to it any time soon, but that it's a legitimate improvement that someone
could make, if they were interested.

Comment 8

[Håkan Waara]

I think I *could* make this improvement, but I think it should stay as it is.
That was my point with my latest post.

I think we should have it as it is, and just let it be. I know a lot of people
who actually liked this "feature" in NS4.x.

Comment 9

[Phil Peterson]

They liked that shift-delete works with the keyboard, but not the toolbar
button? IMO, this is inconsistent behavior, and we should fix that. Personally,
I use shift-delete all the time, and have trained myself not to use the toolbar
Delete button for that reason.

Comment 10

[scottputterman]

reassigning to ssu.

Comment 11

[Kathleen :Brade]

For YEARS I have been wondering why I couldn't use shift-delete to delete
messages without putting them in the trash can.  This should be pretty easy to
fix.  

Sean Su, Håkan--let me know if you need assistance.

Can we at least reset the milestone for 1.1 or ?

Comment 12

[Kathleen :Brade]

*** Bug 86660 has been marked as a duplicate of this bug. ***

Comment 13

[Kathleen :Brade]

nominate for nsbeta1

Comment 14

[scottputterman]

marking nsbeta1-

Comment 15

[Bruno 'Aqualon' Escherl]

Attachment: WIP patch

This is a work in progress patch to solve this bug and cleanup the code for deleting messages/message folders on the way.

Comment 16

[Bruno 'Aqualon' Escherl]

Attachment: Fix the bug and cleanup of message deletion code

The patch fixes the following issues:
- Clicking the delete button with pressed shift key deletes messages and doesn't move it to the trash (like for shift-delete key).
- shift-delete with focused folder and selected message doesn't delete the message anymore.

Following things were changed for the patch:
- DefaultController doesn't delete the message directly, but uses mailWindowOverlay.js' MsgDeleteMessage().
- Delete button calls MsgDeleteFromToolbar(event) and that function decides, if and how a message or a folder is to be deleted.
- MsgDeleteMessageFromMessageWindow() was removed from messageWindow.js, cause MessageWindowController can also use MsgDeleteMessage(). Saves duplicated code for doing the same thing.
- Some indentation glitches were fixed.

Comment 17

[[e-mail]]

Comment on attachment 276115 [details] [diff] [review]
Fix the bug and cleanup of message deletion code

This makes it so that the Delete button cancels messages. Is that intentional?

What was wrong with making the delete button's oncommand be this:
goDoCommand(event.shiftKey ? "cmd_shiftDelete" : "cmd_delete");

You don't want to call goSetMenuValue for cmd_shiftDelete.

Comment 18

[David :Bienvenu]

I just assumed that you were going to do what Neil suggested - that's pretty simple...

Comment 19

[Bruno 'Aqualon' Escherl]

Attachment: Simple fix

Comment 20

[Bruno 'Aqualon' Escherl]

(In reply to comment #17)
> What was wrong with making the delete button's oncommand be this:
> goDoCommand(event.shiftKey ? "cmd_shiftDelete" : "cmd_delete");
Nothing, I just didn't know it's that easy to do :)

I still think some of the code cleaning is useful, but that should probably be made in another bug.

One question, at the moment shift-delete (and shift-'button_delete') delete the selected message even when there's focus on the folder pane (while just delete asks the user, if the selected folder should be moved into trash). Is that intentional or is it a bug? I would prefer, that shift-delete shows the equal behaviour to delete, hence delete the selected folder. Deleting the selected but not focused message without a chance to get it back from the trash seems to be a bad idea.

Comment 21

[Bruno 'Aqualon' Escherl]

(In reply to comment #17)
> (From update of attachment 276115 [details] [diff] [review])
> This makes it so that the Delete button cancels messages. Is that intentional?
Do we ever show the delete button for newsgroups? Preventing something that never occurs seems to be quite redundant.

Comment 22

[[e-mail]]

(In reply to comment #20)
>One question, at the moment shift-delete (and shift-'button_delete') delete the
>selected message even when there's focus on the folder pane (while just delete
>asks the user, if the selected folder should be moved into trash).
To fix that, simply add cmd_shiftDelete to the folder pane controller.

Comment 23

[[e-mail]]

Comment on attachment 276131 [details] [diff] [review]
Simple fix

I wrote this in comment #17. How can I review it?

Comment 24

[[e-mail]]

(In reply to comment #21)
>Do we ever show the delete button for newsgroups? Preventing something that
>never occurs seems to be quite redundant.
A good point. Let's remove that code too while you're at it.

Comment 25

[Bruno 'Aqualon' Escherl]

I think that was closed accidently, reopening.

Comment 26

[David :Bienvenu]

Comment on attachment 276131 [details] [diff] [review]
Simple fix

thx, loooks good.

Comment 27

[Frank Wein [:mcsmurf]]

Checking in mailWindowOverlay.xul;
/cvsroot/mozilla/mailnews/base/resources/content/mailWindowOverlay.xul,v  <--  mailWindowOverlay.xul
new revision: 1.334; previous revision: 1.333
done