763236 - Thunderbird 13 - accounts get mixed when using menu "get mail"

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, defect)

Description

[Pepe]

Attachment: 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]

Attachment: 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]

Comment on attachment 631697 [details]
accounts under "get mail".jpg

account sorting under menu "get mail" is mixed up

Comment 3

[:aceman]

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]

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]

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]

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]

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]

Attachment: 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]

(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]

Attachment: right click "move to"

Global Inbox, so no sorting of POP3 accounts visible

Comment 11

[Pepe]

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:World Anti-bad-Duping Agency]

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:World Anti-bad-Duping Agency]

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]

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:World Anti-bad-Duping Agency]

(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]

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]

> 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]

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]

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]

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]]

(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]

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]

Attachment: 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]

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]]

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]

Attachment: patch v2

Thanks.

Comment 27

[David :Bienvenu]

Comment on attachment 635502 [details] [diff] [review]
patch v2

looks reasonable.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/2a7420635947

Comment 29

[Lee_Dailey]

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]

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]

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]

They can make a copy of their profile and run the test version on that.

Comment 33

[Mark Banner (:standard8)]

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]

OK, thanks.
Lee, maybe your folks may be more willing to try the TB14beta when this lands there.

Comment 35

[Lee_Dailey]

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]

That is why I proposed *beta*, which will become TB14 in 2 weeks :)

Comment 37

[Mark Banner (:standard8)]

Checked in:

https://hg.mozilla.org/releases/comm-aurora/rev/fdc46e0d58ad
https://hg.mozilla.org/releases/comm-beta/rev/292da56356d0

Comment 38

[Lee_Dailey]

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]

Thanks for the feedback Lee. I am also glad this could be fixed so quickly :)