383938 - Unable to close account settings dialog

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Account Configuration, defect)

Description

[Andrew Schultz]

Attachment: javascript stack

When I try to close the account settings dialog after selecting the Local Folders account with current SeaMonkey trunk, I get the exception:

Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMsgAccount.defaultIdentity]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://messenger/content/AccountManager.js :: saveAccount :: line 563"  data: no]
Source File: chrome://messenger/content/AccountManager.js
Line: 563

I've attached the javascript stack.  .defaultIdentity fails because
m_identities is empty (mCount=0 according to gdb), which is reasonable given that my prefs.js lists no identities for the Local Folders account.

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/mailnews/base/src/nsMsgAccount.cpp&rev=1.81&mark=255#241

This regressed between 2007-06-06-11-trunk and 2007-06-07-11-trunk, which would finger bug 379070, although I haven't tracked down how it could be related.

Comment 1

[Eyal Rozenberg]

Same here.

Comment 2

[Eyal Rozenberg]

Same here - with WinXP.

Comment 3

[Ian Neal]

Looks like the checkin for bug 383485 that caused this, new code does not seem to cope with accounts with no default identity.

Comment 4

[Andrew Schultz]

Attachment: patch

just return null if there are no identities

Comment 5

[Scott MacGregor]

Is it better to return NS_OK in nsMsgAccount::GetDefaultIdentity even if identity is null after calling do_QueryElementAt? 

-  nsCOMPtr<nsIMsgIdentity> identity( do_QueryElementAt(m_identities, 0));
+  nsCOMPtr<nsIMsgIdentity> identity( do_QueryElementAt(m_identities, 0, &rv));
   identity.swap(*aDefaultIdentity);
   return rv;
 }

would become:

+  nsCOMPtr<nsIMsgIdentity> identity( do_QueryElementAt(m_identities, 0));
   identity.swap(*aDefaultIdentity);
   return NS_OK;
 }

I'm trying to think what best matches the behavior of the code before Bug 379070.

Comment 6

[Andrew Schultz]

Right.  I was avoiding that because
1. I wasn't sure what other possible reasons do_QueryElementAt might return failure (or might return failure in the future).  Currently, it would only return failure if m_identities is null (it isn't) or the array isn't long enough (which is the case we care about).
2. I didn't want to depend on the rest of the code doing something sane in the case of failure (it seems to).

The code I attached seems more robust.  It should match behavior of the code before Bug 379070, but without depending on do_QueryElementAt's behavior when m_identities is empty.

Comment 7

[Scott MacGregor]

Comment on attachment 267889 [details] [diff] [review]
patch

cool. Thanks for the patch!

Comment 8

[Andrew Schultz]

FIXED