200989 - nsImapProtocol::DiscoverMailboxList can't list all folders

Status: RESOLVED DUPLICATE of bug 165161

(MailNews Core :: Networking: IMAP, defect)

Description

[Calvin Liu]

Hi, Bienvenu,

A piece of code in this method like this.
...
if (usingSubscription) // default, 
//set the option "Show only subscribed folders" checked.
  {
    pattern.Append(prefix);
    pattern.Append("*");
  }
else //set the option "Show only subscribed folders" unchecked.
  {
    pattern.Append(prefix);
    pattern.Append("%"); // mscott just need one percent right?
    // pattern = PR_smprintf("%s%%", prefix);
    char delimiter = ns->GetDelimiter();
    if (delimiter)
    {
      // delimiter might be NIL, in which case there's no hierarchy anyway
      pattern2 = prefix;
      pattern2 += "%";
      pattern2 += delimiter;
      pattern2 += "%";
 // pattern2 = PR_smprintf("%s%%%c%%", prefix, delimiter);
    }
}
                                                                               
                                                                              ...
Don't know why we assign pattern2 a value like "prefix%delimiter%" here, but not
"prefix*" or something like that. I guess there must be some reasons. But what's
that?

Currently, if user has the option "Show only subscribed folders" unchecked,
he'll see only 2 level folders. If he wants to see more, he has to expand the
tree manually. Sounds inconvenient, right?

Calvin

Comment 1

[Calvin Liu]

Attachment: patch for testing

Please review or give some feedback. I'll make the patch better. Thanks.

Comment 2

[Calvin Liu]

*** Bug 200988 has been marked as a duplicate of this bug. ***

Comment 3

[Calvin Liu]

Forget the repro desc!!!
Reproduce steps:
1. login to your mail account, create some nested folders, more than 4 levels, like
a1
a1/a2
a1/a2/a3
a1/a2/a3/a4
2. open mail client, go to [edit]->[mail and newsgroups preferences]-><your
server account>->[server settings]->[Advance]
3. uncheck "show only subscribled folders"
4. restart mozilla and relogin to your mail account
You'll find only 2 level folders showed.

Comment 4

[Henry Jia]

This is for performance. We can't let mozilla hold for too long for access the
whole folders.

Comment 5

[Calvin Liu]

Yes. I guess so.

Comment 6

[Calvin Liu]

Attachment: workaround, set a trigger in prefs.js

For special cases if user wants to list all folders.
Need to set "mail.enable_expand_all" as "true" in prefs.js file.
r=?

Comment 7

[Lars Uffmann]

Could this be related to these two similar bugs?

http://bugzilla.mozilla.org/show_bug.cgi?id=138759
http://bugzilla.mozilla.org/show_bug.cgi?id=121104

Both are unresolved since allmost a year.

Lars

Comment 8

[Lars Uffmann]

Add /me to Cc: List

Comment 9

[David :Bienvenu]

ah, the reason this happens is because you created your folder hierarchy and
then turned off subscription, which should be a rare case.

The reason we don't list all folders is that it can be very hard on the server
and slow. There's no reason to list all the sub-folders, if we list folders when
you expand the parent. Why would you want to list all the folders? I find it
much cleaner to have the parents collapsed, so it's easier to find the top-level
folders. There was a bug caused by the fact that parent folders used to show up
by default as expanded, so we'd never list the children...

I don't really like having a hidden pref for this. We should be figuring it out
ourselves. The problem is that on startup we're not listing the children of
folders that are already expanded on startup (as determined by the remembered
expanded/collapsed state of the folder).  I'm not sure how 4.x worked in this
respect. Once we started using subscription as the default, hardly anyone used
non-subscription. And there was a while when 4.x didn't remember the
expand/collapse state of folders.

All that being said, if it's too hard to fix this the right way (only listing
children/grand-children of expanded folders), we could just list *, as you
suggest, since not using subscription should be the exception.

