244347 - Cannot change sort order of accounts (implemented fix: DnD in account manager, pref: mail.accountmanager.accounts)

Status: RESOLVED FIXED

(MailNews Core :: Account Manager, enhancement)

Description

[Florian Effenberger]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de-DE; rv:1.6) Gecko/20040206 Firefox/0.8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de-DE; rv:1.6) Gecko/20040206 Firefox/0.8

I cannot change the sort order of accounts. I'd like to change the order my
accounts appear in the folder pane.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Comment 1

[Benedikt Kantus]

Florian, *please* search for bugs before submitting them.

*** This bug has been marked as a duplicate of 61078 ***

Comment 2

[Frédéric Buclin]

Not a dupe of bug 61078. Bug 61078 is about the Mozilla Suite while this one is about Thunderbird. Reopening and nominating for 2.0.

david, any chance to get this feature in Tb 2.0? This looks like a "core" feature rather than something for an extension.

Comment 3

[Scott MacGregor]

not going to block the release for this.

Comment 5

[Roman]

Aaah, such a basic feature, not available in major version 2. Alas. But it's open source after all, I /could/ fix this myself... Disappointment.....

Comment 6

[Felix Schwarz]

There is an extension called "Folderpane Tools" which provides this: https://addons.mozilla.org/de/firefox/addon/258

Comment 7

[Pete Riley]

That extension is nice.  However, it's a hack that has the side effect of changing Thunderbird's default account in such a way that it breaks the Lightning extension (which is an integrated calendar for Thunderbird).  That is, when I set Folderpane tools to change the order of accounts, it sets the default account to 'Local Folders' and Lightning can't send meeting invitations because 'Local Folders' isn't a real account.

Yes, Lightning could probably solve this problem by allowing the user to choose from which account to send meeting invitations, but IMO it would be better if Thunderbird offered this as core functionality so that extensions like Folderpane tools and Lightning didn't have to make hacks to make it work, plus there would be one less extension that we would have to install.  And, Thunderbird could implement it with nice drag&drop to re-order accounts (and hopefully to re-order mailbox folders too).

Comment 9

[peter luttrell]

While the extension is great, I think this feature needs to be part of the core application. Every modern email client that I've used that supports multiple email accounts has this feature.

Comment 14

[Dustin Steiner]

I hope this feature will come with Thunderbird 3 'cause it's really a feature that I don't want to miss any longer.

Comment 15

[Michel]

Still not in the TB3beta4 and not even in the last 4th november nightly build...

Any chance for the next release ?

