Bug 1854064 - Don't load accounts that don't have a server hostname. r=john.bieling

The UI doesn't allow such accounts. If somehow we got into the state of having no hostname,
the account is doomed and can never do anything useful, so don't load it at all.
If we allow such accounts, they will blow up the UI since we then end up with folders
that don't know which server they are belong to.

Differential Revision: https://phabricator.services.mozilla.com/D188957

--HG--
extra : rebase_source : e479e8294841d67b7b69f1c1e435efd2052a00c3

mailnews/base/public/nsIMsgAccountManager.idl

@@ -167,11 +167,14 @@ interface nsIMsgAccountManager : nsISupports {
   // Create the account for that special server.
   void createLocalMailAccount();
 
-  /* load accounts kicks off the creation of all accounts. You do not need
-   * to call this and all accounts should be loaded lazily if you use any
-   * of the above.
+  /**
+   * Kicks off the creation of all accounts. You do not need to call this and
+   * all accounts should be loaded lazily if you use any of the above.
    */
-  void LoadAccounts();
+  void loadAccounts();
+
+  /** Frees all the account manager data structures. */
+  void unloadAccounts();
 
   /**
    * When the server for an account could not be loaded, typically because the
@@ -180,15 +183,12 @@ interface nsIMsgAccountManager : nsISupports {
    * rechecked at startup but this function can be used to recheck all servers
    * at any time to avoid having to restart to reactivate an account.
    */
-  void ReactivateAccounts();
+  void reactivateAccounts();
 
   void setSpecialFolders();
 
   void loadVirtualFolders();
 
-  /* unload accounts frees all the account manager data structures */
-  void UnloadAccounts();
-
   void WriteToFolderCache(in nsIMsgFolderCache folderCache);
   void saveVirtualFolders();
   void closeCachedConnections();

mailnews/base/src/nsMsgAccount.cpp

@@ -11,7 +11,7 @@
 #include "nsCRTGlue.h"
 #include "nsCOMPtr.h"
 #include "nsIMsgFolderNotificationService.h"
-
+#include "nsPrintfCString.h"
 #include "nsIPrefService.h"
 #include "nsIPrefBranch.h"
 #include "nsMsgAccount.h"
@@ -92,6 +92,15 @@ nsresult nsMsgAccount::createIncomingServer() {
   rv = accountManager->GetIncomingServer(serverKey, getter_AddRefs(server));
   NS_ENSURE_SUCCESS(rv, rv);
 
+  nsCString hostname;
+  rv = server->GetHostName(hostname);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (hostname.IsEmpty()) {
+    NS_WARNING(
+        nsPrintfCString("Server had no hostname; key=%s", serverKey.get()));
+    return NS_ERROR_UNEXPECTED;
+  }
+
   // store the server in this structure
   m_incomingServer = server;
   accountManager->NotifyServerLoaded(server);

mailnews/base/test/unit/test_accountMgr2.js

@@ -86,19 +86,34 @@ add_task(async function () {
   Assert.equal(MailServices.accounts.allIdentities.length, 2);
 
   // Test a special hostname.
-  const acc3 = MailServices.accounts.createAccount();
-  acc3.incomingServer = MailServices.accounts.createIncomingServer(
+  const acc4 = MailServices.accounts.createAccount();
+  acc4.incomingServer = MailServices.accounts.createIncomingServer(
     "bob_unavail",
     "0.0.0.0.", // Note ending dot which would not do anything for an IP.
     "pop3"
   );
   const id4 = MailServices.accounts.createIdentity();
   id4.email = "bob_unavail@example.com";
-  acc3.addIdentity(id4);
+  acc4.addIdentity(id4);
 
   Assert.equal(
     MailServices.accounts.accounts.length,
     3,
-    "acc3 should be in accounts"
+    "acc4 should be in accounts"
+  );
+
+  // Test that an account with empty server hostname doesn't even get listed.
+  const serverKey = acc4.incomingServer.key;
+  Services.prefs.setStringPref(`mail.server.${serverKey}.hostname`, "");
+  MailServices.accounts.unloadAccounts();
+  MailServices.accounts.loadAccounts();
+  Assert.equal(
+    MailServices.accounts.accounts.length,
+    2,
+    "invalid acc4 should have been removed"
+  );
+  Assert.equal(
+    Services.prefs.getCharPref("mail.accountmanager.accounts"),
+    "account2,account3"
   );
 });

mailnews/base/test/unit/test_accountMgrCustomTypes.js

@@ -66,17 +66,17 @@ function run_test() {
   Assert.notEqual(newAccount.key, "account2");
   Assert.equal(MailServices.accounts.accounts.length, 2);
 
-  MailServices.accounts.UnloadAccounts();
+  MailServices.accounts.unloadAccounts();
 
   // Set the unavailable account to a valid type, and watch it appear.
   Services.prefs.setCharPref("mail.server.server2.type", "pop3");
   Assert.equal(MailServices.accounts.accounts.length, 3);
 
   // Make it bad again, and reload it to restart the timeout before delete.
-  MailServices.accounts.UnloadAccounts();
+  MailServices.accounts.unloadAccounts();
   Services.prefs.setCharPref("mail.server.server2.type", "invalid");
   Assert.equal(MailServices.accounts.accounts.length, 2);
-  MailServices.accounts.UnloadAccounts();
+  MailServices.accounts.unloadAccounts();
 
   // Now let the bad type timeout, and watch it magically disappear!
   do_test_pending();

mailnews/imap/test/unit/test_imapAuthMethods.js

@@ -129,7 +129,7 @@ function nextTest() {
     incomingServer = null;
     MailServices.accounts.closeCachedConnections();
     MailServices.accounts.shutdownServers();
-    MailServices.accounts.UnloadAccounts();
+    MailServices.accounts.unloadAccounts();
     server.stop();
   } catch (e) {
     // server.stop();

mailnews/import/src/SeamonkeyImport.jsm

@@ -198,8 +198,8 @@ SeamonkeyImportSettings.prototype = {
     );
 
     // Reload accounts so that `CheckIfLocalFolderExists` in importDialog works
-    MailServices.accounts.UnloadAccounts();
-    MailServices.accounts.LoadAccounts();
+    MailServices.accounts.unloadAccounts();
+    MailServices.accounts.loadAccounts();
     return true;
   },
 };

mailnews/local/test/unit/test_pop3Password2.js

@@ -185,7 +185,7 @@ add_task(async function () {
   handler.kUsername = "testpop3";
   handler.kPassword = "pop3test";
 
-  MailServices.accounts.LoadAccounts();
+  MailServices.accounts.loadAccounts();
 
   localAccountUtils.incomingServer = MailServices.accounts.localFoldersServer;
 

mailnews/news/test/unit/test_nntpPassword2.js

@@ -68,7 +68,7 @@ add_task(async function () {
     test = "news:*";
 
     // Get the existing incoming server
-    MailServices.accounts.LoadAccounts();
+    MailServices.accounts.loadAccounts();
 
     // Create the incoming server with "original" details.
     var incomingServer = MailServices.accounts.getIncomingServer("server2");