369620 - (pref unthreads=True): Threading via column header forces Sort by "Order Received", should be "Date"

Status: VERIFIED FIXED

(Thunderbird :: Mail Window Front End, enhancement)

Description

[Gervase Markham [:gerv]]

Steps to reproduce:

- Pull down Thunderbird thread page column picker and choose "Order Received", 
  so you can see 
- Click the header of the thread column to sort by thread

Expected: Date column acquires "sort arrow"; Thunderbird sorts the threads in order of "earliest date in thread". After all, threading is an inherently date-based thing.

Actual: Order Received column acquires "sort arrow"; Thunderbird sorts the threads in order of "lowest order-received number in thread". 

I don't know how the order received numbers are allocated, by as they are not congruent with date, you can get some very strange sort orders. Examples and screenshots available on request. This is particularly bad if you are the sort of person who triages their inbox into a "to do" folder, because the Order Received number gets reallocated whenever you move a message from one folder to another.

This may be a duplicate of or related to bug 57898, but I'd like a Thunderbird developer to comment on this particular common scenario.

Gerv

Comment 1

[Mike Cowperthwaite]

This behavior is "how it's always been" in the suite as well as TB.

If you set the value of the pref
  mailnews.thread_pane_column_unthreads
to False, clicking the Thread column header won't change the sort criterion; it will simply toggle between Threaded and Unthreaded.  By default, this pref is True to provide the old behavior.  (xref bug 219787.)

Additionally, clicking on any other column header won't change the threading, it will simply change the sort criterion or reverse the direction.

The suite additionally provides prefs UI for changing that setting.

Comment 2

[Gervase Markham [:gerv]]

Mike - thanks for that. Changing that pref helps - in that I can sort by date and then thread, and get the behaviour I want. But it doesn't solve the underlying problem.

"This is how it's always been" is true of many things, including a load of long-standing bugs. It doesn't make it necessarily right. Has anyone come up with a reason why people might ever want to sort by the date/time a file has arrived in a folder? Or might (without the hint of the column being present) even *guess* that this is what's going on? Do we know of any other mailers that behave that way?

The default sort should be by the date on the mail, not by order received. 

Gerv

Comment 3

[Magnus Melin [:mkmelin]]

Sounds like a duplicate of bug 262316.

Comment 4

[Mike Cowperthwaite]

I believe Gerv is talking about which order the threads appear in relative to one another; that bug is about ordering of the items within the thread.

Comment 5

[Gervase Markham [:gerv]]

Exactly.

Gerv

Comment 6

[Mike Cowperthwaite]

> Has anyone come up with a reason why people might ever want to sort by the
> date/time a file has arrived in a folder?

I'm speculating that this happened primarily because the developers of the Netscape era were dogfooding with IMAP, where (if I understand this correctly) "order received" *is* "date received" even for messages moved to folders.

Similarly, Order Received is the tiebreaker for all other sort criteria -- subject, sender, whatever (xref bug 363991).  Arguably, that should be "by date" as well.

Also: it used to be that threads sorted by date used the date of the topmost item in the thread, so sorting by date didn't necessarily gain you anything in terms of visibility of recent messages -- in which case, sorting by Order Received isn't much worse.  That behavior was changed (around the time of Mozilla 1.5, I think) to use the most-recent item in the thread.  (Bug 254159 seeks the same technique for using Order Received sorting.)

Even sorting by date is contentious -- see bug 166254.  A lot of people want sort by "Received Date" because Date headers are often broken; others want the Date header to always be used.


Anyway, Gerv: the patch at bug contains the line that sets the sort criterion for sort-by-thread with unthreads=False:
  sortType = nsMsgViewSortType.byId;
If you want to submit a patch and drive it past the gauntlet, more power to you.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Duplicate of bug 264941 ?

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

See also bug 341548 (especially bug 341548 comment 10).  It's related to 
the symptom of messages being (or appearing) out of order.  David Bienvenu's 
patch for that problem is attached to bug 166254 (which it also fixes).  
It has been waiting there for SR for 15 months.  Maybe a governance issue?

Comment 9

[Mike Cowperthwaite]

