Last Comment Bug 763236 - Thunderbird 13 - accounts get mixed when using menu "get mail"
: Thunderbird 13 - accounts get mixed when using menu "get mail"
Status: RESOLVED FIXED
: regression
Product: Thunderbird
Classification: Client Software
Component: Account Manager (show other bugs)
: 13 Branch
: All All
: -- normal with 1 vote (vote)
: Thunderbird 16.0
Assigned To: :aceman
:
:
Mentors:
: 772891 (view as bug list)
Depends on:
Blocks: 749200
  Show dependency treegraph
 
Reported: 2012-06-09 16:17 PDT by Pepe
Modified: 2012-07-11 14:48 PDT (History)
12 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed
+
fixed


Attachments
accounts under "get mail".jpg (9.13 KB, image/jpeg)
2012-06-09 16:17 PDT, Pepe
no flags Details
the sorting in the account manager is alphabetic (25.92 KB, image/jpeg)
2012-06-09 16:24 PDT, Pepe
no flags Details
folderpane.js from Folderpane Tools. (1.70 KB, text/plain)
2012-06-10 20:50 PDT, Philip Chee
no flags Details
right click "move to" (13.23 KB, image/jpeg)
2012-06-11 04:03 PDT, Pepe
no flags Details
patch (4.65 KB, patch)
2012-06-21 14:34 PDT, :aceman
florian: feedback+
Details | Diff | Splinter Review
patch v2 (4.69 KB, patch)
2012-06-21 15:25 PDT, :aceman
mozilla: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Splinter Review

Description Pepe 2012-06-09 16:17:23 PDT
Created attachment 631697 [details]
accounts under "get mail".jpg

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:12.0) Gecko/20120515 Firefox/12.0 AvantBrowser/Tri-Core
Build ID: 20120521181842

Steps to reproduce:

I have multiple E-Mail-accounts that I sorted with the add-on "Folderpane Tools" alphabetically.

Since I installed TB 13, the sorting is mixed up when I klick on the arrow of menu "get mail" (what you do when I only want to check one mail account).

Account manager still shows the accounts in the correct order, only the menu "get mail" ist mixed up!

I have 30 accounts and Use Windows XP SP3. 

Till TB 12 all was fine, and by the way, I didn't change any settings or something in menu "View"
Comment 1 Pepe 2012-06-09 16:24:19 PDT
Created attachment 631698 [details]
the sorting in the account manager is alphabetic

So here you can see that the sorting in the account manager is different and still correct (i. e. alphabatically), like it always was from TB 3 - 12

So it is a BUG in TB13
Comment 2 Pepe 2012-06-09 16:25:20 PDT
Comment on attachment 631697 [details]
accounts under "get mail".jpg

account sorting under menu "get mail" is mixed up
Comment 3 :aceman 2012-06-09 17:18:16 PDT
That depends on what the extension does and how it forces the ordering of the accounts. Maybe it assumes some things about Thunderbird's internals that were never promised, only incidentally worked in versions 3-12. We'd need to extension author to comment here to tell how the extension works. Then we can decide if it is really a bug in TB or an intentional change.
Comment 4 Pepe 2012-06-09 21:55:21 PDT
So you should just add a native function to order the accounts in TB, without needing to use an add-on. Like it is possible in many other programs (move up/down E-Mail-accounts).
Comment 5 :aceman 2012-06-10 04:17:32 PDT
And that is what bug 244347 requests.
Also note that TB reorders accounts automatically according to their type, e.g. the default account first, then all POP3+IMAP4 accounts, Local Folders, RSS, IM, News. But if you only care about the order inside POP3 (or IMAP4) accounts that should ordering should be stable.

