271988 - compacting folder other than selected one clears message and thread panes

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Florian Effenberger]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de-DE; rv:1.7.5) Gecko/20041108 Firefox/1.0
Build Identifier: Thunderbird 0.9

Compacting/compressing a folder jumps back to Inbox without displaying messages.

Reproducible: Always
Steps to Reproduce:
1. Compact/compress a folder via the context menu (right-click).

Actual Results:  
The mark goes back to the Inbox, however, no messages are shown. As soon as you
clock on another folder and go back to Inbox, all messages are there again. I
guess the "jumpback" to Inbox is just wrong.

Comment 1

[Mike Cowperthwaite]

IMAP or POP?  Is the folder being compacted a peer of Inbox, or a subfolder?

Comment 2

[Florian Effenberger]

It is a POP account. The folder is "under" the Inbox:

Inbox
 Folder1
 Folder2
 MyFolder
Draft

Comment 3

[Mike Cowperthwaite]

And which folder is selected at the point you right-click to get the menu?
Right-click doesn't change the persistent selection.

I think what's happening is, you have the Inbox selected, right-click on a 
subfolder, select Compact, and the thread pane and message pane are cleared 
(only if there is actually any compaction to be done).  So it's not that it's 
jumping back -- it's that the panes are either being unnecessarily cleared, or 
not being refreshed after clearing, when the selected folder is different from 
the compacting folder.

Does that match your experience?

Comment 4

[Florian Effenberger]

You seem to be right. I just checked it out. When the to-be-compacted folder is
not selected before the right click, the problem is occurs. If it is, there is
no problem.

Comment 5

[Mike Cowperthwaite]

Reproduced with TB 0.9+ (20041126), Win2K.
I also see the symptom in Moz 1.8a5.

Comment 6

[eric gorr]

I just wanted to confirm what I believe is the same bug happening under the
latest version of Thunderbird (1.0.2) on MacOSX.

I have the compaction feature turned on and the inbox view set to just show me
unread messages. When I click on the folder, I see the dialog appear:

 'Do you wish to compact all local and offline folders to save disk space?'

I click on OK.

It compaction takes place, but the message view either shows me no messages or
shows me all of the messages even though the view is set to 'unread' only.

I generally have to change views or switch to a different folder and back again
to get things to display properly.

This is a rather annoying bug.

Comment 7

[Lee_Dailey]

howdy y'all,

my tb info ...
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8) Gecko/20051201 Thunderbird/1.5 - Build ID: 2005120115

this problem is also in tb150rc2 and is really quite disconcerting. it LOOKS like tbird has locked up or hung. one must either select a diff view OR a diff fodler to get tb to redisplay the contents of the msg-list pane.

take care,
lee

Comment 8

[Wayne Mery (:wsmwk)]

Mike do you still see this problem?  I think reporter is gone.

Comment 9

[Florian Effenberger]

Reporter is there. ;-) But I think bug is fixed, can't reproduce anymore with 2.0.0.6

Comment 10

[Wayne Mery (:wsmwk)]

WFM per comment 6

Comment 11

[Ilja Sekler]

I can unmistakeably reproduce this bug with a fresh Linux trunk build of Thunderbird version 3.0a2pre (2008070715), checkout start: Mo 7. Jul 15:36:23 CEST 2008 and saw it all the time on 1.8 branch builds. The bug is still there, please reopen.

A bit clearer STR:

1. move a message from folder A to folder B

2. select a message in folder B

3. right-click folder A -> Compact

Actual Results: the thread pane and the message pane are cleared.

Expected Results: neither the thread pane nor the message pane are affected by compacting another folder, the selected message is still displayed.

Comment 12

[Wayne Mery (:wsmwk)]

Ilja, thanks for those steps. I see this on LOCAL folders version 3.0a2pre (2008070205).  Don't see this with imap

Comment 13

[David :Bienvenu]

Attachment: proposed fix

This fix does two things - if the currently displayed folder isn't getting compacted, don't clear the thread pane and message pane. And when trying to reload the folder in HandleCompactCompleted, I just deselect and reselect the current folder, so that we'll do the right thing with single folder saved searches (currently, we reload the real folder w/o the search terms, not the virtual folder).

I've attached changes for SM and TB, but I haven't tried the SM patch.

Comment 14

[[e-mail]]