BTW : it would be really cool that this possibility also be available in SmartFolders view too (see bug https://bugzilla.mozilla.org/show_bug.cgi?id=526659)

Michel

Comment 16

[Maarten Kossen]

Any chance this is going to be looked at? It's been nagging me for a long, long time and editing the prefs.js file is not a real joy. I work with different companies and e-mail addresses and occasionally want to re-order my account list. A feature to do this would be extremely appreciated.

If I could help, I would like to, but I am unsure how I can. I am a programmer but have never developed anything Mozilla-related yet.

Comment 17

[ian douglas]

I'd love this feature as well, it's been on the books for 6 years now.

Comment 18

[David :Bienvenu]

First, we'd need a UI. Two obvious UI's - drag drop in the folder pane to rearrange accounts (not highly discoverable), or drag drop in account settings, or buttons in account settings to move accounts up or down. But Bryan very likely has better ideas...we'd like the whole folder pane to be much more customizable.

Comment 19

[Wayne Mery (:wsmwk)]

one further thought, is if it's done in folder pane then it adds the complexity of dealing with unified folder and other modes. 

On the other hand, there are other bugs/requests that have asked for customization of folders/order within those modes.

Comment 20

[Bryan Clark (DevTools PM) [:clarkbw]]

I'm not usually a fan of drag & drop because it often lacks discoverability and undo features such that people will accidentally drag something and drop it somewhere else without ever knowing.

I'd probably like to see some kind of edit mode where you turn on the folder pane edit mode and then can re-order the accounts.  Perhaps this could be done in account settings instead?

Just some thoughts.

Comment 21

[Jim Porter (:squib)]

Why not have an option in Tools -> Account Settings? You could just add "Move up" and "Move down" to the Account Actions menu button.

Comment 22

[Karel Koubek]

Agreed. Account Settings -> Account Actions -> "Change Account order"  seems like the best place for this with minimum confusion.

Preferably a small window should open, similar to Options -> Display -> Tags, listing all the available accounts with the buttons "Move up" and "Move down" to its side. The default account name could be in bold letters with word "default" in parentheses. Even a button "Make default" could be added.

Comment 23

[Thomas D. (:thomas8)]

This is basic enough and has enough votes (currently 27) to warrant wanted-thunderbird+, which should be set.

Comment 24

[Bryan Clark (DevTools PM) [:clarkbw]]

Attachment: account settings

(In reply to comment #21)
> Why not have an option in Tools -> Account Settings? You could just add "Move
> up" and "Move down" to the Account Actions menu button.

Yeah, that's what I meant at the end.

I was thinking we could pull it out from the account actions so it's just two up/down buttons connected to the list.

Comment 26

[Thomas D. (:thomas8)]

http://getsatisfaction.com/mozilla_messaging/tags/bug_244347

Sorry for spam, but I cannot add the link to the equivalent MoMo getsatisfaction report anywhere else but here in a comment, due to:

Bug 571740  - Cannot add getsatisfaction links/URLs to "See also" field (e.g. for problems reported at Mozilla Messaging's Thunderbird support site)
Bug 577847 - "See also" field should accept any URL
Bug 577842 - Allow adding multiple URLs in URL field (that would be the easiest fix, I'd think; it could be rephrased as providing an optional *custom* URL field for multiple URLs *in addition* to the current default URL field.)

Comment 27

[Marcus Yoo]

https://addons.mozilla.org/en-US/thunderbird/addon/15102/

Manually soft folders

This addon seem to fix the issue addressed here for the most part.
It was last updated on 2010-11-1, by author mozjonathan.
I've just tried it, and it works great.

Comment 29

[Wayne Mery (:wsmwk)]

(looks like "After Thunderbird 1" got translated to "Thunderbird 11")

Comment 30

[Bob]

So this bug was entered WAYYYYY back on  5-22-2004, almost a decade ago, and this feature has STILL not been added.

I was about to request this myself, but this bug report already exists, as does Bug 535117.

It's WAYYYYY past time for this basic function to be added!

Comment 31

[Pepe]

It's even worse, now the sorting order under menu "get mail" is totally mixed up, although I ordered all accounts with the add-on "Folderpane Tool"!

https://bugzilla.mozilla.org/show_bug.cgi?id=763236

Comment 32

[posita]

For those posting "+1" or "me too" comments, please consider voting instead of adding such comments (see the "Importance" field above for links to add your votes).

Comment 34

[Geert-Emo]

Same here as Pepe reported.
I suspect code regression to before <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=428887">bug 428887</a>

Comment 35

[:aceman]

I try to look at this after bug 749200 lands (so that we can be sure the account order is interpreted the same way at all places.

Also I need to note that in the current implementation (and bug 749200 will not change that), the various types of accounts are forcefully sorted in specific positions, i.e. you have all mail (POP/IMAP) accounts, Local folders, all News accounts, all IM account, all RSS accounts, etc.
So what you can influence is only the order of accounts inside the groups, i.e. within all mail accounts, you can set which account comes first, which second etc.
If this is enough for everyone, then I could try to implement something.

Comment 36

[:aceman]

I'd go in the way of comment 21, just menuitems/buttons in the Account manager, no drag and drop anywhere.

Comment 37

[Lee_Dailey]

howdy :aceman,

comment 21 would fit my use case quite well without introducing any unneeded complexity. plus, that type of sorting is used elsewhere in tbird, so there may be reusable code in there somewhere.

take care,
lee

Comment 38

[Ulf Zibis]

(In reply to :aceman from comment #35)
> So what you can influence is only the order of accounts inside the groups,
> i.e. within all mail accounts, you can set which account comes first, which
> second etc.
> If this is enough for everyone, then I could try to implement something.

+1
+ [Default] button
Please also consider bug 706791
(It should include the renumbering of X-Account-Key tags in the Mbox files)

Comment 39

[:aceman]

Comment on attachment 463688 [details]
account settings

Bwinton, could you comment on whether this UI proposed would be OK? Or should the "move up/down" items be included into the "account actions" menu?

Comment 40

[Thomas D. (:thomas8)]

(In reply to :aceman from comment #39)
> Comment on attachment 463688 [details]
> account settings
> 
> Bwinton, could you comment on whether this UI proposed would be OK? Or
> should the "move up/down" items be included into the "account actions" menu?

Hiding "move up/down" in "account actions" menu imho would make a rather unfortunate UX: For those scenarios where this function is most relevant (users with many accounts who want to move account 10 up to position 1), it would be most difficult to use: To move an account up by one, this would require the following procedure:
1 click "account actions"
2 delicate mouse move to hover exactly "Move up" while avoiding both "Move down" and "Remove Account"
3 click "Move up"
To move an account up from position 1 to position 10, that's 20 clicks and 10 delicate mouse moves in between. Outch.
I suggest we'd be better off with only 10 safe and straightforward mouse clicks in exactly the same position, on the up/down arrow buttons as proposed by attachment 463688 [details].

Don't forget to make this keyboard-accessible. up/down buttons need to be focusable, and when they have focus, both Enter or Space should press the buttons, so that user can just repeatedly press enter to move up or down.

Btw, I find that "Account actions" button pretty clumsy, so if anything, I would get things outa there to be individual buttons (as they used to be) instead of hiding more buttons inside.

Comment 41

[Thomas D. (:thomas8)]

For full keyboard access, I recommend:
With focus on account...
- alt+cursor down to move account down
- alt+cursor up to move account up

That's plausible, easy to memorize, and highly efficient.
It may need a hint somewhere to be discoverable, e.g. the following tooltips on up/down arrow buttons:
"Move account up (Alt+Cursor Up)"
"Move account down (Alt+Cursor Down)"

Comment 42

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 463688 [details]
account settings

Yeah, I think that might be okay.  I worry a little about what people would expect to happen when they were on, say, "Junk Settings", and clicked the button, and I'm not a huge fan of having the up/down buttons on the bottom there.  So while this is better than the current state, I think I would prefer some sort of "Reorder Accounts…" in the "Account Actions" dropdown, which took us to a more focused popup (similar to the "Manage Identities…" popup, but with more/different buttons).

Would you mind doing a mockup of what you think the new dialog should look like, and I'll give you some feedback on that?

Thanks,
Blake.

Comment 43

[:aceman]

What about disabling the buttons when the selection is not a node that is the name of an account?

Or, even if the selection is on "Junk settings" the whole account will move. If the tooltips to the buttons say "Move account up/down" the expectations could be managed.

Or, if the "reorder accounts" command temporarily collapsed the accounts so that only the names are visible.

I think I would be able to copy the "Manage identities" dialog, but I think it would be a duplication of code when we already have one for the accounts (the current AM). The identities do not have any other list, so need the subdialog.

Comment 44

[Blake Winton (:bwinton) (:☕️)]

(In reply to :aceman from comment #43)
> What about disabling the buttons when the selection is not a node that is
> the name of an account?

That wouldn't give people much indication of how to enable them, so I'm not a giant fan of that.

> Or, even if the selection is on "Junk settings" the whole account will move.
> If the tooltips to the buttons say "Move account up/down" the expectations
> could be managed.

That would certainly surprise me if it happened to me.  Can you think of another product that does something like that?

> Or, if the "reorder accounts" command temporarily collapsed the accounts so
> that only the names are visible.

That could work, if there's a reason to not open a new dialog…

> I think I would be able to copy the "Manage identities" dialog, but I think
> it would be a duplication of code when we already have one for the accounts
> (the current AM). The identities do not have any other list, so need the
> subdialog.

Perhaps we can re-use the code in both places?

Thanks,
Blake.

Comment 45

[Thomas D. (:thomas8)]

I'd much prefer to see this land with a little bit of UX oddness rather than wait even longer till we establish something better. It's not everyday that users will need to change the order of their accounts... But for that one time, we should let them do it without more delay after 8 years of waiting...

Comment 46

[Mitra Ardron]

Ditto what Thomas says, its a major pain as I've been using a TB a long long time, and have accounts that rarely get emal, but sit high on the list, so that the important current accounts are pushed off the bottom of the screen where I won't notice incoming email. 

I don't know if changing the order would have other implications but why not keep it simple ..We have a "Account Actions" button - adding Move Up as suggested in Comment #21 in 2010 would presumably be a trivial UI step

Comment 47

[:aceman]

Yes, that would be easy.

Bwinton, what about that? Adding a "Move account up/down" menu items into the Account actions menu. Those items could be active for all panes of an account (the first line with name, including "server settings", "Junk settings, etc.".
Would that still be unexpected?

Comment 48

[:aceman]

But Thomas would better like the arrow buttons outside the Account actions.

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #44)
> (In reply to :aceman from comment #43)
> > What about disabling the buttons when the selection is not a node that is
> > the name of an account?
> 
> That wouldn't give people much indication of how to enable them, so I'm not
> a giant fan of that.

What about having the tooltip hint that the selection must be on the account name to activate the buttons?

Comment 49

[:aceman]

Re-using the "manage identities" dialog may be possible but it would need a lot of work. E.g. if Bwinton proposes a new "manage accounts" dialog for managing accounts, then it would need to move the existing "add", "remove", "make default" account actions to this new subdialog. They already have code to work in the current AM. Adapting them to a different list would be some work and could produce new bugs. I think just adding move up/down to the existing AM would be easier.

Comment 50

[Thomas D. (:thomas8)]

(In reply to :aceman from comment #49)
> Re-using the "manage identities" dialog may be possible but it would need a
> lot of work. E.g. if Bwinton proposes a new "manage accounts" dialog for
> managing accounts, then it would need to move the existing "add", "remove",
> "make default" account actions to this new subdialog.

I understood from Blake's comment 42 that he is suggesting "Reorder accounts..." command in Account actions which would trigger a *"Reorder accounts"* dialogue exclusively for that very purpose (no need to move other commands around), which I think makes a nice and easy UI because the current AM is so cluttered as we have all the nested options open by default (and those nested subcategories repeated for each account are probably unfortunate design anyway). If we want to move accounts within the current AM, we really need to collapse those subcategories. We could collapse them after user clicks "move account" first time.

Imo there is no problem if we enable moving of accounts only when the main account entries are focused - that's the general principle of using software: Select, then act. It doesn't make much sense to start "account move" with "Composition & Adressing" subcategory selected (although it wouldn't hurt much either).

Comment 51

[:aceman]

It is the same. If we shared with "manage identities" but then only wanted to have the "move up" and "move down" operations there, then we'd have to hide the "add", "edit", "delete", "set default" buttons in there ;)
And add "move up" and "move down" buttons as they are not yet there (and aren't planned for identities).

Comment 52

[:aceman]

But there is one thing in favor of the separate dialog: there are various saves and checks of data in the account manager that trigger when the selection is changed (moving from one pane to another). If we now suddenly start to move rows around in the tree I don't know what would happen and if those checks would be properly triggered. We would need to really make sure the same pane (row) is selected again after the movement.

Also, opening a subdialog, making the shuffles and returning back to the AM with reordered accounts will not solve this. I would feel best when the main AM is closed before we open this new dialog.

CCing Seamonkey guys as this will affect them too, even worse because they do not have the Account actions menu, but buttons instead.

Comment 53

[Atul Jangra [:atuljangra]]

This bug looks interesting. Just pinging to know if anybody is currently working on this.
As it is a student project, maybe we can add it to SummerOfCode13 ideas list.

Comment 54

[:aceman]

Hi. We are discussing how this should look and work. After that settles, I suppose I able to implement this. I just forgot about this a little (had to wait for dependency to land). I'll look at the design using a small separate dialog to sort the accounts.

Of course, if there is interest in getting new contributors to the account manager then this is a good bug (and not that hard as it is possible to copy the code/pattern from e.g. the identity list dialog).

I'd just wonder who would mentor it :)

Comment 55

[Atul Jangra [:atuljangra]]

(In reply to :aceman from comment #54)

> I'd just wonder who would mentor it :)

You can mentor it as you know about the problem and how to implement this :-)

Comment 56

[:aceman]

Theoretically yes, depends how much time it needs and if I am an officially acceptable mentor for Mozilla.

Comment 57

[:aceman]

Bwinton, to avoid misunderstanding, could you revoke your ui-r+ on the screenshot as you no longer agree with the buttons below the account list?

Comment 58

[:aceman]

Atuljangra, coding what is needed for this bug is work for maybe 1-2 days so I am not sure it qualifies for a full project.

On the other hand I hate the semantic duplication that we have several such small dialogs with Add/Edit/Delete/Move up/down/make default or something. In each of them we must code the same hacks for selecting items after deletion, workarounds for focus problems in the listbox widget, etc.

My idea for the student would be to come up some abstraction layer for this. That provides the framework having the dialog, some buttons and managing focus/selection/redraw etc. Then we could just send to this dialog the proper JS list to manage and code specific for the list in question (e.g. some checks/questions before deletion)
See comment 49 where I started with this.
I am not sure how much code can be saved/abstracted in this way and if all this is worth it. But finding that out may take some more weeks and could be a better project.

Comment 59

[Blake Winton (:bwinton) (:☕️)]

(Just commenting to clear the needinfo request, since I revoked my ui-r+…  Sorry about that, folks.)

Comment 60

[Josiah Bruner [:jsbruner]]

Aceman, I'm unassigning you from this bug since A. You aren't actually working on it, so that confuses others who might want too. And B. Someone has volunteered to take this, so clearing your name makes things simpler.

Comment 61

[Thomas D. (:thomas8)]

Current statistics - This bug: 44 votes,  8 dupes.
See also: SeaMonkey bug 61078 (60 votes, 15 dupes).

Comment 62

[Thomas D. (:thomas8)]

So per comment 42 from Blake Winton (ux lead), and aceman's comment 52, the preferred UI direction this should take is to implement a separate "Reorder accounts" dialogue (with [Move Up] and [Move Down] buttons). The dialogue would be triggered from a "Reorder Accounts..." menuitem to be added in Tools | Account Settings | Account Actions (button popup menu).

Volunteers with XUL and JavaScript coding skills are most welcome to start working on this (the community will assist you). 

The "Folder Pane Tools" Addon provides a "Rearrange Accounts" dialogue which looks very much like what we want here (still works for TB24), so anyone willing to pick this up can use that as a proof-of-concept model UI/code:

https://addons.mozilla.org/en-US/thunderbird/addon/258/

Comment 64

[Yan Pas]

+1. I would also like to see reordering of RSS subscriptions

Comment 66

[Greeneye57]

I found a very crude tedious way for a user to re-order accounts - until a simple re-order version is released. Every time a mail account is "set as default" it is moved to the top next time you re-open Thunderbird. So after lots of juggling, closing and re-opening Thunderbird you get the order you want. Sorry guys but I am a tech-head not a coder. ;-)

Comment 67

[Greeneye57]

And I'm the one who suggested re-ordering which created this thread - see my opening comment.

Comment 72

[Thomas D. (:thomas8)]

We might borrow some ideas or code from bug 663695, where I implemented reordering of attachments in their list.

Comment 73

[Thomas D. (:thomas8)]

(In reply to :aceman from comment #52)
> But there is one thing in favor of the separate dialog: there are various
> saves and checks of data in the account manager that trigger when the
> selection is changed (moving from one pane to another). If we now suddenly
> start to move rows around in the tree I don't know what would happen and if
> those checks would be properly triggered. We would need to really make sure
> the same pane (row) is selected again after the movement.
> 
> Also, opening a subdialog, making the shuffles and returning back to the AM
> with reordered accounts will not solve this. I would feel best when the main
> AM is closed before we open this new dialog.

I'm not sure how good that would really feel, the main Account Settings dialogue going away from underneath. More importantly, I think it would be quite hard to get this right: Account Settings is currently designed as ONE big dialogue (like it or not), where ANY changes only apply after pressing OK, or will be discarded after pressing Cancel (some exceptions might apply, but essentially violate this logic already). Closing the main Account Settings dialogue to do account sorting hence would have to take care of that "unsaved changes" problem - we can't just apply them without user's explicit OK, and obviously we can't just discard them either. But nagging with a question before going into reordering dialogue also isn't feasible. And actually, do we really need to force users to dig deep into "Account Settings > Account Actions > Reorder Accounts" for a rare but essential action like controlling the layout of your primary UI? Other layout options are much more exposed (View > Layout).

Here's an UX idea which will make implementation of this significantly easier:
* Just add "Reorder Accounts..." to the accounts context menu:

Get messages
Open in new tab
Open in new window
Search messages...
---------------------
New folder...
---------------------
Reorder Accounts...
Settings...

Benefits:
- Avoid the "unsaved changes" and settings tree updating cans of worms described above, easier to code -> easier to deliver
- KISS UX: Intuitive but minimalist access to this essential function from primary UI via account context menu (ux-discovery)
- nicely tucked away in a rarely-used context menu with very few entries so far

That said, I'm not against having "Reorder Accounts" *also* in Account settings, but I think it's quite intuitive, plausible and sufficient to only have this on the account context menu for now, which would offer a much leaner UX compared to "three-levels-deep-into-account-settings-and-shaking-the-settings-tree" approach.

Richard, Aceman, what do you think?

Comment 74

[Thomas D. (:thomas8)]

(Please note: In this comment, I'll discuss a minor detail of access to the new functionality, which is NOT essential to get started with this).

In comment 73, I suggested a minimal UI for accessing "Reorder Accounts" from accounts context menu as a nice and easy way of implementing this. In addition, we should also consider offering /main menu/ access to "Reorder Accounts..." (especially if "Reorder Accounts" is not (yet) available from Account Settings"). Candidates:

1) View > Layout > o Classic View
                     Wide View
                     Vertical View
                     --------------------
                   ✓ Folder Pane
                   ✓ Folder Pane Columns
                   ✓ Message Pane
                     --------------------
                     Reorder Accounts...

2) View > Folders > o All
                      Unified
                      Unread
                      Favorite
                      Recent
                      ---------------------
                      Compact View
                      ---------------------
                      Reorder Accounts...

3) Tools > [Snip]
           Import...
           Developer Tools   >
           Clear Recent History...
           ------------------------
           Reorder Accounts...
           Account Settings...
           Options...

I guess all of these have their pros and cons. My thoughts:

1) View > Layout: This came to my mind first, as reordering accounts is a general layout thing. "Reorder Accounts..." arguably fits here as we also have "Folder pane" and "Folder pane columns" which change visibility and layout of folder pane. However, we also have a separate View > Folders menu with a lot of options changing the layout of folder pane.

2) View > Folders: Logically (in terms of UI hierarchy), this might seem like the best fit, as all of the options here directly manipulate the layout of the folder pane. However, I am not sure how discoverable this is: Would users look for "folders" for changing order of "Accounts"?

3) Tools: I'm not a big fan of adding yet another entry to the already crowded Tools menu, and it does look somewhat redundant here with "Reorder Accounts..." and "Account Settings...". Notwithstanding, there's a plausible relation/difference between the two, so it's possible to put it here. I think I'd prefer 1) or 2) over 3).

Between 1) and 2), maybe 2) is best after all.

Comment 75

[Magnus Melin [:mkmelin]]

The easiest (in every regard) to do implement it is drag-n-drop, no? Yes not keyboard-only, but this is something one would do once/twice in a lifetime - tops!

Comment 76

[Richard Marti (:Paenglab)]

Yes, directly move them with d-n-d like we do already with the newsgroup folders. That's the most minimalistic UI.

Comment 77

[Ulf Zibis]

(In reply to Thomas D. (currently busy elsewhere) from comment #74)
> 1) View > Layout > o Classic View
>                      Wide View
>                      Vertical View
>                      --------------------
>                    ✓ Folder Pane
>                    ✓ Folder Pane Columns
>                    ✓ Message Pane
>                      --------------------
>                      Reorder Accounts...
This would be my favourite ! Yes, reordering the accounts is a layout concern.

> 2) View > Folders > o All
>                       Unified
>                       Unread
>                       Favorite
>                       Recent
>                       ---------------------
>                       Compact View
>                       ---------------------
>                       Reorder Accounts...
Possible, but doesn't really fit to the other functions

> 3) Tools > [Snip]
>            Import...
>            Developer Tools   >
>            Clear Recent History...
>            ------------------------
>            Reorder Accounts...
>            Account Settings...
>            Options...
A tool is something to do something, not a setting, as "Reorder Accounts" is.

About the d-n-d approach: This would be also smart, but then this possibility should be available consistently for all folders/items in the folder pane.
Even if it would be implemented, I only would like it as 2nd option to View > Layout > Reorder Accounts...

Comment 78

[Thomas D. (:thomas8)]

(In reply to Magnus Melin from comment #75)
> The easiest (in every regard) to do implement it is drag-n-drop, no? Yes not
> keyboard-only, but this is something one would do once/twice in a lifetime -
> tops!

I'm not against trying drag and drop, but I have big doubts if that's "easier" in any way, both wrt implementation and the resulting UX. And I believe that we should honor our Mozilla roots and affiliation by ensuring that every aspect of our application is "open and accessible to all". So having d-n-d only and no keyboard-only equivalent is a no-go imho.

d-n-d implementation/UX concerns:
a) There's at least 2 entirely different folder layout modes, "All" and "Unified", so both of them would have to support d-n-d, isn't it? That's significantly more complex.
b) We want to let users drag accounts, but in between there's tons of other folders, which makes for confusing/complex UX and difficult implementation, as all the folders in between can't be dragged nor can they be drop targets so you'll be dragging needles through the haystack.

