59694 - IMAP Mark as Deleted Mode: Need to use "Delete" button for toggle mark/unmark for "mark as deleted" msgs

Status: VERIFIED FIXED

(MailNews Core :: Networking: IMAP, defect, P2)

Description

[Karen Huang]

11-08-01-MN6 build:

Overview:
From 4.x, Selecting "Delete" button on the tool bar will unmark "mark as delete"  
messages on the thread pane. Netscape 6 should provide this functionality as 
well, so users still can toggle "mark as deleted" messages between mark or 
unmark for those deleted messages.

Setup Info:
1) Use an IMAP account.
2) Set up "Mark it as deleted" mode for deleting message from Mail/News Account 
settings

Steps:
1) Login to above setup IMAP mail account
2) Delete messages(It will display a "x" sign for those mark as deleted 
messages)
3) Select/highlight that mark deleted messages
4) Select the "delete" button again from the tool bar should unmark the x sign 
in order to undo the "mark as deleted" messages
5) Actual Results: "Delete" button is not working for unmarking "mark as 
deleted" messages in the thread pane.

Expected results: "Delete" button should work for unmarking "mark as deleted" 
messages in the thread pane.

Comment 1

[Scott MacGregor]

triaging to a future release.

Comment 2

[David :Bienvenu]

yes, this is important to fix.

Comment 3

[David :Bienvenu]

changing priorities

Comment 4

[Matthew T (active 1999-2002)]