(In reply to comment #13)
> This fix does two things - if the currently displayed folder isn't getting
> compacted, don't clear the thread pane and message pane.
OK, this sounds reasonable.

> And when trying to reload the folder in HandleCompactCompleted, I just
> deselect and reselect the current folder, so that we'll do the right
> thing with single folder saved searches (currently, we reload the real
> folder w/o the search terms, not the virtual folder).
But isn't deselecting and reselecting the folder generating a lot of unwanted extra work, such as checking for new messages?

Comment 15

[David :Bienvenu]

well, we will check again for new messages, but since we generally have a cached connection to that folder, it 's just a IMAP NOOP, not a full folder select and flag sync.

But you're right; we should only do this in the case of non imap folders, which is the only case when we clear the thread pane anyway...

Comment 16

[David :Bienvenu]

Attachment: only refresh selection for non-imap folders

only do this for non-imap folders.

Comment 17

[[e-mail]]

Comment on attachment 330519 [details] [diff] [review]
only refresh selection for non-imap folders

IMHO it still sucks that the DB view can't be updated invisibly - it's not as if the list of messages changes, but the logic is so twisted I can't even see where the search terms get set, let alone cleared.

>+  if (folder  && folder.server.type != "imap")
Nit: doubled space before &&

>+      // force the selection to change, to reselect the current folder.
>+        var folderSelection = GetFolderTree().view.selection;
Nit: indentation of comment seems wrong

>+          folderSelection.select(curFolderIndex);
>         LoadCurrentlyDisplayedMessage();
Presumably this reroots the folder synchronously so that you can redisplay?

Comment 18

[David :Bienvenu]

There are bigger problems than where the search terms get set - compacting local folders changes the message keys (which are integral to the view), and more importantly, closes the db, deletes it, and copies a new db over the old db. In order to update the view invisibly, we'd need two methods on the view - one to prepare for update, where it would let go of the db and any msg hdrs, and one to reload itself once the new db was created. These methods would look an awful lot like close and open, followed by re-issuing the search, for saved searches. Alternatively, we could rewrite the way compact works so that it works "in place" for the db, but that would complicate (or at least, change significantly) error handling.

The way saved searches set and run their search terms is very odd, I agree. It's all done in js. The c++ view classes are listeners and not instigators. The search terms are set up in FolderPaneSelectionChange() by setting gVirtualFolderTerms or gXFVirtualFolderTerms. They're executed in onEnterInSearchBar(), which allows us to combine saved searches with views and quick searches, which the c++ classes don't know about. 

>Presumably this reroots the folder synchronously so that you can redisplay?
Yes, it does.

Comment 19

[[e-mail]]

(In reply to comment #18)
> The search terms are set up in FolderPaneSelectionChange() by setting
> gVirtualFolderTerms or gXFVirtualFolderTerms. They're executed in
> onEnterInSearchBar(), which allows us to combine saved searches with views and
> quick searches, which the c++ classes don't know about. 
But I guess we can't just call onEnterInSearchBar ourselves instead?

Comment 20

[David :Bienvenu]

(In reply to comment #19)
> But I guess we can't just call onEnterInSearchBar ourselves instead?
> 
We can, but we'd still need to re-open the view first, so it wouldn't be any less code, or any less cpu work.

Comment 21

[David :Bienvenu]

Attachment: meet Neil half-way

re our IRC discussion, I can't call RerootFolder w/o duplicating all the work it does to figure out what the current view, sort order, sort type, etc, are. But I can pretend the selection changed, w/o monkeying with the actual selection :-)

Comment 22

[[e-mail]]

Comment on attachment 330994 [details] [diff] [review]
meet Neil half-way

>-        var msgdb = msgFolder.getMsgDatabase(msgWindow);
>-        if (msgdb)
>-        {
>-          var dbFolderInfo = msgdb.dBFolderInfo;
>-          sortType = dbFolderInfo.sortType;
>-          sortOrder = dbFolderInfo.sortOrder;
>-          viewFlags = dbFolderInfo.viewFlags;
>-          viewType = dbFolderInfo.viewType;
>-          dbFolderInfo = null;
>-        }
>-
>-        RerootFolder(uri, msgFolder, viewType, viewFlags, sortType, sortOrder);
Well, my question was really "why not just stick in an onEnterInSearchBar();"?

But think I'm OK with your latest approach.

Comment 23

[David :Bienvenu]

fixed, change set 60:c20fa1ac845c

Comment 24

[Ilja Sekler]

I'm very sad to report that I still get the thread pane (only the thread pane) cleared compacting a folder other then the selected one. Even worse: now, it doesn't happen always but only sometimes - still often enough. The chance to see it increases stongly together with the size of the MBOX file of the compacted mail folder.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b1pre) Gecko/20080920111459 Thunderbird/3.0b1pre

Would you prefer to reopen this bug or should I file a new one?