Comment 10

[Lars Uffmann]

Hi, Bienvenu,

for most IMAP servers it is a fairly cheap operation to list all subfolders.
For Maildir++ based servers (courier) it is a simple readdir. Other servers
as Cyrus use a BerkelyDB to look up folders. I'm not sure for UW though.

> Why would you want to list all the folders?

I would be nice to have a "expand/collapse all" Option. Either for _all_
folders, or for a specific subfolder. Also, the subscribe dialog could
benefit from a "expand all" feature. Also, on startup the Mailfilter engine
needs to know if a folder, which is target of a filter rule, does exist or
not.

> And there was a while when 4.x didn't remember the
> expand/collapse state of folders

After startup, Mozilla keeps track of the "expand/collapse" state until the
IMAP session is initialized. After the session is opened, the folder tree is
collapsed! This is with "show only subscribed folders" disabled.
Then you have to deal with Bug 138759: Folders are marked as having no
subfolders until the parent folder is once collapsed and the re-expanded.

> we could just list *, as you suggest, since not using subscription should be
> the exception

Not using subscription is what users expect. The first question users comming
from Netscappe 4.X ask is: "Where are my folders!?". It is the most frequently
asked question in our internal mail FAQ. IMHO subscription schould be used
to selectivly hide some folders, for the advanced user, but not as the default.

Lars

Comment 11

[hjp]

Re comment #9
> There's no reason to list all the sub-folders, if we list folders when
> you expand the parent. Why would you want to list all the folders?

1) Folding/unfolding the parent is rather unintuitive.

2) Mozilla forgets which all info about folders on login, so the user has to
unfold those he wants to see and fold those doesn't want to see.

3) And this is IMHO the biggest problem: Mozilla thinks the folders aren't there
and disables all filters which try to access them.

(And btw I also think this is a dup of bug 138759)

Comment 12

[David :Bienvenu]

re the last comment, I'm not suggesting that the user collapse and expand the
parent folder to see if the children have children. Mozilla should be doing that
automatically, by listing two levels below the expanded folders. That would NOT
require listing all folders.

LSUB is much more efficient than LIST on UW servers, among others. It just spits
out the subscribe list. LIST actually has to grovel through the directory
structure on disk.

Comment 13

[Calvin Liu]

Bienvenu,
Maybe you think it's a rare case that a user create folder hierarchy and
then turned off subscription. But if one user need to shift from one mail server
to another, in case of a mail server upgrading in a big company, this feature is
useful for the deployment guys. They can just transfer user data to the new
server and uncheck "Show only subscribed folders" with auto-config feature. Then
the user will see their folders without any trouble.
So that's a real case, no matter how rare it is. :)
I think the optional preference item in prefs.js is acceptable, right?
Calvin

Comment 14

[David :Bienvenu]

I don't like adding a hidden preference for something we should be handling
automatically in the code: list children and grandchildren of folders that we
expand, when in list mode. So I'd say, either fix it so we list children and
grandchildren, or just turn on list * all the time when in non-subscription
mode. People aren't going to discover the hidden pref.

Comment 15

[Calvin Liu]

Bienvenu,
>I don't like adding a hidden preference for something we should be handling
>automatically in the code: list children and grandchildren of folders that we
>expand, when in list mode. So I'd say, either fix it so we list children and
>grandchildren, 
What do you mean for "fix it"? Currently mozilla act as this.
>or just turn on list * all the time when in non-subscription
If we turn on list * when in non-subscription mode, there is no difference with
subscription mode, right? So the code is simpler than before. :) But the
performance issue still remains so I don't prefer this way.
>mode. People aren't going to discover the hidden pref.
Right, it's the last way. But maybe it's a good balance between satisfying
users' requirement and reducing the workload (of adding UI to handle it), since
it's rare case.
Too lazy? :)
Calvin

Comment 16

[David :Bienvenu]

