Bug 244347 - allow changing sort order of accounts. r=mkmelin

mail/base/modules/MailMigrator.jsm

@@ -24,6 +24,10 @@ var { migrateMailnews } = ChromeUtils.import(
   "resource:///modules/MailnewsMigrator.jsm"
 );
 
+const { compareAccounts } = ChromeUtils.import(
+  "resource:///modules/folderUtils.jsm"
+);
+
 var MailMigrator = {
   /**
    * Switch the given fonts to the given encodings, but only if the current fonts
@@ -118,7 +122,7 @@ var MailMigrator = {
   _migrateUI() {
     // The code for this was ported from
     // mozilla/browser/components/nsBrowserGlue.js
-    const UI_VERSION = 23;
+    const UI_VERSION = 24;
     const MESSENGER_DOCURL = "chrome://messenger/content/messenger.xhtml";
     const MESSENGERCOMPOSE_DOCURL =
       "chrome://messenger/content/messengercompose/messengercompose.xhtml";
@@ -517,6 +521,15 @@ var MailMigrator = {
         this._migrateSMTPToOAuth2("smtp.aol.com");
       }
 
+      if (currentUIVersion < 24) {
+        let accountList = MailServices.accounts.accounts.filter(
+          a => a.incomingServer && a.incomingServer.type != "im"
+        );
+        accountList.sort(compareAccounts);
+        let accountKeyList = accountList.map(account => account.key);
+        MailServices.accounts.reorderAccounts(accountKeyList);
+      }
+
       // Update the migration version.
       Services.prefs.setIntPref(UI_VERSION_PREF, UI_VERSION);
     } catch (e) {

mail/components/extensions/test/browser/browser_ext_compose_begin.js

@@ -26,11 +26,11 @@ add_task(async function testIdentity() {
       browser.test.assertEq(2, accounts.length, "number of accounts");
       browser.test.assertEq(
         2,
-        accounts[0].identities.length,
+        accounts[1].identities.length,
         "number of identities"
       );
-      let [defaultIdentity, nonDefaultIdentity] = accounts[0].identities;
-      let folder = accounts[0].folders.find(f => f.name == "test");
+      let [defaultIdentity, nonDefaultIdentity] = accounts[1].identities;
+      let folder = accounts[1].folders.find(f => f.name == "test");
       let { messages } = await browser.messages.list(folder);
       browser.test.assertEq(4, messages.length, "number of messages");
 
@@ -114,7 +114,7 @@ add_task(async function testHeaders() {
 
       let accounts = await browser.accounts.list();
       browser.test.assertEq(2, accounts.length, "number of accounts");
-      let folder = accounts[0].folders.find(f => f.name == "test");
+      let folder = accounts[1].folders.find(f => f.name == "test");
       let { messages } = await browser.messages.list(folder);
       browser.test.assertEq(4, messages.length, "number of messages");
 
@@ -265,11 +265,11 @@ add_task(async function testBody() {
       browser.test.assertEq(2, accounts.length, "number of accounts");
       browser.test.assertEq(
         2,
-        accounts[0].identities.length,
+        accounts[1].identities.length,
         "number of identities"
       );
-      let [htmlIdentity, plainTextIdentity] = accounts[0].identities;
-      let folder = accounts[0].folders.find(f => f.name == "test");
+      let [htmlIdentity, plainTextIdentity] = accounts[1].identities;
+      let folder = accounts[1].folders.find(f => f.name == "test");
       let { messages } = await browser.messages.list(folder);
       browser.test.assertEq(4, messages.length, "number of messages");
 
@@ -595,8 +595,8 @@ add_task(async function testBody() {
 add_task(async function testAttachments() {
   let extension = ExtensionTestUtils.loadExtension({
     background: async () => {
-      let [account] = await browser.accounts.list();
-      let folder = account.folders.find(f => f.name == "test");
+      let accounts = await browser.accounts.list();
+      let folder = accounts[1].folders.find(f => f.name == "test");
       let { messages } = await browser.messages.list(folder);
 
       let newTab = await browser.compose.beginNew({

mail/components/extensions/test/browser/browser_ext_compose_onBeforeSend.js

@@ -211,8 +211,8 @@ add_task(async function testChangeDetails() {
         return window.sendMessage("checkWindow", expected);
       }
 
-      let [account] = await browser.accounts.list();
-      let [defaultIdentity, nonDefaultIdentity] = account.identities;
+      let accounts = await browser.accounts.list();
+      let [defaultIdentity, nonDefaultIdentity] = accounts[1].identities;
 
       // Add a listener that changes the headers and body. Sending should
       // continue and the headers should change. This is largely the same code

mail/components/extensions/test/xpcshell/test_ext_accounts.js

@@ -9,6 +9,8 @@ var { ExtensionTestUtils } = ChromeUtils.import(
 );
 
 add_task(async function test_accounts() {
+  // Here all the accounts are local but the first account will behave as
+  // an actual local account and will be kept last always.
   let files = {
     "background.js": async () => {
       let [account1Id, account1Name] = await window.waitForMessage();
@@ -50,7 +52,7 @@ add_task(async function test_accounts() {
       );
       let result2 = await browser.accounts.list();
       browser.test.assertEq(2, result2.length);
-      window.assertDeepEqual(result1[0], result2[0]);
+      window.assertDeepEqual(result1[0], result2[1]);
       window.assertDeepEqual(
         {
           id: account2Id,
@@ -65,13 +67,13 @@ add_task(async function test_accounts() {
             },
           ],
         },
-        result2[1]
+        result2[0]
       );
 
       let result3 = await browser.accounts.get(account1Id);
       window.assertDeepEqual(result1[0], result3);
       let result4 = await browser.accounts.get(account2Id);
-      window.assertDeepEqual(result2[1], result4);
+      window.assertDeepEqual(result2[0], result4);
 
       await window.sendMessage("create folders");
       let result5 = await browser.accounts.get(account1Id);
@@ -152,7 +154,7 @@ add_task(async function test_accounts() {
       }
 
       defaultAccount = await browser.accounts.getDefault();
-      browser.test.assertEq(result2[1].id, defaultAccount.id);
+      browser.test.assertEq(result2[0].id, defaultAccount.id);
 
       browser.test.notifyPass("finished");
     },

mail/test/browser/account/browser.ini

@@ -47,6 +47,7 @@ subsuite = thunderbird
 support-files = xml/**
 
 [browser_abWhitelist.js]
+[browser_accountOrder.js]
 [browser_accountTelemetry.js]
 [browser_actions.js]
 [browser_archiveOptions.js]

mail/test/browser/account/browser_accountOrder.js

@@ -0,0 +1,96 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/**
+ * This test checks proper operation of the account ordering functionality in the Account manager.
+ */
+
+"use strict";
+
+var {
+  click_account_tree_row,
+  get_account_tree_row,
+  open_advanced_settings,
+} = ChromeUtils.import(
+  "resource://testing-common/mozmill/AccountManagerHelpers.jsm"
+);
+
+var { MailServices } = ChromeUtils.import(
+  "resource:///modules/MailServices.jsm"
+);
+
+var { mc } = ChromeUtils.import(
+  "resource://testing-common/mozmill/FolderDisplayHelpers.jsm"
+);
+
+var gPopAccount, gOriginalAccountCount;
+
+add_task(function setupModule(module) {
+  // There may be pre-existing accounts from other tests.
+  gOriginalAccountCount = MailServices.accounts.allServers.length;
+
+  // Create a POP server
+  let popServer = MailServices.accounts
+    .createIncomingServer("nobody", "foo.invalid", "pop3")
+    .QueryInterface(Ci.nsIPop3IncomingServer);
+
+  let identity = MailServices.accounts.createIdentity();
+  identity.email = "tinderbox@foo.invalid";
+
+  gPopAccount = MailServices.accounts.createAccount();
+  gPopAccount.incomingServer = popServer;
+  gPopAccount.addIdentity(identity);
+
+  // Now there should be one more account.
+  Assert.equal(
+    MailServices.accounts.allServers.length,
+    gOriginalAccountCount + 1
+  );
+});
+
+registerCleanupFunction(function teardownModule(module) {
+  if (gPopAccount) {
+    // Remove our test account to leave the profile clean.
+    MailServices.accounts.removeAccount(gPopAccount);
+    gPopAccount = null;
+  }
+  // There should be only the original accounts left.
+  Assert.equal(MailServices.accounts.allServers.length, gOriginalAccountCount);
+});
+
+add_task(function test_account_open_state() {
+  open_advanced_settings(function(tab) {
+    subtest_check_account_order(tab);
+  });
+});
+
+/**
+ * Check the order of the accounts.
+ *
+ * @param {Object} tab - The account manager tab.
+ */
+function subtest_check_account_order(tab) {
+  let accountRow = get_account_tree_row(gPopAccount.key, null, tab);
+  click_account_tree_row(tab, accountRow);
+
+  let prevAccountList = MailServices.accounts.accounts.map(
+    account => account.key
+  );
+
+  // Moving the account up to reorder.
+  EventUtils.synthesizeKey("VK_UP", { altKey: true });
+  mc.sleep(0);
+  let curAccountList = MailServices.accounts.accounts.map(
+    account => account.key
+  );
+  Assert.notEqual(curAccountList.join(), prevAccountList.join());
+
+  accountRow = get_account_tree_row(gPopAccount.key, null, tab);
+  click_account_tree_row(tab, accountRow);
+  // Moving the account down, back to the starting position.
+  EventUtils.synthesizeKey("VK_DOWN", { altKey: true });
+  mc.sleep(0);
+  curAccountList = MailServices.accounts.accounts.map(account => account.key);
+  Assert.equal(curAccountList.join(), prevAccountList.join());
+}

mail/test/browser/folder-widget/browser_messageFilters.js

@@ -74,7 +74,7 @@ add_task(function test_message_filter_shows_newsgroup_server() {
   Assert.ok(popup.exists());
   filterc.click(popup);
 
-  let nntp = new elib.Elem(popup.node.children.item(2));
+  let nntp = new elib.Elem(popup.node.children.item(1));
   Assert.ok(nntp.exists());
   // We need to get the newsgroups to pop up somehow.
   // These all fail.

mailnews/base/prefs/content/AccountManager.js

@@ -37,9 +37,6 @@ var { BrowserUtils } = ChromeUtils.import(
   "resource://gre/modules/BrowserUtils.jsm"
 );
 var { Gloda } = ChromeUtils.import("resource:///modules/gloda/Gloda.jsm");
-var { fixIterator } = ChromeUtils.import(
-  "resource:///modules/iteratorUtils.jsm"
-);
 var { MailServices } = ChromeUtils.import(
   "resource:///modules/MailServices.jsm"
 );
@@ -868,6 +865,23 @@ function setAccountLabel(aAccountKey, aAccountNode, aLabel) {
   aAccountNode.setAttribute("label", aLabel);
 }
 
+/**
+ * Notify the UI to rebuild the account tree.
+ */
+function rebuildAccountTree() {
+  for (let win of Services.wm.getEnumerator("mail:3pane")) {
+    win.gFolderTreeView._rebuild();
+    let tabmail = win.document.getElementById("tabmail");
+    for (let tabInfo of tabmail.tabInfo) {
+      let tab = tabmail.getTabForBrowser(tabInfo.browser);
+      if (tab && tab.urlbar && tab.urlbar.value == "about:accountsettings") {
+        tab.browser.reload();
+        return;
+      }
+    }
+  }
+}
+
 /**
  * Make currentAccount (currently selected in the account tree) the default one.
  */
@@ -880,11 +894,19 @@ function onSetDefault(event) {
   let previousDefault = MailServices.accounts.defaultAccount;
   MailServices.accounts.defaultAccount = currentAccount;
   markDefaultServer(currentAccount, previousDefault);
+  let accountList = allAccountsSorted(true);
+  let accountKeyList = accountList
+    .map(account => account.key)
+    .filter(key => key != currentAccount.key);
+  accountKeyList.unshift(currentAccount.key);
+  MailServices.accounts.reorderAccounts(accountKeyList);
+  rebuildAccountTree();
   // Update gloda's myContact with the new default account's default identity.
   Gloda._initMyIdentities();
 
   // This is only needed on Seamonkey which has this button.
   setEnabled(document.getElementById("setDefaultButton"), false);
+  gAccountTree.load();
 }
 
 function onRemoveAccount(event) {
@@ -1701,6 +1723,19 @@ function getValueArrayFor(account) {
   return accountArray[serverId];
 }
 
+function accountReordered() {
+  let accountIds = [];
+  for (let account of document.getElementById("account-tree-children")
+    .children) {
+    if (account.hasAttribute("id")) {
+      accountIds.push(account.getAttribute("id"));
+    }
+  }
+
+  MailServices.accounts.reorderAccounts(accountIds);
+  rebuildAccountTree();
+}
+
 var gAccountTree = {
   load() {
     this._build();
@@ -1763,6 +1798,73 @@ var gAccountTree = {
       mainTree.lastChild.remove();
     }
 
+    let accountTree = document.getElementById("accounttree");
+
+    // By default, data/elements cannot be dropped in other elements.
+    // To allow a drop, we must prevent the default handling of the element.
+    accountTree.addEventListener("dragover", event => {
+      event.preventDefault();
+    });
+
+    accountTree.addEventListener("drop", event => {
+      let row = accountTree.getRowAt(event.clientX, event.clientY);
+      let dragId = event.dataTransfer.getData("text/account");
+      if (!dragId) {
+        return;
+      }
+      let dropId = null;
+      let length = 0;
+      for (let childElement of mainTree.children) {
+        length += childElement.querySelectorAll("treerow").length;
+        if (length > row) {
+          dropId = childElement.getAttribute("id");
+          break;
+        }
+      }
+      if (dropId && dragId != dropId) {
+        let dragitem = mainTree.querySelector("#" + dragId);
+        let dropItem = mainTree.querySelector("#" + dropId);
+        mainTree.insertBefore(dragitem, dropItem);
+        accountReordered();
+      }
+    });
+
+    accountTree.addEventListener("dragstart", event => {
+      if (getCurrentAccount()) {
+        event.dataTransfer.effectAllowed = "move";
+        event.dataTransfer.dropEffect = "move";
+        event.dataTransfer.setData("text/account", getCurrentAccount().key);
+      }
+    });
+
+    accountTree.addEventListener(
+      "keydown",
+      event => {
+        if (event.code == "ArrowDown" && event.altKey && getCurrentAccount()) {
+          let treeItem = mainTree.querySelector("#" + getCurrentAccount().key);
+          if (
+            treeItem &&
+            treeItem.nextElementSibling &&
+            treeItem.nextElementSibling != mainTree.lastElementChild
+          ) {
+            mainTree.insertBefore(treeItem.nextElementSibling, treeItem);
+          }
+          accountReordered();
+        } else if (
+          event.code == "ArrowUp" &&
+          event.altKey &&
+          getCurrentAccount()
+        ) {
+          let treeItem = mainTree.querySelector("#" + getCurrentAccount().key);
+          if (treeItem && treeItem.previousElementSibling) {
+            mainTree.insertBefore(treeItem, treeItem.previousElementSibling);
+          }
+          accountReordered();
+        }
+      },
+      true
+    );
+
     for (let account of accounts) {
       let accountName = null;
       let accountKey = account.key;
@@ -1874,6 +1976,7 @@ var gAccountTree = {
             "src",
             server.wrappedJSObject.imAccount.protocol.iconBaseURI + "icon.png"
           );
+          treeitem.id = accountKey;
         }
       }
 

mailnews/base/public/nsIMsgAccountManager.idl

@@ -242,6 +242,13 @@ interface nsIMsgAccountManager : nsISupports {
 
   // Used to sort servers (accounts) for e.g. the folder pane
   long getSortOrder(in nsIMsgIncomingServer server);
+
+  /**
+   * Sets new order of accounts.
+   *
+   * @param accountKeys - Account keys in the new preferred order.
+   */
+  void reorderAccounts(in Array<ACString> accountKeys);
 };
 
 %{C++

mailnews/base/src/folderUtils.jsm

@@ -12,6 +12,7 @@ const EXPORTED_SYMBOLS = [
   "allAccountsSorted",
   "getMostRecentFolders",
   "folderNameCompare",
+  "compareAccounts",
 ];
 
 const { MailServices } = ChromeUtils.import(
@@ -168,7 +169,7 @@ function allAccountsSorted(aExcludeIMAccounts) {
     accountList = accountList.filter(a => a.incomingServer.type != "im");
   }
 
-  return accountList.sort(compareAccounts);
+  return accountList;
 }
 
 /**

mailnews/base/src/nsMsgAccountManager.cpp

@@ -1547,16 +1547,50 @@ nsresult nsMsgAccountManager::createKeyedAccount(const nsCString& key,
 
   account->SetKey(key);
 
-  m_accounts.AppendElement(account);
-
-  // add to string list
-  if (mAccountKeyList.IsEmpty())
-    mAccountKeyList = key;
-  else {
-    mAccountKeyList.Append(',');
-    mAccountKeyList.Append(key);
+  nsCString localFoldersAccountKey;
+  nsCOMPtr<nsIMsgIncomingServer> localFoldersServer;
+  rv = GetLocalFoldersServer(getter_AddRefs(localFoldersServer));
+  if (NS_SUCCEEDED(rv)) {
+    for (auto account : m_accounts) {
+       nsCOMPtr<nsIMsgIncomingServer> server;
+       rv = account->GetIncomingServer(getter_AddRefs(server));
+       if (NS_SUCCEEDED(rv) && server == localFoldersServer) {
+         account->GetKey(localFoldersAccountKey);
+         break;
+       }
+    }
   }
 
+  // Extracting the account key of the last mail acoount.
+  nsCString lastFolderAccountKey;
+  for (int32_t index = m_accounts.Length() - 1; index >= 0; index--) {
+    nsCOMPtr<nsIMsgIncomingServer> server;
+    rv = m_accounts[index]->GetIncomingServer(getter_AddRefs(server));
+    if (NS_SUCCEEDED(rv) && server) {
+      nsCString accountType;
+      rv = server->GetType(accountType);
+      if (NS_SUCCEEDED(rv) && !accountType.EqualsLiteral("im")) {
+        m_accounts[index]->GetKey(lastFolderAccountKey);
+        break;
+      }
+    }
+  }
+
+  if (!localFoldersAccountKey.IsEmpty() && !lastFolderAccountKey.IsEmpty() && lastFolderAccountKey == localFoldersAccountKey) {
+    m_accounts.InsertElementAt(m_accounts.Length() - 1, account);
+  } else {
+    m_accounts.AppendElement(account);
+  }
+
+  nsCString newAccountKeyList;
+  nsCString accountKey;
+  for (uint32_t index = 0; index < m_accounts.Length(); index++) {
+    m_accounts[index]->GetKey(accountKey);
+    if (index) newAccountKeyList.Append(ACCOUNT_DELIMITER);
+    newAccountKeyList.Append(accountKey);
+  }
+  mAccountKeyList = newAccountKeyList;
+
   m_prefs->SetCharPref(PREF_MAIL_ACCOUNTMANAGER_ACCOUNTS, mAccountKeyList);
   account.forget(aAccount);
   return NS_OK;
@@ -3282,3 +3316,34 @@ nsMsgAccountManager::GetSortOrder(nsIMsgIncomingServer* aServer,
 
   return NS_OK;
 }
+
+NS_IMETHODIMP
+nsMsgAccountManager::ReorderAccounts(const nsTArray<nsCString> &newAccounts) {
+  // Check that the new account list contains all the existing accounts,
+  // just in a different order.
+  if (newAccounts.Length() != m_accounts.Length())
+    return NS_ERROR_INVALID_ARG;
+
+  for (uint32_t i = 0; i < m_accounts.Length(); i++) {
+    nsCString accountKey;
+    m_accounts[i]->GetKey(accountKey);
+    if (!newAccounts.Contains(accountKey))
+      return NS_ERROR_INVALID_ARG;
+  }
+
+  // In-place swap the elements in m_accounts to the order defined in newAccounts.
+  for (uint32_t i = 0; i < newAccounts.Length(); i++) {
+    nsCString newKey = newAccounts[i];
+    for (uint32_t j = i; j < m_accounts.Length(); j++) {
+      nsCString oldKey;
+      m_accounts[j]->GetKey(oldKey);
+      if (newKey.Equals(oldKey)) {
+        if (i != j)
+          std::swap(m_accounts[i], m_accounts[j]);
+        break;
+      }
+    }
+  }
+
+  return OutputAccountsPref();
+}

mailnews/base/test/unit/test_accountMgr.js

@@ -53,14 +53,16 @@ function run_test() {
     "account1,account2,account4,account5,account5,account6"
   );
   // Set the default account to one we're going to get rid of. The account
-  //  manager should recover relatively gracefully.
+  // manager should recover relatively gracefully.
   Services.prefs.setCharPref("mail.accountmanager.defaultaccount", "account6");
 
   // This will force the load of the accounts setup above.
   Assert.equal(MailServices.accounts.accounts.length, 3);
+  // Here all the accounts are local but the first account will behave as
+  // an actual local account and will be kept last always.
   Assert.equal(
     Services.prefs.getCharPref("mail.accountmanager.accounts"),
-    "account1,account4,account5"
+    "account4,account5,account1"
   );
   let server5 = MailServices.accounts
     .getIncomingServer("server5")
@@ -86,9 +88,9 @@ function run_test() {
   Assert.equal(MailServices.accounts.accounts.length, 2);
   Assert.equal(
     Services.prefs.getCharPref("mail.accountmanager.accounts"),
-    "account1,account5"
+    "account5,account1"
   );
-  // make sure cleaning up duplicate accounts didn't hork accounts
+  // Make sure cleaning up duplicate accounts didn't hork accounts.
   Assert.equal(
     Services.prefs.getCharPref("mail.account.account1.server"),
     "server1"

mailnews/base/test/unit/test_accountMgrCustomTypes.js

@@ -24,10 +24,10 @@ function run_test() {
   );
 
   // Here we are simulating a server and account that is added by an
-  //  extension, but that extension is currently unloaded. The extension
-  //  added "secondsToLeaveUnavailable" (though a typical value would be
-  //  one month, not 2 seconds!) to tell the core code to leave this alone
-  //  for awhile if the extension is unloaded.
+  // extension, but that extension is currently unloaded. The extension
+  // added "secondsToLeaveUnavailable" (though a typical value would be
+  // one month, not 2 seconds!) to tell the core code to leave this alone
+  // for awhile if the extension is unloaded.
   Services.prefs.setCharPref("mail.server.server2.hostname", "pop3.host.org");
   Services.prefs.setCharPref("mail.server.server2.type", "invalid");
   Services.prefs.setIntPref("mail.server.server2.secondsToLeaveUnavailable", 2);
@@ -39,17 +39,19 @@ function run_test() {
   Services.prefs.setCharPref("mail.accountmanager.defaultaccount", "account1");
 
   // This will force the load of the accounts setup above.
-  // We don't see the invalid account
+  // We don't see the invalid account.
   Assert.equal(MailServices.accounts.accounts.length, 1);
 
-  // but it is really there
+  // But it is really there.
+  // Here all the accounts are local but the first account will behave as
+  // an actual local account and will be kept last always.
   Assert.equal(
     Services.prefs.getCharPref("mail.accountmanager.accounts"),
-    "account1,account2"
+    "account2,account1"
   );
 
-  // add a new account (so that we can check if this clobbers the existing
-  //  inactive account or its server)
+  // Add a new account (so that we can check if this clobbers the existing
+  // inactive account or its server).
   let newAccount = MailServices.accounts.createAccount();
   let newIdentity = MailServices.accounts.createIdentity();
   newAccount.addIdentity(newIdentity);
@@ -60,7 +62,7 @@ function run_test() {
     "pop3"
   );
 
-  // no collisions with the inactive account
+  // No collisions with the inactive account.
   Assert.notEqual(newIdentity.key, "id2");
   Assert.notEqual(newAccount.incomingServer.key, "server2");
   Assert.notEqual(newAccount.key, "account2");
@@ -68,25 +70,25 @@ function run_test() {
 
   MailServices.accounts.UnloadAccounts();
 
-  // set the unavailable account to a valid type, and watch it appear.
+  // 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
+  // Make it bad again, and reload it to restart the timeout before delete.
   MailServices.accounts.UnloadAccounts();
   Services.prefs.setCharPref("mail.server.server2.type", "invalid");
   Assert.equal(MailServices.accounts.accounts.length, 2);
   MailServices.accounts.UnloadAccounts();
 
-  // now let the bad type timeout, and watch it magically disappear!
+  // Now let the bad type timeout, and watch it magically disappear!
   do_test_pending();
   do_timeout(3000, function() {
     Assert.equal(MailServices.accounts.accounts.length, 2);
 
-    // it is now gone
+    // It is now gone.
     Assert.equal(
       Services.prefs.getCharPref("mail.accountmanager.accounts"),
-      "account1," + newAccount.key
+      newAccount.key + ",account1"
     );
 
     do_test_finished();

mailnews/base/test/unit/test_accountMgrRemoveDefault.js

@@ -35,9 +35,11 @@ function run_test() {
 
   // Load of the accounts setup above.
   Assert.equal(MailServices.accounts.accounts.length, 3);
+  // Here all the accounts are local but the first account will behave as
+  // an actual local account and will be kept last always.
   Assert.equal(
     Services.prefs.getCharPref("mail.accountmanager.accounts"),
-    "account1,account2,account3"
+    "account2,account3,account1"
   );
   Assert.equal(MailServices.accounts.defaultAccount?.key, "account3");