From 5d21f962db500a22697221d985709d5f24fa27f5 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Sun, 10 Nov 2019 22:51:10 -0500 Subject: Bug 342632 - Allow defaultAccount to return success with nullptr result when there is no usable account. Tag #1273 --- mailnews/base/prefs/content/AccountManager.js | 20 +---- mailnews/base/prefs/content/AccountWizard.js | 26 +++--- mailnews/base/public/nsIMsgAccountManager.idl | 10 ++- mailnews/base/search/content/searchWidgets.xml | 8 +- mailnews/base/src/nsMessengerWinIntegration.cpp | 3 +- mailnews/base/src/nsMsgAccountManager.cpp | 110 ++++++++++++------------ mailnews/base/src/nsMsgAccountManager.h | 10 +++ 7 files changed, 94 insertions(+), 93 deletions(-) (limited to 'mailnews/base') diff --git a/mailnews/base/prefs/content/AccountManager.js b/mailnews/base/prefs/content/AccountManager.js index 83117f4ad..81fb115de 100644 --- a/mailnews/base/prefs/content/AccountManager.js +++ b/mailnews/base/prefs/content/AccountManager.js @@ -704,7 +704,7 @@ function onSetDefault(event) { if (event.target.getAttribute("disabled") == "true") return; - let previousDefault = getDefaultAccount(); + let previousDefault = MailServices.accounts.defaultAccount; MailServices.accounts.defaultAccount = currentAccount; markDefaultServer(currentAccount, previousDefault); @@ -771,7 +771,7 @@ function onRemoveAccount(event) { // Either the default account was deleted so there is a new one // or the default account was not changed. Either way, there is // no need to unmark the old one. - markDefaultServer(getDefaultAccount(), null); + markDefaultServer(MailServices.accounts.defaultAccount, null); } function saveAccount(accountValues, account) @@ -928,7 +928,7 @@ function updateItems(tree, account, addAccountItem, setDefaultItem, removeItem) // problem. Either way, we don't want the user to act on it. let server = account.incomingServer; - if (account != getDefaultAccount() && + if (account != MailServices.accounts.defaultAccount && server.canBeDefaultServer && account.identities.length > 0) canSetDefault = true; @@ -1378,18 +1378,6 @@ function getCurrentAccount() return currentAccount; } -/** - * Returns the default account without throwing exception if there is none. - * The account manager can be opened even if there are no account yet. - */ -function getDefaultAccount() { - try { - return MailServices.accounts.defaultAccount; - } catch (e) { - return null; // No default account yet. - } -} - /** * Get the array of persisted form elements for the given page. */ @@ -1605,7 +1593,7 @@ var gAccountTree = { treeitem._account = account; } - markDefaultServer(getDefaultAccount(), null); + markDefaultServer(MailServices.accounts.defaultAccount, null); // Now add the outgoing server node. var treeitem = document.createElement("treeitem"); diff --git a/mailnews/base/prefs/content/AccountWizard.js b/mailnews/base/prefs/content/AccountWizard.js index 1903cbc88..90d729418 100644 --- a/mailnews/base/prefs/content/AccountWizard.js +++ b/mailnews/base/prefs/content/AccountWizard.js @@ -53,7 +53,9 @@ var gPrefsBundle, gMessengerBundle; // the current nsIMsgAccount var gCurrentAccount; -// default account +// The default account before we create a new account. +// We need to store this as just asking for the default account may switch +// it to the newly created one if there was none before. var gDefaultAccount; // the current associative array that @@ -85,14 +87,9 @@ function onAccountWizardLoad() { checkForInvalidAccounts(); - try { - gDefaultAccount = MailServices.accounts.defaultAccount; - } - catch (ex) { - // no default account, this is expected the first time you launch mail - // on a new profile - gDefaultAccount = null; - } + // It is fine if there is no default account, this is expected the first + // time you launch mail on a new profile. + gDefaultAccount = MailServices.accounts.defaultAccount; // Set default value for global inbox checkbox var checkGlobalInbox = document.getElementById("deferStorage"); @@ -799,8 +796,7 @@ function getPreConfigDataForAccount(account) function AccountToAccountData(account, defaultAccountData) { - dump("AccountToAccountData(" + account + ", " + - defaultAccountData + ")\n"); + dump("AccountToAccountData(" + account + ", " + defaultAccountData + ")\n"); var accountData = defaultAccountData; if (!accountData) accountData = new Object; @@ -966,10 +962,10 @@ function onFlush() { */ function EnableCheckMailAtStartUpIfNeeded(newAccount) { - // Check if default account exists and if that account is alllowed to be - // a default account. If no such account, make this one as the default account + // Check if default account existed. + // If no such account, make this one the default account // and turn on the new mail check at startup for the current account - if (!(gDefaultAccount && gDefaultAccount.incomingServer.canBeDefaultServer)) { + if (!gDefaultAccount) { MailServices.accounts.defaultAccount = newAccount; newAccount.incomingServer.loginAtStartUp = true; newAccount.incomingServer.downloadOnBiff = true; @@ -981,7 +977,7 @@ function SetSmtpRequiresUsernameAttribute(accountData) // If this is the default server, time to set the smtp user name // Set the generic attribute for requiring user name for smtp to true. // ISPs can override the pref via rdf files. - if (!(gDefaultAccount && gDefaultAccount.incomingServer.canBeDefaultServer)) { + if (!gDefaultAccount) { accountData.smtpRequiresUsername = true; } } diff --git a/mailnews/base/public/nsIMsgAccountManager.idl b/mailnews/base/public/nsIMsgAccountManager.idl index d5a06d2c8..53ea2351e 100644 --- a/mailnews/base/public/nsIMsgAccountManager.idl +++ b/mailnews/base/public/nsIMsgAccountManager.idl @@ -71,9 +71,13 @@ interface nsIMsgAccountManager : nsISupports { /* account list stuff */ - /* defaultAccount should always be set if there are any accounts - * in the account manager. You can only set the defaultAccount to an - * account already in the account manager */ + /** + * Returns the account that was marked as the default one. + * Only some server types can serve as default account. + * If there is no such account, null is returned. + * You can only set the defaultAccount to an + * account already in the account manager. + */ attribute nsIMsgAccount defaultAccount; /** diff --git a/mailnews/base/search/content/searchWidgets.xml b/mailnews/base/search/content/searchWidgets.xml index 80ebe38c4..69f3b2ae2 100644 --- a/mailnews/base/search/content/searchWidgets.xml +++ b/mailnews/base/search/content/searchWidgets.xml @@ -224,10 +224,12 @@ let identitiesRaw = MailServices.accounts .getIdentitiesForServer(gFilterList.folder.server); let identities = Array.from(this.fixIterator(identitiesRaw, - Components.interfaces.nsIMsgIdentity)); + Ci.nsIMsgIdentity)); - if (!identities.length) // typically if this is Local Folders - identities.push(MailServices.accounts.defaultAccount.defaultIdentity); + if (identities.length == 0) { // typically if this is Local Folders + if (MailServices.accounts.defaultAccount) + identities.push(MailServices.accounts.defaultAccount.defaultIdentity); + } let templateFound = false; let foldersScanned = []; diff --git a/mailnews/base/src/nsMessengerWinIntegration.cpp b/mailnews/base/src/nsMessengerWinIntegration.cpp index 98888fcb3..ff0c08041 100644 --- a/mailnews/base/src/nsMessengerWinIntegration.cpp +++ b/mailnews/base/src/nsMessengerWinIntegration.cpp @@ -1089,7 +1089,8 @@ nsMessengerWinIntegration::SetupInbox() nsCOMPtr account; rv = accountManager->GetDefaultAccount(getter_AddRefs(account)); - if (NS_FAILED(rv)) { + NS_ENSURE_SUCCESS(rv, rv); + if (!account) { // this can happen if we launch mail on a new profile // we don't have a default account yet mDefaultAccountMightHaveAnInbox = false; diff --git a/mailnews/base/src/nsMsgAccountManager.cpp b/mailnews/base/src/nsMsgAccountManager.cpp index 9dd73d6f9..4ebac58a5 100644 --- a/mailnews/base/src/nsMsgAccountManager.cpp +++ b/mailnews/base/src/nsMsgAccountManager.cpp @@ -654,9 +654,9 @@ nsMsgAccountManager::RemoveAccount(nsIMsgAccount *aAccount, return rv; } - // if it's the default, clear the default account + // If it's the default, choose a new default account. if (m_defaultAccount.get() == aAccount) - SetDefaultAccount(nullptr); + AutosetDefaultAccount(); // XXX - need to figure out if this is the last time this server is // being used, and only send notification then. @@ -727,7 +727,9 @@ nsMsgAccountManager::OutputAccountsPref() mAccountKeyList.get()); } -/* get the default account. If no default account, pick the first account */ +/** + * Get the default account. If no default account, return null. + */ NS_IMETHODIMP nsMsgAccountManager::GetDefaultAccount(nsIMsgAccount **aDefaultAccount) { @@ -737,74 +739,72 @@ nsMsgAccountManager::GetDefaultAccount(nsIMsgAccount **aDefaultAccount) NS_ENSURE_SUCCESS(rv, rv); if (!m_defaultAccount) { - uint32_t count = m_accounts.Length(); - if (!count) { - *aDefaultAccount = nullptr; - return NS_ERROR_FAILURE; - } - nsCString defaultKey; rv = m_prefs->GetCharPref(PREF_MAIL_ACCOUNTMANAGER_DEFAULTACCOUNT, getter_Copies(defaultKey)); - if (NS_SUCCEEDED(rv)) + if (NS_SUCCEEDED(rv)) { rv = GetAccount(defaultKey, getter_AddRefs(m_defaultAccount)); - - if (NS_FAILED(rv) || !m_defaultAccount) { - nsCOMPtr firstAccount; - uint32_t index; - bool foundValidDefaultAccount = false; - for (index = 0; index < count; index++) { - nsCOMPtr account(m_accounts[index]); - - // get incoming server - nsCOMPtr server; - // server could be null if created by an unloaded extension - (void) account->GetIncomingServer(getter_AddRefs(server)); - - bool canBeDefaultServer = false; - if (server) - { - server->GetCanBeDefaultServer(&canBeDefaultServer); - if (!firstAccount) - firstAccount = account; - } - - // if this can serve as default server, set it as default and - // break out of the loop. - if (canBeDefaultServer) { - SetDefaultAccount(account); - foundValidDefaultAccount = true; - break; - } - } - - if (!foundValidDefaultAccount) { - // Get the first account and use it. - // We need to fix this scenario, e.g. in bug 342632. - NS_WARNING("No valid default account found."); - if (firstAccount) { - NS_WARNING("Just using the first one (FIXME)."); - SetDefaultAccount(firstAccount); - } + if (NS_SUCCEEDED(rv) && m_defaultAccount) { + bool canBeDefault = false; + rv = CheckDefaultAccount(m_defaultAccount, canBeDefault); + if (NS_FAILED(rv) || !canBeDefault) + m_defaultAccount = nullptr; } } } - if (!m_defaultAccount) { - // Absolutely no usable account found. Error out. - NS_ERROR("Default account is null, when not expected!"); - *aDefaultAccount = nullptr; - return NS_ERROR_FAILURE; + NS_IF_ADDREF(*aDefaultAccount = m_defaultAccount); + return NS_OK; +} + +/** + * Check if the given account can be default. + */ +nsresult +nsMsgAccountManager::CheckDefaultAccount(nsIMsgAccount *aAccount, bool &aCanBeDefault) +{ + aCanBeDefault = false; + nsCOMPtr server; + // Server could be null if created by an unloaded extension. + nsresult rv = aAccount->GetIncomingServer(getter_AddRefs(server)); + NS_ENSURE_SUCCESS(rv, rv); + if (server) { + // Check if server can be default. + rv = server->GetCanBeDefaultServer(&aCanBeDefault); + } + return rv; +} + +/** + * Pick the first account that can be default and make it the default. + */ +nsresult +nsMsgAccountManager::AutosetDefaultAccount() +{ + for (nsIMsgAccount* account : m_accounts) { + bool canBeDefault = false; + nsresult rv = CheckDefaultAccount(account, canBeDefault); + if (NS_SUCCEEDED(rv) && canBeDefault) { + return SetDefaultAccount(account); + } } - NS_ADDREF(*aDefaultAccount = m_defaultAccount); return NS_OK; } NS_IMETHODIMP nsMsgAccountManager::SetDefaultAccount(nsIMsgAccount *aDefaultAccount) { + if (!aDefaultAccount) + return NS_ERROR_INVALID_ARG; + if (m_defaultAccount != aDefaultAccount) { + bool canBeDefault = false; + nsresult rv = CheckDefaultAccount(aDefaultAccount, canBeDefault); + if (NS_FAILED(rv) || !canBeDefault) { + // Report failure if we were explicitly asked to use an unusable server. + return NS_ERROR_INVALID_ARG; + } nsCOMPtr oldAccount = m_defaultAccount; m_defaultAccount = aDefaultAccount; (void) setDefaultAccountPref(aDefaultAccount); @@ -3677,7 +3677,7 @@ nsMsgAccountManager::GetSortOrder(nsIMsgIncomingServer* aServer, int32_t* aSortO if (NS_SUCCEEDED(rv) && defaultAccount) { nsCOMPtr defaultServer; rv = m_defaultAccount->GetIncomingServer(getter_AddRefs(defaultServer)); - if (NS_SUCCEEDED(rv) && defaultServer && (aServer == defaultServer)) { + if (NS_SUCCEEDED(rv) && (aServer == defaultServer)) { *aSortOrder = 0; return NS_OK; } diff --git a/mailnews/base/src/nsMsgAccountManager.h b/mailnews/base/src/nsMsgAccountManager.h index d5a116e57..11d19590b 100644 --- a/mailnews/base/src/nsMsgAccountManager.h +++ b/mailnews/base/src/nsMsgAccountManager.h @@ -142,6 +142,16 @@ private: nsresult GetLocalFoldersPrettyName(nsString &localFoldersName); + /** + * Check if the given account can be the set as the default account. + */ + nsresult CheckDefaultAccount(nsIMsgAccount* aAccount, bool &aCanBeDefault); + + /** + * Find a new account that can serve as default. + */ + nsresult AutosetDefaultAccount(); + // sets the pref for the default server nsresult setDefaultAccountPref(nsIMsgAccount *aDefaultAccount); -- cgit v1.2.3