Closed Bug 516923 Opened 11 years ago Closed 11 years ago

Some folders do not appear in Smart folder view

Categories

(Thunderbird :: Folder and Message Lists, defect, P1, major)

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0rc1

People

(Reporter: gerv, Assigned: Bienvenu)

Details

(Whiteboard: [no l10n impact])

Attachments

(2 files, 1 obsolete file)

Setup:

Thunderbird 3.0b4pre on Ubuntu Linux 9.04. Two IMAP accounts - "gerv@gerv.net" and "gerv@mozilla.org", plus two feed accounts and two newsgroup accounts. 

In "All" folder view:

gerv@gerv.net account has folders "Sent" (empty) and "sent-mail" (full; defined in prefs as sent mail folder), both with stamp icon. It has folders "archive" (full; defined in prefs as archive folder) and "Archives" (empty), both with belt buckle icon.

gerv@mozilla.org account has only "sent-mail" with Stamp icon, but has same arrangement with archive folders.

The context menu does not allow me to delete or rename any of the above-mentioned folders. 

The user prefs in prefs.js seem correct, so I don't know where this "two special folders" thing is coming from.

In "Smart" folder view:

"Sent" top-level folder picks up empty "Sent" folder from gerv@gerv.net and calls it "gerv@gerv.net".

"Archives" top-level folder similarly picks up empty "Archives" folders from both accounts.

"Drafts" top-level folder has picked up the gerv@gerv.net defined Drafts folder ("Drafts-gerv@gerv.net", which is actually in Local Folders) but for some reason the parallel "Drafts-gerv@mozilla.org" folder remains a top-level folder in this view, and is not subsumed under the "Drafts" top level.

Neither gerv@gerv.net's "sent-mail" or either "archive" folder is visible anywhere else in the folder tree. As these folders contain important messages, this could be considered dataloss. 

A screenshot is attached of the two views. Do let me know what other information I can provide.

Gerv
Gerv
we need to figure out how to handle having multiple special folders in the same account.
Flags: blocking-thunderbird3+
Target Milestone: --- → Thunderbird 3.0rc1
Whiteboard: [no l10n impact]
This makes it so we look for all special folders in an account. 

