340886 - Junk status changes to "unknown" when I delete or separate an attachment

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[Gabriela]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; de; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Thunderbird Version 1.5.0.4 (20060516)

When deleting or separating an attachment from an incoming mail, the junk status of this mail always changes from "no junk" (or "junk") to "unknown" (it shows the red question mark). 

Reproducible: Always

Steps to Reproduce:
1. You get mail which is automatically or manually marked as "junk" (j) or "no junk" (shift+j)
2. Now delete or separate an attachment from this mail


Actual Results:  
You'll see the junk status will change to "unknown"

Expected Results:  
Thunderbird shouldn't forget the junk status of a mail.

This is Thunderbird Version 1.5.0.4 with the Mnenhy 0.7.4 extension installed, but I don't think that Mnenhy is responsable for this behaviour.

Comment 1

[Tilmann Reh]

SeaMonkey (any version from 1.0a to 1.0.2) behaves exactly the same - I assume this is an issue related to the core (not only the Thunderbird application).

Comment 2

[David :Bienvenu]

is this IMAP or POP3?

Comment 3

[Tilmann Reh]

It's perfectly reproducible with POP3.
I can't tell if the bug is also present with IMAP since I am using only POP3.

Comment 4

[Gabriela]

Yes, this bug is reproducible with POP3 and IMAP.

Comment 5

[Robert Rothenberg]

I can reproduce something similar too:

1. Mark messages as junk (the messages in particular were
   from a mailing list, and moved to a folder by filters)

2. Go to Junk folder

3. Select attachments in messages and choose Delete All

The junk flag is unmarked.

Comment 6

[Kent James (:rkent)]

I can reproduce the behavior in comment 5 in a build based on the 2008-03-27 Thunderbird trunk.

Comment 7

[Kent James (:rkent)]

I want to fix this as part of my general effort to properly maintain message metadata. I can still confirm that junk info is lost when detaching attachments. Presumably any other metadata that is not saved in the message is also lost.

Comment 8

[Kent James (:rkent)]

Attachment: Support msgToReplace in CopyFileMessage

This fix seems to work initially, on both local and IMAP folders, but I want to check it a little more before review.

Comment 9

[Kent James (:rkent)]

Attachment: rev C, fixes issues in IMAP

There's deceptively little code changed here, which masks the complexity of the change. That's because so much of the infrastructure was in place to support adding a source header to CopyFileMessage - it just didn't work and wasn't used. I've tested this on IMAP (both saving offline and not) and local messages, and it all seems to work now. junkscore was one little complication. It was not being preserved in IMAP CopyFileMessage, so I just let the SetPendingHeaders do the work regardless of whether custom flags were supported. That seems to work as far as I can tell.

Comment 10

[David :Bienvenu]

Comment on attachment 382360 [details] [diff] [review]
rev C, fixes issues in IMAP

can you just remove the commented out line here?

-      rv = DeleteMessage(msgToReplace, msgWindow, PR_TRUE, PR_TRUE);
+      //rv = DeleteMessage(msgToReplace, msgWindow, PR_TRUE, PR_TRUE);
+      rv = OnCopyCompleted(fileSupport, PR_TRUE);

otherwise, this looks good (if a little scary).

Comment 11

[Kent James (:rkent)]

I left that deleted line intentionally, since the original intent of the code was to delete the message at that point, but I did not want to take the time now to make sure that delete worked, plus fix all of the places where there are workarounds because delete did not work.

Perhaps if I expanded the comment to make this clear?

Comment 12

[David :Bienvenu]

sure, an expanded comment is fine, thx. But you really don't need to leave commented out code - you can say "this code used to call DeleteMessage()...

Comment 13

[Tilmann Reh]

Not directly related to the junk status, but to another status information:

When a message with attachment was received with "Disposition-Notification" and the attachment is deleted, the notification process is started again (in my case, I get the according dialog with the current version of SM).

Perhaps you can also check the behaviour in this case, while looking at status changes upon attachment delete. Maybe it's just another flag that must be copied to the new message.

Comment 14

[Kent James (:rkent)]

The patch follows the pattern I have done in similar bugs (in fact uses the same code) which iterates through all properties, and copies all except those that are specifically disabled. So it should solve most database-related issues that occur from attachment deleting or detaching.

But it would be worth testing other cases after the bug lands. It would be great if you would do that, Tilmann. The bug will probably land in the next few days.

Comment 15

[Tilmann Reh]

I will have a look at the latest SM 2.0 trunk builds next week.

Comment 16

[Kent James (:rkent)]

Attachment: Fixed nits, patch to checkin (carrying over reviews)

Comment 17

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/0a6bd5fc4c4d

Comment 18

[Tilmann Reh]

I just tested with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090618 SeaMonkey/2.0b1pre - Build 20090618001425.

The junk status does not change upon deletion of an attachment any more, so this issue really is fixed.
However, disposition notification requests still cause a second question dialog after an attachment has been deleted. This applies to both POP3 and IMAP.

Additionally, when deleting a received message (moving it to the trash folder) that had a disposition notification request, a new question dialog appears when the message is later viewed in the trash folder (tested with IMAP). The current branch versions (at least SM 1.1.16) behave the same way, so it's not a new bug.