Don't forget to change `Delete' to `Undelete' in the main menus and context menu 
as well.

Comment 5

[laurel]

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

Comment 6

[laurel]

Regarding the menu change comment: 4.7 win32 doesn't toggle Delete menu 
item/context to Undelete.  

Comment 7

[David :Bienvenu]

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

Comment 8

[scottputterman]

marking nsbeta1+ and moving to mozilla0.8 milestone.

Comment 9

[laurel]

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

Comment 10

[scottputterman]

reassigning to naving

Comment 11

[benc]

re: menu toggle, Communicator 4.7 didn't but it should have. Thinking back 
really far, I can't remember if pine required you to use separate commands for 
delete or undelete...

Outlook has separate "Delete" and "Undelete" items, and they light 
appropriately. Only Delete has a shortcut.

What I would really like is for the "Delete" key and "delete button" or any 
other non-menu UI to toggle the delete flag. That is the fastest way to use the 
delete mode. I don't think the menu design is that important, because everyone 
I know that uses this mode is going for speed.

Comment 12

[scottputterman]

moving to mozilla0.9

Comment 13

[Navin Gupta]

We already have "undo delete messages". Why add another button that does the 
work of undo ?

Comment 14

[[e-mail]]

Because sometimes people want to delete messages that they did not immediately 
delete.

For example, if a filter moved a message, you might decide you want it to remain 
in your INBOX.

Or you might have deleted a message in a previous session, and now you can't 
UNDO DELETE.

The feature was in Communicator 4 + it's consistent with what outlook, eudora 
and pine allow.

Comment 15

[scottputterman]

marking nsbeta1- and moving to future milestone.

Comment 16

[benc]

I have to use communicator to open the same mailbox and undelete messages I 
can't "undo delete" on.

This annoys me every day,
marking for dogfood, using the new criteria.

I'd like to see people use IMAP in this mode for a week and then make a nsbeta 
decision after getting a taste of what this is like.

I also imagine this is not a huge request for engineering time, it's just an 
IMAP flag.

Comment 17

[selmer (gone)]

converting from dogfood to catfood nomination.

Comment 18

[[e-mail]]

I found a workaround:

Select the deleted message you want to undelete.

Delete it "again".
Then undelete.

This suggets that the delete command is somewhat mindless and tries to delete
already deleted stuff. I can investigate this further at some later date.

Comment 19

[[e-mail]]

(spam apology) Cool workaround!

Comment 20

[Karen Huang]

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

Comment 21

[[e-mail]]

I don't think this new dupe is the same bug...

Comment 22

[Tom Mraz]

The workaround doesn't work now Linux nightly 2001061821. Also not platform
specific bug. And setting severity to enh.

Comment 23

[benc]

The workaround is dependent on undo working, which doesn't work all the time.

Comment 24

[Chase Tingley]

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

Comment 25

[Karen Huang]

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

Comment 26

[Karen Huang]

Adding "Undo" in the summary since "Undo" "mark as delete" is not working 
either....
Raising severity to major since there are many people complaining about this 
problem and there are many dup bugs of this bug.....
So, is the target milestone still "Future"? 

Comment 27

[James Nord]

Karen,

Undo delete is working - but the UI does not show this.

Delete a message, choose undo (thew message is marked as deleted) 
change folder and change folder back and the message is not marked as deleted.

Comment 28

[Karen Huang]

Updating the summary to make it more clear.
I am wondering to know whether we can fix this bug earlier...
Are you sure this will be a "Future" bug?

I am nominating this bug for nsBranch since without this bug fix, user won't be 
able to "Undo" the "Mark as delete" messages. (users won't know they need to 
select the other folder to refresh this functionality on UI...)
If this won't be fixed on next release, at least this shouldn't be a "future" 
bug......

Comment 29

[Navin Gupta]

so I found out that we are marking imap delete twice once before we go to the 
server and once after, so it unsets during the deletion, and when we do undo it 
sets back the delete flag.

Comment 30

[Karen Huang]

Undo/Redo is working from bug 94662.
I am updating summary back for toggle.....

Comment 31

[Scott MacGregor]

cleaning up nsbranch keywords. 

Comment 32

[laurel]

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

Comment 33

[Kai Engert [:KaiE:]]

Changing my e-mail address.

Comment 34

[Kai Engert [:KaiE:]]

Removing old e-mail address.

Comment 35

[Gregory Labzovsky]

Attachment: proposed fix

Comment 36

[Gregory Labzovsky]

I've just attached a fix for this bug. It checks if the selected message already
has IMAP_DELETED flag set and unsets it when necessary.

Comment 37

[[e-mail]]

Does this code have the same effect on multiple messages as mark and flag?

Comment 38

[Gregory Labzovsky]

Yes, it does.

Comment 39

[[e-mail]]

Purely as a spin-off from this bug it might be an idea to mark the delete
menuitem and/or mail button as "checked" so that it's more obvious that clicking
delete will actually undelete. Although there is then the question of whether
the news mark button should show as "checked" when the current message is read.

Comment 40

[Gregory Labzovsky]

This is about another bug. See 82056.
Anyone can review the fix?

Comment 41

[Navin Gupta]

your patch does nothing about the undo object, I mean if the user has 
undeleted the message, if he does Edit | Undo delete message what happens?

Also i don't think that updateFolder() is needed. 

Comment 42

[laurel]

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

Comment 43

[laurel]

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

Comment 44

[Karen Huang]

Nominating nsbeta1 since there are many dup bugs in this bug and there are users 
expecting this to work.

Comment 45

[[e-mail]]

Stuff that this needs to work:
1. The "Delete" menuitems needs to be type="checkbox"
2. The context checked states need to be updated when the popups open
3. The main checked state needs to be updated by the controllers
4. Create SelectedMessagesAreDeleted() function in mailWindowOverlay.js
5. isIMAPDeleted attribute needs to be added to nsIMsgHdr.idl
6. C code written to support isIMAPDeleted attribute

I can't do point 6 but I've got a patch for the rest, should you want it.

Comment 46

[David :Bienvenu]

these two are not needed:

5. isIMAPDeleted attribute needs to be added to nsIMsgHdr.idl
6. C code written to support isIMAPDeleted attribute


all you need to do is get the flags and check the imap deleted flag. (Plus, we
don't write c code :-) )

Comment 47

[[e-mail]]

OK, so why does mark read/unread and flagged do it like that then?

Comment 48

[David :Bienvenu]

"to make our js cleaner", according to the comment in the idl. However, it's not
needed. I'd rather not expose all 32 msg flags as individual attributes and
methods in the idl and we have to draw the line somewhere. If the js is terribly
ugly, I guess we could add an attribute, or add a boolean method that takes a
flag and tells you if it's set. Also, the UI changes you're describing should
only be in affect if the imap server of the folder selected is using the imap
delete model, and you'll need to do lots of work to determine that (not everyone
uses the imap delete model).

Comment 49

[[e-mail]]

You'll find it difficult to select a deleted message if you're not using the
IMAP delete model. And this is the only case in which the UI would be different.

Comment 50

[laurel]

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

Comment 51

[Jaime Rodriguez, Jr.]

Is this really a nsbeta1+, P2? If yes, then we need to try and schedule it for
the MachV release. If its not gonna happen then we should nsbeta1- it, or chage
the priority to P3.

Comment 52

[scottputterman]

We need to do this.

Comment 53

[Navin Gupta]

Attachment: proposed fix

The fix is to basically toggle the deleteFlag when deleting already deleted
imap messages. For undo and redo to undo/redo last action I had to add another
method that gives the deleteFlag for the hdr so that we can toggle. 

david, can you review?

Comment 54

[David :Bienvenu]

+    if (flags & MSG_FLAG_IMAP_DELETED)
+      *aResult = PR_TRUE;
+    else
+      *aResult = PR_FALSE;

this can just be 
*aResult = (flags & MSG_FLAG_IMAP_DELETED) != 0;

the rest looks good, r=bienvenu

Comment 55

[Mark Shuttleworth]

Would it be difficult also to toggle the menu and button to reflect the
'undelete' function when a 'marked deleted' message is highlighted?

Comment 56

[Navin Gupta]

That would be a separate bug. this bug is a request to make it behave like 4.x

Comment 57

[laurel]

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

Comment 58

[Navin Gupta]

fixed

Comment 59

[laurel]

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

Comment 60

[Michael Buckland]

Adding to status whiteboard AOLTW and plussing bugs definite requirements,
adding just AOLTW for possibles.

Comment 61

[David :Bienvenu]

Michael, has anyone at AOLTW said they use the IMAP delete model? It's not
something that many users use.

Comment 62

[Michael Buckland]

David, I think we will probably want this fix if we take bug 11926 for AOLTW. 
And it's not all sure we will take bug 11926, hence I just AOLTW'ed this bug,
with no plus.  Hope this helps.

Comment 63

[Michael Buckland]

Removed AOLTW

Comment 64

[Dawn Endico]

removed the item for this bug from the release notes for 0.9.9 and beyond,
because the bug is fixed now. Let me know if you think the item should be 
re-added.

Comment 65

[Karen Huang]

Verified on Windows 05-12-20-branch-pr1
Linux 05-12-23-branch-pr1
Mac 05-12-17-branch-pr1