Closed Bug 1266698 Opened 4 years ago Closed 3 years ago

Attempting to sort messages on the Date field whilst a quick filter is applied gets stuck on sort descending


(Thunderbird :: Mail Window Front End, defect)

Not set


(thunderbird49 wontfix, thunderbird50 fixed, thunderbird_esr4550+ fixed)

Thunderbird 51.0
Tracking Status
thunderbird49 --- wontfix
thunderbird50 --- fixed
thunderbird_esr45 50+ fixed


(Reporter: standard8, Assigned: alta88)


(Keywords: regression)


(1 file)

Thunderbird 45.0 on a Mac.


1) On a large folder, start a quick filter for a set of messages.
2) Try and sort the folder by date by clicking the column heading

Expected Results

- The messages are sorted by date, and then changes the sort order every time the column is clicked.

Actual Results

- The messages are sorted by date, and then get stuck on sort ascending, even though the arrow is changing direction.
I can confirm this behavior.
I'm not going to block the next release on this, but this is an important regression we should investigate.

Alta88, is there any way that you could investigate?
Flags: needinfo?(alta88)
This happens only if the quickfilter view is threaded.  The "quicksearch" view code is fundamentally broken for threading; in Tb38 and likely much earlier.

1) leaving and re-entering a quickfilter folder, stickied, will show the same behavior when toggling a column for sort,
2) in the same folder, merely toggling a filter (say, Sender search) off and back on will also do the same when trying to column sort.

Note that repairing the folder is the only way to fix subsequent broken threaded sorts. The above two str are on 38.7.2. It isn't even necessary for any items to actually thread, merely being in threaded mode breaks it. Anyway, it doesn't seem like a regression from the group view changes, perhaps they make the breakage more obvious in the adamance there to persist state for the single saved search folder case (which is the only other use of "quicksearch").

IMO, the solution is to kill the 1000 lines of "quicksearch" contract id code and replace the usage with "xfvf". I wanted to do it in the group patches, asuth wanted to do it way back as well.  It's easy enough to try, just replace the string in dbViewWrapper.js.  The one known problem is that status bar counts don't get updated, which is probably the same issue as the ancient permanent Searching... bug in virtual folders.
Flags: needinfo?(alta88)
Unfortunately, replacing "quicksearch" with "xfvf" or "search" contractids creates subtle errors in either threading toggle (2+ deep), or delete/insert of messages re counts, status counts, and probably thread deletion.  So it's not worth pursuing.

It turns out that when thread handling (nsMsgQuickSearchDBView::SortThreads) was duplicated for quicksearch, it left out a reset which exists in nsMsgDBView::SortThreads. So threaded reverse column sorting was broken for (only) byDate and byId, due to using ReverseThreads().
Assignee: nobody → alta88
Attachment #8784825 - Flags: review?(rkent)
Comment on attachment 8784825 [details] [diff] [review]

Seems to work and make sense, but I don't really have a great overall grasp of the view design.
Attachment #8784825 - Flags: review?(rkent) → review+
Keywords: checkin-needed
Closed: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 51.0
Version: 45 Branch → Trunk
Comment on attachment 8784825 [details] [diff] [review]

[Approval Request Comment]
Regression caused by (bug #): none
User impact if declined: failure in sorts during quicksearch
Testing completed (on c-c, etc.): only by hand
Risk to taking this patch (and alternatives if risky): low

Since this is tracking esr45 let's move it to aurora to eventually get it considered for tb45
Attachment #8784825 - Flags: approval-comm-aurora?
Comment on attachment 8784825 [details] [diff] [review]

Aurora (TB 50):
Landed with DONTBUILD, next blocklist update will compile it.
Attachment #8784825 - Flags: approval-comm-aurora? → approval-comm-aurora+
Comment on attachment 8784825 [details] [diff] [review]

[Approval Request Comment]
Tracking-tb45 and has been through beta, consider for tb45.
Attachment #8784825 - Flags: approval-comm-esr45?
Comment on attachment 8784825 [details] [diff] [review]
Attachment #8784825 - Flags: approval-comm-esr45? → approval-comm-esr45+
You need to log in before you can comment on or make changes to this bug.