495020 - Unread Folder population causes focused folder to change

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect)

Description

[John L. Galt]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1pre) Gecko/20090526 Firefox/3.5b4pre (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1pre) Gecko/20090526 Shredder/3.0b3pre

I participated in the discussion of https://bugzilla.mozilla.org/show_bug.cgi?id=466644 extensively, until a patch was written to fix the issue.

However, after the patch was introduced into the current nightly codebase, a new error has emerged, and even though I am almost absolutely sure this is because of said patch, I was instructed to create a new bug since this is a new issue.

cf: https://bugzilla.mozilla.org/show_bug.cgi?id=466644#c42

Using all builds since this fix was added to the codebase, including the most
current one, Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1pre)
Gecko/20090525 Shredder/3.0b3pre:

If I have a folder selected in the unread folders view, and Tb starts checking
mail, and folders appear above the currently selected folder in the tree, the
view remains the same.  However, if folders appear after the currently selected
folder in the tree, then for some reason the currently selected folder moves on
its own accord to the next one lower in the tree.

This is reproducible 100% on my machine.

Stats:  Windows 7 64bit RC1, Tb build as noted above, running on a Quad Core
6600 OC'd to 3.21 GHz w/ 4 GB RAM (1000MHz, PC8000) installed.

Thunderbird is running a combination of IMAP, POP and Webmail for access to 12 different accounts (Not 12 different user profiles, 12 accounts for the *same* user profile).

Reproducible: Always

Steps to Reproduce:
1.  Run Tb, change to unread folders view, select a folder randomly (easiest manifestation is to select the top folder, as this will then reproduce a lot more quickly).
2.  Check mail.  
3.  
Actual Results:  
If any folders appear *above the currently selected folder in the tree, focus will not change.  If any folders appear *below* the currently selected folder, the selection focus will move to the next folder in the tree - and not only does the selection focus move, the actually view in the Preview changes to accommodate the currently selected *message* in the newly selected folder.

Expected Results:  
The selection focus should not change upon checking mail in the unread folder view.  I can think of one *possible* exception, and that would be if you have marked everything in the currently selected folder as 'Read' - in which case, upon a new mail check Tb could intelligently realize that the folder no longer applies to the current view and would remove it from view.....

I do not use a theme, and this does not cause a 'crash'.  Obviously, as noted from bug 466644 above, this did not occur before, b/c Tb 3 was simply not updating the user folders view at all.  So, this only started occurring after the patch from 466644 was introduced into the codebase about a week ago or so.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

cc'ing all cc'ed folks from bug 466644

Comment 2

[Aurimas]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1pre) Gecko/20090525 Lightning/1.0pre Shredder/3.0b3pre

I can confirm the same behavior.

Comment 3

[Wayne Mery (:wsmwk)]

still see this in beta 3?
 http://www.mozillamessaging.com/en-US/thunderbird/early_releases/

Comment 4

[Paul Roub]

I am.  

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.1) Gecko/20090715 Lightning/1.0pre Thunderbird/3.0b3

In my case, all accounts are IMAP, with server-side filtering.  A new IMAP message shows up in a folder further down the list, and off we go.

Comment 5

[John L. Galt]

Sorry for the late reply - yes, this is still, absolutely 100% reproducible - regardless of which Tb I use (currently loading:

Shredder 3.1a1 - Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20090903 Shredder/3.1a1pre;

also testing in Tb 3.0b3 - Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.1) Gecko/20090715 Thunderbird/3.0b3;

also testing in Shredder 3.0b4pre - Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.4pre) Gecko/20090903 Shredder/3.0b4pre

I have a supposition, though.  I am currently allowing Tb to manage my filtering, even on IMAP accounts, and I have just started foraying into the world of GMail filters right in the app itself - I am going to try to run Tb tomorrow after moving all filters to server side to see if it makes a difference in terms of how the unread folder operates - perhaps it is not the appearance of the messages themselves, but the quick appearance and then filtration of messages from the Inbox to other locations that may be causing this....

Comment 6

[Paul Roub]

>  I am going to try to run Tb
> tomorrow after moving all filters to server side to see if it makes a
> difference in terms of how the unread folder operates

Unlikely - see my comment (#4).  *All* of my filters are server-side, and I see this too.

Comment 7

[John L. Galt]

Hmmm - I only killed the filters for the 3 GMail accounts that I had in Thunderbird, and even in spite of your comment, Paul, this time it did not happen at all.

All filters for other accounts are still in place.  However, I was up pretty late last night building server side filters, and am only partially completed, so a lot of my previously filtered mail was dumped in the inbox, so it could also be the fact that not all my filters are in place in the first place.  I'll continue to build the filters and hopefully by late Sat I should be able to verify again on my end.

Comment 8

[Wayne Mery (:wsmwk)]

I don't recall ever seeing anything causes a focus change in folders, other than explicit selection. Do you see this started in safe mode? http://support.mozilla.com/en-US/kb/Safe+Mode

Comment 9

[Paul Roub]

Yes, I just tested it in safe mode.  Same result.

Comment 10

[John L. Galt]

I am unsure as to why this is marked unconfirmed - is it possible that Paul and I are the only ones seeing this b/c of some ironic configuration setting?

I have converted all of my GMail account to server side filtering and this still occurs on a regular basis....

Comment 11

[Carson Gaspar]

I can confirm this same behaviour on:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.5pre) Gecko/20091026 Shredder/3.0pre