I've cc'ed clarkbw because I think we need some UI advice about how to present multiple special folders in the same account. For example, I can have the default Sent folder in an account, and then specify some other random folder in that same account as the sent folder for a different account. Prior to this patch, the second folder wouldn't appear at all in the smart folders view. With this patch, there will be two sub-folders of the Smart sent folder with the same account name, so the user can't distinguish between them (but at least they can see both of them). We could try to detect having multiple special folders in an account and do something like adding the folder name in parenthesis (e.g., user@server.com(Sent) and user@server.com(random sent))
Attachment #401981 - Flags: review?(mkmelin+mozilla)
Whiteboard: [no l10n impact] → [no l10n impact][has partial patch for review][needs UI input clarkbw]
(In reply to comment #3)
> We could try to detect having multiple special
> folders in an account and do something like adding the folder name in
> parenthesis (e.g., user@server.com(Sent) and user@server.com(random sent))

We could at least make it the tooltip, I think the additional text is likely to be cropped off and out of view.

Is there some way to get out of this situation? i.e. right click, [x] Sent Folder, uncheck.
Right now we use the tooltip to give the new msg summary for the folder. I suppose we could show the folder name if there aren't any new messages, which would be the common case for everything but the Inbox.

The way out of this situation for the user is not intuitive - for every folder that has a special folder flag inappropriately set, they must first set it as the special folder in account settings, then set a different folder as the special folder.

We could add code to verify that every folder that has various special folder flags set is actually a special folder of that type, i.e., verify the flags against the prefs.
Priority: -- → P1
Hmm, I'm a little scared of a solution like that this far into the release schedule but that really depends on how you feel about it.

Perhaps we could always insert the additional text before the summary for the folder so that it is always visible.
(In reply to comment #6)
> Hmm, I'm a little scared of a solution like that this far into the release
> schedule but that really depends on how you feel about it.
It's a little scary.
> 
> Perhaps we could always insert the additional text before the summary for the
> folder so that it is always visible.
I'll code that up.
Maybe i'm missing something, but why wouldn't we just have it show which account it is a special folder for.

So

 Sent 
     Account1          <-- Sent on Account1
     Account2          <... e.g. MoreSent on Account1 (special sent for account2)
     ...

Or why would that be a problem (ui-wise)?
We could do that if there are duplicates on the same account, though Bryan was worried it would be too long and the account name might not be completely visible.

I kinda prefer that solution to the tooltip one, personally.
I guess my question was more of: why would it look any different than normal?
(In reply to comment #10)
> I guess my question was more of: why would it look any different than normal?

Yeah, I was worried it wouldn't be seen at all because of the length but you make a good point.  We don't need to do the tooltip change if we make this change instead.
fortunately, the code already knew how to do this for unread/favorites views
Attachment #401981 - Attachment is obsolete: true
Attachment #402183 - Flags: review?(mkmelin+mozilla)
Attachment #401981 - Flags: review?(mkmelin+mozilla)
Status: NEW → ASSIGNED
Whiteboard: [no l10n impact][has partial patch for review][needs UI input clarkbw] → [no l10n impact][has patch for review]
Duplicate of this bug: 518348
I still don't understand why this needs special casing.

With a pop and an imap account, i set the Sent folder of two pop-identities to be on the imap account (folders otheraccount_sent and other2_account_sent)

With patch: http://imagebin.ca/view/M0rY-o5.html
Without: http://imagebin.ca/view/TP1yDRm.html

Why is it a problem like as is? (Except that it only lists one of the folders.)
With the patch it says on which server the folder is - but that's irrelevant imo. You just want to know which account it belongs to. The only case the "folder - account" name is needed is if (due to multiple identity sent settings) end up with several folders named the same.
If I have two sent folders on the same account, whether because of a bug, or deliberately, we will show just the account twice (assuming the first patch), and I won't know what folder it is. In other words, take my first patch, apply it, and then go to account 2, set it's sent folder to some random folder on account 1, and the sub-folders of the smart sent folder will show acount1 twice.
let me try a little ascii art. If I have two sent folders in an account foo@bar, from two identities, I'll see the following, with my first patch:

Sent
  foo@bar
  foo@bar

With the second patch, I'll see

Sent
  Sent - foo@bar
  random sent - foo@bar

So the user can tell that the folder name is, as well as the account it's in (though not the account/identity it's for).
Comment on attachment 402183 [details] [diff] [review]
show "folder - account" for duplicate folders w/ special flags

Ah, i see now.

>+     let foldersWithFlag = acct.incomingServer.rootFolder.getFoldersWithFlags(aFolderFlag);
>+     if (foldersWithFlag.length > 0) {
>+       for each (folderWithFlag in fixIterator(foldersWithFlag.enumerate(),
>+                                               Components.interfaces.nsIMsgFolder)) {
>+         folders.push(folderWithFlag);
>+        // add sub-folders of Sent and Archive to the result.
>+        if (deep && (aFolderFlag & (nsMsgFolderFlags.SentMail | nsMsgFolderFlags.Archive)))
>+          this.addSubFolders(folderWithFlag, folders);
>+        }

Indention is off (and please capitalize Add).

But why is it only sent and archive, not the other special folders?

>       }
>     }
>     return folders;
>   },
> 
>   _smartFoldersGenerator: function ftv_smartFoldersGenerator(ftv)
>   {
>     let map = [];
>@@ -890,26 +893,35 @@ let gFolderTreeView = {
>       map.push(smartFolderItem);
>     else
>       map[position] = smartFolderItem;
>     // Add the actual special folders as sub-folders of the saved search.
>     // By setting _children directly, we bypass the normal calculation
>     // of subfolders.
>     smartFolderItem._children = [new ftvItem(f) for each (f in subFolders)];
> 
>-    // sortFolderItems(this._children);
>+    let prevChild = null;
>     // Each child is a level one below the smartFolder
>     for each (let child in smartFolderItem._children) {
>       child._level = smartFolderItem._level + 1;
>       child._parent = smartFolderItem;
>       // don't show sub-folders of the inbox, but I think Archives/Sent, etc
>       // should have the sub-folders.
>       if (flag & nsMsgFolderFlags.Inbox)
>         child.__defineGetter__("children", function() []);
>-      child.useServerNameOnly = true;
>+      // If we have consecutive children with the same server, then both
>+      // should display as folder - server.
>+      if (prevChild && (child._folder.server == prevChild._folder.server)) {
>+        child.addServerName = true;
>+        prevChild.addServerName = true;
>+        prevChild.useServerNameOnly = false;
>+      }
>+      else
>+        child.useServerNameOnly = true;

Braces for else too.

r=mkmelin with that sorted out
Attachment #402183 - Flags: review?(mkmelin+mozilla) → review+
fix checked in. Re why it's only sent and archive, not the other special folders, I haven't changed anything there. It doesn't affect the folders you see in smart folders mode, but it will affect folders are included in the top level smart folder contents, i.e., the scope of the saved search.  Sent/Archives are the ones where I think it's important to see the contents of the sub-folders in the top-level smart folder.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [no l10n impact][has patch for review] → [no l10n impact]
You need to log in before you can comment on or make changes to this bug.