Bug 1231592 - Implement mail.server.default.forceSelect for misbehaving IMAP servers. Use UidExpunge(). r=jorgk, sr=rkent
authorGene Smith <gds@chartertn.net>
Wed, 15 Mar 2017 14:42:00 +0100
changeset 27956 4fd458dcdd4a95a833e577ed0ef2e2b0d176b85e
parent 27955 04d93e773a776bb86827873ba9d7a7d1246c7458
child 27957 0cf73e610aa1cfa7a7b2d9dc6b13c59a70d21e55
push id1966
push userclokep@gmail.com
push dateMon, 12 Jun 2017 16:57:35 +0000
treeherdercomm-beta@32d9b8d10da1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorgk, rkent
bugs1231592
Bug 1231592 - Implement mail.server.default.forceSelect for misbehaving IMAP servers. Use UidExpunge(). r=jorgk, sr=rkent
mail/locales/en-US/chrome/messenger/am-server-advanced.dtd
mailnews/base/prefs/content/am-server-advanced.xul
mailnews/base/prefs/content/am-server.js
mailnews/base/prefs/content/am-server.xul
mailnews/imap/public/nsIImapIncomingServer.idl
mailnews/imap/src/nsImapIncomingServer.cpp
mailnews/imap/src/nsImapProtocol.cpp
mailnews/imap/src/nsImapProtocol.h
mailnews/mailnews.js
suite/locales/en-US/chrome/mailnews/pref/am-server-advanced.dtd
--- a/mail/locales/en-US/chrome/messenger/am-server-advanced.dtd
+++ b/mail/locales/en-US/chrome/messenger/am-server-advanced.dtd
@@ -7,16 +7,19 @@
 <!ENTITY serverDirectory.label "IMAP server directory:">
 <!ENTITY serverDirectory.accesskey "d">
 <!ENTITY usingSubscription.label "Show only subscribed folders">
 <!ENTITY usingSubscription.accesskey "w">
 <!ENTITY dualUseFolders.label "Server supports folders that contain sub-folders and messages">
 <!ENTITY dualUseFolders.accesskey "f">
 <!ENTITY maximumConnectionsNumber.label "Maximum number of server connections to cache">
 <!ENTITY maximumConnectionsNumber.accesskey "M">
+<!ENTITY forceSelect.label "Send IMAP select to server when checking for new mail">
+<!ENTITY forceSelect.tooltip "Some servers need an additional IMAP select so new email is reliably received, or is shown as read after it has been read on another device. Selecting this option will cause some increased network traffic.">
+<!ENTITY forceSelect.accesskey "F">
 <!-- LOCALIZATION NOTE (namespaceDesc.label): Do not translate "IMAP" -->
 <!ENTITY namespaceDesc.label "These preferences specify the namespaces on your IMAP server">
 <!ENTITY personalNamespace.label "Personal namespace:">
 <!ENTITY personalNamespace.accesskey "P">
 <!ENTITY publicNamespace.label "Public (shared):">
 <!ENTITY publicNamespace.accesskey "u">
 <!ENTITY otherUsersNamespace.label "Other Users:">
 <!ENTITY otherUsersNamespace.accesskey "O">
--- a/mailnews/base/prefs/content/am-server-advanced.xul
+++ b/mailnews/base/prefs/content/am-server-advanced.xul
@@ -92,16 +92,22 @@
           <row align="center">
             <separator class="indent"/>
             <checkbox amsa_persist="true" id="overrideNamespaces"
                       label="&overrideNamespaces.label;"
                       accesskey="&overrideNamespaces.accesskey;"/>
           </row>
         </rows>
       </grid>
+
+      <separator class="groove"/>
+      <checkbox amsa_persist="true" id="forceSelect"
+                label="&forceSelect.label;"
+                tooltiptext="&forceSelect.tooltip;"
+                accesskey="&forceSelect.accesskey;"/>
     </vbox>
 
 
     <!-- POP3 Panel -->
     <vbox id="pop3Panel">
       <label flex="1" control="folderStorage">&pop3DeferringDesc.label;</label>
       <hbox align="center">
         <radiogroup id="folderStorage"
