fix handling of stat errors and don't stat noselect folders when checking individual folders for new messsages, r/sr=standard8 bug 581707

mailnews/imap/src/nsImapIncomingServer.cpp

@@ -2366,7 +2366,7 @@ nsImapIncomingServer::OnStopRunningUrl(nsIURI *url, nsresult exitCode)
       }
       // if we get an error running the url, it's better
       // not to chain the next url.
-      if (NS_FAILED(exitCode))
+      if (NS_FAILED(exitCode) && exitCode != NS_MSG_ERROR_IMAP_COMMAND_FAILED)
         m_foldersToStat.Clear();
       if (m_foldersToStat.Count() > 0)
         m_foldersToStat[0]->UpdateStatus(this, nsnull);
@@ -2994,19 +2994,20 @@ nsImapIncomingServer::GetNewMessagesForNonInboxFolders(nsIMsgFolder *aFolder,
   // or if we are forced to check all folders
   PRUint32 flags = 0;
   aFolder->GetFlags(&flags);
-  if ((forceAllFolders &&
+  nsresult rv;
-      !(flags & (nsMsgFolderFlags::Inbox | nsMsgFolderFlags::Trash | nsMsgFolderFlags::Junk |
+  nsCOMPtr<nsIMsgImapMailFolder> imapFolder = do_QueryInterface(aFolder, &rv);
-               nsMsgFolderFlags::ImapNoselect | nsMsgFolderFlags::Virtual))) ||
+  NS_ENSURE_SUCCESS(rv, rv);
-      (flags & nsMsgFolderFlags::CheckNew))
+  PRBool canOpen;
+  imapFolder->GetCanOpenFolder(&canOpen);
+  if (canOpen && ((forceAllFolders &&
+                 !(flags & (nsMsgFolderFlags::Inbox | nsMsgFolderFlags::Trash |
+                   nsMsgFolderFlags::Junk | nsMsgFolderFlags::Virtual))) ||
+                flags & nsMsgFolderFlags::CheckNew))
   {
     // Get new messages for this folder.
     aFolder->SetGettingNewMessages(PR_TRUE);
     if (performingBiff)
-    {
+      imapFolder->SetPerformingBiff(PR_TRUE);
-      nsCOMPtr<nsIMsgImapMailFolder> imapFolder(do_QueryInterface(aFolder));
-      if (imapFolder)
-        imapFolder->SetPerformingBiff(PR_TRUE);
-    }
     PRBool isOpen = PR_FALSE;
     nsCOMPtr <nsIMsgMailSession> mailSession = do_GetService(NS_MSGMAILSESSION_CONTRACTID);
     if (mailSession && aFolder)
@@ -3022,9 +3023,7 @@ nsImapIncomingServer::GetNewMessagesForNonInboxFolders(nsIMsgFolder *aFolder,
     }
     if (gUseStatus && !isOpen)
     {
-      nsCOMPtr <nsIMsgImapMailFolder> imapFolder = do_QueryInterface(aFolder);
+      if (!isServer && m_foldersToStat.IndexOf(imapFolder) == -1)
-      if (imapFolder && !isServer &&
-          m_foldersToStat.IndexOf(imapFolder) == -1)
         m_foldersToStat.AppendObject(imapFolder);
     }
     else
@@ -3033,7 +3032,7 @@ nsImapIncomingServer::GetNewMessagesForNonInboxFolders(nsIMsgFolder *aFolder,
 
   // Loop through all subfolders to get new messages for them.
   nsCOMPtr<nsISimpleEnumerator> enumerator;
-  nsresult rv = aFolder->GetSubFolders(getter_AddRefs(enumerator));
+  rv = aFolder->GetSubFolders(getter_AddRefs(enumerator));
   if (NS_FAILED(rv))
     return rv;
 

mailnews/imap/test/unit/test_dontStatNoSelect.js

@@ -0,0 +1,119 @@
+// This file tests that checking folders for new mail with STATUS
+// doesn't try to STAT noselect folders.
+
+var gServer, gImapServer;
+var gIMAPInbox, gIMAPFolder1, gIMAPFolder2;
+var gFolder2Mailbox;
+
+load("../../mailnews/resources/messageGenerator.js");
+
+const nsIIOService = Cc["@mozilla.org/network/io-service;1"]
+                     .getService(Ci.nsIIOService);
+
+function run_test() {
+  var daemon = new imapDaemon();
+  daemon.createMailbox("folder 1", {subscribed : true});
+  let folder1Mailbox = daemon.getMailbox("folder 1");
+  folder1Mailbox.flags.push("\\Noselect");
+  daemon.createMailbox("folder 2", {subscribed : true});
+  gFolder2Mailbox = daemon.getMailbox("folder 2");
+  addMessageToFolder(gFolder2Mailbox);
+  gServer = makeServer(daemon, "");
+
+  gImapServer = createLocalIMAPServer();
+  gImapServer.maximumConnectionsNumber = 1;
+
+  loadLocalMailAccount();
+
+  // We need an identity so that updateFolder doesn't fail
+  let acctMgr = Cc["@mozilla.org/messenger/account-manager;1"]
+                  .getService(Ci.nsIMsgAccountManager);
+  let localAccount = acctMgr.createAccount();
+  let identity = acctMgr.createIdentity();
+  localAccount.addIdentity(identity);
+  localAccount.defaultIdentity = identity;
+  localAccount.incomingServer = gLocalIncomingServer;
+  acctMgr.defaultAccount = localAccount;
+
+  // Let's also have another account, using the same identity
+  let imapAccount = acctMgr.createAccount();
+  imapAccount.addIdentity(identity);
+  imapAccount.defaultIdentity = identity;
+  imapAccount.incomingServer = gImapServer;
+
+  // Get the folder list...
+  gImapServer.performExpand(null);
+  gServer.performTest("SUBSCRIBE");
+  // pref tuning: one connection only, turn off notifications
+  let prefBranch = Cc["@mozilla.org/preferences-service;1"]
+                     .getService(Ci.nsIPrefBranch);
+  // Make sure no biff notifications happen
+  prefBranch.setBoolPref("mail.biff.play_sound", false);
+  prefBranch.setBoolPref("mail.biff.show_alert", false);
+  prefBranch.setBoolPref("mail.biff.show_tray_icon", false);
+  prefBranch.setBoolPref("mail.biff.animate_dock_icon", false);
+
+  let rootFolder = gImapServer.rootFolder;
+  gIMAPInbox = rootFolder.getFolderWithFlags(Ci.nsMsgFolderFlags.Inbox);
+  gFolder1 = rootFolder.getChildNamed("folder 1");
+  gFolder2 = rootFolder.getChildNamed("folder 2");
+  gFolder1.setFlag(Ci.nsMsgFolderFlags.CheckNew);
+  gFolder2.setFlag(Ci.nsMsgFolderFlags.CheckNew);
+  do_test_pending();
+  // imap fake server's resetTest resets the authentication state - charming.
+  // So poke the _test member directly.
+  gServer._test = true;
+  gIMAPInbox.getNewMessages(null, null);
+  gServer.performTest("STATUS");
+  // We want to wait for the STATUS to be really done before we issue
+  // more STATUS commands, so we do a NOOP on the
+  // INBOX, and since we only have one connection with the fake server,
+  // that will essentially serialize things.
+  gServer._test = true;
+  gIMAPInbox.updateFolder(null);
+  gServer.performTest("NOOP");
+  do_timeout_function(0, testCheckStatError);
+}
+
+function testCheckStatError() {
+  // folder 2 should have been stat'd, but not folder 1. All we can really check
+  // is that folder 2 was stat'd and that its unread msg count is 1
+  do_check_eq(gFolder2.getNumUnread(false), 1);
+  addMessageToFolder(gFolder2Mailbox);
+  gFolder1.clearFlag(Ci.nsMsgFolderFlags.ImapNoselect);
+  gServer._test = true;
+  // we've cleared the ImapNoselect flag, so we will attempt to STAT folder 1,
+  // which will fail. So we verify that we go on and STAT folder 2, and that
+  // it picks up the message we added to it above.
+  gIMAPInbox.getNewMessages(null, null);
+  gServer.performTest("STATUS");
+  gServer._test = true;
+  gServer.performTest("STATUS");
+  do_timeout_function(0, endTest);
+}
+
+function addMessageToFolder(mbox) {
+  // make a couple messges
+  let messages = [];
+  let gMessageGenerator = new MessageGenerator();
+  messages = messages.concat(gMessageGenerator.makeMessage());
+
+  let msgURI =
+    nsIIOService.newURI("data:text/plain;base64," +
+                     btoa(messages[0].toMessageString()),
+                     null, null);
+  let message = new imapMessage(msgURI.spec, mbox.uidnext++);
+  mbox.addMessage(message);
+}
+
+function endTest()
+{
+  do_check_eq(gFolder2.getNumUnread(false), 2);
+  // Clean up the server in preparation
+  gServer.resetTest();
+  gImapServer.closeCachedConnections();
+  gServer.performTest();
+  gServer.stop();
+
+  do_test_finished();
+}

mailnews/test/fakeserver/imapd.js

@@ -750,7 +750,7 @@ IMAP_RFC3501_handler.prototype = {
       this._lastCommand = command;
       // Are we allowed to execute this command?
       if (this._enabledCommands[this._state].indexOf(command) == -1)
-        return this._tag + " BAD illegal command for current state";
+        return this._tag + " BAD illegal command for current state " + this._state;
 
       try {
         // Format the arguments nicely
@@ -1001,8 +1001,11 @@ IMAP_RFC3501_handler.prototype = {
     var mbox = this._daemon.getMailbox(args[0]);
     if (!mbox || mbox.name == "")
       return "NO no such mailbox";
-    if (mbox._children.length > 0 && "\\Noselect" in mbox.flags)
+    if (mbox._children.length > 0) {
-      return "NO cannot delete mailbox";
+      for (let i = 0; i < mbox.flags.length; i++)
+        if (mbox.flags[i] == "\\Noselect")
+          return "NO cannot delete mailbox";
+    }
     this._daemon.deleteMailbox(mbox);
     return "OK DELETE completed";
   },
@@ -1055,6 +1058,9 @@ IMAP_RFC3501_handler.prototype = {
     var box = this._daemon.getMailbox(args[0]);
     if (!box)
       return "NO no such mailbox exists";
+    for (let i = 0; i < box.flags.length; i++)
+      if (box.flags[i] == "\\Noselect")
+        return "NO STATUS not allowed on Noselect folder";
     var parts = [];
     for each (var status in args[1]) {
       var line = status + " ";