A simple reordering dialogue with a plain vanilla list of all accounts (w/o any distracting folders) looks simple enough both for implementation and UX.

Comment 79

[Thomas D. (:thomas8)]

I'm also concerned about ux-error-prevention when things become dragable in the folders list. I've seen my parents accidentally dragging things around too often, and even for myself, I would never want something as crucial as my messaging system to be vulnerable to havoc from accidental drags.

Comment 80

[Derek v Giroulle]

As a user reordering accounts doesn't need to be done with drag and drop.   
it could be done by  pressing an up/down arrow in a list box or even typing numbers next to the account in a list box. 
Derek

Comment 81

[Magnus Melin [:mkmelin]]

Well you could do the d-n-d only in the account settings. Then you don't have to worry about views or long folder trees.

Comment 82

[Thomas D. (:thomas8)]

(In reply to Magnus Melin from comment #81)
> Well you could do the d-n-d only in the account settings. Then you don't
> have to worry about views or long folder trees.

Yes, that's better in terms of ux-error-prevention, but even less discoverable - I doubt I would ever try that and it wouldn't be easy to let users know about that feature. And unfortunately, account settings DOES have the same problem of large numbers of setting subnodes in the tree which are obstructing plain and simple d-n-d.

Some of this has been discussed already and I don't see drag and drop coming out as the favored solution, whereas there has been some notable concurrence about implementing a plain vanilla "Reorder Accounts" dialogue. If someone wants to add d-n-d to that - fine - but it's not required, whereas keyboard-accessibility IS a UX requirement.

Comment 83

[Ulf Zibis]

(In reply to Thomas D. (currently busy elsewhere) from comment #82)
> ... but even less discoverable - I doubt I would ever try that and it wouldn't be easy to let
> users know about that feature.
+1

Comment 84

[Magnus Melin [:mkmelin]]

How many? Well how many people really want to reorder? And if they don't find it them selves there's documentation.
Before adding a lot of dialogs you should consider the *overall* experience of what you're creating. Nobody really wants a jungle of irrelevant options to wade through.

Comment 85

[Greeneye57]

There is credit in all sides of this discussion. However, many of my clients would love this feature (including me). A simpler way without drag n drop would be to be able to right click on the account and "set as default". Then it moves to the top (like some other mail programs eg: the old win live mail). By doing this a few times, the accounts would eventually be in the order the user wants them to be. Sort of a mailbox Tetris. And clumsy (old?) users can't drag n drop anything to the wrong place.

Comment 86

[Thomas D. (:thomas8)]

(In reply to Magnus Melin from comment #84)
> How many? Well how many people really want to reorder?

Quite many, judging from the increasing popularity of this bug:
TB bug 244347: 49 votes, 14 duplicates all across the years, more recent duplicates (obviously, as people have more accounts than 10 years ago)
SM bug 61078:  64 votes, 15 duplicates

This bug isn't some edge case, but a likely scenario at some point for any user of TB. For those few times where a significant number of users will need it, it must be discoverable and accessible.

> And if they don't find it themselves there's documentation.

Ymmv, but I prefer in-product ux-discovery over some hidden documentation somewhere. And I as an advanced user would not easily try dragging any tree item with a dozen of subnodes, that just feels odd and unnecessarily complex. And I'm not sure what you are really arguing against, assuming that you couldn't possibly suggest to make this feature keyboard-inaccessible by design, which would be in open violation of our own accessibility guidelines, which imho are relevant even in terms of mozilla's mission ("open and accessible for all").

If you're against having a dedicated "Reorder accounts" dialogue (whilst bwinton, aceman and myself are in favor after weighing pros and cons of alternatives), please offer a dialogue-less proposal which secures keyboard access. I don't see why we would deliberately exclude e.g. blind users from reorganizing their accounts via keyboard, when there's popular demand for this feature, and we can make it accessible at the low cost of a single menu entry in some rarely used corner of account settings, account context menu, or View submenus.

> Before adding a lot of dialogs

Pls explain "a lot" - afasics we could never add more than maximally *one* such dialogue for any given list, and some lists can do without a dialogue if it's feasible to expose the item-moving UI on the main list (e.g. filters). Not feasible here.

> you should consider the *overall* experience of what you're creating.

Considered with detailed reasons on record, and not just me.

> Nobody really wants a jungle of irrelevant options to wade through.

Pls be more specific where "jungle of irrelevant options" would be caused by any of the current proposals of this bug, as I'm failing to see that.
- My comment 73 - account context menu entry - suggests to add 1 entry to a menu of 7. Account context menu is probably rarely used, and 7 entries isn't a jungle imho.
- My comment 74 - favored solutions 1) View > Layout or 2) View > folders - proposes adding 1 entry to a menu of 6 - again where's the jungle to wade through? Moreover, none of these menus are likely to be frequently used.

Personally I don't see how commonplace exaggerations will contribute to establish a discoverable and accessible UI for the legitimate use case of this bug.

Comment 87

[Thomas D. (:thomas8)]

(In reply to Greeneye57 from comment #85)
> There is credit in all sides of this discussion.

Thanks for the appreciation and your contribution of another user perspective.

> However, many of my clients
> would love this feature (including me). A simpler way without drag n drop
> would be to be able to right click on the account and "set as default". Then
> it moves to the top (like some other mail programs eg: the old win live
> mail). By doing this a few times, the accounts would eventually be in the
> order the user wants them to be. Sort of a mailbox Tetris.

Unfortunately, "Mailbox Tetris" doesn't comply with our UX principles as yet. ;-)

> And clumsy (old?) users can't drag n drop anything to the wrong place.

Yes, ux-error-prevention is one reason why allowing dragging of accounts in the folder pane isn't a good idea, and Magnus seems to concur.

Comment 88

[Wayne Mery (:wsmwk)]

Seeing the support requests, I agree it is a popular request. And it must be keyboard accessible. D+D is not.

Comment 89

[Thomas D. (:thomas8)]

PROPOSED IMPLEMENTATION DETAILS

Derived from consensus proposal of "Reorder Accounts" dialogue. Given that d+d on folder pane or Accounts Manager's main account list is not keyboard accessible, error-prone, hard to discover and odd UX with lots of obfuscating subnodes. As hinted by aceman, we could "clone" the existing "Manage identities" dialogue and adapt to our purposes here, which shouldn't be very hard.

a) XUL: Add <menuitem>s and <command>
        - |View > Layout > Reorder Accounts...| menuitem or some such (see comment 74)
        - |Reorder Accounts...| menuitem in accounts context menu
        - consider adding |Reorder Accounts...| menuitem to root folders' context menu if "Unified" view (as account nodes aren't available there)
        - <command id="cmd_reorderAccounts" oncommand="reorderAccountsDialog()">

