553633 - [PATCH] adding smart folder with space in name fails

Status: RESOLVED DUPLICATE of bug 553747

(Thunderbird :: Folder and Message Lists, defect)

Description

[Shane Caraveo (:mixedpuppy)]

I'm adding a new smart folder type with the following code:

let smartMode = gFolderTreeView._modes["smart"];
smartMode._flagNameList.push([Components.interfaces.nsMsgFolderFlags.Unused2,
                              "Mailing Lists", false]);

This works for inserting a new type and having the folder created, however folderPane.js fails to find the folder after the initial creation, causing a failure to display the unified folders pane.  patch to follow.

Comment 1

[Shane Caraveo (:mixedpuppy)]

Attachment: fix finding a virtual folder in smart folders account

Comment 2

[Shane Caraveo (:mixedpuppy)]

Attachment: fix dynamically adding new smart folder types

There were a few more areas that had to be addressed to support adding new virtual folder types

Comment 3

[Bryan Clark (DevTools PM) [:clarkbw]]

sid, do you have time to take a look at the patch, attachment 433590 [details] [diff] [review]

Comment 4

[Shane Caraveo (:mixedpuppy)]

Attachment: fix dynamically adding new smart folder types (hg diff)

Comment 5

[Shane Caraveo (:mixedpuppy)]

also see bug 553747, where I'm looking into ability to add new smart folder types easily without having to muck around with nsMsgFolderFlags.idl.

Comment 6

[Rain [:rain1] (inactive)]

Comment on attachment 433599 [details] [diff] [review]
 fix dynamically adding new smart folder types   (hg diff)

Thanks for fixing this!

>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
>@@ -959,18 +959,17 @@ let gFolderTreeView = {
>       else
>         map[position] = folderItem;
>       // No smart folder was added
>       return null;
>     }
> 
>     let smartFolder;
>     try {
>-      let folderUri = smartRootFolder.URI + "/" + folderName;
>-      smartFolder = smartRootFolder.getChildWithURI(folderUri, false, true);
>+      smartFolder = smartRootFolder.getChildNamed(folderName);

getChildWithURI's third parameter is true, which implies a case-insensitive search. However I don't think getChildNamed is case insensitive -- could you double-check that this won't cause problems? I don't think there should be any, since I believe we've been using the same capitalization everywhere all along.

>+    let specialFlags = gFolderTreeView._modes["smart"]._allFlags;

So I'm fine with this here, but looking at bug 553747 it seems like we'll need a method for this -- extensions really shouldn't be poking around in underscored stuff if they can avoid it. getFolderTreeMode(aCommonName) maybe?

Comment 7

[Rain [:rain1] (inactive)]

Comment on attachment 433599 [details] [diff] [review]
 fix dynamically adding new smart folder types   (hg diff)

This should be trivially testable with Mozmill, so by our test policy it'll need one.

Comment 8

[Shane Caraveo (:mixedpuppy)]

(In reply to comment #6)

> >     let smartFolder;
> >     try {
> >-      let folderUri = smartRootFolder.URI + "/" + folderName;
> >-      smartFolder = smartRootFolder.getChildWithURI(folderUri, false, true);
> >+      smartFolder = smartRootFolder.getChildNamed(folderName);
> 
> getChildWithURI's third parameter is true, which implies a case-insensitive
> search. However I don't think getChildNamed is case insensitive -- could you
> double-check that this won't cause problems? I don't think there should be any,
> since I believe we've been using the same capitalization everywhere all along.


Looking at nsMsgDBFolder.cpp#3329, it is using a case insensitive comparison.

Comment 9

[Shane Caraveo (:mixedpuppy)]

this bug was fixed along with some extension work