Last Comment Bug 593525 - Deleting a mail message from a collapsed mail thread causes all the other messages to disappear from the subject pane but the next message in the thread appears in the message display window
: Deleting a mail message from a collapsed mail thread causes all the other mes...
Status: RESOLVED FIXED
: regression
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Message Display (show other bugs)
: Trunk
: x86 Windows 7
: -- major with 1 vote (vote)
: seamonkey2.1b2
Assigned To: neil@parkwaycc.co.uk
:
:
Mentors:
Depends on:
Blocks: 573392 650703
  Show dependency treegraph
 
Reported: 2010-09-03 17:47 PDT by Ed Novak
Modified: 2013-07-10 00:36 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
UI fix [Checked in: See comment 10] (508 bytes, patch)
2010-12-09 14:44 PST, neil@parkwaycc.co.uk
mnyromyr: review+
Details | Diff | Splinter Review
Remove useless code [Checked in: Comment 7] (520 bytes, patch)
2010-12-09 14:46 PST, neil@parkwaycc.co.uk
mozilla: review+
Details | Diff | Splinter Review

Description Ed Novak 2010-09-03 17:47:15 PDT
User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b4pre) Gecko/20100817 NOT Firefox/3.6  SeaMonkey/2.1a3
Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0b4pre) Gecko/20100817 NOT Firefox/3.6  SeaMonkey/2.1a3

When deleting threaded mail messages one at a time while the thread is collapsed causes the remaining messages in the thread to disappear from the subject window but the actual messages are still shown in the message display pane.  Clicking delete again will most times cause the messages to reappear in the subject pane but not always.  If the thread of messages is expanded then this does not occur.

Reproducible: Always

Steps to Reproduce:
1.View a mail message from a thread of e-mails and make sure the thread is collapsed.
2.Delete the message and the remaining messages in the thread will disappear from the subject line pane but the next message in the thread will be in the message display pane.
3.Expand the thread of messages and then they can be deleted one at a time without the subject line disappearing from the subject pane.


Expected Results:  
Messages should be able to be deleted from a collapsed thread one at a time without the subject of the thread disappearing.

This problem did not occur until I updated from Seamonkey 2.1a2 to 2.1a3 and I have the same problem on my home computer and my work computer - one machine is running Windows 7 Ultimate and the other is running Windows XP SP3.

I am using the Skypilot Theme on both machines.
Comment 1 Ed Novak 2010-10-22 10:29:33 PDT
Went back to 2.0.8 when the above problem occurred.  Updated to 2.1b1 yesterday - same problem still occurs.  Now using default theme since Skypilot does not work with 2.1b1
Comment 2 neil@parkwaycc.co.uk 2010-12-09 08:19:42 PST
This is all tabmail's fault.

What happens on the 1.9.1 branch is that nsMsgDBView::DeleteMessages stashes the index of the collapsed thread in mIndiciesToNoteChange so that it can remove it from the view when the delete completes. But when the header is actually deleted nsMsgThreadedDBView steps in, removes the index from mIndicesToNoteChange, and notes it as a change instead.

What happens on trunk is that when we then get notified that the total message count changed, tabmail wants to update its tab title. This triggers an update of the mail toolbar, which then tries to update the file button, which then tries to collect the selected messages. Unfortunately this makes nsMsgDBView re-add all the deleted indices to the list of changes to note. So we may end up trying to remove the rows twice, or (as in this case) when we shouldn't.
Comment 3 neil@parkwaycc.co.uk 2010-12-09 14:42:21 PST
Actually there are two ways of fixing this bug.

For some reason GetHeadersFromSelection adds the selection to the list of indices to be deleted if we're deleting rows. Now nobody normally calls GetHeadersFromSelection while we're deleting rows; they all call it first, and then delete the rows, maintaining the list of indices to be deleted.

The other issue is that the UI doesn't actually need to call GetMsgHdrsForSelection (which calls GetHeadersFromSelection). This was accidentally caused by bug 573392 :-(
Comment 4 neil@parkwaycc.co.uk 2010-12-09 14:44:57 PST
Created attachment 496629 [details] [diff] [review]
UI fix
[Checked in: See comment 10]

We don't need to do archive command handling for the file button.
Comment 5 neil@parkwaycc.co.uk 2010-12-09 14:46:35 PST
Created attachment 496630 [details] [diff] [review]
Remove useless code
[Checked in: Comment 7]

Because nobody actually sets m_deletingRows until after calling this.
Comment 6 David :Bienvenu 2010-12-14 08:15:56 PST
This code was introduced in bug 205877 but I don't hit the code proposed for removal when I toggle the junk status, so I suspect the code to handle toggling junk status has changed.
Comment 7 neil@parkwaycc.co.uk 2010-12-16 15:42:14 PST
Comment on attachment 496630 [details] [diff] [review]
Remove useless code
[Checked in: Comment 7]

Pushed changeset a11c5d467280 to comm-central.

Leaving bug open since attachment 496629 [details] [diff] [review] is still wanted.
Comment 8 Karsten Düsterloh 2010-12-19 10:52:34 PST
The steps given in comment 0 don't work for me.
If I collapse the thread, either the selected messages changes to the thread starter or there's no selected message in the thread pane anymore, hence I can't delete it anyway?

And what is "the remaining messages in the thread will disappear from the subject line pane" supposed to mean?
Comment 9 Karsten Düsterloh 2010-12-19 16:35:17 PST
Comment on attachment 496629 [details] [diff] [review]
UI fix
[Checked in: See comment 10]

I can reproduce now using 2.1b1 when deleting the thread starter message.

>+++ b/suite/mailnews/mail3PaneWindowCommands.js	Thu Dec 09 22:44:04 2010 +0000
>       case "cmd_file":
>+        return (GetNumSelectedMessages() > 0);

No braces necessary.
Comment 10 neil@parkwaycc.co.uk 2010-12-25 16:06:53 PST
Pushed changeset 8310ed9cac86 to comm-central.
Comment 11 Michael 2013-07-09 22:57:39 PDT
A couple of comments:
1) Deleting all the attached threads just lost five days worth of work transferring from another imap server.  I was deleting ONLY the duplicates and lost thousands of messages instead.
2) Adding a button to turn-off threads and ONLY delete the TRASH to the delete pop-up would have been a nice gesture instead of just "warning" me.
3) I had no idea how to turn off threads...  There is a bug in the float on threads that says click to see threads even if they are already turned on.  It DOES NOT SAY "click to TURN OFF threads."

OK:
Three feature requests that I am too tired to write up:
1) Pop-up Must have an extra TURN-OFF Threads button to ONLY DELETE THESE Messages (in my case they were in the TRASH folder already.
2) Fix the bug on the threads mouse float/title to say TURN-OFF Threads instead of always "click to see threads" when they are ALREADY ON.
3) I get threads, but really, in the trash folder???  Delete all threads...???
Comment 12 neil@parkwaycc.co.uk 2013-07-10 00:36:12 PDT
(In reply to Michael from comment #11)
> A couple of comments:
Sorry but these comments don't appear to be related to this particular issue, and mentioning them here won't help to get them addressed.

> 3) I had no idea how to turn off threads...  There is a bug in the float on
> threads that says click to see threads even if they are already turned on. 
> It DOES NOT SAY "click to TURN OFF threads."
This probably dates back to the time when it didn't turn off threads, and nobody thought to update the tooltip when the behaviour was changed.

Note You need to log in before you can comment on or make changes to this bug.