b) XUL: Implement the "Reorder Accounts" dialogue
        - create reorderAccountsDlg.xul
        - <listbox id="accountsList">
        - <button>s: [Move to Top]
                     [Move Up]
                     [Move Down]
                     [Move to Bottom]

c) JS:  Implement backend functionality
        - function reorderAccountsDialog() to call the "Reorder Accounts" dialogue, something like:
>         window.openDialog("reorderAccounts.xul", "", "chrome,modal,resizable=no,centerscreen", args);
        - create reorderAccountsDlg.js
          - therein: onLoad(): get accounts list (caveat: Chat accounts! see below)
          - function to effect the actual reordering (via prefs, I think; e.g. copy from current 'set as default' routines, or existing addon)

Caveat: Devil in the detail: There are different account types, some of which don't appear in the main folder pane (e.g. Chat accounts). So the Reorder dialog needs to reflect that somehow. Maybe this:

Proposed UI/UX for handling special case of Chat accounts:
- UI: At the top of reorder dialog, add a dropdown:
[Email and News accounts v]
|Email and News accounts  |
|Chat accounts            |
+-------------------------+
- UX: Chose "Email and News accounts" to see their list and reorder them.
      Chose "Chat accounts" to see their list and reorder them.

Questions:
- Any other account types which need special handling?
- Any other UI ideas how to handle these special cases? (I'm fine with my proposal, but I may not be aware of all account types and their special needs)

Aceman (wrt implementation), Richard (wrt UX), does that sound like a plan?

Comment 90

[Thomas D. (:thomas8)]

(In reply to Thomas D. (currently busy elsewhere) from comment #89)
> Proposed UI/UX for handling special case of Chat accounts:
> - UI: At the top of reorder dialog, add a dropdown:
> [Email and News accounts v]
> |Email and News accounts  |
> |Chat accounts            |
> +-------------------------+
> - UX: Chose "Email and News accounts" to see their list and reorder them.
>       Chose "Chat accounts" to see their list and reorder them.

Another possible UI/UX:
- Add an attribute to account types if they appear in the main folder list or not
- For reordering, have a all accounts in a single list
- Add separators for any account types NOT in the main folder list
- Move up/down/top/bottom will only move special accounts within their separators, never beyond.

Mail account 1
Mail account 2
---------------
Chat account 1
Chat account 2

I think this is LESS intuitive than the account type dropdown with dedicated lists.
If we are able to know the account type, it's friendly name, and if it's on the main folder list or not, dynamically filling the dropdown shouldn't be a problem for implementation.

Comment 91

[:aceman]

Yes, for simplicity and avoiding side effects, I proposed some simple dialog that would visually be a clone of Manage identities (and not the folder pane, not the account manager tree). But I am very reluctant to see a patch cloning the code too. We should not make just another block of code managing just another list for reordering some objects.
So that is the main problem why this bug is stalled.

Also, currently TB will not allow you to reorder accounts in every way you want, until something like bug 1359410 is implemented. You can only reorder account inside their types (like within Mail, IM, News, etc). For the list of account types see the function getServerSortOrder() in folderUtils.jsm. And then there is the default account. So even if you make a nice dialog with buttons for moving accounts around, TB will still fight with you about what you can and cannot do :)
But you seem to know about that according to your proposal which already separates the accounts by type. Yes, I can see this would be a difference from the Manage identities dialog, which only has a single list.

Comment 92

[Ulf Zibis]

(In reply to Thomas D. (currently busy elsewhere) from comment #89)
> b) XUL: Implement the "Reorder Accounts" dialogue
>         - create reorderAccountsDlg.xul
>         - <listbox id="accountsList">
>         - <button>s: [Move to Top]
>                      [Move Up]
>                      [Move Down]
>                      [Move to Bottom]

I'm not sure if the [Move to Top] and [Move to Bottom] buttons are needed (and will not work as expected because of the account type constraints), as reordering accounts will be a vary rare action, even much less used as e.g. the reorder functionality for the search engines in Firefox. There is also only [Up] and [Down]. But it may be comfortable to mark multiple accounts to move them at same time, and - very sophisticated - with SHIFT over "type-groups".

I think, we should allow the user any order he wants. Maybe one wants the News-Folders or the Local Folders in top position or group some News account with some Mail account, why not? Even having multiple locale "accounts" to "attach" them e.g. to a IMAP account could be interesting to group things by there content.
To recreate an account type order I suggest an [Group by Type] or [Reset to default Order] button. The type of an account may be indicated in the order list by some icon or a colour.


> Proposed UI/UX for handling special case of Chat accounts:
> - UI: At the top of reorder dialog, add a dropdown:
> [Email and News accounts v]
> |Email and News accounts  |
> |Chat accounts            |
> +-------------------------+
> - UX: Chose "Email and News accounts" to see their list and reorder them.
>       Chose "Chat accounts" to see their list and reorder them.

The more above said, I would not like this pre-selection, additionally because it's something in direction "a jungle of irrelevant options to wade through"

(In reply to :aceman from comment #91)
> Also, currently TB will not allow you to reorder accounts in every way you
> want, until something like bug 1359410 is implemented. You can only reorder
> account inside their types (like within Mail, IM, News, etc).
Good point. First step should be to rely the account order on mail.accountmanager.accounts. With this, advanced users could modify the order by hacking prefs.js and have "beta experience" with reordering accounts, so could report possible problems in advance.

Comment 93

[Ulf Zibis]

2 additional suggestions:
- mark all items of same type in neighbourhood with double-click on order list item
- mark all items of same type in all places with triple-click on order list item

Comment 94

[Richard Marti (:Paenglab)]

(In reply to Thomas D. (currently busy elsewhere) from comment #89)
> PROPOSED IMPLEMENTATION DETAILS
> 
> Derived from consensus proposal of "Reorder Accounts" dialogue. Given that
> d+d on folder pane or Accounts Manager's main account list is not keyboard
> accessible, error-prone, hard to discover and odd UX with lots of
> obfuscating subnodes. As hinted by aceman, we could "clone" the existing
> "Manage identities" dialogue and adapt to our purposes here, which shouldn't
> be very hard.
> 
> a) XUL: Add <menuitem>s and <command>
>         - |View > Layout > Reorder Accounts...| menuitem or some such (see
> comment 74)
>         - |Reorder Accounts...| menuitem in accounts context menu
>         - consider adding |Reorder Accounts...| menuitem to root folders'
> context menu if "Unified" view (as account nodes aren't available there)
>         - <command id="cmd_reorderAccounts"
> oncommand="reorderAccountsDialog()">

This is a seldom used feature and shouldn't be added to the main menu. In folder pane's account context menu and in the account manager's menulist on the bottom is enough.
 
> Caveat: Devil in the detail: There are different account types, some of
> which don't appear in the main folder pane (e.g. Chat accounts). So the
> Reorder dialog needs to reflect that somehow. Maybe this:
> 
> Proposed UI/UX for handling special case of Chat accounts:
> - UI: At the top of reorder dialog, add a dropdown:
> [Email and News accounts v]
> |Email and News accounts  |
> |Chat accounts            |
> +-------------------------+
> - UX: Chose "Email and News accounts" to see their list and reorder them.
>       Chose "Chat accounts" to see their list and reorder them.

The dropdown is too much. A sentence on the top of the dialog which says, that the accounts can only be moved inside their type would be okay. Separating them like you proposed here is simpler:

(In reply to Thomas D. (currently busy elsewhere) from comment #90)
> Another possible UI/UX:
> - Add an attribute to account types if they appear in the main folder list
> or not
> - For reordering, have a all accounts in a single list
> - Add separators for any account types NOT in the main folder list
> - Move up/down/top/bottom will only move special accounts within their
> separators, never beyond.
> 
> Mail account 1
> Mail account 2
> ---------------
> Chat account 1
> Chat account 2
> 
> I think this is LESS intuitive than the account type dropdown with dedicated
> lists.

How many accounts does an average user have? To show, after selecting the dropdown, two or three accounts in the listbox looks not so appealing. But showing all accounts in their order they have in the folder pane like your second proposal is more intuitive.

Maybe you look how the extension "Manually Sort Folder" has done this with two listboxes and the "LocalFolder" between them. This extension has also a menulist to select the default account which is at the top of the list (with this also a news account can be the first in the list).

Comment 95

[Ulf Zibis]

(In reply to Richard Marti (:Paenglab) from comment #94)
> This is a seldom used feature and shouldn't be added to the main menu. In
> folder pane's account context menu and in the account manager's menulist on
> the bottom is enough.
I don't think the context menu of some folder is the right place, because IMHO the actions in a context menu of a folder should relate to exactly that folder. The order of all folders is a more general setting.

> How many accounts does an average user have?
This is a setting for advanced users and should not be measurerd against simple users.

Comment 96

[Thomas D. (:thomas8)]

This comment discusses details, there's agreement on most of the basics already.

(In reply to Ulf Zibis from comment #93)
> 2 additional suggestions:
> - mark all items of same type in neighbourhood with double-click on order
> list item
> - mark all items of same type in all places with triple-click on order list
> item

These are nice ideas for a followup bug, so please open one so as to avoid more discussion here.
- somewhat non-standard, hence: ux-discovery? But I'm not against it, my position is that if it doesn't hurt anyone or hinder us in the future, why not do something useful instead of doing nothing?
- depends on multiple selection, which is much harder to implement than single selection; so we might choose the path of least resistance first to get this landed. Note that multiple selection automatically also allows disjunct multiple selection (item 1 and item 3) which always needs some special handling in code. I've just done this for bug 663695 so I know what I'm talking about.

(In reply to Richard Marti (:Paenglab) from comment #94)
> This is a seldom used feature and shouldn't be added to the main menu.

With that argument, we could probably clear out 50% of the main menus... I don't think that's how it works. I understand that in the traditional understanding of main menus, which TB is retaining for good reason, *all* functionality must be accessible from the main menus somehow. Hence there are loads of rarely used features in the menus, and what is rarely used also depends very much on individual users. If you ask Michelle (reporter of Bug 1435194), she'll probably add a new account every day and might want to sort that into existing accounts. Thunderbird exists because it offers a lot of features which may only be used by some low percentage of users (say 5%), but those features will be 100% essential for them, so if we design only for what everybody needs, we'd probably have to go back to the bare bones of Google's webmail.

> In folder pane's account context menu and in the account manager's menulist on
> the bottom is enough.

So we agree on having this in the account context menu. Aceman and I already explained why having it in the account managers "Account Actions" menu, while desirable, might pose significant problems of UX and/or implementation.

(In reply to Ulf Zibis from comment #95)
> I don't think the context menu of some folder is the right place, because
> IMHO the actions in a context menu of a folder should relate to exactly that
> folder. The order of all folders is a more general setting.

Yes and no.
Yes, I agree with Ulf that we should have a main menu access for this, and I have explored our options in comment 74, with |View > Layout > Reorder Accounts...| being my current favorite. There are only a handful of rarely used options in that menu, so adding one more doesn't hurt in any way, but will be tremendously helpful for ux-discovery, also in the case where accounts are covert as in "Unified folders" view - do you really want to add a "Reorder accounts" context menu on all inboxes in unified view? If not, how do users of "Unified folders" access "Reorder accounts..."? Main menu covers all views so it's a good way out, always available. This is also about access: Keyboard-only access to some specific context menu can be pretty clumsy (pls try it) compared to the ease and systematic approach of main menus. Plus, not everybody has a windows menu key, and not everybody knows about Shift+F10.
Notwithstanding, we definitely want "Reorder Accounts" on the context menu of accounts in "All folders" view, as it relates directly to the position of that account in the list, so that's a very intuitive place to start from, but not always available depending on folder views.

> > How many accounts does an average user have?
> This is a setting for advanced users and should not be measurerd against
> simple users.

The number of accounts is pretty irrelevant for the general need to reorder them (although more accounts will make that need more likely). I can have two which I just happened to add in the wrong order. Btw, I do NOT share the notion that the default account must always be on top, and I hope that account reordering will allow to position the default account anywhere in the list. E.g., my first account is my bugzilla account because I'm checking that most frequently; whereas my default account should be my private account which comes second in the list, as defaulting to my bugzilla account isn't helpful.

Comment 97

[:aceman]

(In reply to Thomas D. (currently busy elsewhere) from comment #96)
> So we agree on having this in the account context menu. Aceman and I already
> explained why having it in the account managers "Account Actions" menu,
> while desirable, might pose significant problems of UX and/or implementation.

Actually I am for this being in the Account Actions menu, but the Account manager window must be closed and the Reordering dialog opened when this action is chosen.

Comment 98

[Thomas D. (:thomas8)]

(In reply to :aceman from comment #97)
> (In reply to Thomas D. (currently busy elsewhere) from comment #96)
> > So we agree on having this in the account context menu. Aceman and I already
> > explained why having it in the account managers "Account Actions" menu,
> > while desirable, might pose significant problems of UX and/or implementation.
> 
> Actually I am for this being in the Account Actions menu, but the Account
> manager window must be closed and the Reordering dialog opened when this
> action is chosen.

Pls read my comment 73 which points out UX/implementation problems with that approach.
Imho, as long as account settings is a dialogue with a central OK/Cancel button, we can neither just save users changes without explicit confirmation, nor discard them. How do you handle "unsaved changes" anywhere in AM before closinng it? Plus this is pretty deep in settings hence hard to find, and I also think it feels odd when clicking some menu in AM makes AM suddenly disappear from underneath...

Comment 99

[Ulf Zibis]

(In reply to Thomas D. (currently busy elsewhere) from comment #96)

> These are nice ideas for a followup bug, so please open one so as to avoid
> more discussion here.
Agreed! What you think about a [Reset to default Order] button or similar?

> Btw, I do NOT share the
> notion that the default account must always be on top, and I hope that
> account reordering will allow to position the default account anywhere in
> the list. E.g., my first account is my bugzilla account because I'm checking
> that most frequently; whereas my default account should be my private
> account which comes second in the list, as defaulting to my bugzilla account
> isn't helpful.

+1

Comment 100

[:aceman]

(In reply to Thomas D. (currently busy elsewhere) from comment #98)
> Pls read my comment 73 which points out UX/implementation problems with that
> approach.
> Imho, as long as account settings is a dialogue with a central OK/Cancel
> button, we can neither just save users changes without explicit
> confirmation, nor discard them. How do you handle "unsaved changes" anywhere
> in AM before closing it?

Yes, that is the problem. You could throw up a dialog saying all changes will be lost.
You could make the reordering inside of the account manager window directly, 
Also note the plan is to put the account manager inside a tab, together with prefs (like Firefox has it), bug 1096006.

> Plus this is pretty deep in settings hence hard to
> find, and I also think it feels odd when clicking some menu in AM makes AM
> suddenly disappear from underneath...

This being deep isn't such a problem, this is supposed to be a rarely needed action. You also have identity reordering deep in the account manager, inside a panel.

Comment 102

[Solway]

can we please have something added to be able to change the order of the accounts.
i shouldnt have to rely on addons to do such a simple thing

https://addons.thunderbird.net/en-US/thunderbird/addon/manually-sort-folders/
this addon is now out of date for version 68 and up

Comment 103

[Jozsi]

Thunderbird is getting old and featureless...
It's annoying that users don't have the possibility to rearrange the accounts. Thunderbird forgets every month the gmail authorization so the account must be deleted and added again. This procedure always changes the order of accounts.
Counterproductive.
Users want to rearrange the accounts with a simple drag-and-drop.

Comment 104

[markus.oberhauser]

All this back and forth of ideas and yet this simple task has not been implemented since 16 (!) years.... why not exactly?

Couldn't be a simple approach of [- +] or [˄ ˅] icons beside the account panel in AM possible, yet a shortcut like ALT+"+" and ATL+"-", icons accessible with tabulator?
Please, can we have this in 2020?

Comment 105

[Andre Klapper]

Welcome Markus. To answer the "Why not exactly": Because you have not implemented this "simple task" by providing a patch. Furthermore, age of a ticket is entirely irrelevant because anyone can propose any random idea. That does not mean that someone decides to work on it though.
It is up to you to provide patches if you want to see something fixed. See https://www.thunderbird.net/en-US/get-involved/#development

Comment 106

[Magnus Melin [:mkmelin]]

After all discussion, I think we should as a first step add dnd in the account manager. Maybe with keyboard shortcuts like comment 41.
Not too keen on any further dialogs, and also the whole Account Actions button, which is disconnected from the account...

We'll want to extract backend code from the patch in bug 1359410.

Comment 107

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-0.patch

In the account manager, doing "option + up" and "option + down" will change the order of the accounts and that will be reflected in the Message Pane WIndow. If the user wants default order, "mail.accountmanager.accounts.ordered" can be set to false. Right now, we don't have any preferences but we can have a checkbox in the Preferences for Default Thunderbird Accounts order or Manual Accounts order which will trigger "mail.accountmanager.accounts.ordered".

Comment 108

[Magnus Melin [:mkmelin]]

Comment on attachment 9161415 [details] [diff] [review]
Bug-244347_change-account-order-0.patch

Review of attachment 9161415 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/base/prefs/content/AccountManager.js
@@ +1,1 @@
> +/* eslint-disable prettier/prettier */

debug leftover?

@@ +1682,5 @@
>  }
>  
> +function accountReordered() {
> +  let accountItems = document.getElementById("account-tree-children")
> +    .childNodes;

children

@@ +1692,5 @@
> +  }
> +
> +  if (MailServices.accounts.accounts.length != accountIds.length) {
> +    return;
> +  }

what is this for?

@@ +1765,4 @@
>        mainTree.lastChild.remove();
>      }
>  
> +    if (Services.prefs.getBoolPref("mail.accountmanager.accounts.ordered")) {

do an early return instead

@@ +1779,5 @@
> +              "account-tree-children"
> +            );
> +            let treeItem = accountTreeNode.querySelector(
> +              "#" + getCurrentAccount().key
> +            );

document.getElementById(getCurrentAccount().key)

@@ +1801,5 @@
> +              "account-tree-children"
> +            );
> +            let treeItem = accountTreeNode.querySelector(
> +              "#" + getCurrentAccount().key
> +            );

here too

::: mailnews/base/public/nsIMsgAccountManager.idl
@@ +245,5 @@
> +
> +  /**
> +   * Sets new order of accounts.
> +   *
> +   * @param accounts - Account keys in the new preferred order.

please change it to say accountKeys

::: mailnews/base/src/nsMsgAccountManager.cpp
@@ +3282,5 @@
> +  // just in a different order.
> +  if (newAccounts.Length() != m_accounts.Length())
> +    return NS_ERROR_INVALID_ARG;
> +
> +  for (uint32_t index = 0; index < m_accounts.Length(); index++) {

would just use "i"

@@ +3292,5 @@
> +
> +  // In-place swap the elements in m_accounts to the order defined in newAccounts.
> +  for (uint32_t indexN = 0; indexN < newAccounts.Length(); indexN++) {
> +    nsCString newKey = newAccounts[indexN];
> +    for (uint32_t indexO = indexN; indexO < m_accounts.Length(); indexO++) {

i and j for the index variable names please

::: mailnews/mailnews.js
@@ +472,5 @@
>  pref("mail.accountmanager.accounts", "");
>  
> +// If false, we ignore the order of accounts defined in mail.accountmanager.accounts
> +// and nicely group accounts by type in the UI.
> +pref("mail.accountmanager.accounts.ordered", true);

I think the idea would be to not have this pref but instead do a one time setup of the pref for how the account are ordered.
(Also need need to remember to update that pref when accounts are added/removed).

Comment 109

[Khushil Mistry [:khushil324]]

(In reply to Magnus Melin [:mkmelin] from comment #108)

I think the idea would be to not have this pref but instead do a one time
setup of the pref for how the account are ordered.
(Also need need to remember to update that pref when accounts are
added/removed).

Should we use "mail.accountmanager.accounts" pref for this? It already contains the list of account keys in the order of their creation date/time.
So initially, we have 1 Email account with "account1" and the local folder account with "account2". If we add another account which will be "account3". We are sorting in a way that the local folder with "account2" will come last always, so the order will be in the view: "account1, account3, account2". And the pref "mail.accountmanager.accounts" will be "account1, account2, account3". So how do you want to order it by default and when account is added and removed?

And we also need to indicate users how they can change the order. What should we do for that?

Comment 110

[pfc]

Honestly, allowing that even by changing some entry in advanced prefs and putting account numbers like 1,5,2,4,3,6 instead of 1,2,3,4,5,6 would be good. There was an add-on for that, mentioned in this thread (Manually sort folders) and it worked great. If you'd like to check it out but won't be able to find it, I can upload it somewhere and share a link. Implementing something like that into the app would be the most elegant solution but just making it possible to change the order in any way is infinitely better than having no option whatsoever to change that.

Comment 111

[Geert-Emo]

(In reply to pufcio from comment #110)

Honestly, allowing that even by changing some entry in advanced prefs and putting account numbers like 1,5,2,4,3,6 instead of 1,2,3,4,5,6 would be good. There was an add-on for that, mentioned in this thread (Manually sort folders) and it worked great. If you'd like to check it out but won't be able to find it, I can upload it somewhere and share a link. Implementing something like that into the app would be the most elegant solution but just making it possible to change the order in any way is infinitely better than having no option whatsoever to change that.

I agree with pufcio.

Comment 112

[Ulf Zibis]

I too agree with pufcio.
If I understand right, he first wants to have bug 1359410 fixed promptly. Don't wait until we have a GUI according this bug.

Comment 113

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-1.patch

Comment 114

[:aceman]

Comment on attachment 9167536 [details] [diff] [review]
Bug-244347_change-account-order-1.patch

Do I understand this correctly, that it is dropping the default sorting of accounts and the order in mail.accountmanager.accounts is always honored, without any pref? This wasn't said in the discussion and also must not be the initial behaviour. Please be sure not to regress bug 749200 again.

Comment 115

[Magnus Melin [:mkmelin]]

Comment on attachment 9167536 [details] [diff] [review]
Bug-244347_change-account-order-1.patch

Review of attachment 9167536 [details] [diff] [review]:
-----------------------------------------------------------------

I now get Local Folders on top. I think you need to do a one time migration of the current "normal order". 

Currently set as default moves an account to top. I guess it should continue to do that (but you'd be able to reorder it after that)

::: mailnews/base/prefs/content/AccountManager.js
@@ +1776,5 @@
>      }
>  
> +    let accountTree = document.getElementById("accounttree");
> +    accountTree.addEventListener("dragover", event => {
> +      event.preventDefault();

what is this for?

Maybe it should only show drop allowed where it can, i.e. not the account manager sections (server settings, copies & folders etc.)

@@ +1787,5 @@
> +        return;
> +      }
> +      let dropId = null;
> +      let length = 0;
> +      for (let childElement of mainTree.childNodes) {

children I think

@@ +1788,5 @@
> +      }
> +      let dropId = null;
> +      let length = 0;
> +      for (let childElement of mainTree.childNodes) {
> +        length = length + childElement.querySelectorAll("treerow").length;

length +=

Comment 116

[Khushil Mistry [:khushil324]]

(In reply to Magnus Melin [:mkmelin] from comment #115)

what is this for?

Maybe it should only show drop allowed where it can, i.e. not the account
manager sections (server settings, copies & folders etc.)

By default, data/elements cannot be dropped in other elements. To allow a drop, we must prevent the default handling of the element.

Comment 117

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-2.patch

Comment 118

[:aceman]

(In reply to Magnus Melin [:mkmelin] from comment #115)

I now get Local Folders on top. I think you need to do a one time migration
of the current "normal order".

Really guys, what are you trying to achieve here? The order of accounts must not change after the patch, only there must appear a possibility to reorder them manually. But users that don't touch that, should get the automatic ordering. Also remember new accounts get appended to the end of the list, but it will be wrong if they always appear on the bottom of the UI (for some account types it will be wrong).
So why are you undoing bug 749200 (automatic ordering by account type)? Where is the UI and UX discussion of this?

Comment 119

[Magnus Melin [:mkmelin]]

That's why I requested that the current order is migrated into the normal ordering - that way there is not really a change in UI/UX on that part. Except that if you do decide to rearrange accounts you get to use an ordering of your choice.

Re adding accounts, I think new accounts should not go below Local Folders, but other than that there's little sense in putting news accounts below other accounts. If I add it second, why wouldn't I want it second? (And in the new world, I'd just move it up/down anyway.)

Comment 120

[Anje]

protz mentions:
For sorting accounts: Thunderbird developers have committed to allowing sorting accounts (not folders) directly within Thunderbird, without the need for an addon. This will reuse some code that I (and others) wrote, which allows arbitrary reordering of accounts, meaning that this will be fixed automatically when the next version of Thunderbird comes out.

Manually Sort Folders had a good solution for reordering accounts in the Folder Pane.
Can I ask...are you making use of this code so kindly proffered?

Comment 121

[Magnus Melin [:mkmelin]]

Comment on attachment 9167732 [details] [diff] [review]
Bug-244347_change-account-order-2.patch

Review of attachment 9167732 [details] [diff] [review]:
-----------------------------------------------------------------

Can you please add some automated test for this

::: mailnews/base/prefs/content/AccountManager.js
@@ +878,5 @@
>    MailServices.accounts.defaultAccount = currentAccount;
>    markDefaultServer(currentAccount, previousDefault);
> +  let accountList = allAccountsSorted(true);
> +  let accountKeyList = accountList.map(account => account.key);
> +  let index = accountKeyList.indexOf(currentAccount.key);

you can just filter after map, so 

 accountList.map(account => account.key).filter(key => key != currentAccount.key);

@@ +882,5 @@
> +  let index = accountKeyList.indexOf(currentAccount.key);
> +  if (index > -1) {
> +    accountKeyList.splice(index, 1);
> +  }
> +  let newAccountKeyList = [currentAccount.key].concat(accountKeyList);

accountKeyList.unshift(currentAccount.key);

would be nicer

::: mailnews/base/util/folderUtils.jsm
@@ +175,5 @@
> +    Services.prefs.setBoolPref("mail.accountmanager.accounts.ordered", true);
> +    accountList.sort(compareAccounts);
> +    let accountKeyList = accountList.map(account => account.key);
> +    MailServices.accounts.reorderAccounts(accountKeyList);
> +  }

I'd do migration around https://searchfox.org/comm-central/source/mail/base/modules/MailMigrator.jsm#116

Comment 122

[:aceman]

(In reply to Magnus Melin [:mkmelin] from comment #119)

Re adding accounts, I think new accounts should not go below Local Folders, but other than that there's little sense in putting news accounts below other accounts.

So with the patch, everything will go below Local Folders, as Local Folders is created first (or second).

If I add it second, why wouldn't I want it second? (And in the new world, I'd just move it up/down anyway.)

Because in bug 749200 it was decided we do not want such mess? Yes, it will now be possible to manually reorder the accounts.

Comment 123

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-3.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=517e6580d3bc8df2e8c5b38c21a9e10e611c6500

Comment 125

[Magnus Melin [:mkmelin]]

Comment on attachment 9170293 [details] [diff] [review]
Bug-244347_change-account-order-3.patch

Review of attachment 9170293 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/modules/MailMigrator.jsm
@@ +520,5 @@
> +        ) {
> +          Services.prefs.setBoolPref(
> +            "mail.accountmanager.accounts.ordered",
> +            true
> +          );

doing anything with mail.accountmanager.accounts.ordered seems pointless. It's not used anywhere and will always just be set to true going forwareds.

@@ +523,5 @@
> +            true
> +          );
> +          let accountList = MailServices.accounts.accounts.filter(
> +            a => a.incomingServer && a.incomingServer.type != "im"
> +          );

why not the im accounts? Even if not shown in the 3 pane, one should be able to reorder them in the account manager and have that stick. 

Oh, I see we also have an messenger.accounts pref for IM, and that can already be reordered in the im account win.
We should probably unify this here as well so that we migrate that away to using the main list.

::: mail/test/browser/account/browser_accountOrder.js
@@ +76,5 @@
> +
> +  let prevAccountList = MailServices.accounts.accounts.map(
> +    account => account.key
> +  );
> +

Please add comments for that this is moving the account up

@@ +82,5 @@
> +  let curAccountList = MailServices.accounts.accounts.map(
> +    account => account.key
> +  );
> +  Assert.notEqual(curAccountList.join(), prevAccountList.join());
> +  EventUtils.synthesizeKey("VK_DOWN", { altKey: true });

.. and down

Comment 126

[Khushil Mistry [:khushil324]]

(In reply to Magnus Melin [:mkmelin] from comment #125)

why not the im accounts? Even if not shown in the 3 pane, one should be able
to reorder them in the account manager and have that stick.

Oh, I see we also have an messenger.accounts pref for IM, and that can
already be reordered in the im account win.
We should probably unify this here as well so that we migrate that away to
using the main list.

Keys in "messenger.accounts" and keys from "MailServices.accounts.accounts" are different. "messenger.accounts" value is "account1,account2"(in increasing order of all the chat accounts) in this order whereas in "MailServices.accounts.accounts", im accounts keys can be "account3", "account4"(in the increasing order of all the accounts). So this is not possible.

Comment 127

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-4.patch

Comment 128

[Magnus Melin [:mkmelin]]

Comment on attachment 9172362 [details] [diff] [review]
Bug-244347_change-account-order-4.patch

Review of attachment 9172362 [details] [diff] [review]:
-----------------------------------------------------------------

I think when adding new accounts, if Local Folders is the last account, the new account should be added *before* Local Folders. 
If local folders is not last, then the order has been customized and we'd just add it last.

::: mail/test/browser/account/browser_tree.js
@@ +200,4 @@
>    let accountRow = accountTree.view.selection.currentIndex;
>    Assert.equal(
>      accountTree.view.getItemAtIndex(accountRow)._account,
> +    accountList[accountIndex - 1]

Not sure why this changed, but the comment above here is wrong then

::: mailnews/base/prefs/content/AccountManager.js
@@ +1793,5 @@
> +    accountTree.addEventListener("dragover", event => {
> +      event.preventDefault();
> +    });
> +
> +    accountTree.addEventListener("drop", event => {

The UX of trying to drag something to first position is a bit weird (can't be done). I think usually if you drag something and drop it on the first itme, the expectation is the dropped item moves to top.

There is also no drop indicator. Can we add one?

@@ +1808,5 @@
> +          dropId = childElement.getAttribute("id");
> +          break;
> +        }
> +      }
> +      if (dragId != dropId && dropId) {

&& dropId looks like it's not needed

@@ +1809,5 @@
> +          break;
> +        }
> +      }
> +      if (dragId != dropId && dropId) {
> +        let dragitem = mainTree.querySelector("#" + dragId);

getElementById fore these please

@@ +1842,5 @@
> +          event.code == "ArrowUp" &&
> +          event.altKey &&
> +          getCurrentAccount()
> +        ) {
> +          let treeItem = mainTree.querySelector("#" + getCurrentAccount().key);

getELementById

Comment 129

[Magnus Melin [:mkmelin]]

Actually, since local folders isn't listed in the account manager it can't be anywhere else except last. We should just always add new accounts before it, never after.

Comment 130

[Thomas D. (:thomas8)]

(In reply to Magnus Melin [:mkmelin] from comment #129)

Actually, since local folders isn't listed in the account manager it can't be anywhere else except last. We should just always add new accounts before it, never after.

From an UX perspective, not being able to move local folders freely might look like an unwarranted limitation (ux-implementation-level).

Comment 131

[Mike]

(In reply to Thomas D. (:thomas8) from comment #130)

(In reply to Magnus Melin [:mkmelin] from comment #129)

Actually, since local folders isn't listed in the account manager it can't be anywhere else except last. We should just always add new accounts before it, never after.

From an UX perspective, not being able to move local folders freely might look like an unwarranted limitation (ux-implementation-level).

This can be handled with a simple dialog box popup stating that Local Folders are always at the end due to a limitation forced by the structure of the application or program. The dialog box options could be as simple as {check box}Do Not show Again and [Next}

Comment 132

[Magnus Melin [:mkmelin]]

If we do dnd of account in the folder pane later we could allow it to be moved. But atm since it's not in the account manager, we should just keep it where it is for now. There is also the complication that it's (I think, unfortunately still) possible to set up pop accounts that are using Local Folders and not their own account node (the global inbox mode).

Comment 133

[Khushil Mistry [:khushil324]]

(In reply to Magnus Melin [:mkmelin] from comment #129)

Actually, since local folders isn't listed in the account manager it can't be anywhere else except last. We should just always add new accounts before it, never after.

"Local Folders" is listed in the the account manager for me. Can you explian this a bit more?

Comment 134

[WTBart]

still don't understand. When Mozila owned Thunderbird I could move accouns to any order I wanted. Now I can't.

Comment 135

[Jonathan Protzenko [:protz]]

FYI, the ability to move Local Folders on top was offered by my addon, until some Thunderbird changes made it impossible, at which points users started complaining pretty loudly. So I suspect it's a common feature request to be able to move all accounts freely (which was the intention behind my previous patch).

Comment 136

[Magnus Melin [:mkmelin]]

(In reply to Khushil Mistry [:khushil324] from comment #133)

"Local Folders" is listed in the the account manager for me. Can you explian this a bit more?

Sorry, ignore me, I forgot they are :)
I still think that if Local Folders are last (== no customize happened), new accounts should be inserted before them in the list.

Comment 137

[Khushil Mistry [:khushil324]]

(In reply to Magnus Melin [:mkmelin] from comment #128)

I think when adding new accounts, if Local Folders is the last account, the
new account should be added before Local Folders.
If local folders is not last, then the order has been customized and we'd
just add it last.

Currently, we put RSS and News Feed after local folders. So what to do in that case? I feel that we should not have any special cases for local folders and treat them like the normal account.(In reply to Magnus Melin [:mkmelin] from comment #128)

The UX of trying to drag something to first position is a bit weird (can't
be done). I think usually if you drag something and drop it on the first
itme, the expectation is the dropped item moves to top.

There is also no drop indicator. Can we add one?

I will try for a drop-indicator but it seems a little difficult. Let me check again.

getElementById fore these please

It's showing an error when we try to use getElementById: JavaScript error: chrome://messenger/content/AccountManager.js, line 1815: TypeError: mainTree.getElementById is not a function

Comment 138

[Magnus Melin [:mkmelin]]

(In reply to Khushil Mistry [:khushil324] from comment #137)

Currently, we put RSS and News Feed after local folders. So what to do in that case? I feel that we should not have any special cases for local folders and treat them like the normal account.

I'd just keep Local Folder last, until a user modification moves it elsewhere.
I think what we want to avoid here is that Local Folders ends up as the first account for all new profiles. You only have local folders initially, then you add more, which would go below, which is not nice.

Aleca, wdyt?

Comment 139

[Alessandro Castellani [:aleca]]

I agree with having Local Folder always at last position unless a user changes it.
I think we can safely assume that, as a default, users are more interest in their remote accounts than the automatically created local folder.
Having the ability to reorder it is good, but by default it should always be pushed at the last position.

Comment 140

[Alessandro Castellani [:aleca]]

Regarding the drop indicator, we should use the tab-drag-indicator.svg we currently use for tab and copied from Firefox.
You can easily rotate it with CSS. https://searchfox.org/comm-central/search?q=tab-drag-indicator.svg&path=
I don't think it's possible to use :after/before pseudo elements in the richlist XUL element, which is a shame as that would have been very easy to handle.

Comment 141

[Ulf Zibis]

What's about positioning a new created account always directly above Local Folders regardless it's position?
If the user wants the new created account at another position, he can move it manually.

(In reply to Alessandro Castellani (:aleca) from comment #140)

I don't think it's possible to use :after/before pseudo elements in the richlist XUL element, which is a shame as that would have been very easy to handle.

In Firefox at the search engine settings, the is the possibility, to move an element to the top position. Maybe this code can be used to move the folders in TB.
Also with bookmarks in FF I see this functionality.

Comment 142

[Anje]

Idea: People maybe used to the import function in address book.
The import address book text file uses a three column table structure with Move up and Move down buttons.
If far left column (checkbox) could be used as a means of setting of default account. Only one checkbox can be selected.
If inner left column was a numbered position in list, then right column could be list of accounts names.
Move up/Move down buttons move selected mail account (third/right column) to match position number.

Maybe additional buttons 'Move to Top' & 'Move to bottom' to save some clicking.

I'm not saying life is that simple, but............. :)

Comment 143

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-5.patch

There is a problem with drop indicator, we are not able to apply styling to the tree element. So I have skipped that part right now.

Comment 144

[Richard Marti (:Paenglab)]

Maybe https://searchfox.org/mozilla-central/rev/3d53187b90605ccef321c9cadbba762ad06a6381/toolkit/themes/shared/tree.inc.css#100-112 helps you to style them.

Comment 145

[Khushil Mistry [:khushil324]]

Great, thanks for this. Let me try!

Comment 146

[bugzilla-fwd]

I would just like to add support for protz' code / the ability to change the order of accounts to be implemented (particularly moving Local folders to the top of the accounts list) in whatever is the simplest fashion - add the ability for users to manually change and leave the rest of the ordering as is if that can be done. I can see there is alot of debate about various options, but we just want to see the ability to sort implemented and further tweaks to be applied later if needed.

There is a significant (200k+) userbase of protz's manually sort folder add-on, which it no longer functions for users running a current or near-current version of TB. This is significantly causing angst and noise in a large portion of the TB community. Thanks

Comment 147

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-6.patch

Comment 148

[Magnus Melin [:mkmelin]]

Comment on attachment 9182864 [details] [diff] [review]
Bug-244347_change-account-order-6.patch

Review of attachment 9182864 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/test/browser/account/browser_accountOrder.js
@@ +21,5 @@
> +);
> +
> +var { mc } = ChromeUtils.import(
> +  "resource://testing-common/mozmill/FolderDisplayHelpers.jsm"
> +);

unused

::: mailnews/base/src/folderUtils.jsm
@@ +176,5 @@
> +  if (
> +    Services.prefs.getBoolPref(
> +      "mail.accountmanager.accounts.localfolderlast",
> +      true
> +    )

I don't understand the purpose of this pref. 
If local folders is last, nothing to do. If that account is not last, it was moved and order should be honoured. The migration should take care that the list of accounts is what is should be.

I believe the incominserver.sortOrder attribute can (and should be) be removed
https://searchfox.org/comm-central/rev/25df033ac98c8cb346a8e0c338d1e7647ca23bfb/mailnews/base/public/nsIMsgIncomingServer.idl#537

Comment 149

[Khushil Mistry [:khushil324]]

(In reply to Magnus Melin [:mkmelin] from comment #148)

I don't understand the purpose of this pref.
If local folders is last, nothing to do. If that account is not last, it was
moved and order should be honoured. The migration should take care that the
list of accounts is what is should be.

When we add a new account, it gets appended in MailServices.accounts.accounts list. So now, "local folders" will not be last on the list. allAccountsSorted function is the util function that returns the required account order. So this pref is set up in the account setting window when the user changes the order and allAccountsSorted util function will use this pref to return the required account order despite "local folders" is not last in the MailServices.accounts.accounts list.
We can do another thing: when we append the account in the MailServices.accounts.accounts, check at that time only. But I am not sure how much complexity it will add and how much c++ work will it require.
Any thoughts?

Comment 150

[Khushil Mistry [:khushil324]]

(In reply to Magnus Melin [:mkmelin] from comment #148)

I believe the incominserver.sortOrder attribute can (and should be) be
removed
https://searchfox.org/comm-central/rev/
25df033ac98c8cb346a8e0c338d1e7647ca23bfb/mailnews/base/public/
nsIMsgIncomingServer.idl#537

Let's do it in the follow-up bug. It will add more complexity here.

Comment 151

[Khushil Mistry [:khushil324]]

We can do something here related to my second suggestion: https://searchfox.org/comm-central/source/mail/components/accountcreation/content/createInBackend.js#21

Comment 152

[Ulf Zibis]

(In reply to Khushil Mistry [:khushil324] from comment #149)

When we add a new account, it gets appended in MailServices.accounts.accounts list. So now, "local folders" will not be last on the list. allAccountsSorted function is the util function that returns the required account order. So this pref is set up in the account setting window when the user changes the order and allAccountsSorted util function will use this pref to return the required account order despite "local folders" is not last in the MailServices.accounts.accounts list.

Wouldn't the suggestion from comment 141 solve the problem?

Comment 153

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-7.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=2af3da7e9d190b9774fe8549ee3e7d1491c4f7f9

I have updated the patch and removed the usage of extra pref.

Comment 154

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-8.patch

Comment 155

[Magnus Melin [:mkmelin]]

Looks to me such checks should be around here: https://searchfox.org/comm-central/rev/25df033ac98c8cb346a8e0c338d1e7647ca23bfb/mailnews/base/src/nsMsgAccountManager.cpp#1525-1545
Something like

  nsCString localFoldersAccountKey;
  nsCOMPtr<nsIMsgIncomingServer> localFoldersServer;
  rv = GetLocalFoldersServer(getter_AddRefs(localFoldersServer));
  if (NS_SUCCEEDED(rv)) {
    for (auto account : m_accounts) {
       nsCOMPtr<nsIMsgIncomingServer> server;
       rv = account->GetIncomingServer(getter_AddRefs(server));
       if (NS_SUCCEEDED(rv) && server == localFoldersServer) {
         account->GetKey(localFoldersAccountKey);
         break;
       }
    }
  }

Then, if local is the only account, or last, insert the new account before it. Otherwise append the new account last.

Comment 156

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-9.patch

Comment 157

[Magnus Melin [:mkmelin]]

Comment on attachment 9189242 [details] [diff] [review]
Bug-244347_change-account-order-9.patch

Review of attachment 9189242 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/test/browser/account/browser_accountOrder.js
@@ +80,5 @@
> +    account => account.key
> +  );
> +  Assert.notEqual(curAccountList.join(), prevAccountList.join());
> +  // Moving the account down, back to the starting position.
> +  EventUtils.synthesizeKey("VK_DOWN", { altKey: true });

I'd think these require some time to make sure the event was actually acted upon, like
await new Promise(resolve => setTimeout(resolve));

Same for VK_UP above

::: mailnews/base/src/nsMsgAccountManager.cpp
@@ +1550,5 @@
> +  }
> +
> +  if (!localFoldersAccountKey.IsEmpty() && !lastFoldersAccountKey.IsEmpty() && lastFoldersAccountKey == localFoldersAccountKey) {
> +    m_accounts.InsertElementAt(m_accounts.Length() - 1, account);
> +    mAccountKeyList.ReplaceSubstring(localFoldersAccountKey, lastFoldersAccountKey);

This could go wrong, e.g. for key2, key22 would get replaced.
It's probably best to instead look through the accounts again after the modifications, and create the mAccountKeyList from the list of values.

Comment 158

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-10.patch

Comment 159

[Khushil Mistry [:khushil324]]

(In reply to Magnus Melin [:mkmelin] from comment #157)

I'd think these require some time to make sure the event was actually acted
upon, like
await new Promise(resolve => setTimeout(resolve));

Same for VK_UP above

Using mc.sleep(0) as we are using it with EventUtils.synthesizeKey at various places.

Comment 160

[Alessandro Castellani [:aleca]]

Using mc.sleep(0) as we are using it with EventUtils.synthesizeKey at various places.

Quick fly by suggestion.
I did implement some mc.sleep(0) in the past but they ended up biting me after a while.
It's better to not set an arbitrary timeout since we open up to the possibility of intermittent failures if something runs slightly slower, and that happens more often than none especially when all the tests from the same folder are running.

I suggest to use waitForEvent or waitForCondition, if applicable to this situation, so we don't have to worry about timings.

Comment 161

[L A Walsh]

(In reply to Magnus Melin [:mkmelin] from comment #157)

::: mail/test/browser/account/browser_accountOrder.js

• account => account.key
• Assert.notEqual(curAccountList.join(), prevAccountList.join());
• // Moving the account down, back to the starting position.
• EventUtils.synthesizeKey("VK_DOWN", { altKey: true });

I'd think these require some time to make sure the event was actually acted
upon, like await new Promise(resolve => setTimeout(resolve));
Same for VK_UP above

---

That would be odd, since synthesize key seems for moving through a data table and wouldn't involve an actual key
being pressed. Even if it does generate output, it's intermediate output that the user wouldn't usually be
interested in.

• if (!localFoldersAccountKey.IsEmpty() && !lastFoldersAccountKey.IsEmpty() && lastFoldersAccountKey == localFoldersAccountKey) {
• m_accounts.InsertElementAt(m_accounts.Length() - 1, account);
• mAccountKeyList.ReplaceSubstring(localFoldersAccountKey, lastFoldersAccountKey);

This could go wrong, e.g. for key2, key22 would get replaced.
It's probably best to instead look through the accounts again after the
modifications, and create the mAccountKeyList from the list of values.

---

It's possible. It depends on how keys are delimited and whether or not partial matches are supported. Otherwise there should be a way to specify the entire substring value, i.e. in a key like "xxyz.keyNNN.foo", will it do partial field replacement or does the substring have to line up
to the value between "dots" (in that example).

(In reply to Alessandro Castellani (:aleca) from comment #160)

Using mc.sleep(0) as we are using it with EventUtils.synthesizeKey at various places.

I did implement some mc.sleep(0) in the past but they ended up biting me after a while.
It's better to not set an arbitrary timeout ... so we don't have to worry about timings.

---

Isn't '0' a special value (not arbitrary), to mean to not really wait any time unless something else is demanding attention? I.e. it's a signal from a running thread that it could "wait" if something else needs attention, but otherwise, resume execution after checking for higher priority events. It could still generate non-determinant behavior for other reasons, but it certainly shouldn't be in the same class as mc.sleep(X), X!=0.

Comment 162

[Magnus Melin [:mkmelin]]

(In reply to L A Walsh from comment #161)

That would be odd, since synthesize key seems for moving through a data table and wouldn't involve an actual key
being pressed. Even if it does generate output, it's intermediate output that the user wouldn't usually be
interested in.

What's immediate to an actual user is not necessarily immediate on the code level. Tests easily fail intermittently because events were triggered on one row but weren't handled yet in time for checking the reaction, on the next line.

Isn't '0' a special value (not arbitrary), to mean to not really wait any time unless something else is demanding attention?

It works like setTimeout. So 0 = next event cycle.

Comment 163

[Magnus Melin [:mkmelin]]

Comment on attachment 9189957 [details] [diff] [review]
Bug-244347_change-account-order-10.patch

Review of attachment 9189957 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/base/prefs/content/AccountManager.js
@@ +874,5 @@
> +    let tabmail = win.document.getElementById("tabmail");
> +    for (let tabInfo of tabmail.tabInfo) {
> +      let tab = tabmail.getTabForBrowser(tabInfo.browser);
> +      if (tab && tab.urlbar && tab.urlbar.value == "about:accountsettings") {
> +        tab.browser.reload();

and return

@@ +1722,5 @@
>    return accountArray[serverId];
>  }
>  
> +function accountReordered() {
> +  let accountItems = document.getElementById("account-tree-children").children;

would just inline this one

::: mailnews/base/src/nsMsgAccountManager.cpp
@@ +1575,5 @@
> +    for (auto account : m_accounts) {
> +      account->GetKey(accountKey);
> +      newAccountKeyList.Append(accountKey);
> +      newAccountKeyList.Append(',');
> +    }

This will leave a trailing comma, not sure we handle that properly later. You can use https://searchfox.org/comm-central/rev/1fa5ebe1384434e904b33bb7de8f0a3d6e8bfdc5/mailnews/base/src/nsMsgAccountManager.cpp#651-653 instead.
Once the append/inserting of the accounts is done, I would call that for both of the if-else clause cases

Comment 164

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-11.patch

Comment 165

[Magnus Melin [:mkmelin]]

Comment on attachment 9190484 [details] [diff] [review]
Bug-244347_change-account-order-11.patch

Review of attachment 9190484 [details] [diff] [review]:
-----------------------------------------------------------------

This seems good now! r=mkmelin

Comment 166

[Khushil Mistry [:khushil324]]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c2cd309ecd362b50c24aad23e8140364c4de5d25

Comment 167

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-12.patch

https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=d8164ce9a9310d5fa7032bfe686ed061cd6558b9&selectedTaskRun=SJLvyNkAQHaRkkLSJjzNdw.0

Corrected unit test failure.
Here unit tests were failing because all the accounts are local but the first account will behave as an actual local account and will be kept last always. Previously, we were not saving the keys in the order but in Javascript, we used a helper function to always keep the local account at the last. Now we are doing it directly through C++ so account keys strings will be also in such order.

Comment 168

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-13.patch

Corrected the mail/test/browser/folder-widget/browser_messageFilters.js test failure.

Comment 169

[Khushil Mistry [:khushil324]]

https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=e01ec50eb08ce6e10e04698454fac2df066a192a

Comment 170

[Magnus Melin [:mkmelin]]

It's reproducible locally, but you have to run the whole dir. So some kind of interaction with another test

./mach test --headless comm/mail/components/extensions/test/browser/

Comment 171

[Khushil Mistry [:khushil324]]

Attachment: Bug-244347_change-account-order-14.patch

https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=16c3659d382785a895d886dfedc50ce30974fc71

Comment 172

[Magnus Melin [:mkmelin]]

Comment on attachment 9192698 [details] [diff] [review]
Bug-244347_change-account-order-14.patch

Review of attachment 9192698 [details] [diff] [review]:
-----------------------------------------------------------------

Alright, let's get this landed!

Comment 173

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/05adf8cffed2
allow changing sort order of accounts. r=mkmelin

Comment 174

[Magnus Melin [:mkmelin]]

Unfortunately I think we'll need to back this out. The way nsMsgAccountManager::ReorderAccounts rejects the new accounts if the new vs old length is incorrect doesn't work out if one has chat accounts set up. In a test profile I had one other case as well, but that's likely a leftover from some other unfinished patch (server24 - type none - hostname "smart mailboxes").
The effect is that the accounts are only re-ordered as desired in the account settings UI, but the change doesn't really happen.

Comment 175

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/comm-central/rev/55dc6d93c924
Backed out changeset 05adf8cffed2 to fix problems with chat accounts. rs=backout

Comment 176

[Daniel]

Hello! It doesn't work on TB 78.6.0 on Mac Catalina... Is that normal ?
Thank's, have a nice day.
Daniel.

Comment 177

[John Bieling (:TbSync)]

@Daniel: It has not been backported to ESR yet. But it looks like it was backed out completely due to some issues. So those have to be fixed first and when everything works, it may be backported to ESR.

Comment 178

[Daniel]

Good! Thank's for the infos.
Nice day,
Daniel.

Comment 179

[Martin Giger [:freaktechnik]]

Attachment: Bug 244347 - change sort order of accounts. r=mkmelin

Comment 180

[Martin Giger [:freaktechnik]]

Attachment: Bug 244347 - Don't filter IM accounts and add hidden accounts to reordering. r=mkmelin

Hidden accounts include the unified folder account. Further this fixes reordering after an account
is marked as default.

Depends on D108943

Comment 181

[Martin Giger [:freaktechnik]]

Comment on attachment 9192698 [details] [diff] [review]
Bug-244347_change-account-order-14.patch

The second new patch fixes the issues that lead to the backout:

• Fix migration when there are IM accounts in the profile
• Fix reordering when the user has used the unified folders view
• Bonus fix: reordering when setting a default account with IM accounts

Comment 182

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/800f2782950f
change sort order of accounts. r=mkmelin
https://hg.mozilla.org/comm-central/rev/3643bba90447
Don't filter IM accounts and add hidden accounts to reordering. r=mkmelin

Comment 183

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/bbd147b0355c
follow-up - Fix a broken test. rs=bustage-fix

Comment 184

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d3383b71397c
followup to clang-format. rs=clang-format