Closed
Bug 500083
Opened 16 years ago
Closed 16 years ago
delete message from Search Messages results doesn't update view
Categories
(Thunderbird :: Folder and Message Lists, defect)
Thunderbird
Folder and Message Lists
Tracking
(Not tracked)
VERIFIED
FIXED
Thunderbird 3.0b3
People
(Reporter: Bienvenu, Assigned: Bienvenu)
References
(Blocks 1 open bug)
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
7.58 KB,
patch
|
Bienvenu
:
review+
standard8
:
superreview+
|
Details | Diff | Splinter Review |
2.20 KB,
patch
|
Details | Diff | Splinter Review |
If I do an advanced search, and delete an entry from the search results, the view doesn't update. This is also true for gloda search results, which use a search results view as well.
The attached patch fixes it for me - I'm going to work on a mozmill test for this, but I wanted to get asuth's opinion on the basic approach.
Attachment #384776 -
Flags: review?(bugmail)
Comment 1•16 years ago
|
||
I'm pretty sure even in en-UK it'd be curiosity, not curiousity.
Assignee | ||
Comment 2•16 years ago
|
||
heh, ok, changed locally. There is already a mozmill test for search results, so I just need to add a delete test to that.
Putting in b3.
Status: NEW → ASSIGNED
Flags: blocking-thunderbird3+
Comment 3•16 years ago
|
||
Comment on attachment 384776 [details] [diff] [review]
proposed fix
In response to https://bugzilla.mozilla.org/show_bug.cgi?id=474701#c163:
Since the gloda synthetic view is basically force-feeding the headers, it could make sure the registrations happens for the folders in its results view. It's a toss-up on simplicity versus fake view rebuilds, but since the advanced search window is not commonly used, I'll say simplicity is the right thing.
The down-side to this implementation is that view rebuilds (which includes new searches) will happen on:
- any folder being moved
- any folder being deleted
- any folder being compacted (the search view will actually go away until the compaction completes)
The gloda search case has it easy because view rebuilds are low-cost for it. It just crams the headers in again. Arguably the gloda search won't be crazy about the compaction making us go away, but I rather suspect things won't be usable while that is happening anyways.
on file: mailnews/base/src/dbViewWrapper.js line 268
> dump("view wrapper got delete completed\n");
please remove debug dump
Attachment #384776 -
Flags: review?(bugmail) → review+
Assignee | ||
Comment 4•16 years ago
|
||
carrying forward asuth's r+, requesting sr.
In the case of compact, at least for local folders, rebuilding the view is needed. I don't think advanced search is going to be that common, so right now I like the implementation simplicity trade-off.
Attachment #384776 -
Attachment is obsolete: true
Attachment #384796 -
Flags: superreview?(bugzilla)
Attachment #384796 -
Flags: review+
Assignee | ||
Comment 5•16 years ago
|
||
I have not been able to test this since mozmill broke overnight for me - it fails cleaning up the mozmill extension dir in the objdir, at the start of the test run. I'm not sure why that started happening...but I'm clobbering and rebuilding.
Assignee | ||
Comment 7•16 years ago
|
||
it turns out to be very difficult to write a test case that fails - basically, the failure is that we're missing some invalidation calls on the view, and it's pretty hard to tell if a paint has happened. It's a lot easier to write tests for the gloda search results because we can tell if the next message gets loaded...
Updated•16 years ago
|
Attachment #384796 -
Flags: superreview?(bugzilla) → superreview+
Comment 8•16 years ago
|
||
I've pushed the patch in time for today's nightly: http://hg.mozilla.org/comm-central/rev/06ef7c212993
We still need to sort out the testcase though.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Comment 9•16 years ago
|
||
The mozmill test is unhappy; was the patch refreshed before submission? The use of "swc.dbView" would require that a getter helper is added to the bottom of test-window-helpers.js. When that is added, press_delete is unhappy by way of wait_for_message_display_completion because it assumes all kinds of things about messageDisplays and docshells. press_delete/wait_for_message_dispay_completion should either be more aware of these things or the MessageDisplayWidget subclass used for the search view may need to change its behaviour slightly to not be misleading.
Assignee | ||
Comment 10•16 years ago
|
||
shoot, I didn't mean for the mozmill test part of the patch to land - that part should just be reverted.
Comment 11•16 years ago
|
||
v.fixed
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1pre) Gecko/20090626 Shredder/3.0b3pre
Status: RESOLVED → VERIFIED
Updated•16 years ago
|
OS: Windows Vista → All
Hardware: x86 → All
Comment 13•15 years ago
|
||
filed similar new Bug 515400.
noting it here as a reminder about needing a test
Flags: in-litmus?
Summary: delete from search results doesn't update view → delete message from Search Messages results doesn't update view
Comment 14•15 years ago
|
||
Added https://litmus.mozilla.org/show_test.cgi?id=9509 in BFT and FFT.
Flags: in-litmus? → in-litmus+
Updated•10 years ago
|
Flags: in-testsuite?
You need to log in
before you can comment on or make changes to this bug.
Description
•