747362 - No account selected when opening the account settings dialog while the inbox of the unified view is selected

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, defect)

Description

[Florian Quèze [:florian]]

Attachment: Patch

I see this in the error console when this happens:
JavaScript error: chrome://messenger/content/AccountManager.js, line 176: childrenNode.childNodes[i]._account is undefined


I was afraid this was caused by some of my changes for bug 742238 so I investigated. The problem is that when the 'Inbox' of the unified view is selected, the code opening the Account Settings… dialog wants to select a "Unified folders" account that isn't in the list, so the code looking for this account iterates through the whole list of accounts and fails on the "Outgoing Server (SMTP)" tree item that doesn't have an _account property.

The attached patch prevents the code from failing on tree items that don't have the _account property and to fall back to the first account when the requested account isn't in the list.

Comment 1

[:aceman]

I can confirm the error. I like the patch.
But as you "document.getElementById("account-tree-children")" twice in the common path, wouldn't it be better to:
var childrenNode = document.getElementById("account-tree-children");
var accountNode = childrenNode.firstChild;

(I have seen cases where even "document.getElementById("account-tree-children") is null" (when playing with edge cases in bug 723277 comment 44), but this patch does not make that worse, I think. I'll look at that in other bug.)

Comment 2

[Florian Quèze [:florian]]

Attachment: Patch v2

Updated patch taking into account the suggestion from comment 1.

(In reply to :aceman from comment #1)

> (I have seen cases where even
> "document.getElementById("account-tree-children") is null" (when playing
> with edge cases in bug 723277 comment 44), but this patch does not make that
> worse, I think.

It seems you meant bug 713277 comment 44.

Comment 3

[:aceman]

(In reply to Florian Quèze from comment #2)
> It seems you meant bug 713277 comment 44.
Yes, sorry.

Comment 4

[David :Bienvenu]

Comment on attachment 616962 [details] [diff] [review]
Patch v2

looks reasonable, though I'd prefer let for the local vars, thx!

Comment 5

[Florian Quèze [:florian]]

http://hg.mozilla.org/comm-central/rev/9bb214321f4c