So I'd say, either fix it so we list children and
>grandchildren, 
What do you mean for "fix it"? Currently mozilla act as this.

Mozilla acts this way when you, the user, expand or collapse a folder in the
folder pane. But we don't do this when you startup and some folders are
automatically expanded because they were expanded when you last shut down. That
is the whole source of this bug, and others like it.

But I may be misunderstanding what you're saying. Are you saying you want all
folders to automatically be expanded when you startup? So that your whole
hiearchy is expanded? I don't want that.

Comment 17

[Calvin Liu]

OK, can we make mozilla show a "+" in front of the 2nd level folder if it
returns "HasChildren" flag?

Comment 18

[Lars Uffmann]

Hi Calvin,

this should be working, of course. It does not if
"show only subscribed folders" is unchecked. See
for example 154778, or 138759.

"show only subscribed folders" unchecked breaks
folder-handling in Mailnews.

After all I've read in many other Bugzilla reports,
it's maybee time for an "show only subscribed folders"
Metabug. What do you think?

Lars

Comment 19

[Aleksander Adamowski]

What about limiting depth of traversal based on folders count found so far,
instead of the currently used fixed depth limit == 2 levels?

E.g. stop travesring into deeper levels if number of subfolders found so far
exceeds 512?

When people have small numbers of subfolders at depth >= 3, descending there
wouldn't mean a performance hit, and it is common that people only place a very
small number of folders below depth level 2 - the majority is usually at level 1
and 2...

Comment 20

[Calum Mackay]

Calvin's patch, in comment #1, certainly fixes for me the problem reported in
bug 138759, which suggests that these should have a dependency, if not a
duplication.

Not sure yet whether there's a performance issue; we seem to have some network
problems too :)

Comment 21

[Calvin Liu]

Lars,
It's a good idea to create a meta bug. It's bug 201332. :)
But the issue is even if mozilla just open 2 level folders, it should display a
"+" in front of the folder, not a "-". This is a reasonable requirement.
Calvin

Comment 22

[Lars Uffmann]

Hi Calvin,

I finally managed it to build the beast. Of course I applied your patch from #1
and it does fix the problem! Seems like you "hit the nail on the head". One of 
the most efficient patches I've sees so far :-)

Lars

Comment 23

[Calvin Liu]

Lars,
Thanks for encouraging!
Nice to see it works in your case. But actually this patch  doesn't meet someone
else's requirement.
I think the correct way is show a "+" before the 2nd level folder and if user
click on it, mozilla will expand next level.
Further, a key binding "*" which will expand the whole tree should work for all
tree-style widget. Don't know why it doesn't work now. Anybody can help?
Calvin

Comment 24

[David :Bienvenu]

Let me state the problem one more time. Please let me know what you don't
understand about this, so we can go from there. (The following is only for the
non-subscription case)

1. Mozilla remembers the expand/collapse state of each parent folder when we
start up, and expands parent folders that were expanded when you shut down.

2. Mozilla only lists two levels of folders on startup.

3. Mozilla removes all folders from local disk that are not listed at startup.
It thinks they are no longer on the server.

4. From 2 and 3, you can see that all folders beneath the second level are
removed on startup.

If we did not do 1), then this would not be a problem. If all folders were
collapsed at startup, then when you expanded a top-level folder by clicking on
the twisty, we would list the children and children of the children, and the
sub-folders would have the correct twisty. But, since parent folders are
expanded at startup, but DON'T go through the same code that lists children of
expanded folders (think of that code as being hooked up to the UI, not to the
backend code that expands folders on startup), we don't know that the children
folders have children of their own.

So, if you're still with me here, the possible fixes are to 

a) Always list all sub-folders, the list "*" approach.
b) At startup, if we expand a parent folder, we need to do the same list "%.%"
thing for each parent folder, as if the user had expanded the folder in the UI.

Comment 25

[Lars Uffmann]

