80451 - Attachements menu not updated correctly

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P2)

Description

[Christos Cheretakis]

This is how to reproduce:

1. Open a message with say 3 attachments
2. Go to attachements menu, you see 3 entries there
3. Save all attachements, one after the other
4. Delete message, which moves you to the next
5. Open attachements menu again, you see one of the files from the previous message

If you follow the same procedure with next message, sometimes you get two
entries in the att. menu from previous messages

Just checked with 2001051008 on linux

Comment 1

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

attachment UI -> mscott

Comment 2

[Scott MacGregor]

that's not good. should only take a couple lines of JS to fix. I must have
messed up my logic for destroying entries in the attachment popup. 

Comment 3

[scottputterman]

I was able to reproduce this on Windows 2000 using 200151404. changing OS to ALL.

Comment 4

[Asa Dotzler [:asa]]

This morning's builds had a bug ( bug 80746 ) which may have led to 
a Bugzilla user inadvertantly 
changing this bug from the Assignbed/Accepted status to the New status.  If you 
are the owner of this bug please check to see that it is in the correct Status.  
Thanks.

Comment 5

[scottputterman]

reassigning to prass

Comment 6

[prass]

Attachment: proposed patch

Comment 7

[prass]

Attachment: updated patch.

Comment 8

[prass]

MScott, can you please r/sr the patch? Thanks!

Comment 9

[Scott MacGregor]

thanks for looking into this prassana. Couple questions....I still don't
understand why what I had previously written doesn't work? 

-     while ( popup.childNodes.length > 2 ) 
-       popup.removeChild(popup.childNodes[0]); 

shouldn't that remove all but the last 2 elements in the popup? The last 2
elements being the separator and the save All Attachments menu item.

Looking at your patch, why don't we set len = .childNodes.length in all cases
instead of setting it sometimes to .childNodes.length and sometimes to
len=GetNumberOfAttachmentsForDisplayedMessage().

Comment 10

[scottputterman]

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

Comment 11

[prass]

Thanks for the update Scott.
1. This should work:
-     while ( popup.childNodes.length > 2 ) 
-       popup.removeChild(popup.childNodes[0]); 


but because the items are inserted at length-2
869       item = popup.insertBefore(item, popup.childNodes[length-2]);

some items keep remaining

2. You are right I should just set it to length instead of getting the biggest one

Here is a new patch

Comment 12

[prass]

Attachment: patch

Comment 13

[prass]

Attachment: final patch

Comment 14

[Scott MacGregor]

sr=mscott

Comment 15

[prass]

fixed

Comment 16

[esther]

Using build 2001-05-17 on win, mac and linux this is fixed.  Note: I didn't have 
to save all the attachments and delete the msg to see this problem.  I verified 
it by selecting a message with 5 attachments, selecting File|Attachment from the 
menu to see the list of attachments.  Then changing to another message with one 
attachment, I use to see all 6 when selecting the File|Attachment.  Now I see 
the right attachments with the right message. Verified