739258 - nsMsgAccountManager::GetDefaultAccount can make an account with canBeDefaultServer = false the default

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, defect)

Description

[Florian Quèze [:florian]]

Because of this code:
http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgAccountManager.cpp#779

This can cause an IM account to become the default account (and cause various errors) if it was created before the first email account.

779       if (!foundValidDefaultAccount) {
780         // get the first account and use it.
781         // we need to fix this scenario.
782         NS_WARNING("No valid default account found, just using first (FIXME)");
783         nsCOMPtr<nsIMsgAccount> firstAccount( do_QueryElementAt(m_accounts, 0));
784         SetDefaultAccount(firstAccount);
785       }

Can we just return null as the default account in that case, or would it cause crashes/other complications?

Comment 1

[:aceman]

I think TB goes to great lengths to ensure at least one account is created, be it just Local Folders.

Instead of picking the one at '0', what about iterating over the accounts and using the first that has canBeDefaultServer = true?

Comment 2

[Florian Quèze [:florian]]

(In reply to :aceman from comment #1)

> Instead of picking the one at '0', what about iterating over the accounts
> and using the first that has canBeDefaultServer = true?

It's what the 20 or so lines above the code I've quoted do, see http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgAccountManager.cpp#756

Comment 3

[:aceman]

I think returning NULL is the same question as bug 342632 is proposing.

Comment 4

[:aceman]

But yes, 342632 wants to return NULL due to other reasons and other code line so this is not a dupe.

But once you return NULL for any reason you will probably cause the potential complications discussed in bug 342632, therefore you may need that bug to land first because it will check and update problematic callers too.

Comment 5

[:aceman]

I indeed fixed this in bug 342632.
An account with 'canBeDefaultServer'==false should not be chosen as a default server, even if SetDefaultServer is called explicitly on such an account.

Can you please verify the fix?