351692 - Need to see Account Names in the Move To / Copy To Recent Folder menus

Status: VERIFIED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Scott MacGregor]

If you have multiple folders with the same name, you don't know which folder you are copying or moving into when you use the recent folder drop down.

Comment 1

[Scott MacGregor]

In order to fix this, I think we should think about how we implemented:

nsMsgFlatFolderDataSource::GetFolderDisplayName

it currently only appends the account name if in the current folder view (recent, etc) you have two folders with the same name. But in the case or views like recent and unread if you have multiple folders even if they both aren't in the view, you don't know which one is which.

Maybe GetFolderDisplayName should append the account name if there's a folder in all folders with the same name and not just a folder in the current view.

Comment 2

[Frederic Da Vitoria]

This is a big problem if you use separate accounts. You can then have as many In, Out, Sent, Trash folders as accounts! (sorry, I have a French version so I don't know the exact names of the boxes) Currently, I have 5 In, 2 Out, 5 Trash in my recent! So my recent menu is almost useless.

Comment 3

[ggerard]

Why not just use the account name + IMAP folder name (in the case of IMAP) as this is already available and perfectly descriptive.

Comment 4

[Peter Tandler]

As the duplicate folder names are most likely Inbox etc., adding the "path" might be sufficient, e.g.

Inbox (Account1)
Inbox (Account2)
SomeFolder (Account1/SubFolder)
SomeFolder (Account2/AnotherSubFolder)

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

David seems to have code which could solve this (see bug 378605 comment 8).

Comment 7

[Scott MacGregor]

I think David's code is for the folder pane and it still works. This bug is about the recent folder menu items for moving & copying. 

Comment 8

[David :Bienvenu]

Attachment: [checked in]possible fix

this makes the menu items use the folder tree display name, which has the account for folders which are duplicates. It also includes the unread count for each folder in the recent menu, which is a little odd (assuming you aren't displaying the separate unread column in the folder pane).

An alternative would be to make an other attribute of the folder resources for the recent datasource, that showed the account name. I'm ambivalent. I think this patch is an improvement as is...

Comment 9

[Scott MacGregor]

Comment on attachment 265405 [details] [diff] [review]
[checked in]possible fix

I think the reason I didn't fix it this way before was because folderTreeName only appends the account name if there are two folders in the recent folders list that have the same name.

So this won't fix the case where you are trying to move a message to say a folder called Inbox and you open up the menu to find a single folder called Inbox in the menu but not know which account that was tied too....

It was either that or that this change shows the account for all folders, all the time in the recent folder move menu. I can't remember which :)

In any case your right, it is an improvement.

Comment 10

[David :Bienvenu]

Comment on attachment 265405 [details] [diff] [review]
[checked in]possible fix

I've checked this change in, but I'll leave the bug open since I think there's room for improvement.

Comment 11

[Mike Cowperthwaite]

With 3a1-0527, I notice that the folder name in the menu includes the unread count in parentheses.

Comment 12

[David :Bienvenu]

Mike, yes, I mentioned that comment #8 - to fix that, we'd need to use yet an other rdf property.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

David, to avoid the display of the unread count we could use ?folderTreeSimpleName. Then we have an output like "Inbox - Local Folders". But do we want to have '-' as separator or are brackets better: "Inbox [Local Folder]"? For the first proposal I could come up with a patch on yours one. :)

Comment 14

[David :Bienvenu]

Henrik, yes, it looks like that would work - a patch would be appreciated. Re [] vs - : if people think [] is better, we could change it for both. I believe I picked '-' because it was simple.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

David, I have another idea but don't know if it is possible. Are we able to change the font color for parts of a menu item? I think it would be nice if the account name is greyed out. So you can concentrate at the real folder name while also having a low contrast account name.

For the separator part we could also use () like it is implemented for the unread message count. Otherwise we will have to many different separators. Scott, what is your suggestion?

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: [checked in]Account names without numbers

Comment 17

[David :Bienvenu]

Comment on attachment 267596 [details] [diff] [review]
[checked in]Account names without numbers

thx, Henrik

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Can we try to get this into the next release of Thunderbird 2?

Comment 19

[Scott MacGregor]

this is not a security bug blocker so i'm going to minus this.

Comment 20

[Phil Ringnalda (:philor)]

Comment on attachment 267596 [details] [diff] [review]
[checked in]Account names without numbers

mail/base/content/mailWindowOverlay.xul 1.209

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Scott, are you happy with the current state? Or should we leave this bug open for further improvements?

For myself it looks good and I have to revert my comment 15 in using separators like [] or (). The '-' is more inconspicuous and doesn't disturb too much. It would be better if the account names could be displayed in lower contrast but as David stated on IRC this is probably not possible.

Comment 22

[Frédéric Buclin]

David, Scott, any chance to see this fix implemented in 2.0.0.x? As other commenters said already, this bug is really annoying when having several accounts. This is really a UE issue. If you don't plan to backport it, maybe setting the TM to Tb3 would make this clear.

Comment 23

[David :Bienvenu]

Comment on attachment 267596 [details] [diff] [review]
[checked in]Account names without numbers

probably pretty trivial to put into a 2.0.0.x release

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #23)
> (From update of attachment 267596 [details] [diff] [review])
> probably pretty trivial to put into a 2.0.0.x release

David, my patch is a follow-up of your one. Should I provide a new one for 1.8 branch instead which will apply correctly? 

Comment 25

[David :Bienvenu]

Henrik, that would be helpful, and save me some time, yes.

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: [checked in]Patch against 1.8 branch

Update for attachment 267596 [details] [diff] [review] against 1.8 branch.

Comment 27

[David :Bienvenu]

Comment on attachment 274823 [details] [diff] [review]
[checked in]Patch against 1.8 branch

thx!

Comment 28

[Scott MacGregor]

Comment on attachment 274823 [details] [diff] [review]
[checked in]Patch against 1.8 branch

assuming David is ok with this going into the branch...a=mscott

Comment 29

[Henrik Skupin [:whimboo][⌚️UTC+1]]

David, I mark this bug as resolved. I don't think that we have more improvements on this for now? Otherwise lets file another bug.

Comment 30

[Karsten Düsterloh]

Comment on attachment 274823 [details] [diff] [review]
[checked in]Patch against 1.8 branch

Landed on MOZILLA_1_8_BRANCH.

Comment 31

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified with Thunderbird version 2.0.0.7pre (20070913).

Comment 33

[Mark]

If I read this correctly, this patch has been incorporated into the source,
and if I build 2.0.0.12 now, this change should be there.  Is that right?

If so, then I would like to suggest that this change didn't go far enough.
I now have one "Sent" folder when I do Move To -> Recent.  I have multiple
email accounts, each with a "Sent" folder.  Which "Sent" folder is that in
the Recent list?

It's actually quite commonh for me to have regular folders with the same
name, just different parent folderss.  Why not show the whole "path"
for each Recent folder, as in comment #4?

But perhaps bug 378605 is the place for further discussion?