465177 - Mixed up (default) folders in folderpane after landing 414038

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Alex Ihrig [:Thunderbird_Mail_DE]]

After landing Bug 414038 in trunk, folders are mixed up in folderpane. Maybe this is only for IMAP accounts.

Steps to reproduce:
1. Expand all subfolders in your (IMAP) account.
2. Collapse the account
3. Re-expand the account -> now the mixed up folders occur

Switching between "All folders" and "Unread folders" and back again to "All folders" removes the mix-up. Doing steps 1-3 again will mix it up again.

Comment 1

[Alex Ihrig [:Thunderbird_Mail_DE]]

Attachment: The correct folderpane

Comment 2

[Alex Ihrig [:Thunderbird_Mail_DE]]

Attachment: Mixed up (default) folders

Comment 3

[Alex Ihrig [:Thunderbird_Mail_DE]]

Marked importance as "major" - users could be confused and think their folders are in trash.

Comment 4

[Mark Banner (:standard8)]

Please state your full build id, available from Help -> About. This may have been fixed in today's builds by bug 465057.

Comment 5

[Alex Ihrig [:Thunderbird_Mail_DE]]

Ups, sorry. Build-ID is:
Mozilla/5.0 (Windows; U; Windows NT 6.0; de; rv:1.9.1b2pre) Gecko/20081116 Lightning/1.0pre Shredder/3.0b1pre ID:20081116031554

Comment 6

[Alex Ihrig [:Thunderbird_Mail_DE]]

And yes, creating a new profile (without add-ons) leads to exactly the same behaviour.

Comment 7

[Alex Ihrig [:Thunderbird_Mail_DE]]

Bug 465057 doesn't fix this bug - my folderPane.js includes http://hg.mozilla.org/comm-central/rev/72ea4a5eee81

Comment 8

[Joey Minta]

Attachment: patch

This happens when there are sub-sub-folders already opened. We don't consider the number of grandkids we've added to the tree in determining where to add the next folder.

Comment 9

[Joey Minta]

Attachment: mozmill test

Mozmill test, fails on trunk but passes with this patch.

Comment 10

[Alex Ihrig [:Thunderbird_Mail_DE]]

(In reply to comment #8)
> Created an attachment (id=348440) [details]
> patch

Seems to be working with this patch.

Comment 11

[David :Bienvenu]

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

thx, jminta.

Comment 12

[Joey Minta]

Committed as rev a2e8cdb8337a. Thanks for the great bug report!

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 348441 [details] [diff] [review]
mozmill test

This test fails with:

fail :: undefined (Toggle..)
exception: TypeError: this._rowMap[aIndex] is undefined

Comment 14

[Wayne Mery (:wsmwk)]

This FIXED bug is flagged with in‑testsuite?   It would be great if assignee or someone else can clear the flag if a test is not appropriate.  And if appropriate, create a test and plus the flag to finish off the bug.

Comment 15

[Joey Minta]

Attachment: mozmill test v2

I was working locally with a profile with no accounts, so I thought it was necessary to create the local account myself. With the automatic mozmill-created profile, the createLocalAccount() line would throw. This test now works with that profile.

Comment 16

[Mark Banner (:standard8)]

Comment on attachment 390543 [details] [diff] [review]
mozmill test v2

Andrew's more up to date on the folder pane than I am, so I'm going to punt this to him.

Please fix the no newline at the end of the mozmilltests.list file though.

Comment 17

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 390543 [details] [diff] [review]
mozmill test v2

http://reviews.visophyte.org/r/bzexport/38/

Thanks for writing tests before they were cool and making the effort to get it
in now that we're on the bandwagon too! :)


on file: mail/test/mozmill/folder-pane/test-bug465177.js line 1
> /* ***** BEGIN LICENSE BLOCK *****

Please change the file's name so it provides an indicator of what is being
tested for; you can keep the bug number in there if you think it provides a
serious benefit.  For example, test-collapse-and-expand.js is intuitive for me
and is also well-suited to having more tests added to it in the future.


on file: mail/test/mozmill/folder-pane/test-bug465177.js line 40
> // We have to do this manually, MozMill doesn't have a function for non-browser windows.
> var mainWindow = Components.classes["@mozilla.org/appshell/window-mediator;1"]
>                            .getService(Components.interfaces.nsIWindowMediator)
>                            .getMostRecentWindow("mail:3pane");
> var  MC = new controller.MozMillController(mainWindow);

You can actually use controller.getMail3PaneController() for this.  However,
to deal with pathological cases (which tend to happen on our test boxes) where
the window has not yet loaded sufficiently to have a windowtype, it's probably
better to use something like window-display-helper.

For simplicity, I'd just suggest using folder-display-helper since it
automatically exposes an "mc" variable to your normal namespace when you use
its installInto method.

Example boilerplate can be found here:
http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/folder-display/test-selection.js


on file: mail/test/mozmill/folder-pane/test-bug465177.js line 47
>   const Ci = Components.interfaces;

mozmill magically defines Ci at the top-level.  You do not need to do this.


on file: mail/test/mozmill/folder-pane/test-bug465177.js line 48
>   let am = Components.classes["@mozilla.org/messenger/account-manager;1"]

It also defines Cc...