But you say the order in the Account manager is correct, only that one popup (get mail) is wrong. What about other account lists? E.g. when you rightclick a message and select "Move to", what is the order of the accounts?
Comment 6 Philip Chee 2012-06-10 06:07:22 PDT
Folderpane Tools only changes the display order in the folder pane. As far as I know it doesn't touch the Get Mail drop-down.
Comment 7 :aceman 2012-06-10 11:00:30 PDT
In what way does it do it? Does it change the server IDs or does it reimplement some function of the folder pane to produce the wished ordering? But the user says also the Account manager is affected by the extension.
Comment 8 Philip Chee 2012-06-10 20:50:16 PDT
Created attachment 631797 [details]
folderpane.js from Folderpane Tools.

> Does it change the server IDs or does it reimplement some
> function of the folder pane to produce the wished ordering?

Yes we override gFolderTreeView._sortedAccounts()
I would guess something in _sortedAccounts changed:

  window.loadStartFolder = fptLoadStartFolder;
  if (window.gFolderTreeView)
    window.gFolderTreeView._sortedAccounts = fptSortedAccounts;
Comment 9 Pepe 2012-06-11 04:02:01 PDT
(In reply to :aceman from comment #5)
> And that is what bug 244347 requests.
> Also note that TB reorders accounts automatically according to their type,
> e.g. the default account first, then all POP3+IMAP4 accounts, Local Folders,
> RSS, IM, News. But if you only care about the order inside POP3 (or IMAP4)
> accounts that should ordering should be stable.
> 
> But you say the order in the Account manager is correct, only that one popup
> (get mail) is wrong. What about other account lists? E.g. when you
> rightclick a message and select "Move to", what is the order of the accounts?

The order of the folders when I right click is okay, I must say that I use a "Global Inbox", so I don't see all the accounts separately when I try to move.

Yes, the account manager shows the order alphabetically like in TB versions 3 - 12, so the account manager is NOT affected, only the "Get Mail" drop down is messed up. I use that "Get it" function quite often.

I have only POP3 accounts (29 I think) and 1 IMAP account.
Comment 10 Pepe 2012-06-11 04:03:03 PDT
Created attachment 631851 [details]
right click "move to"

Global Inbox, so no sorting of POP3 accounts visible
Comment 11 Pepe 2012-06-16 04:08:03 PDT
I have uninstalled the "Folderpane Tools" add-on and after that have added a new POP3-account.

In the account manager the new accoutn appears at the bottom (like it should), but in the drop down menu "Get Mail" it appears somewhere in the middle of all accounts, although normally, without Folderpane Tools, the account should also appear right at the bottom...

I don't think the bug is caused by the add-on.
Comment 12 WADA 2012-06-19 17:21:15 PDT
Ordered in "associated mail.server.serverX number", instead of "account order in mail.accountmanager.accounts" which is used for "account order at folder pane" and is modified by add-on?

It looks;
 (1) Default mail account
 (2) Mail accounts/RSS accounts in order of associated server.serverX number
 (3) News accounts
And, when mail.account.account1.server=server1 and account1 is placed at top of list, this account was placed at bottom of above (2) by following changes in prefs.js.
 mail.account.account1.server=server1 => mail.account.account1.server=server99
 mail.server.server1.xxx              => mail.server.server99.xxx
Comment 13 WADA 2012-06-19 21:14:13 PDT
Even when "order of non-default mail accounts & RSS accounts" was incidentally "order of server number", order was chaged by default mail change and was not "order of server number". Order was altered as if random for me.

Because problem was reported in a forum by an user and I could observe phenomenon with Tb 13.0.1 on Win-XP(all add-ons are disabled), changing to New.
Comment 14 :aceman 2012-06-19 23:50:41 PDT
As I said TB reorders the accounts by type, not by the server/account number. So the extension should not rely on some incidental ordering.
I am consolidating the ordering in various places of the UI in bug 749200. Once that lands, the extension should override the new single getServerPrecedence() function in folderUtils.jsm so that it assesses the server by the number, not type.

But I'll check if that ordering even applies to the "get mail" button.
Comment 15 WADA 2012-06-20 00:55:26 PDT
(In reply to :aceman from comment #14)
> As I said TB reorders the accounts by type, not by the server/account
> number. So the extension should not rely on some incidental ordering.

As we already said, we are testing without add-on too.
Problem is different order among next in pure Tb.
(a) Order of accounts listed at folder pane in any Tb release :
    based on mail.accountmanager.accounts and type
    (1) Default mail account
    (2) Mail/RSS accounts in mail.accountmanager.accounts order
    (3) Local Folders
    (4) News accouns in mail.accountmanager.accounts order
    Even if this is "incidental", this was almost same since initial of Mozilla
    Mail&News which was inherited from Netscape Messenger.
(b) Order of accounts at Get Msg Drop Down list
(b-1) Tb 12 or before : same as (a)
(b-2) Tb 13 : Different from (2) of (a). 
              Changed by default merely mail account change.
              Order is unpredictable for me.
(a) and (b-2) should be consistent.

Please note that order in mail.accountmanager.accounts may be altered by Tb due to account deletion/re-definition, and that manual modification of mail.accountmanager.accounts is never prefs.js corruption.
Comment 16 :aceman 2012-06-20 01:20:24 PDT
Yes, I am going to investigate (b), i.e. how does "get mail" dropdown list determine the order.

(1) - (4) is not incidental, it is by design and forced by the various ordering functions, that I try to merge in bug 749200.
Only RSS and IM accounts are going to be updated/fixed, see bug 749200 comment 7.
Comment 17 Philip Chee 2012-06-20 06:17:47 PDT
> Once that lands, the extension should override the new single
> getServerPrecedence() function in folderUtils.jsm
IIRC methods/function in JSMs can't be overridden by design.
Comment 18 :aceman 2012-06-20 13:57:41 PDT
I can also confirm that in TB13 the "get mail" account list has different order than the folder pane and account manager. I do not remember how it was in older versions.
Comment 19 :aceman 2012-06-20 14:11:06 PDT
I think I see what is going on:
http://hg.mozilla.org/releases/comm-release/diff/ef0b70ed8c0e/mailnews/base/content/folderWidgets.xml (this is the regression patch in my opinion).

In this patch (for TB13, due to IM landing) the sorting is changed from sorting accounts (as all other lists do) to sorting servers. Maybe normally the produced order mostly matched regardless of the method used. But if you have crafted serverX and accountX IDs the lists may diverge (it even diverges in my production profile where I did not craft anything:)). The divergence is inside server type groups (yes, Mail accounts/servers must come before News accounts, but inside the Mail group the order is undefined and may just depend on the serverX IDs).

I have not tested this theory yet.

Florian, would it be a problem to revert it back and just filter out IM accounts? That is even what Neil suggests in bug 749200 comment 37.
Comment 20 :aceman 2012-06-20 14:17:06 PDT
This affects all folder pickers, like those in the "Move to" context menu, or in Account manager, Copies & folders. Ugly.
Comment 21 Florian Quèze [:florian] [:flo] 2012-06-20 14:30:36 PDT
(In reply to :aceman from comment #19)

> Florian, would it be a problem to revert it back and just filter out IM
> accounts?

That's OK with me. I think we (and especially David) were happy in that bug to reduce the number of specific cases for type == "im", but if you are factoring all the code and this turns out to keep only a single test for type == "im", it's not really adding back all the specific cases :).
Comment 22 :aceman 2012-06-20 14:56:44 PDT
If this is fixed quickly (basically revert of the linked commit), does it have any change of getting in a branch sooner than TB16?
Otherwise I can just do it as part of bug 749200 as it would be easier there.
Comment 23 :aceman 2012-06-21 14:34:40 PDT
Created attachment 635475 [details] [diff] [review]
patch

This reverts only the sorting part of Florian's patch. The other part using .allServers can stay, the order is not important there.

I can confirm this fixes the problem in my tests (I have reordered account in the "mail.accountmanager.accounts" pref).
Comment 24 :aceman 2012-06-21 14:48:11 PDT
This is a just quick fix to get the function back into original ordering.

I'll clean up this function better in bug 749200.
Comment 25 Florian Quèze [:florian] [:flo] 2012-06-21 15:04:52 PDT
Comment on attachment 635475 [details] [diff] [review]
patch

>-            var defaultServerKey = acctMgr.defaultAccount.incomingServer.key;
>-            function serverCompare(a, b) {
>-              if (a.key == defaultServerKey)
>+            function accountCompare(a, b) {
>+              if (a.key == acctMgr.defaultAccount.key)
>                 return -1;
>-              if (b.key == defaultServerKey)
>+              if (b.key == acctMgr.defaultAccount.key)

You can still keep acctMgr.defaultAccount.key in a variable to avoid retrieving it several times.

This is OK with me otherwise, but I'm not a mailnews peer, I think you will want to request review from David who reviewed the original patch.
Comment 26 :aceman 2012-06-21 15:25:07 PDT
Created attachment 635502 [details] [diff] [review]
patch v2

Thanks.
Comment 27 David :Bienvenu 2012-06-25 15:00:44 PDT
Comment on attachment 635502 [details] [diff] [review]
patch v2

looks reasonable.
Comment 28 Ryan VanderMeulen [:RyanVM] 2012-06-26 18:06:25 PDT
https://hg.mozilla.org/comm-central/rev/2a7420635947
Comment 29 Lee_Dailey 2012-06-28 07:16:18 PDT
howdy y'all,

comment 22 asked if this could get into tbird earlier than v16. has that been decided? 

there is a thread over at mozillazine on this and it seems to be a remarkably jarring effect. while i'm not seeing it - apparently my account types don't trigger the hiccup - the number who are seeing it seems to make it worth porting to tb14 at least.

- TB 13 - Move/Copy To feature - email accounts jumbled
http://forums.mozillazine.org/viewtopic.php?f=31&t=2484947

take care,
lee
Comment 30 :aceman 2012-06-28 12:52:31 PDT
Yes, I want to let it bake on Trunk (TB16) for some days so that everybody can test it. If it works good, I'll nominate the patch for TB14 and TB15.

Lee, can you get some of the reporters in the forum to test it on trunk?
Comment 31 Lee_Dailey 2012-06-28 12:58:33 PDT
howdy :aceman,

i'll give it a try. [*grin*] most folks who have mentioned the problem seem willing to test things ... but going with a test version of tbird seems to be more than most are willing to risk. understandably so.

take care,
lee
Comment 32 :aceman 2012-06-28 13:17:26 PDT
They can make a copy of their profile and run the test version on that.
Comment 33 Mark Banner (:standard8, afk until Dec) 2012-07-03 04:33:05 PDT
Comment on attachment 635502 [details] [diff] [review]
patch v2

[Triage Comment]
We've only got another two weeks until we release, so I'd like to get this into this beta so we've got some time for testing on the beta channel. Therefore a=me for aurora/beta.
Comment 34 :aceman 2012-07-03 04:46:43 PDT
OK, thanks.
Lee, maybe your folks may be more willing to try the TB14beta when this lands there.
Comment 35 Lee_Dailey 2012-07-03 06:02:18 PDT
howdy :aceman,

i will give it a try. [*grin*] from what i can tell, none of the folks who have the problem were willing to take the risk with the alfa versions.

take care,
lee
Comment 36 :aceman 2012-07-03 11:24:53 PDT
That is why I proposed *beta*, which will become TB14 in 2 weeks :)
Comment 38 Lee_Dailey 2012-07-05 18:10:01 PDT
howdy y'all,

one of the folks over at mozillazine tested with the current beta and it fixed his scrambled get mail account list sequence. here is the post ...
http://forums.mozillazine.org/viewtopic.php?p=12117611#p12117611

he's quite pleased, as am i. thanks, folks! [*grin*] 

take care,
lee
Comment 39 :aceman 2012-07-06 02:24:58 PDT
Thanks for the feedback Lee. I am also glad this could be fixed so quickly :)
Comment 40 WADA 2012-07-11 14:48:27 PDT
*** Bug 772891 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.