This makes the current builds unusable, as emails vanish from the window while I'm reading them. And if it was the last unread email in that folder, I now have to go back to the full folder list and navigate back to where I was.

Comment 12

[Berry Kercheval]

I can confirm it too on:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.5) Gecko/20091204 Thunderbird/3.0

This is the released image!

Comment 13

[Wayne Mery (:wsmwk)]

gmail users, can anyone confirm this scenario which might be related?  and sid0, any thoughts ...

1. Be in All Folders
2. get a new gmail message
3. folder view changes to "Smart Folders"

Comment 14

[John L. Galt]

Wayne, LTNS - I'll get back to the queue in the next couple of days....

As to this particular scenario:

Are you referring to being in the <b>All Mail</b> folder?  If so, I currently have those disabled from shwoing in Thunderbird (<b>Settings --> Labels --> All Mail --> <i>uncheck </i>Show in IMAP</b>)

If that is what you meant, I'll enable and test.

Comment 15

[Wayne Mery (:wsmwk)]

I mean that the folder pane's header is All folders, and not smart folders.

Comment 16

[John L. Galt]

Oh - no, this is in *Unread* Folders.

Comment 17

[John L. Galt]

(In reply to comment #16)
> Oh - no, this is in *Unread* Folders.

NM.  I answered before I remembered what was going on - I'll check out this and see if I can reproduce.

Comment 18

[Paul Roub]

Attachment: Screen shot, prior to the unwanted folder change

One of three. A single message in "folder 1" is selected and previewed,

Comment 19

[Paul Roub]

Attachment: Screen shot, adding a message to folder 2 on the server

Adding a simple message, via Panda IMAP's dmail command, which is what normally delivers my mail to the folders.

Comment 20

[Paul Roub]

Attachment: Screen shot, selection is lost.

Another account's new folder didn't seem to trigger the change in the meantime

Comment 21

[Paul Roub]

Attachment: Thunderbird IMAP logs during and after delivery of the new message

Tossed irrelevant stuff, just grepped for "folder 2" with 7 lines of context before/after.  All the notifications/etc. are there.

Comment 22

[Emilio Pozuelo Monfort]

I have this issue too in 3.0.3 on Linux.

Comment 23

[John L. Galt]

Any new details on this?  This is the most annoying bug I have run into in Thunderbird - I basically have to let it completely synchronize all of my IMAP accounts before I can start processing mail.  Otherwise, as I am attempting to click on an unread mail, TB is changing the focus from one folder to another and in the process I either open a different email entirely, or else it freezes as it tries to sort out just where exactly I am trying to (erroneously) go....

Comment 24

[John L. Galt]

Also, currently using v 3.0.3 final, plus 3.1 beta *and* 3.2a1 - all three are exhibiting this behavior.  OS and other information have to changed from above (other than yet another re-install to allow for upgraded hardware).

Comment 25

[David :Bienvenu]

confirming

Comment 26

[David :Bienvenu]

A little more detail - when a folder is newly marked as having unread, that causes a complete rebuild of the unread folders view, which removes all the folders that no longer have unread. rebuilding attempts to maintain the selected folder, but it seems to be failing miserably.

Comment 27

[David :Bienvenu]

OK, I think this code is the issue - 
      if (oldCount !== null)
          this._tree.rowCountChanged(0, this._rowMap.length - oldCount);

It basically updates the row count after rebuilding  by saying that rows were added or removed at row 0, which is not usually the case, and completely messes up the tree's selected row.

The easiest way to fix this is to avoid getting into the rebuild code in this case. We can simply add the newly unread folder to the view. This would mean that folders no longer with unread messages would stay in the unread view, which is analogous to what we do for unread messages only views. Or, we could update the view by iterating over the folders and removing folders no longer with unread messages one by one, instead of rebuilding the view. Or we could make _rebuild just save and restore the selected folder instead of letting the tree do it, and lying to the tree about what changed.

Comment 28

[David :Bienvenu]

Attachment: make rebuild maintain the selected folders

imap logs irrelevant, so I've obsoleted it. This patch fixes the bug. I'll try to come up with a mozmill test for this, though mozmill tests can't get new mail.

Comment 29

[David :Bienvenu]

I'm tempted to mark this blocking since I don't know how anyone would live in that view with this bug, but I'll just leaved it at wanted for now, since there haven't been dupes of this, afaik.

Comment 30

[Berry Kercheval]

I live with it because
1) I REALLY like this view
2) I set the update time pretty large to minimize this bug's effect
3) It's only REALLY annoying when I've just selected the last unread message in a folder, and the bug removes the folder and the open message from the view before I can read it.

If there are still unread messages, the folder stays in the view and if I re-select it the message I was reading comes back.

