Closed Bug 505321 Opened 14 years ago Closed 14 years ago

new message added to an open threaded cross-folder saved search doesn't get added in the right place


(Thunderbird :: Folder and Message Lists, defect)

Windows Vista
Not set


(Not tracked)

Thunderbird 3.0b4


(Reporter: Bienvenu, Assigned: Bienvenu)


(Keywords: regression)


(3 files)

If I have a threaded cross-folder saved search open (e.g., the smart folders Inbox, threaded), and I'm reverse sorted by date, so the newest messages are at the top, new messages in new threads don't get added in the right place. Usually, they should be added at the top, but they end up at or near the bottom.

There's a rather simple fix, but getting a test case to fail was much more complicated. So first I'm going to attach a patch that adds a test to show the failure, fixes the test harness so that it detects the failure, and some backend changes to fix a backend problem that was horking the test. Then I'll attach a patch that fixes the actual bug. Finally, I need to fix the regression that caused the bug...

some of the issues I ran into:

test_nsMsgDBView.js was crunching gTestFolder when it ran the extra group mutation test, so I moved the group test to the end, and also tried to fix the test loop so that it would restore a crunched gTestFolder.

Several of the assert methods were using an Iterator on arguments, which doesn't work, so we weren't checking the things we thought we were checking. 

Sorting a threaded search view was causing the groupedbysort flag to get set on the view, which meant we weren't testing the non-grouped case for search views. To fix this, I had to move OpenWithHdrs from the xfvfview class to the searchdbview class, since that code was doing the right thing.

requesting sr from standard8 for the core change...
Flags: blocking-thunderbird3+
Attachment #389550 - Flags: superreview?(bugzilla)
Attachment #389550 - Flags: review?(bugmail)
This fixes the problem by using the date of the message as the thread date if we can't find the thread - since the message must be a singleton, this is a reasonable thing to do.

The underlying issue is that the header is getting added to the search db view w/o going through the code that creates the cross-folder thread object - I need to fix that next, but that might end up being slightly more involved.
Attachment #389552 - Flags: superreview?(bugzilla)
Attachment #389552 - Flags: review?(bugmail)
this fixes the underlying issue, per the comment in the code.
Attachment #389563 - Flags: superreview?(bugzilla)
Attachment #389563 - Flags: review?(bugmail)
Comment on attachment 389550 [details] [diff] [review]
get a test to fail

My apologies about the broken tests.  They indeed do break quite nicely with this patch.  Thanks for splitting it out as a separate patch.
Attachment #389550 - Flags: review?(bugmail) → review+
Attachment #389552 - Flags: review?(bugmail) → review+
Comment on attachment 389563 [details] [diff] [review]
fix underlying issue

A very useful comment.

Verified the test succeeds without your more immedate fix too.
Attachment #389563 - Flags: review?(bugmail) → review+
Whiteboard: [needs sr standard8]
Attachment #389550 - Flags: superreview?(bugzilla) → superreview+
Attachment #389563 - Flags: superreview?(bugzilla) → superreview+
Attachment #389552 - Flags: superreview?(bugzilla) → superreview+
Whiteboard: [needs sr standard8] → [ready to land]
all three patches landed.
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [ready to land]
You need to log in before you can comment on or make changes to this bug.