--- a/mailnews/base/prefs/content/am-server.js
+++ b/mailnews/base/prefs/content/am-server.js
@@ -128,16 +128,17 @@ function onAdvanced()
   serverSettings.serverPrettyName = gServer.prettyName;
   serverSettings.account = top.getCurrentAccount();
 
   if (serverType == "imap")
   {
     serverSettings.dualUseFolders = document.getElementById("imap.dualUseFolders").checked;
     serverSettings.usingSubscription = document.getElementById("imap.usingSubscription").checked;
     serverSettings.maximumConnectionsNumber = document.getElementById("imap.maximumConnectionsNumber").getAttribute("value");
+    serverSettings.forceSelect = document.getElementById("imap.forceSelect").checked;
     // string prefs
     serverSettings.personalNamespace = document.getElementById("imap.personalNamespace").getAttribute("value");
     serverSettings.publicNamespace = document.getElementById("imap.publicNamespace").getAttribute("value");
     serverSettings.serverDirectory = document.getElementById("imap.serverDirectory").getAttribute("value");
     serverSettings.otherUsersNamespace = document.getElementById("imap.otherUsersNamespace").getAttribute("value");
     serverSettings.overrideNamespaces = document.getElementById("imap.overrideNamespaces").checked;
   }
   else if (serverType == "pop3")
@@ -149,16 +150,17 @@ function onAdvanced()
   window.openDialog("chrome://messenger/content/am-server-advanced.xul",
                     "_blank", "chrome,modal,titlebar", serverSettings);
 
   if (serverType == "imap")
   {
     document.getElementById("imap.dualUseFolders").checked = serverSettings.dualUseFolders;
     document.getElementById("imap.usingSubscription").checked = serverSettings.usingSubscription;
     document.getElementById("imap.maximumConnectionsNumber").setAttribute("value", serverSettings.maximumConnectionsNumber);
+    document.getElementById("imap.forceSelect").checked = serverSettings.forceSelect;
     // string prefs
     document.getElementById("imap.personalNamespace").setAttribute("value", serverSettings.personalNamespace);
     document.getElementById("imap.publicNamespace").setAttribute("value", serverSettings.publicNamespace);
     document.getElementById("imap.serverDirectory").setAttribute("value", serverSettings.serverDirectory);
     document.getElementById("imap.otherUsersNamespace").setAttribute("value", serverSettings.otherUsersNamespace);
     document.getElementById("imap.overrideNamespaces").checked = serverSettings.overrideNamespaces;
   }
   else if (serverType == "pop3")
--- a/mailnews/base/prefs/content/am-server.xul
+++ b/mailnews/base/prefs/content/am-server.xul
@@ -323,16 +323,19 @@
       <label hidden="true" wsm_persist="true" id="imap.maximumConnectionsNumber"/>
       <label hidden="true" wsm_persist="true" id="imap.personalNamespace"/>
       <label hidden="true" wsm_persist="true" id="imap.publicNamespace"/>
       <label hidden="true" wsm_persist="true" id="imap.otherUsersNamespace"/>
       <label hidden="true" wsm_persist="true" id="imap.serverDirectory"/>
       <checkbox hidden="true" wsm_persist="true" id="imap.overrideNamespaces"
                 prefattribute="value"
                 prefstring="mail.server.%serverkey%.override_namespaces"/>
+      <checkbox hidden="true" wsm_persist="true" id="imap.forceSelect"
+                prefattribute="value"
+                prefstring="mail.server.%serverkey%.forceSelect"/>
     </hbox>
 
     <!-- NNTP -->
     <hbox hidefor="pop3,imap,movemail" align="center">
       <checkbox wsm_persist="true" id="nntp.notifyOn"
                 label="&maxMessagesStart.label;"
                 accesskey="&maxMessagesStart.accesskey;"
                 oncommand="onCheckItem('nntp.maxArticles', [this.id]);"
--- a/mailnews/imap/public/nsIImapIncomingServer.idl
+++ b/mailnews/imap/public/nsIImapIncomingServer.idl
@@ -20,16 +20,17 @@ interface nsMsgImapDeleteModels
   const long MoveToTrash = 1;   /* delete moves message to the trash */
   const long DeleteNoTrash = 2; /* delete is shift delete - don't create or use trash */
 };
 
 [scriptable, uuid(ea6a0765-07b8-40df-924c-9004ed707251)]
 interface nsIImapIncomingServer : nsISupports {
 
   attribute long maximumConnectionsNumber;
+  attribute boolean forceSelect;
   attribute long timeOutLimits;
   attribute ACString adminUrl;
   attribute ACString serverDirectory;
   /// RFC 2971 ID response stored as a pref
   attribute ACString serverIDPref;
   attribute boolean cleanupInboxOnExit;
   attribute nsMsgImapDeleteModel deleteModel;
   attribute boolean dualUseFolders;
--- a/mailnews/imap/src/nsImapIncomingServer.cpp
+++ b/mailnews/imap/src/nsImapIncomingServer.cpp
@@ -271,16 +271,19 @@ nsImapIncomingServer::GetMaximumConnecti
 }
 
 NS_IMETHODIMP
 nsImapIncomingServer::SetMaximumConnectionsNumber(int32_t aMaxConnections)
 {
   return SetIntValue("max_cached_connections", aMaxConnections);
 }
 
