Bug 340886 - "Junk status changes to "unknown" when I delete or separate an attachment" [r+sr=bienvenu]
authorKent James <kent@caspia.com>
Wed, 17 Jun 2009 09:53:52 +0100
changeset 2860 0a6bd5fc4c4d4fe10a1b03f96a2d024f334cec16
parent 2859 d97f55ec5ca0873495e9dc9fdeb9c14737c8b53a
child 2861 6129e94e587028b1bbb1b39c5d10bcb39e88f14f
push id2319
push userbugzilla@standard8.plus.com
push dateWed, 17 Jun 2009 08:58:30 +0000
treeherdercomm-central@9f8df696c12b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs340886
Bug 340886 - "Junk status changes to "unknown" when I delete or separate an attachment" [r+sr=bienvenu]
mailnews/base/public/nsIMsgCopyService.idl
mailnews/base/src/nsMessenger.cpp
mailnews/base/src/nsMsgCopyService.cpp
mailnews/imap/src/nsImapMailFolder.cpp
mailnews/local/src/nsLocalMailFolder.cpp
--- a/mailnews/base/public/nsIMsgCopyService.idl
+++ b/mailnews/base/public/nsIMsgCopyService.idl
@@ -89,21 +89,22 @@ interface nsIMsgCopyService : nsISupport
                     in nsIMsgWindow msgWindow);
 
   /**
    * Copies message in rfc format from file to folder.
    *
    * @param aFile             A file which contains message in rfc format which
    *                          will copied to destFolder.
    * @param dstFolder         Destination folder where a message will be copied.
-   * @param msgToReplace      Header which identifies message which will be
-   *                          replaced by newly copied message or null. It is
-   *                          used generally for drafts, when saving a new copy
-   *                          of a draft message should replace the old
-   *                          draft message.
+   * @param msgToReplace      Header which identifies a message to use as a source
+   *                          of message properties, or null. For example, when
+   *                          deleting an attachment, the processed message is
+   *                          stored in a file, but the metadata should be copied
+   *                          from the original message. This method will NOT delete
+   *                          the original message.
    * @param isDraftOrTemplate Specifies whether a message is a stored in draft
    *                          folder or not. If is true listener should
    *                          implement GetMessageId and return unique id for
    *                          message in destination folder. This is important
    *                          for IMAP servers which doesn't support uidplus.
    *                          If destination folder contains message with the
    *                          same message-id then it is possible that listener
    *                          get wrong message key in callback
--- a/mailnews/base/src/nsMessenger.cpp
+++ b/mailnews/base/src/nsMessenger.cpp
@@ -2423,35 +2423,35 @@ nsDelAttachListener::OnStopRequest(nsIRe
   mMessageFolder->CopyDataDone();
   if (NS_FAILED(aStatusCode))
     return aStatusCode;
 
   // called when we complete processing of the StreamMessage request.
   // This is called before OnStopRunningUrl().
   nsresult rv;
 
-  // copy the file back into the folder. Note: if we set msgToReplace then
-  // CopyFileMessage() fails, do the delete ourselves
+  // copy the file back into the folder. Note: setting msgToReplace only copies
+  // metadata, so we do the delete ourselves
   nsCOMPtr<nsIMsgCopyServiceListener> listenerCopyService;
   rv = this->QueryInterface( NS_GET_IID(nsIMsgCopyServiceListener), getter_AddRefs(listenerCopyService) );
   NS_ENSURE_SUCCESS(rv,rv);
 
   mMsgFileStream->Close();
   mMsgFileStream = nsnull;
   mNewMessageKey = PR_UINT32_MAX;
   nsCOMPtr<nsIMsgCopyService> copyService = do_GetService(NS_MSGCOPYSERVICE_CONTRACTID);
   m_state = eCopyingNewMsg;
   // clone file because nsIFile on Windows caches the wrong file size.
   nsCOMPtr <nsIFile> clone;
   mMsgFile->Clone(getter_AddRefs(clone));
   if (copyService)
   {
     nsCString originalKeys;
     mOriginalMessage->GetStringProperty("keywords", getter_Copies(originalKeys));
-    rv = copyService->CopyFileMessage(clone, mMessageFolder, nsnull, PR_FALSE,
+    rv = copyService->CopyFileMessage(clone, mMessageFolder, mOriginalMessage, PR_FALSE,
                                       mOrigMsgFlags, originalKeys, listenerCopyService, mMsgWindow);
   }
   return rv;
 }
 
 //
 // nsIStreamListener
 //
--- a/mailnews/base/src/nsMsgCopyService.cpp
+++ b/mailnews/base/src/nsMsgCopyService.cpp
@@ -600,17 +600,20 @@ nsMsgCopyService::CopyFileMessage(nsIFil
   if (NS_FAILED(rv)) goto done;
 
   rv = copyRequest->Init(nsCopyFileMessageType, fileSupport, dstFolder,
                          isDraft, aMsgFlags, aNewMsgKeywords, listener, window, PR_FALSE);
   if (NS_FAILED(rv)) goto done;
 
   if (msgToReplace)
   {
-    copySource = copyRequest->AddNewCopySource(dstFolder);
+    // The actual source of the message is a file not a folder, but
+    // we still need an nsCopySource to reference the old message header
+    // which will be used to recover message metadata.
+    copySource = copyRequest->AddNewCopySource(nsnull);
     if (!copySource)
     {
         rv = NS_ERROR_OUT_OF_MEMORY;
         goto done;
     }
     copySource->AddMessage(msgToReplace);
   }
 
--- a/mailnews/imap/src/nsImapMailFolder.cpp
+++ b/mailnews/imap/src/nsImapMailFolder.cpp
@@ -6698,17 +6698,17 @@ void nsImapMailFolder::SetPendingAttribu
   dontPreserveEx.AppendLiteral(" ");
 
   // these properties are set as integers below, so don't set them again
   // in the iteration through the properties
   dontPreserveEx.AppendLiteral("offlineMsgSize msgOffset flags priority ");
 
   // these fields are either copied separately when the server does not support
   // custom IMAP flags, or managed directly through the flags
-  dontPreserveEx.AppendLiteral("keywords label junkscore ");
+  dontPreserveEx.AppendLiteral("keywords label ");
 
   PRUint32 i, count;
 
   rv = messages->GetLength(&count);
   if (NS_FAILED(rv)) 
     return;
 
   // check if any msg hdr has special flags or properties set
@@ -6716,20 +6716,16 @@ void nsImapMailFolder::SetPendingAttribu
   for (i = 0; i < count; i++)
   {
     nsCOMPtr <nsIMsgDBHdr> msgDBHdr = do_QueryElementAt(messages, i, &rv);
     if (mDatabase && msgDBHdr)
     {
       if (!(supportedUserFlags & kImapMsgSupportUserFlag))
       {
         nsMsgLabelValue label;
-        nsCString junkScore;
-        msgDBHdr->GetStringProperty("junkscore", getter_Copies(junkScore));
-        if (!junkScore.IsEmpty()) // ignore already scored messages.
-          mDatabase->SetAttributeOnPendingHdr(msgDBHdr, "junkscore", junkScore.get());
         msgDBHdr->GetLabel(&label);
         if (label != 0)
         {
           nsCAutoString labelStr;
           labelStr.AppendInt(label);
           mDatabase->SetAttributeOnPendingHdr(msgDBHdr, "label", labelStr.get());
         }
         nsCString keywords;
@@ -7300,20 +7296,31 @@ nsImapMailFolder::CopyFileMessage(nsIFil
       return OnCopyCompleted(srcSupport, rv);
 
     rv = QueryInterface(NS_GET_IID(nsIUrlListener), getter_AddRefs(urlListener));
 
     if (msgToReplace)
     {
         rv = msgToReplace->GetMessageKey(&key);
         if (NS_SUCCEEDED(rv))
-            messageId.AppendInt((PRInt32) key);
-    }
-
-    rv = InitCopyState(srcSupport, messages, PR_FALSE, isDraftOrTemplate,
+        {
+          messageId.AppendInt((PRInt32) key);
+          // Perhaps we have the message offline, but even if we do it is
+          // not valid, since the only time we do a file copy for an
+          // existing message is when we are changing the message.
+          // So set the offline size to 0 to force SetPendingAttributes to
+          // clear the offline message flag.
+          msgToReplace->SetOfflineMessageSize(0);
+          messages->AppendElement(msgToReplace, PR_FALSE);
+          SetPendingAttributes(messages, PR_FALSE);
+        }
+    }
+
+    PRBool isMove = (msgToReplace ? PR_TRUE : PR_FALSE);
+    rv = InitCopyState(srcSupport, messages, isMove, isDraftOrTemplate,
                        PR_FALSE, aNewMsgFlags, aNewMsgKeywords, listener, 
                        msgWindow, PR_FALSE);
     if (NS_FAILED(rv))
       return OnCopyCompleted(srcSupport, rv);
 
     m_copyState->m_streamCopy = PR_TRUE;
     nsCOMPtr<nsISupports> copySupport;
     if( m_copyState )
--- a/mailnews/local/src/nsLocalMailFolder.cpp
+++ b/mailnews/local/src/nsLocalMailFolder.cpp
@@ -2180,18 +2180,21 @@ nsMsgLocalMailFolder::CopyFileMessage(ns
 
     if (NS_SUCCEEDED(rv))
       rv = CopyData(inputStream, (PRInt32) fileSize);
 
     if (NS_SUCCEEDED(rv))
       rv = EndCopy(PR_TRUE);
 
     //mDatabase should have been initialized above - if we got msgDb
+    // If we were going to delete, here is where we would do it. But because
+    // existing code already supports doing those deletes, we are just going
+    // to end the copy.
     if (NS_SUCCEEDED(rv) && msgToReplace && mDatabase)
-      rv = DeleteMessage(msgToReplace, msgWindow, PR_TRUE, PR_TRUE);
+      rv = OnCopyCompleted(fileSupport, PR_TRUE);
 
     if (inputStream)
       inputStream->Close();
   }
 
   if(NS_FAILED(rv))
     (void) OnCopyCompleted(fileSupport, PR_FALSE);
 
@@ -2528,18 +2531,17 @@ NS_IMETHODIMP nsMsgLocalMailFolder::EndC
         mCopyState->m_fileStream->Flush();
       else
         seekableStream->Seek(nsISeekableStream::NS_SEEK_CUR, 0);
     }
   }
   //Copy the header to the new database
   if (copySucceeded && mCopyState->m_message)
   {
-    //  CopyMessages() goes here; CopyFileMessage() never gets in here because
-    //  the mCopyState->m_message will be always null for file message
+    //  CopyMessages() goes here, and CopyFileMessages() with metadata to save;
     nsCOMPtr<nsIMsgDBHdr> newHdr;
     if(!mCopyState->m_parseMsgState)
     {
       if(mCopyState->m_destDB)
       {
         rv = mCopyState->m_destDB->CopyHdrFromExistingHdr(mCopyState->m_curDstKey,
           mCopyState->m_message, PR_TRUE,
           getter_AddRefs(newHdr));