672022 - Thunderbird 5 does not sort address columns correctly if showing only display name

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect)

Description

[Avner Falk]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:5.0) Gecko/20100101 Firefox/5.0
Build ID: 20110615151330

Steps to reproduce:

Click on the From column header


Actual results:

The messages were not sorted alphabetically by the sender's name


Expected results:

The messages should be sorted alphabetically by the sender's name

Comment 1

[David :Bienvenu]

Are you showing only the display name, per options/preferences, advanced, general? We sort by the full sender address, not the display name.

Comment 2

[Avner Falk]

The From column displays the sender's name, not his e-mail address. All previous versions of Thunderbird sorted the From column by the display name. Why would TB5 have to be any different in this respect? This is a real bug and needs to be fixed rather than trying to find excuses for it.

Comment 3

[David :Bienvenu]

It would be helpful if you could answer my question. It's unfortunate that you interpreted a question attempting to diagnose your problem as an excuse. But I'll keep trying. If you turn off "show only display name", do you see the correct sorting? TB 5 did indeed change what is displayed in the thread pane, but what we sort on did not change (that's this bug) - see https://bugzilla.mozilla.org/show_bug.cgi?id=243631

Comment 4

[Avner Falk]

It's unfortunate that we are arguing instead of trying to solve the problem together. In your previous message you wrote me, "Are you showing only the display name, per options/preferences, advanced, general?" There is no such option under the Tools > Options > Advanced > General tab. How can I turn off the "show only display name" option? If you want me to use the Configuration Editor, you should say so and tell me which settings to change and how. How can I answer your question when it is phrased in a way which makes it impossible to understand?

Comment 5

[David :Bienvenu]

tools, options, advanced, reading & display.

Boying, I looked into what it's going to take to fix this - unforutnately, I think some of the collation key generation code is going to need to be lifted into the view code, in particular, nsMsgDBView is going to need an equivalent of nsMsgDatabase::RowCellColumnToCollationKey() that takes the display name string instead of a column directly from the database.

Comment 6

[Avner Falk]

Dear Boying and David, if you really want to save Thunderbird's position as the best e-mail client available, then you will do what it takes to fix this bug (as well as Bug 672023, the absence of the message-download-progress bar, which has erroneously been marked as a duplicate of an earlier bug). Once you have done this, I shall be delighted to reinstall TB5 on my computer and to recommend it to the thousands of people in my TB Address Book. Thank you very much.

Comment 7

[Mitchell Baker]

David

I'm using TB 5 on a Mac 10.6.5.  Sorting on "from" works fine for me.  In the Tools menu dropdow however, I do not an "options" option.  I have:  saved files, add-ons, activity manager, message filters, run filters on folder, run filters on msg, run junk mail controls, delete marked mail as junk, import, error console, account settings.

but as I said, i'm not experiencing this bug.

Comment 8

[David :Bienvenu]

Mitchell, it's preferences on the mac (tools, options, on windows, and Avner is using windows, according to the bug). We use different menus to conform to standard app behavior on the OS, but it makes it awkward to tell users how to get to options/preferences.

Comment 9

[Mitchell Baker]

Yup, figured that!

Comment 10

[Avner Falk]

Mitchell, this bug definitely exists in the Windows version of TB5 and many users have complained about it on various sites (e.g. http://forums.mozillazine.org/viewtopic.php?f=29&t=2210501&start=15). I'd love to see your people fix this bug ASAP. Thanks, Avner

Comment 11

[David :Bienvenu]

Attachment: proposed fix

this will fix the sort issues, though it may cause a pretty big performance hit the first time you sort by sender, and thereafter if you change an address book card. Before this can get reviewed, I need a unit test for it.

Comment 12

[Avner Falk]

Thanks for including me on this code-change proposition. I am not a programmer. However, if you send me a beta version of TB 5.1 I shall be happy to test it.

Comment 13

[David :Bienvenu]

Attachment: fix with unit test

I noticed that we also weren't sorting by recipients correctly, and so I added both kinds of sort to a unit test.

Comment 14

[Avner Falk]

When you have the next version kindly send me a download link. Thanks.

Comment 15

[Mark Banner (:standard8)]

Comment on attachment 548803 [details] [diff] [review]
fix with unit test

>-        rv = msgHdr->GetRecipientsCollationKey(len, result);
>         rv = msgHdr->GetAuthorCollationKey(len, result);

We should be able to remove these functions on trunk, unless you've got a specific reason for keeping them.

I'm a little concerned by the performance issues, presumably there's no easy way to batch these keys/changes up when we download messages (we could cover that in a follow-up bug).

I think this is a reasonable first step for getting in for 7 and seeing what feedback we get.

Comment 16

[David :Bienvenu]

(In reply to Mark Banner (:standard8) from comment #15)
> Comment on attachment 548803 [details] [diff] [review]
> fix with unit test
> 
> >-        rv = msgHdr->GetRecipientsCollationKey(len, result);
> >         rv = msgHdr->GetAuthorCollationKey(len, result);
> 
> We should be able to remove these functions on trunk, unless you've got a
> specific reason for keeping them.

I don't know if any extensions are using these. But I can file a bug to remove them.

> 
> I'm a little concerned by the performance issues, presumably there's no easy
> way to batch these keys/changes up when we download messages (we could cover
> that in a follow-up bug).

Since we cache the result of figuring out the display name in the .msf file, I'm not sure what you would batch...once the display name is cached, the new code performs very much like the old code.
> 
> I think this is a reasonable first step for getting in for 7 and seeing what
> feedback we get.

Comment 17

[Mark Banner (:standard8)]

Checked into trunk: http://hg.mozilla.org/comm-central/rev/18242a1afda0

Comment 18

[Avner Falk]

Congratulations. Is your new version compatible with the European language spelling dictionaries and with BiDi Mail UI? Where can I download it? Thanks.

Comment 19

[Mark Banner (:standard8)]

Checked into aurora: http://hg.mozilla.org/releases/comm-aurora/rev/47fc904950ff

Comment 20

[Avner Falk]

Even with the latest Beta of TB7, the sorting of messages in a given folder by the sender's name takes an inordinate amount of time (from 5 to 30 seconds, depending on the number of messages in the folder). In TB3 it takes less than a second. This is a serious problem which will prevent users from upgrading to TB7 or will make them revert to TB3. Therefore this bug cannot be marked as fixed before it is resolved.

Comment 21

[Mark Banner (:standard8)]

Please file a new bug for other issues so they can be tracked properly.