When I try to create a search folder using maildir, it creates the .msf file. If I restart, that search folder is not found. mbox seems to create an empty directory entry with the name of the search folder. If I manually create this for a maildir search folder, then the folder is now recognized, but it is not recognized as a search folder, so it appears empty.

As a workaround, I can create a dummy POP3 mbox local store account and store saved searches there. The saved searches can then search and display maildir folders from other accounts just fine.

So this turns out to be a little different than I expected. The problem is not creating virtual folders, it is deleting them. When you try to delete a virtual folder under maildir, the info is deleted from the virtualFolders.dat file, but the .msf file does not get deleted, and the folder continues to show in the folder tree. Upon restart, the folder no longer shows - but you cannot create a new one with the same name because the existing .msf file opens as invalid.

Fix was simple, I added also virtual folder deletes to a test so that I get a test that fails without the fix.

Bug 1124105 is probably not needed for this fix, but these patches were done on top of that bug. You'll need it if you want to apply this patch.

Comment on attachment 8553917 [details] [diff] [review] fix and test I'm going to remove the review request for now. Although this fix works fine in my test profile, I am still having problems creating virtual folders in maildir in my main profile.

Turns out there was also a problem creating folders as well as deleting them. I'd fixed that in the dist folder of the build but not the source, then forgot about it when I started to focus on the delete problem. Simple change though.

Comment on attachment 8554031 [details] [diff] [review] with create issue solved as well Review of attachment 8554031 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/base/test/unit/xpcshell_maildir.ini @@ +15,5 @@ > +#[test_viewWrapper_virtualFolderCustomTerm.js] > +#run-sequentially = Avoid bustage. > +#[test_windows_font_migration.js] > +#skip-if = os != "win" > +#[test_mailGlue_distribution.js] What's with all the commented-out lines? ::: mailnews/local/src/nsLocalMailFolder.cpp @@ +778,5 @@ > + NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Could not delete msg summary file"); > + > + rv = msgStore->DeleteFolder(this); > + if (rv == NS_ERROR_FILE_NOT_FOUND) > + rv = NS_OK; // virtual folders do not have a msgStore file When I see stuff like this, I'm reminded that we really ought to have a separate implementation of nsIMsgFolder for virtual folders.

(In reply to Joshua Cranmer [:jcranmer] from comment #7) > Comment on attachment 8554031 [details] [diff] [review] > with create issue solved as well > > Review of attachment 8554031 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mail/base/test/unit/xpcshell_maildir.ini > @@ +15,5 @@ > > +#[test_viewWrapper_virtualFolderCustomTerm.js] > > +#run-sequentially = Avoid bustage. > > +#[test_windows_font_migration.js] > > +#skip-if = os != "win" > > +#[test_mailGlue_distribution.js] > > What's with all the commented-out lines? > They are there as a reminder that these tests are specifically not being run for maildir. But I see I did not do the same thing when I added a maildir manifest in imap, so I should remove them. > ::: mailnews/local/src/nsLocalMailFolder.cpp > @@ +778,5 @@ > > + NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Could not delete msg summary file"); > > + > > + rv = msgStore->DeleteFolder(this); > > + if (rv == NS_ERROR_FILE_NOT_FOUND) > > + rv = NS_OK; // virtual folders do not have a msgStore file > > When I see stuff like this, I'm reminded that we really ought to have a > separate implementation of nsIMsgFolder for virtual folders.

Comment on attachment 8554031 [details] [diff] [review] with create issue solved as well I've got another patch coming that incorporates comments as well as fixes on small issue in the tests.

See try run https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=da34039a1436 Those errors are also on trunk.

(In reply to Kent James (:rkent) from comment #2) > So this turns out to be a little different than I expected. > The problem is not creating virtual folders, it is deleting them. > When you try to delete a virtual folder under maildir, the info is deleted from the virtualFolders.dat file, > but the .msf file does not get deleted, and the folder continues to show in the folder tree. > Upon restart, the folder no longer shows - but you cannot create a new one with the same name > because the existing .msf file opens as invalid. Delete virtual folder by whom? Manual delete of Virtual folder? Or delete by Virtual Folder himself? Virtual Folder's design/implementation. If search target folder is not found, remove it from definitin of a Virtual Folder in virtualFolders.dat. If no search target folder, delete the Virtual Folder. This occurs upon rename/move/delete(both move to trash and immediate remove) of Search Target dolder. move/rename is not tracked. If Virtuaal Folder under MaildirStore fails to find Search Target Folder upon restart, above occurs.

Comment on attachment 8577454 [details] [diff] [review] fix tests on Linux Review of attachment 8577454 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/base/test/unit/test_viewWrapper_virtualFolder.js @@ +333,5 @@ > > // now the view wrapper should have closed itself. > do_check_eq(null, viewWrapper.displayedFolder); > + // This fails because virtFolder.parent is null, not sure why > + //virtFolder.parent.propagateDelete(virtFolder, true, null); This test seems to pass if you uncomment this line. ::: mailnews/db/msgdb/src/nsMailDatabase.cpp @@ +116,5 @@ > > NS_IMETHODIMP nsMailDatabase::SetSummaryValid(bool aValid) > { > nsMsgDatabase::SetSummaryValid(aValid); > + EWHITESPACE

Comment on attachment 8577454 [details] [diff] [review] fix tests on Linux We'll take on comm-central after a nightly cycle. https://hg.mozilla.org/comm-central/rev/232a61c2aefa

> > + // This fails because virtFolder.parent is null, not sure why > > + //virtFolder.parent.propagateDelete(virtFolder, true, null); > This test seems to pass if you uncomment this line. This still fails for me on windows, so I left the line commented out.

Comment on attachment 8577454 [details] [diff] [review] fix tests on Linux http://hg.mozilla.org/releases/comm-aurora/rev/bc07ed1b0df8