Another solution c) would be to drop support for non-subscribed mode, if
you can't support it. Outlook does it as well. Brush up the subscribe dialog
and possible the new Account Wizard, all done.

But I would surly vote for a) because I could not follow that LIST "" "*"
bogs down server argument. Are there other arguments against solution a)?

Lars

Comment 26

[Calvin Liu]

Attachment: reproduce steps

This is the reproduce steps, "non-subscription" mode. FYI. Yesterday's trunk.

Comment 27

[Calvin Liu]

Bienvenu,

Actually, this bug is first found in mozilla 1.2.1. In that case, it even worse.
The fist access will be like step 3. So when a user login his imap account, he
may be confused, "where are my folders?" If he is not an expert or won't try
collaspe and expand the parent folders to show subfolders, he'll directly report
bug to his service provider.

I can repro it on win/linux/solaris with current trunk, like the attachment above.

Yes I understand what you mean. As I thought, the major issue is to choose a
good strategy while the minor is to show "+" correctly. That may affect multiple
layers, not only in js or xul, I guess.

Calvin

Comment 28

[Sebastian Hagedorn]

Regarding comment #25: just ask Mark Crispin ... he's the "inventor" of IMAP and the author of UW 
imapd. Doing LIST "" "*" ist inefficient as hell.

Regarding comment #24: option b) seems to be a reasonable thing to do. Are there any downsides 
to it?

Comment 29

[Robert Chien]

My vote also goes to option (a), LIST "" "*".

(In reply to comment #28)
> Regarding comment #25: just ask Mark Crispin ... he's the "inventor" of IMAP and 
> the author of UW imapd. Doing LIST "" "*" ist inefficient as hell.

Depends on your IMAP server. On Sun ONE Messaging Server 5.2, the speed difference 
between a LIST *, a LIST %, and a LSUB * is insignificant:

  LIST "" "*"  -> (0.032 sec)
  LIST "" "%"  -> (0.037 sec)
  LIST "" "%/%"  -> (0.068 sec)
  LSUB "" "*"  -> (0.123 sec)

Fact is, we should focus on fixing the client, and let the server people focus on server performance 
issue. We can't let a good patch sit in the thread based on fear that it may impact server 
performance, esp when LIST "" "*" is a valid IMAP command.

If it really causes problem, we should let Mark Crispin know so he can deprecate LIST "" "*" in the 
next IMAP standard revision. But I doubt this will be the case.

If it impacts your particular server, let the server developers know so they can enhance LIST * 
performance (like storing folder listing in a DB).

Other IMAP clients (mutt, Mail.app) also use LIST "" "*" by default, so I don't think expand_all is as 
bad as you think.

> Regarding comment #24: option b) seems to be a reasonable thing to do. 
> Are there any downsides to it?

Yes. If a user has several parent folders (e.g. A, B, C) expanded upon startup, then Mozilla will 
need to do:

LIST "" "%"
LIST "" "%/%"
LIST "" "A/%"
LIST "" "A/%/%"
LIST "" "B/%"
LIST "" "B/%/%"
LIST "" "C/%"
LIST "" "C/%/%"

which could be more expensive than a simple LIST "" "*".

Comment 30

[David :Bienvenu]

this has already been fixed in 1.7

Comment 31

[Tommy McNeely]

(In reply to comment #30)
> this has already been fixed in 1.7

What method ... "a" or "b" .. I hope we chose "a" since the inefficiency appears
to be a problem with UWIMAP in particular. See comment #10 explaining how Cyrus
uses a DB and courrier is quick.. also Sun ONE (iPlanet) products use a database
to store the mailfolder list so its quite efficeint.

Tommy

Comment 32

[David :Bienvenu]

Neither, really. We just explicitly list folders that we're thinking we should
delete before deleting them. More b than a, really - reopening to mark as dup.

Comment 33

[David :Bienvenu]

duping.

*** This bug has been marked as a duplicate of 165161 ***