Make shift-delete work with toolbar button

RESOLVED FIXED in seamonkey2.0a1

Status

--
enhancement
RESOLVED FIXED
18 years ago
11 years ago

People

(Reporter: phil, Assigned: bugzilla)

Tracking

({polish})

Trunk
seamonkey2.0a1
polish
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

18 years ago
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
(Reporter)

Comment 1

18 years ago
Add keywords 4xp, regression, perf, nsbeta3. I love keywords!
Keywords: 4xp, nsbeta3, perf, regression

Updated

18 years ago
QA Contact: lchiang → nbaca

Comment 2

18 years ago
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. 
(Reporter)

Comment 3

18 years ago
I'm stunned, but you're right. I think that's a bug in 4.x. Adjusting keywords
Keywords: 4xp, nsbeta3, regression → polish
(Reporter)

Updated

18 years ago
Summary: Shift-Delete broken → Make shift-delete work with toolbar button

Updated

18 years ago
QA Contact: nbaca → esther

Comment 4

18 years ago
moving to future milestone.
Target Milestone: --- → Future
QA Contact: esther → sheelar

Comment 5

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

Comment 6

18 years ago
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.
(Reporter)

Comment 7

18 years ago
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.
Severity: normal → enhancement

Comment 8

18 years ago
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.
(Reporter)

Comment 9

18 years ago
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

17 years ago
reassigning to ssu.
Assignee: putterman → ssu

Comment 11

17 years ago
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 ?
OS: Windows NT → All
Hardware: PC → All

Comment 12

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

Comment 13

16 years ago
nominate for nsbeta1
Keywords: nsbeta1

Updated

16 years ago
QA Contact: sheelar → stephend

Comment 14

16 years ago
marking nsbeta1-
Assignee: ssu → sspitzer
Keywords: nsbeta1 → nsbeta1-
Product: Browser → Seamonkey

Updated

14 years ago
Assignee: sspitzer → mail

Updated

12 years ago
QA Contact: stephend
(Assignee)

Comment 15

11 years ago
Created attachment 276016 [details] [diff] [review]
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.
Assignee: mail → aqualon
Status: NEW → ASSIGNED
(Assignee)

Updated

11 years ago
Priority: P3 → --
QA Contact: mail
Target Milestone: Future → seamonkey2.0alpha
(Assignee)

Comment 16

11 years ago
Created attachment 276115 [details] [diff] [review]
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.
Attachment #276016 - Attachment is obsolete: true
Attachment #276115 - Flags: superreview?(neil)
Attachment #276115 - Flags: review?(mnyromyr)

Comment 17

11 years ago
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

11 years ago
I just assumed that you were going to do what Neil suggested - that's pretty simple...
(Assignee)

Comment 19

11 years ago
Created attachment 276131 [details] [diff] [review]
Simple fix
Attachment #276131 - Flags: review?(neil)
(Assignee)

Updated

11 years ago
Attachment #276115 - Flags: superreview?(neil)
Attachment #276115 - Flags: review?(mnyromyr)
(Assignee)

Comment 20

11 years ago
(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.
(Assignee)

Comment 21

11 years ago
(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.
(Assignee)

Updated

11 years ago
Blocks: 391701
(Assignee)

Updated

11 years ago
Attachment #276115 - Attachment is obsolete: true

Comment 22

11 years ago
(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

11 years ago
Comment on attachment 276131 [details] [diff] [review]
Simple fix

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

Comment 24

11 years ago
(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.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Attachment #276131 - Flags: review?(neil) → review?(bienvenu)
(Assignee)

Comment 25

11 years ago
I think that was closed accidently, reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 26

11 years ago
Comment on attachment 276131 [details] [diff] [review]
Simple fix

thx, loooks good.
Attachment #276131 - Flags: review?(bienvenu) → review+
(Assignee)

Updated

11 years ago
Status: REOPENED → ASSIGNED
Keywords: perf → checkin-needed
Checking in mailWindowOverlay.xul;
/cvsroot/mozilla/mailnews/base/resources/content/mailWindowOverlay.xul,v  <--  mailWindowOverlay.xul
new revision: 1.334; previous revision: 1.333
done
Keywords: checkin-needed
(Assignee)

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Depends on: 419379
You need to log in before you can comment on or make changes to this bug.