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

RESOLVED FIXED in Thunderbird 3.0b4

Status

RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Bienvenu, Assigned: Bienvenu)

Tracking

({regression})

Trunk
Thunderbird 3.0b4
x86
Windows Vista
regression
Bug Flags:
blocking-thunderbird3 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

10 years ago
Created attachment 389550 [details] [diff] [review]
get a test to fail

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)
(Assignee)

Comment 1

10 years ago
Created attachment 389552 [details] [diff] [review]
fix immediate problem

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)
(Assignee)

Comment 2

10 years ago
Created attachment 389563 [details] [diff] [review]
fix underlying issue

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+
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
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]
(Assignee)

Comment 5

10 years ago
all three patches landed.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: [ready to land]
You need to log in before you can comment on or make changes to this bug.