350839 - SelectFolder in msgMail3PaneWindow.js fails to select a folder

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Alain Frisch]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.2) Gecko/Debian-1.5.dfsg+1.5.0.2-3 Firefox/1.5.0.2
Build Identifier: 

The function SelectFolder in msgMail3PaneWindow.js is called to open a given folder by selecting it in the tree folder view. To do this, all the ancestors of the folder need to be visible. To achieve this, SelectFold calls the function EnsureFolderIndex. However, this function is buggy.

Here is an example to demonstrate this. Assume you want to select the folder A/B/C, where A is closed but B is open. Since A is closed, A/B/C is not visible. The recursive call in EnsureFolderIndex will make B visible (with a nested recursive call to open A). But then it will toggle the state of B, assuming that it was closed. However, it is already open, and toggling the state actually closes B, making C invisible again. As a consequence, the index of C cannot be retrieved and SelectFolder fails.



Reproducible: Always




I stepped on this bug while developping the Nostalgy extension which makes it possible to open a folder by typing its name (with autocompletion). I don't know whether the bug can be observed with the plain TB interface, though I guess it can be.

The fix is trivial:

function EnsureFolderIndex(builder, msgFolder)
{
  // try to get the index of the folder in the tree
  var index = builder.getIndexOfResource(msgFolder);
  if (index == -1) {
    // if we couldn't find the folder, make all its ancestors visible
    parent_idx = EnsureFolderIndex(builder, msgFolder.parent);
    // maybe the folder is now visible
    index = builder.getIndexOfResource(msgFolder);
    // no: this means that the parent is closed, so open it.
    if (index == -1) { 
      builder.toggleOpenState(parent_idx); 
      index = builder.getIndexOfResource(msgFolder);
    }
  }
  return index;
}

Comment 1

[David Fraser]

In Nostalgy seems this also affects saving - when viewing the Folders in Favorites mode (not All Folders) you can only save to folders that are presents.

Comment 2

[Mike Cowperthwaite]

> The fix is trivial

Please generate a patch, attach the patch to this bug, and request review on it from one of the developers.

Comment 3

[Alain Frisch]

Sorry, I'm not familiar with TB development. Against which version should I produce a patch?

Comment 4

[Mike Cowperthwaite]

Trunk is most straightforward.  Patch should be a cvs diff.

Comment 5

[Joey Minta]

Attachment: mozmill test

Comment 6

[Joey Minta]

As the test shows, this was fixed by bug 414038.

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 348374 [details] [diff] [review]
mozmill test

>diff -r 72ea4a5eee81 mail/test/test_bug350839.js

Please make this mail/tests/mozmill

>+var  MC = new controller.MozMillController(mainWindow);

nit: single space please.

>+  
>+  // Let the folders actually get created and the tree rebuild
>+  MC.sleep(100);
>+  

nit: unnecessary whitespace on blank lines (several places).

r=me with those fixed.