+NS_IMPL_SERVERPREF_BOOL(nsImapIncomingServer, ForceSelect,
+                        "forceSelect")
+
 NS_IMPL_SERVERPREF_BOOL(nsImapIncomingServer, DualUseFolders,
                         "dual_use_folders")
 
 NS_IMPL_SERVERPREF_STR(nsImapIncomingServer, AdminUrl,
                        "admin_url")
 
 NS_IMPL_SERVERPREF_BOOL(nsImapIncomingServer, CleanupInboxOnExit,
                         "cleanup_inbox_on_exit")
--- a/mailnews/imap/src/nsImapProtocol.cpp
+++ b/mailnews/imap/src/nsImapProtocol.cpp
@@ -537,16 +537,17 @@ nsImapProtocol::Initialize(nsIImapHostSe
   nsresult rv = m_downloadLineCache->GrowBuffer(kDownLoadCacheSize);
   NS_ENSURE_SUCCESS(rv, rv);
 
   m_flagState = new nsImapFlagAndUidState(kImapFlagAndUidStateSize);
   if (!m_flagState)
     return NS_ERROR_OUT_OF_MEMORY;
 
   aServer->GetUseIdle(&m_useIdle);
+  aServer->GetForceSelect(&m_forceSelect);
   aServer->GetUseCondStore(&m_useCondStore);
   aServer->GetUseCompressDeflate(&m_useCompressDeflate);
   NS_ADDREF(m_flagState);
 
   m_hostSessionList = aHostSessionList; // no ref count...host session list has life time > connection
   m_parser.SetHostSessionList(aHostSessionList);
   m_parser.SetFlagState(m_flagState);
 
@@ -2541,16 +2542,27 @@ void nsImapProtocol::ProcessSelectedStat
         }
         HeaderFetchCompleted();
       }
       else
       {
         // get new message counts, if any, from server
         if (m_needNoop)
         {
+          // For some IMAP servers, to detect new email we must send imap
+          // SELECT even if already SELECTed on the same mailbox. For other
+          // servers that simply don't support IDLE, doing select here will
+          // cause emails to be properly marked "read" after they have been
+          // read in another email client.
+          if (m_forceSelect)
+          {
+            SelectMailbox(mailboxName.get());
+            selectIssued = true;
+          }
+
           m_noopCount++;
           if ((gPromoteNoopToCheckCount > 0 && (m_noopCount % gPromoteNoopToCheckCount) == 0) ||
             CheckNeeded())
             Check();
           else
             Noop(); // I think this is needed when we're using a cached connection
           m_needNoop = false;
         }
@@ -3003,30 +3015,47 @@ void nsImapProtocol::ProcessSelectedStat
             if (DeathSignalReceived())
               copyState = ImapOnlineCopyStateType::kInterruptedState;
             else
               copyState = GetServerStateParser().LastCommandSuccessful() ?
               (ImapOnlineCopyState) ImapOnlineCopyStateType::kSuccessfulCopy :
             (ImapOnlineCopyState) ImapOnlineCopyStateType::kFailedCopy;
             if (m_imapMailFolderSink)
               m_imapMailFolderSink->OnlineCopyCompleted(this, copyState);
-
-            // Don't mark msg 'Deleted' for aol servers since we already issued 'xaol-move' cmd.
+            // Don't mark message 'Deleted' for AOL servers or standard imap servers
+            // that support MOVE since we already issued an 'xaol-move' or 'move' command.
             if (GetServerStateParser().LastCommandSuccessful() &&
               (m_imapAction == nsIImapUrl::nsImapOnlineMove) &&
               !(GetServerStateParser().ServerIsAOLServer() ||
                 GetServerStateParser().GetCapabilityFlag() & kHasMoveCapability))
             {
+              // Simulate MOVE for servers that don't support MOVE: do COPY-DELETE-EXPUNGE.
               Store(messageIdString, "+FLAGS (\\Deleted \\Seen)",
                 bMessageIdsAreUids);
               bool storeSuccessful = GetServerStateParser().LastCommandSuccessful();
-
-              if (gExpungeAfterDelete && storeSuccessful)
-                Expunge();
-
+              if (storeSuccessful)
+              {
+                if(gExpungeAfterDelete)
+                {
+                  // This will expunge all emails marked as deleted in mailbox,
+                  // not just the ones marked as deleted above.
+                  Expunge();
+                }
+                else
+                {
+                  // Check if UIDPLUS capable so we can just expunge emails we just
+                  // copied and marked as deleted. This prevents expunging emails
+                  // that other clients may have marked as deleted in the mailbox
+                  // and don't want them to disappear.
+                  if (GetServerStateParser().GetCapabilityFlag() & kUidplusCapability)
+                  {
+                    UidExpunge(messageIdString);
+                  }
+                }
+              }
               if (m_imapMailFolderSink)
               {
                 copyState = storeSuccessful ? (ImapOnlineCopyState) ImapOnlineCopyStateType::kSuccessfulDelete
                   : (ImapOnlineCopyState) ImapOnlineCopyStateType::kFailedDelete;
                 m_imapMailFolderSink->OnlineCopyCompleted(this, copyState);
               }
             }
           }
