One thing that came up in the TB UI testing [1] is that a lot of people had issues finding their sent folder. This was because after you've set up a new gmail account, the [Gmail] subfolders where sent, drafts etc. are located are hidden/collapsed by default. 1. http://design.canonical.com/2011/02/thunderbird-in-the-usability-lab/

My suggestion for fixing this would be to put the smarts in folderPane.js - when [Gmail] is added, automatically mark it as expanded. I think we can tell the difference between first run folder discovery, and normal startup. If we don't want to hard-code gmail, or [gmail] gets localized, I'm sure there are other ways we can detect a gmail account.

this works for me, though I need to do a little more testing to make sure it doesn't auto-expand gmail every time.

This fix makes us expand a parent if a special child is added. This is a more general fix than the previous one in that it works for non gmail accounts, but it also means that if we create a special folder on demand (e.g., the sent folder is created with the account collapsed) but I don't think that's too bad. Asking Bryan for his feedback...

Comment on attachment 519420 [details] [diff] [review] alternate fix yeah, this is great

pinging for code review since it has ui-review+

Comment on attachment 521288 [details] [diff] [review] address comments over irc Thanks!

fixed on trunk - http://hg.mozilla.org/comm-central/rev/1d82a1d24312

re-opened due to mozmill failure

FTR, I backed out the folderPane.js part of the patch

This is the stack when _addChildToView is called. ftl_addChildToView([object Object],1,[object Object])@chrome://messenger/content/folderPane.js:1729 ftl_add_folder([object XPCWrappedNative_NoHelper],[object XPCWrappedNative_NoHelper])@chrome://messenger/ content/folderPane.js:1851 IFolderTreeMode_onFolderAdded([object XPCWrappedNative_NoHelper],[object XPCWrappedNative_NoHelper])@chro me://messenger/content/folderPane.js:127 ftl_add([object XPCWrappedNative_NoHelper],[object XPCWrappedNative_NoHelper])@chrome://messenger/content /folderPane.js:1825 configure_message_injection([object Object])@resource://mozmill/modules/frame.js -> file:///c:/Users/Sid/ mozilla/src-2.0/mail/test/mozmill/shared-modules/test-folder-display-helpers.js -> file:///c:/Users/Sid/m ozilla/src-2.0/mailnews/test/resources/messageInjection.js:123 setupAccountStuff()@resource://mozmill/modules/frame.js -> file:///c:/Users/Sid/mozilla/src-2.0/mail/test /mozmill/shared-modules/test-folder-display-helpers.js:308 setupModule()@resource://mozmill/modules/frame.js -> file:///c:/Users/Sid/mozilla/src-2.0/mail/test/mozmi ll/shared-modules/test-folder-display-helpers.js:260 installInto([object Object])@resource://mozmill/modules/frame.js -> file:///c:/Users/Sid/mozilla/src-2.0/ mail/test/mozmill/shared-modules/test-folder-display-helpers.js:267 ... So the reason this happens is that we expand the parent since it is collapsed -- but the folder's already been added, so it'll show up when we do that. It is possible this only happens the first time the folder is expanded and the children are computed, but I'm not sure. (I can't find the code for what happens to an ftvItem's _children if a new folder's created and neither its parent nor its grandparent are visible.) Then we go ahead and add the folder once more to the view, causing the folder to show up twice. Hooray for torn state. I think what we need here is something like <https://mxr.mozilla.org/comm-central/source/mail/base/content/folderPane.js#1825>. I'm also not really convinced _addChildToView is the right place for this -- in any case, we definitely need tests here for the folder pane interactions here, even if they don't use IMAP. test-folder-pane.js is probably a good place to have them in. Note that the test will fail even after the folder's opened, since it doesn't expect the local folders account to be expanded when the inbox is created in configure_message_injection.

Comment on attachment 521288 [details] [diff] [review] address comments over irc withdrawing review for the folder pane part due to comments.

thx for looking at this, Sid. Re needing something like the code in addFolder, we should be executing the code in addFolder in this very case, since we're calling addFolder (addFolder calls _addChildToView, per my understanding of the code, and the stack you included). I suspect we just want to expand the parent and not add the new folder, iff the parent's children haven't been computed. But the folder pane code is complicated...

Comment on attachment 522041 [details] [diff] [review] proposed fix with mozmill test >@@ -1824,26 +1822,40 @@ let gFolderTreeView = { > > // Getting these children might have triggered our parent to build its > // array just now, in which case the added item will already exist > let children = parent.children; > var newChild; > for each (let child in children) { > if (child._folder == aItem) { > newChild = child; >+ haveChild = true; unused var >diff --git a/mail/test/mozmill/folder-pane/test-folder-pane.js b/mail/test/mozmill/folder-pane/test-folder-pane.js >--- a/mail/test/mozmill/folder-pane/test-folder-pane.js >+++ b/mail/test/mozmill/folder-pane/test-folder-pane.js >@@ -62,21 +62,38 @@ function test_all_folders_toggle_folder_ > // Test that we are in All Folders mode by default > assert_folder_mode("all"); > > // All folders mode should give us only 2 rows to start > // (tinderbox account and local folders) > let accounts = 2; > assert_folder_tree_view_row_count(accounts); > >+ let inbox = trash = outbox = archives = folderPaneA = 1; >+ // Create archives folder - this is ugly, but essentially the same as >+ // what mailWindowOverlay.js does. Per the IRC discussion, please add a comment explaining why the usual create_folder method doesn't work. >+ let acctMgr = Cc["@mozilla.org/messenger/account-manager;1"] >+ .getService(Ci.nsIMsgAccountManager); MailServices =)

bug 645838 filed for existing issue Sid noticed when looking at this code...

fixed on trunk - changeset: 7450:d29ba6146ff7 tag: tip user: David Bienvenu <bienvenu@nventure.com> date: Mon Mar 28 16:46:08 2011 -0700 summary: fix bug 637641, auto expand parent folder when special sub-folders are added, r=sid0

oops, forgot the test-folder-display-helpers.js part, which I pushed separately

There were a few follow-ups pushed to fix various bustage in the mozmill tests: http://hg.mozilla.org/comm-central/rev/f4c223296de8 http://hg.mozilla.org/comm-central/rev/00b8e6694bb0 http://hg.mozilla.org/comm-central/rev/58607f25eadf

thx for fixing the collapsing of the initial account - I definitely saw a tinderbox failure with MailServices not being defined, which is why I reverted it, so I wouldn't be shocked if it starts failing again...

Well, when including MailServices, the js file needs to be "mailServices.js" not "MailServices.js"

oh, on non-windows platforms.