205877 - when you toggle junk status from thread pane icon, and you've got it set to move on manual mark, assertion and painting problems

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[(not reading, please use [e-mail] instead)]

when you toggle junk status from thread pane icon, and you've got it set to move
on manual mark, assertion and painting problems

> ###!!! ASSERTION: selected indices for deletion is 0: 'selectionCount', file c:/
> trees/trunk/mozilla/mailnews/base/src/nsMsgDBView.cpp, line 5504

> you can get that if you use the thread pane junk status icon and the message
> isn't selected.

spun off from bug #194273

Comment 1

[(not reading, please use [e-mail] instead)]

accepting

Comment 2

[(not reading, please use [e-mail] instead)]

I want this to block 1.4 final, as I don't want to ship with this.

it doesn't block 1.4, rc1.

Comment 3

[sairuh (rarely reading bugmail)]

agreed. i've accidentally deleted a couple of non-junk messages because of this.

Comment 4

[Jon Granrose]

is this the bug I'm seeing where I mark a mail as junk and it gets moved from my
folder to my junk folder, but the mail subject pane doesn't refesh so it looks
like it's still in the original folder?

Comment 5

[parish]

Jon: Yes, it is.

Seth: FYI in case you haven't seen it happen. This actually works, sometimes.

The situation where it always seems to work is:

1. You receive some mails, none of which get marked as junk.
2. Select one of the messages to read it.
3. Mark one or more of the *other* messages as junk; they get moved and the
thread pane gets updated.

Comment 6

[Samir Gehani]

adt: nsbeta1+/adt1

Comment 7

[emmet]

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

Comment 8

[(not reading, please use [e-mail] instead)]

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

Comment 9

[(not reading, please use [e-mail] instead)]

hope to have this bug fixed on trunk and branch by 6/4/03

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

This is a dataloss bug; raising severity.  Looking forward to a patch.

Comment 11

[(not reading, please use [e-mail] instead)]

sliding out one day, but still going to have for 1.4 final

Comment 12

[(not reading, please use [e-mail] instead)]

Attachment: patch

Comment 13

[(not reading, please use [e-mail] instead)]

Attachment: patch

testing now, I'll seek bienvenu's review tomorrow.

Comment 14

[David :Bienvenu]

Ah, it's all clear to me now - I'd forgotten how this worked. The way this whole
m_deletingRows crap works is that it ignores deletes until the delete is
finished, and then deletes the selected rows. It does this so that we won't
delete each selected row one after the other, and cause the subsequent row to be
selected for each message deleted. That assumption is completely bogus in this
case. It's lucky that things work as well as they do. We remove the correct item
from the view, but we notify the tree about the wrong row getting deleted. 

I can think of two ways to fix this:
1. Actually check if a message is in the selection or not before deciding not to
send notifications. So, we'd check !m_deletingRows or !isSelected before calling
NoteChange. We'd still need a way to avoid sending a delete notification for the
selection...

2. When we start a delete, save away the selection into an array of keys to be
deleted, and then when the delete is completed, instead of notifying about the
delete of the selection, notfiy about the delete of the indexes of the array of
keys we just saved. This is the much more general fix. It would involve looking
up the indexes of the keys when the delete finished, in case something had
changed while the delete was going on.

Your fix works for the case of toggling the junk column, but I think it's still
wrong in the sense that we still send a notification to the tree control about
the selection being deleted. For example, if I select 10 messages, then toggle
the junk column for another message, I think we'll send the tree control a
notification that the 10 rows were deleted.

Comment 15

[(not reading, please use [e-mail] instead)]

david and I have been aiming about this, and agree that the problem is not
calling NoteChange() in one case, or calling it incorrectly in another.

I like his option #2, and will work on it.

Comment 16

[(not reading, please use [e-mail] instead)]

Attachment: david's suggestion, testing now, need to test the "move / delete lots of messages" case

here's a patch (need to test) for david's option #2.

Comment 17

[(not reading, please use [e-mail] instead)]

Comment on attachment 125059 [details] [diff] [review]
david's suggestion, testing now, need to test the "move / delete lots of messages" case

whoops, that patch needs work.

by time we get back into nsMsgDBView::OnDeleteCompleted, keys are already
deleted,
so calling FindKey() returns the "none" index, so calling ::NoteChange()
doesn't work.

Comment 18

[(not reading, please use [e-mail] instead)]

Attachment: patch

Comment 19

[David :Bienvenu]

Comment on attachment 125063 [details] [diff] [review]
patch

sr=bienvenu

Comment 20

[(not reading, please use [e-mail] instead)]

fixed on trunk and branch.

r/sr=bienvenu, a=sspitzer

there are two spin off issues, I'll log them and link back to them here.

Comment 21

[(not reading, please use [e-mail] instead)]

spin off issues:

bug #208515:
need to adjust the mIndicesToNoteChange when hdrs are added or subtracted 

bug #208516:
toggle junk icon when mutliple rows selected, we end up selecting something else
after move / delete

neither of these would block 1.4 final

Comment 22

[[e-mail]]

Hooray! This means that we can remove that silly hack in mailContextMenus.js
(basically anything to do with gThreadPaneMoveOrDeleteOccurred).

Comment 23

[Jon Granrose]

works for me.  thanks, guys!

Comment 24

[Ninoschka Baca]

Branch and Trunk builds: WinMe, Mac 10.1.5, Linux RH 8
This appears to be fixed, but

I noticed on windows and linux that the first time I selected a message, marked
it as junk, then quickly marked a second message as junk that they did not move.
It seemed like I had to unjunk then mark the messages as junk again for it to
work. From then on marking as junk moved the appropriate messages.

Esther, can you try this and see if you get the same results?

Comment 25

[Ninoschka Baca]

Verified Fixed, for initial issue.

Will keep an eye on additional issue observed in comment# 24 but not a showstopper.