--- a/mailnews/imap/src/nsImapProtocol.h
+++ b/mailnews/imap/src/nsImapProtocol.h
@@ -647,16 +647,17 @@ private:
   bool m_needNoop;
   bool m_idle;
   bool m_useIdle;
   int32_t m_noopCount;
   bool    m_autoSubscribe, m_autoUnsubscribe, m_autoSubscribeOnOpen;
   bool m_closeNeededBeforeSelect;
   bool m_retryUrlOnError;
   bool m_preferPlainText;
+  bool m_forceSelect;
 
   int32_t m_uidValidity; // stored uid validity for the selected folder.
 
   enum EMailboxHierarchyNameState {
     kNoOperationInProgress,
       kDiscoverBaseFolderInProgress,
       kDiscoverTrashFolderInProgress,
       kDeleteSubFoldersInProgress,
--- a/mailnews/mailnews.js
+++ b/mailnews/mailnews.js
@@ -488,16 +488,21 @@ pref("mail.server.default.serverFilterNa
 pref("mail.server.default.serverFilterTrustFlags", 1); // 1 == trust positives, 2 == trust negatives, 3 == trust both
 pref("mail.server.default.purgeSpam", false);
 pref("mail.server.default.purgeSpamInterval", 14); // 14 days
 pref("mail.server.default.check_all_folders_for_new", false);
 // should we inhibit whitelisting of the email addresses for a server's identities?
 pref("mail.server.default.inhibitWhiteListingIdentityUser", true);
 // should we inhibit whitelisting of the domain for a server's identities?
 pref("mail.server.default.inhibitWhiteListingIdentityDomain", false);
+// When forceSelect is true, sends extra/redundant imap SELECT when checking for
+// new mail. Needed by some imap servers. Also, if server does not support IDLE,
+// this can help insure messages are marked as "read" after being read in other
+// email clients.
+pref("mail.server.default.forceSelect", false);
 
 // to activate auto-sync feature (preemptive message download for imap) by default
 pref("mail.server.default.autosync_offline_stores",true);
 pref("mail.server.default.offline_download",true);
 
 // -1 means no limit, no purging of offline stores.
 pref("mail.server.default.autosync_max_age_days", -1);
 
--- a/suite/locales/en-US/chrome/mailnews/pref/am-server-advanced.dtd
+++ b/suite/locales/en-US/chrome/mailnews/pref/am-server-advanced.dtd
@@ -7,16 +7,19 @@
 <!ENTITY serverDirectory.label "IMAP server directory:">
 <!ENTITY serverDirectory.accesskey "d">
 <!ENTITY usingSubscription.label "Show only subscribed folders">
 <!ENTITY usingSubscription.accesskey "w">
 <!ENTITY dualUseFolders.label "Server supports folders that contain sub-folders and messages">
 <!ENTITY dualUseFolders.accesskey "f">
 <!ENTITY maximumConnectionsNumber.label "Maximum number of server connections to cache">
 <!ENTITY maximumConnectionsNumber.accesskey "M">
+<!ENTITY forceSelect.label "Send IMAP select to server when checking for new mail">
+<!ENTITY forceSelect.tooltip "Some servers need an additional IMAP select so new email is reliably received, or is shown as read after it has been read on another device. Selecting this option will cause some increased network traffic.">
+<!ENTITY forceSelect.accesskey "F">
 <!-- LOCALIZATION NOTE (namespaceDesc.label): DONT_TRANSLATE "IMAP" -->
 <!ENTITY namespaceDesc.label "These preferences specify the namespaces on your IMAP server">
 <!ENTITY personalNamespace.label "Personal namespace:">
 <!ENTITY personalNamespace.accesskey "P">
 <!ENTITY publicNamespace.label "Public (shared):">
 <!ENTITY publicNamespace.accesskey "u">
 <!ENTITY otherUsersNamespace.label "Other Users:">
 <!ENTITY otherUsersNamespace.accesskey "O">