Still, I'm glad it's fixed and look forward to the next release.

Comment 31

[David :Bienvenu]

Attachment: fix with mozmill test

this adds a mozmill test that fails without the folderPane.js change, and succeeds with it.

Comment 32

[David :Bienvenu]

Berry, I haven't seen your #3 issue, but that could be because I have TB set to remember the last loaded message in a folder (which is the default).

Comment 33

[Berry Kercheval]

David, If I go back to the "All Folders" view and select the folder I was in, I do get the last loaded message back; having to do that and then go back to "Unread Folders" is the annoying part.

Thanks for fixing this.  I looked for folderPane.js on my machine and couldn't find it.  I admit I know almost nothing about the innards of Thunderbird though, since I stopped hacking mail clients after exmh.  If it's easy to find (in some zip file or something) and I can patch it easily let me know, otherwise I'll wait for the next release.

Comment 34

[John L. Galt]

David, thanks for the replies and confirmation.  Glad to know that a new patch is in the works.

Like Berry, I'd like to know if there is a way to easily test this patch without having to compile a new personal build by myself - not currently set up for compiling builds.

If not, I, too, will wait on the next release.

Comment 35

[David :Bienvenu]

pinging for review. Re testing, there's no easy way to test this patch w/o having your own build. You can repackage the jar file with a changed version of folderPane.js, but it's not trivial.

Comment 37

[Rain [:rain1] (inactive)]

Comment on attachment 434963 [details] [diff] [review]
fix with mozmill test

This is an r- because at startup, the getSelectedFolders() call has somewhat hazy semantics. One way to fix it would be for getSelectedFolders() to return [] if no selection is available (and please also fix this._treeElement.view.selection to be this.selection!)

>diff --git a/mail/base/content/folderPane.js b/mail/base/content/folderPane.js
>--- a/mail/base/content/folderPane.js
>+++ b/mail/base/content/folderPane.js
>@@ -1135,32 +1135,44 @@ let gFolderTreeView = {
>       newRowMap = this._modes[this.mode].generateMap(this);
>     } catch(ex) {
>       Components.classes["@mozilla.org/consoleservice;1"]
>                 .getService(Components.interfaces.nsIConsoleService)
>                 .logStringMessage("generator " + this.mode + " failed with exception: " + ex);
>       this.mode = "all";
>       newRowMap = this._modes[this.mode].generateMap(this);
>     }
>+    let selectedFolders = gFolderTreeView.getSelectedFolders();

this.getSelectedFolders();

>+    if (this.selection)
>+      this.selection.clearSelection();

This is an nsITreeView (so the tree element sets the selection attribute on it), but I'd really like to see a selection: null somewhere.

>+    // restore selection.
>+    for (let i = 0; i < selectedFolders.length; i++) {

Using an iterator for this is the in thing these days.

>+function test_newly_added_folder() {

A doc comment for this would be great.

Comment 38

[David :Bienvenu]

Attachment: fix addressing comments

I think this addresses your comments...

Comment 39

[Rain [:rain1] (inactive)]

Comment on attachment 436372 [details] [diff] [review]
fix addressing comments

Looks good, thanks.

>+    // restore selection.
>+    for each (let folder in selectedFolders){

This should be
for (let folder in Iterator(selectedFolders)) {

since we don't want to iterate over properties that evil extensions might add to Array's prototype, and Iterator protects against that.

Comment 40

[David :Bienvenu]

Attachment: patch for checkin

this is what I'll land on the trunk, then. Thx!

Comment 41

[David :Bienvenu]

fixed on trunk.

Comment 42

[David :Bienvenu]

Comment on attachment 436413 [details] [diff] [review]
patch for checkin

nominating for 3.05

Comment 43

[Rain [:rain1] (inactive)]

Attachment: bustage fix

Sorry for messing the iterator syntax up -- I always forget that it returns key-value pairs. :(

This will need to be approved along with the main patch for checkin.

Comment 44

[Rain [:rain1] (inactive)]

Pushed yet another bustage fix for the remaining test failure: http://hg.mozilla.org/comm-central/rev/f4330a8f5022

This doesn't cleanly apply on 1.9.1, though.

Comment 45

[Rain [:rain1] (inactive)]

Attachment: patch that applies cleanly on 1.9.1

Comment 46

[Rain [:rain1] (inactive)]

http://hg.mozilla.org/releases/comm-1.9.1/rev/2cf8bdbe35c3

Comment 47

[Alexander Kriegisch]

Sorry if this is a dumb question, but is this bug also fixed in TB 3.1? In the bug list for RC1 I cannot find it:
https://bugzilla.mozilla.org/buglist.cgi?field0-0-0=cf_status_thunderbird31;query_format=advanced;type0-0-0=equals;value0-0-0=rc1-fixed

Maybe it is just because I am unexperienced in using Bugzilla and was summoning up the wrong list. I also do not understand what "status-thunderbird3.1: wanted" means.

Comment 48

[Rain [:rain1] (inactive)]

This was fixed in beta 2, in fact! Looks like we missed setting the flag, oops.