(In reply to comment #6)
> Anyway, Gerv: the patch at bug contains the line that sets the sort criterion
> for sort-by-thread with unthreads=False [...]

s/b:  ...patch at bug 219787 ...
                      ~~~~~~
(Gerv asked me about this in email, and I think I responded to him with the wrong bug #.  219787's patch contains the source line quoted at the end of comment 6, shown moved from one location in the logic to another.)


(In reply to comment #7)
> Duplicate of bug 264941 ?

No, that bug is about changing to 'Order Received' in response to selections in the   View|Threads   menu.  (Incidentally, the patch there was just checked in this morning.)


(In reply to comment #8)
> See also bug 341548

Not related.  This bug is specifically about picking which sort mode is chosen in response to the click on Thread column header when unthreads=true.  That bug is about which date-containing header is used to fill in the Date header.  (Also, that bug has been fixed.)

Comment 10

[Gervase Markham [:gerv]]

Yes. To be completely clear:

Currently, when unthreads=true, the sort mode chosen when one asks for threads is "Order Received". This works poorly in the Inbox and even worse in other folders, because it gets reset when a message moves between folders. 

This bug requests that, when unthreads=true, the sort mode chosen when one asks for threads is "Date".

I strongly suspect this is a one-line change somewhere. Anyone know where?

Gerv

Comment 11

[Alets]

Hi,

Any update on this? at least an ID for this features?

A/

Comment 12

[WADA:World Anti-bad-Duping Agency]

FYI. Seamonkey version is Bug 389935.

Comment 13

[Jez]

I have a working patch for this bug.  It depends on the fix for Bug #166254 being checked in first, though.  It sorts by Received when possible, otherwise by Date.  I'll post it when Bug #166254 has been fixed.

By the way, it only jumps to 'order received' if you click the thread column button.  If you sort by Date, then choose 'View | Sort by | Threaded', it still sorts by date.

Comment 14

[Nelson Bolyard (seldom reads bugmail)]

> It sorts by Received when possible, otherwise by Date. 

That description, if correct, does not sound like a fix for this bug.
It sounds like it makes things worse.  
Which is wrong: the description, or the proposed fix?

Comment 15

[Jez]

It's better than the fix for this bug suggests, because Received date is what people really want Date to be - a much more reliable indicator of when the e-mail really was received.  I wouldn't say it makes things worse.  Received date is not the same as Order Received.

Comment 16

[Mike Cowperthwaite]

> Received date is what people really want Date to be

You're not necessarily correct in that assertion.

I don't see, at this point, why your patch needs to be dependent on whatever's going on with Received date is a mystery to me.  The patch I made for 
bug 264941, which implemented the menu behavior you cite in comment 13, simply preserves whatever the original sorting mode was.  Do the same thing, don't change the sort criterion at all.

Comment 17

[Nelson Bolyard (seldom reads bugmail)]

This bug very clearly says that the sort order should be by Date.
That's the Date in the Date: header.
That's how this product has worked ever since the beginning of the 
Netscape/mozilla/SeaMonkey/Thunderbird product line.  
That's how the vast majority of users expect it to behave.
It has recently regressed.  This bug addresses the regression.
Using this bug as an opportunity to change the long-time behavior
of the product to suit your personal opinion of a "better" way to 
work neither fixes the bug, nor suits the built-up community of 
users of Mozilla mail/news clients.  
And don't even THINK of saying "but that's how Outlook Express works"!

Comment 18

[Jez]

(In reply to comment #17)
> This bug very clearly says that the sort order should be by Date.
> That's the Date in the Date: header.
> That's how this product has worked ever since the beginning of the 
> Netscape/mozilla/SeaMonkey/Thunderbird product line.

If you read the comments in this bug, apparently that's not true.  'How it has worked' is that it's used Order Received.  However, most people have realised that 'how it has worked' is not always the best way, and that seems to be why this bug is here.

> That's how the vast majority of users expect it to behave.
> It has recently regressed.

Reading the comments to this bug, I don't see anything about a regression.  It looks like the normal behaviour is to use 'Order Received'.  There is a pref to stop that, but this is the traditional behaviour.  I believe it should be changed because it doesn't work well.

> Using this bug as an opportunity to change the long-time behavior
> of the product to suit your personal opinion of a "better" way to 
> work neither fixes the bug, nor suits the built-up community of 
> users of Mozilla mail/news clients.

A Mozillazine vote will see about that.  A *lot* of people prefer Received, maybe even the majority.

> And don't even THINK of saying "but that's how Outlook Express works"!

Bah.  OE does a lot of things right.  :-)

Comment 19

[Jez]

Attachment: Fix v1.0

I think the correct fix for this bug is not always to sort by 'Date', but to be consistent with the 'View | Sort by | Threaded' behaviour; that is, not to change the sort type at all.  This patch does that.

Comment 20

[Jez]

PS. If/when this patch is landed, it fixes bug #389935 as well.

Comment 21

[David :Bienvenu]

I'd much rather change the sort to byDate from byId, as the bug suggests, since I think your proposed behavior change is going to be very surprising for existing users. If I sort by sender to find an e-mail, and then turn threading back on, I'm going to be surprised, if I have your patch. I'd rather do the simple one line change.

Comment 22

[Jez]

David,

Why must you always focus so much on not 'surprising' old users rather than just improving behaviour?  You're insulting their intelligence by implying that they can't adapt to this change.  I don't even see any evidence in this thread that it ever did sort byDate, and especially with the new Received column having been added, I think sorting byDate is a bad move.  You want that retarded one-liner, you write it.

Comment 23

[David :Bienvenu]

Comment on attachment 281084 [details] [diff] [review]
Fix v1.0

minusing - I'll attach the simple patch in a sec...

Comment 24

[David :Bienvenu]

Attachment: one-liner Asa was looking for...

I think this gives us the behavior people were looking for in this bug.

Comment 25

[Jez]

> Created an attachment (id=282722) [details]
> one-liner Asa was looking for...

I'm not sure what you've attached here, but I'm pretty sure it aint what you intended.

Moreover, why aren't you WONTFIXing this bug?  Its desired behaviour is illogical, because people may want to sort by something other than date.  How about accepting my patch for bug #397928 instead?  Plenty of people agree with my behaviour, and when it comes to old behaviour vs easier/more intuative behaviour, you should be going for the latter.  This is stuff being developer for a new *major* version of Thunderbird, version 3.  It doesn't make sense to keep old stuff in for the sake of it.

Comment 26

[David :Bienvenu]

Attachment: right diff...

oops, that was the wrong patch, thx for pointing that out.

I'm not wont-fixing this bug - I'm attaching the one line fix for the bug as described in the report. Your proposed patch is different from what is requested in the bug.

Comment 27

[David :Bienvenu]

Attachment: geez, one day I'll get the right patch.

Comment 28

[Jez]

(In reply to comment #26)
> described in the report. Your proposed patch is different from what is
> requested in the bug.

... which is exactly why I created a new bug for my proposed behaviour.  >:-(

Comment 29

[Wayne Mery (:wsmwk)]

would be good to get this issue and bug 383985 sorted out, and decide if it is wanted for TB3

Comment 30

[Magnus Melin [:mkmelin]]

Yes, this too would be good to get in. Bug 383985 would result in lesser exposure of this bug, but the patch gives a more logical behaviour for users with unthreads true.

David: is this patch waiting for something? (Let me know if you want it checked in.)

Comment 31

[David :Bienvenu]

Thx, Magnus, I believe this can be checked in, and tried out.

Comment 32

[Magnus Melin [:mkmelin]]

I've checked this in for you.

Checking in mailnews/base/resources/content/threadPane.js;
/cvsroot/mozilla/mailnews/base/resources/content/threadPane.js,v  <--  threadPane.js
new revision: 1.93; previous revision: 1.92
done

->FIXED. One less flag to look at:)

Comment 33

[Mike Cowperthwaite]

This part of the code before the patch's change:
   var dbview = GetDBView();
   if(dbview && !dbview.supportsThreading)
     return;
   dbview.viewFlags |= nsMsgViewFlagsType.kThreadedDisplay;

has a potential for failure: if dbview is null, the 'return' will be skipped and then the null pointer will be dereferenced.  The if-clause should change to:
   if (!dbview || !dbview.supportsThreading)

Also, I notice the subsequent routine in the file, MsgSortThreadPane(), also references GetDBView() but doesn't check for null at all.  If the test is unneeded, it should be removed; otherwise, it should be everywhere.


None of which is germane to this bug, which I just checked and can verify.  :)