Last Comment Bug 672022 - Thunderbird 5 does not sort address columns correctly if showing only display name
: Thunderbird 5 does not sort address columns correctly if showing only display...
Status: RESOLVED FIXED
[has patch and unit test for review]
: regression
Product: Thunderbird
Classification: Client Software
Component: Folder and Message Lists (show other bugs)
: 5.0
: All Other
: -- normal (vote)
: Thunderbird 8.0
Assigned To: David :Bienvenu
:
:
Mentors:
: 650627 (view as bug list)
Depends on: 690707 695327
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-16 02:36 PDT by Avner Falk
Modified: 2011-11-05 16:51 PDT (History)
6 users (show)
standard8: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed


Attachments
proposed fix (1.21 KB, patch)
2011-07-24 16:03 PDT, David :Bienvenu
no flags Details | Diff | Splinter Review
fix with unit test (7.81 KB, patch)
2011-07-27 08:57 PDT, David :Bienvenu
standard8: review+
standard8: approval‑comm‑aurora+
Details | Diff | Splinter Review

Description Avner Falk 2011-07-16 02:36:18 PDT
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 2011-07-17 18:39:58 PDT
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 2011-07-17 23:21:51 PDT
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 2011-07-18 06:45:08 PDT
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 2011-07-18 07:09:59 PDT
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 2011-07-18 07:22:24 PDT
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 2011-07-18 07:43:04 PDT
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 2011-07-18 08:39:10 PDT
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 2011-07-18 08:42:50 PDT
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 2011-07-18 08:45:31 PDT
Yup, figured that!
Comment 10 Avner Falk 2011-07-18 08:55:27 PDT
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 2011-07-24 16:03:33 PDT
Created attachment 548053 [details] [diff] [review]
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 2011-07-25 00:47:13 PDT
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 2011-07-27 08:57:31 PDT
Created attachment 548803 [details] [diff] [review]
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 2011-07-27 10:58:22 PDT
When you have the next version kindly send me a download link. Thanks.
Comment 15 Mark Banner (:standard8, limited time in Dec) 2011-08-15 07:56:34 PDT
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 2011-08-15 11:54:02 PDT
(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, limited time in Dec) 2011-08-16 05:09:03 PDT
Checked into trunk: http://hg.mozilla.org/comm-central/rev/18242a1afda0
Comment 18 Avner Falk 2011-08-16 05:16:09 PDT
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, limited time in Dec) 2011-08-16 06:00:14 PDT
Checked into aurora: http://hg.mozilla.org/releases/comm-aurora/rev/47fc904950ff
Comment 20 Avner Falk 2011-09-30 01:17:40 PDT
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, limited time in Dec) 2011-09-30 01:36:33 PDT
Please file a new bug for other issues so they can be tracked properly.
Comment 22 Jim Porter (:squib) 2011-11-05 16:51:11 PDT
*** Bug 650627 has been marked as a duplicate of this bug. ***

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