178758 - Implement IMAP quota support (RFC 2087)

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, enhancement)

Description

[Lorenzo Colitti]

Mozilla is missing IMAP quota support.

We need both backend support for getting quota capabilities and status from the
IMAP server, and some kind of UI. To start with, this could just be a simple
addition to the IMAP folder properties. In the distant future, we could also
have warnings when folders go over quota and other such niceties.

I have started on the backend work, but I could use a help doing the front
end/UI. Also, this would be my first attempt at non-trivial mozilla coding, so I
hope someone can help me out.

Comment 1

[David :Bienvenu]

I can try to help you out, but I do more of the backend stuff and not the front
end stuff.

You probably already know this, but you want to add a flag to
eIMAPCapabilityFlag for the quota capability.

Comment 2

[Lorenzo Colitti]

Attachment: Work in progress (protocol part)

This patch adds quota support to the IMAP protocol class and IMAP parser. It
checks the server has quota capability and if it does it asks the server for
quota data every time a folder is opened.

Unfortunately, I haven't yet figured out how to get the quota information from
the parser to the class that should store it (nsImapMailFolder).

Comment 3

[David :Bienvenu]

this looks like a good start.

  nsCString quotacommand (GetServerCommandTag());

this should be nsCAutoString so that the memory will come from the stack.

Please take out the PRLog stuff you've put in (at least before checking in) -
the log will automatically capture what you need to know so you don't need to
add quota-specific logging.

It looks like you've added the call to the Quota command in the List() command
(if I'm reading the diff correctly; always an iffy proposition). I don't think
we want to do it there if you really want to update the quota information when
the user selects a folder.  I would look at the way we update the acl
information (look at calls to RefreshACLInformationIfNeccesary). Or just update
the quota information when we issue a select.

In order to get the quota information back to nsImapMailFolder, you're going to
need to add a method or two to nsIImapMailFolderSink.idl, and add the methods to
nsImapMailFolder.cpp - the mailFolderSink class is an xpcom proxied interface
that proxies calls from the imap thread (where everything is thread-safe), to
the ui thread (which is not thread-safe). These calls should just set the quota
information on the nsImapMailFolder object. Now, I don't know if you want that
information to be persistent across mozilla sessions, but I'm guessing it should
be. To do that, you need to get the information into the imap mail db for the
folder, and probably also the folder cache. To do this, I'd look at the way we
handle the online name, NS_IMETHODIMP nsImapMailFolder::SetOnlineName and
nsImapMailFolder::ReadFromFolderCacheElem.

Let me know if you have more questions. I'll do my best to help. Thx for working
on this.

Comment 4

[Lorenzo Colitti]

Attachment: Work in progress v2 (protocol and backend)

Bienvenu, thanks for taking the time to help me with this! I wouldn't be able
to do it on my own. :-)

The patch is still work in progress, but a lot of the necessary stuff has been
added.

The only call to GetQuotaData is still in List(), but that will change; the
proper solution is more complicated than I thought, because we should really
update quota data for a folder every time one of the following events occurs:

- New message arrives
- Move message(s) from/to folder
  - Delete (if delete is Move to trash)
- Copy message(s) to folder
- Expunge / compact folder
- Others???

Is it worth checking these events one by one, or should we poll the server if
the user requests the properties of a folder (maybe via a callback so the
dialog appears instantly and the quota info is filled in when the server
replies)?

Or maybe we could do it at the IMAP command level, in nsImapProtocol.cpp: every
time we issue a command which might modify the user's quota, we could ask the
server for quota information on the folder(s) involved. However, that would ask
the server *before* the command, not after, which is pretty pointless.

I have looked through the source to see how the ACL information is updated and
I have narrowed it down to the folderNeedsACLListed attribute in
nsImapMailFolderSink.idl, as that's what nsImapProtocol.cpp checks to see if it
must refresh the ACL information:

http://lxr.mozilla.org/seamonkey/source/mailnews/imap/src/nsImapProtocol.cpp#5252


but I can't seem to find anyone (apart from nsImapMailFolder's constructor)
that sets the attribute, either by calling SetFolderNeedsACLListed from C++ or
by modifying folderNeedsACLListed from JS. I guess I must be missing
something...

Comment 5

[Lorenzo Colitti]

I don't understand... nsImapProtocol::RefreshACLForFolderIfNecessary() calls
GetFolderNeedsACLListed() to determine if it needs to refresh the ACL:

http://lxr.mozilla.org/seamonkey/source/mailnews/imap/src/nsImapProtocol.cpp#5247

This method returns the value of nsImapMailFolder::m_folderNeedsACLListed, which
is only ever set to false or modified by nsImapMailFolder::SetFolderACLListed():

http://lxr.mozilla.org/seamonkey/search?string=m_folderNeedsACLListed

And nobody ever calls SetFolderACLListed() with a true value except the
nsImapMailFolder constructor:

http://lxr.mozilla.org/seamonkey/ident?i=SetFolderNeedsACLListed

There doesn't seem to be any JS code that modifies the folderNeedsACLListed in
nsIImapMailFolderSink, either.

So how is the ACL information updated during the lifetime of the folder, if
m_folderNeedsACLListed is always false except just after the nsImapMailFolder is
instantiated?

Comment 6

[David :Bienvenu]

we instantiate all the nsImapMailFolders every time we start up, so
m_folderNeedsACLListed starts out true. So the first time we ask if we need to
list the acl (generally, when the user selects the folder), the answer is yes,
and we refresh the acl. We only refresh the acl the first time the folder is
selected, because we it's not cheap, and we figure doing it once per session per
folder is more than enough. You probably want to update the quota stuff more
often, but I would say not as often as you are proposing.

Comment 7

[Lorenzo Colitti]

Attachment: patch v0.9 (backend)

This patch actually works. :)
It has been split in two (backend and UI).

This is the backend part.
It still only asks the server for quota when listing a folder. I hope to fix it
so quota is checked whenever biff fires, but I haven't looked at that yet.

Comment 8

[Lorenzo Colitti]

Attachment: patch v0.9 (UI)

This is the UI part.
Quota information is provided via a new tab in the Folder Properties dialog.
This tab, which is only visible if the server is an IMAP server, displays the
quota root, used/total space and a nice "progress bar" which graphically shows
the state of the mailbox (see attached screenshot).

If the server does not support quotas, the tab just says "This server does not
support quotas".

Comment 9

[Lorenzo Colitti]

Attachment: screenshot of feature

Comment 10

[Lorenzo Colitti]

bienvenu: I'm not quite sure how to pass the quota root between C++ and JS
properly. The quota root is a string which starts out as a char * in
nsImapServerResponseParser::quota_data(), then needs to be passed to the folder
sink (attribute folderQuotaRoot), and then needs to be accessed by JS.

The patch uses |char *| in C++ and |string| in IDL, but the quotaroot is an
8-bit string, and alecf's string classes document says that |string| and |char
*| are not 8-bit safe across XPConnect boundaries. Also, in the
SetFolderQuotaRoot method of the sink I'm using PL_strdup to set the member
variable without freeing the previous value, which is clearly unacceptable.

What should I be doing instead? The string classes document says to use
nsACString, but that's an abstract class...

Comment 11

[Lorenzo Colitti]

Attachment: patch v0.91 (backend)

This patch checks for quota information in ProcessMailboxUpdate(), so it
happens on folder select, when biff fires, etc.

Comment 12

[Lorenzo Colitti]

Attachment: patch v0.91 (UI)

Updated UI patch, now localizable.

Comment 13

[David :Bienvenu]

why did you need to add a new .properties file? Can't you use the existing .dtd
file and add a .text or .label entity and get the value attribute out of it or
something? that would make the UI patch a little simpler. Or maybe .label will
give you the string as is...I'm not an expert in this area, obviously :-) but it
would be good not to add another bundle.

Comment 14

[David :Bienvenu]

for this,
+  char *m_folderQuotaRoot; // FIXME
you can use an nsCString, which will handle the memory allocation stuff

some style things:

We are trying to give method arg names a name that starts with a, for arg, so
this should be aMaxKB. Could you fix the rest of the new code to follow this
convention? (I realize most of the code doesn't follow it, but we're trying to
make new code follow it)
+NS_IMETHODIMP nsImapMailFolder::SetFolderQuotaMaxKB(PRUint32 maxKB)

thanks again for working on this!

Comment 15

[David :Bienvenu]

after talking to Seth, we decided you could just add your strings to
messenger.properties and load that instead.

Comment 16

[Lorenzo Colitti]

Sure, I'll do that.

After I fix the string problem, change the method names and change the
properties file, the only problem that remains is the problem of when to update
the quota data.

The current patch updates it in ProcessMailboxUpdate(). This has the advantage
of updating it frequently, so it's always current, but it has the disadvantage
that it issues commands to the server fairly often.

Also, because of the way the patch is written, quota data for a folder is
available only if the folder is open. Is OK just to tell the user or should I 
try to make it connect to the server if the user selects the quota tab in the
folder properties dialog? This doesn't seem like a good idea, because it might
take a long time...

Comment 17

[Lorenzo Colitti]

Who can I ask about frontend/ui issues like these? Should I CC Seth?

Comment 18

[Lorenzo Colitti]

Attachment: patch v1 (backend)

This patch makes a few changes, the most important of which is being able to
display proper error messages to the user: unlike the previous ones, if there
is no quota information available, it discriminates between the folder not
being open, the server not having quota support, and there being no quota on
the folder.

It also fixes one or two minor leaks and cleans up the code a bit.

I think this is ready for someone's attention.

Comment 19

[Lorenzo Colitti]

Attachment: patch v1 (ui)

UI part of same patch.

Comment 20

[Lorenzo Colitti]

Comment on attachment 110472 [details] [diff] [review]
patch v1 (backend)

Seeking reviews.

Comment 21

[David :Bienvenu]

I'll try to look at this sometime soon.

Comment 22

[David :Bienvenu]

thx for working on this; sorry for the delay - here's my first pass at comments
for the last back end code patch:

I find the ? operator produces more readable code:

+      if(capability == kCapabilityUndefined)
+        folderQuotaStatusStringID = IMAP_QUOTA_STATUS_FOLDERNOTOPEN;
+      else
+        folderQuotaStatusStringID = IMAP_QUOTA_STATUS_NOTSUPPORTED;
 so this would be
folderQuotaStatusStringID = (capability == kCapabilityUndefined) ?
IMAP_QUOTA_STATUS_FOLDERNOTOPEN : IMAP_QUOTA_STATUS_NOTSUPPORTED

here, just use NS_ENSURE_ARG_POINTER(aCmdIssued)
+    if (!aCmdIssued)
+        return NS_ERROR_NULL_POINTER;


+void nsImapProtocol::UpdateFolderQuotaData(nsCString aQuotaRoot, PRUint32
aUsed, PRUint32 aMax)

we don't pass around nsCStrings as objects, either pass a pointer or a
reference, or just a const char *

can you use PR_PackedBool here and move these vars with the other packed bools
so we don't make the imap folder object larger than it needs to be?
+  PRBool m_folderQuotaCommandIssued;
+  PRBool m_folderQuotaDataIsValid;

Comment 23

[Lorenzo Colitti]

Attachment: patch v1.01 (backend)

Patch v1.01 addressing bienvenu's comments

Comment 24

[Lorenzo Colitti]

Seth, could you review the UI patch?

It would be nice if this could make it into 1.3, especially considering the code
was ready 1 month ago. :)

Comment 25

[David :Bienvenu]

Comment on attachment 113117 [details] [diff] [review]
patch v1.01 (backend)

r=bienvenu, thx, you should get an sr from Seth.

Comment 26

[(not reading, please use [e-mail] instead)]

sorry for the delay.  

thanks for the patch, Lorenzo.

we definitely want imap quota support.

I'm a bit behind in my code reviews and I'm focusing on 1.3 beta blockers and
other 1.3 bugs.

I'm moving this to 1.4 alpha, when I think I can get to this enhancement.  

(did 4.x have this?  if so, we should add keyword 4xp)

taking from bienvenu (so I see it in my queries), but when this lands, I'll
re-assign to lorenzo for credit.

Comment 27

[David :Bienvenu]

no, 4x didn't have this - at this point, Mozilla has everything and more than
4.x in terms of imap support, that I can think of.

Comment 28

[(not reading, please use [e-mail] instead)]

re-assign to [e-mail], as it's his patch.

I hope to review this during 1.4 alpha.

Comment 29

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 110473 [details] [diff] [review]
patch v1 (ui)

Be sure to add your name & email address to the license boilerplate at the top
of each file you've modified.

>diff -burk .orig mozilla/mailnews/base/resources/locale/en-US/folderProps.dtd
>--- mozilla/mailnews/base/resources/locale/en-US/folderProps.dtd.orig	Thu Jan  2 03:34:28 2003
>+++ mozilla/mailnews/base/resources/locale/en-US/folderProps.dtd	Thu Jan  2 03:17:51 2003
>@@ -44,3 +44,9 @@
> <!ENTITY privileges.button.label                      "Privileges...">
> <!ENTITY permissionsDesc.label                            "You have the following permissions:">
> <!ENTITY folderType.label                             "Folder Type:">
>+
>+<!ENTITY folderQuotaTab.label                               "Quota">
>+<!ENTITY folderSharingTab.accesskey                         "q">

Is the above line correct?  Should that perhaps be "folderQuotaTab.accesskey"? 
Also, I think access keys are case-sensitive, so I suspect it should be "Q".

Take care of those two nits, and this has sr=dmose.

Comment 30

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 113117 [details] [diff] [review]
patch v1.01 (backend)

This is mostly just nitpicking.  The code in general is very well-written.

Be sure to add your name and email address to the license boilerplate of the
files you've touched.

Also, you've added lines that have hard tabs to some of the files.  Please be
sure that all lines you're adding or changing use spaces and not hard tabs.

>diff -rub mozilla-030105-unpatched/mailnews/imap/src/nsImapCore.h mozilla/mailnews/imap/src/nsImapCore.h
>--- mozilla-030105-unpatched/mailnews/imap/src/nsImapCore.h	2003-01-30 20:02:07.000000000 +0100
>+++ mozilla/mailnews/imap/src/nsImapCore.h	2003-01-30 19:36:25.000000000 +0100
>@@ -131,7 +131,8 @@
> 	kLiteralPlusCapability = 0x00004000, /* RFC 2088 LITERAL+ extension */
> 	kAOLImapCapability = 0x00008000,     /* aol imap extensions */
>   kHasLanguageCapability = 0x00010000, /* language extensions */
>-  kHasCRAMCapability     = 0x00020000  /* CRAM auth extension */
>+  kHasCRAMCapability     = 0x00020000, /* CRAM auth extension */
>+	kQuotaCapability       = 0x00040000  /* RFC 2087 quota extension */

No hard tabs please; change the leading whitespace to be the appropriate number
of spaces.

> } eIMAPCapabilityFlag;
> 
> // this used to be part of the connection object class - maybe we should move it into 
>diff -rub mozilla-030105-unpatched/mailnews/imap/src/nsImapMailFolder.cpp mozilla/mailnews/imap/src/nsImapMailFolder.cpp
>--- mozilla-030105-unpatched/mailnews/imap/src/nsImapMailFolder.cpp	2003-01-30 20:02:07.000000000 +0100
>+++ mozilla/mailnews/imap/src/nsImapMailFolder.cpp	2003-01-30 19:54:36.000000000 +0100

>diff -rub mozilla-030105-unpatched/mailnews/imap/src/nsImapProtocol.cpp mozilla/mailnews/imap/src/nsImapProtocol.cpp
>--- mozilla-030105-unpatched/mailnews/imap/src/nsImapProtocol.cpp	2003-01-30 20:02:07.000000000 +0100
>+++ mozilla/mailnews/imap/src/nsImapProtocol.cpp	2003-01-30 19:48:33.000000000 +0100
>@@ -3321,6 +3321,16 @@
> {
>   if (DeathSignalReceived())
>     return;
>+
>+  // Update quota information
>+  if (!DeathSignalReceived())
>+  {
>+    char *boxName;
>+    GetSelectedMailboxName(&boxName);
>+    GetQuotaDataIfSupported(boxName);
>+    PR_Free(boxName);
>+  }
>+
>     // fetch the flags and uids of all existing messages or new ones
>     if (!DeathSignalReceived() && GetServerStateParser().NumberOfMessages())
>     {
>@@ -7245,6 +7255,41 @@
>   return loginSucceeded;
> }
> 
>+void nsImapProtocol::UpdateFolderQuotaData(nsCString& aQuotaRoot, PRUint32 aUsed, PRUint32 aMax)
>+{
>+  NS_ASSERTION(m_imapMailFolderSink, "m_imapMailFolderSink is null!");
>+
>+  if(m_imapMailFolderSink)
>+  {
>+    m_imapMailFolderSink->SetFolderQuotaDataIsValid(PR_TRUE);
>+    m_imapMailFolderSink->SetFolderQuotaRoot(aQuotaRoot);
>+    m_imapMailFolderSink->SetFolderQuotaUsedKB(aUsed);
>+    m_imapMailFolderSink->SetFolderQuotaMaxKB(aMax);
>+  }
>+}
>+
>+void nsImapProtocol::GetQuotaDataIfSupported(const char *aBoxName)
>+{
>+  // If server doesn't have quota support, don't do anything
>+  if (! (GetServerStateParser().GetCapabilityFlag() & kQuotaCapability))
>+    return;
>+
>+  IncrementCommandTagNumber();
>+
>+  nsCAutoString quotacommand (GetServerCommandTag());
>+  quotacommand += " getquotaroot \"";
>+  quotacommand += aBoxName;
>+  quotacommand += "\"" CRLF;

If you condense these previous four lines into a single line with multiple
operator+ calls, you're likely to avoid unnecessary allocations and copies due
to the way the substring machine works under the hood.

>+  NS_ASSERTION(m_imapMailFolderSink, "m_imapMailFolderSink is null!");
>+  if(m_imapMailFolderSink)
>+    m_imapMailFolderSink->SetFolderQuotaCommandIssued(PR_TRUE);

Why both an assertion and an if check?	If the assertion fails, there's a bug,
so don't bother with the if check.

>diff -rub mozilla-030105-unpatched/mailnews/imap/public/nsIImapMailFolderSink.idl mozilla/mailnews/imap/public/nsIImapMailFolderSink.idl
>--- mozilla-030105-unpatched/mailnews/imap/public/nsIImapMailFolderSink.idl	2003-01-30 20:02:07.000000000 +0100
>+++ mozilla/mailnews/imap/public/nsIImapMailFolderSink.idl	2003-01-30 19:36:25.000000000 +0100
>@@ -69,6 +69,11 @@
>   attribute boolean folderNeedsSubscribing;
>   attribute boolean folderNeedsAdded;
>   attribute boolean folderVerifiedOnline;
>+  attribute boolean folderQuotaCommandIssued;
>+  attribute boolean folderQuotaDataIsValid;
>+  attribute ACstring folderQuotaRoot;
>+  attribute unsigned long folderQuotaUsedKB;
>+  attribute unsigned long folderQuotaMaxKB;
>   readonly attribute boolean shouldDownloadAllHeaders;
>   string GetOnlineDelimiter();
>   // Tell mail master about the newly selected mailbox

Please comment each of the new attributes that you've added in some detail
using doxygen/javadoc style comments so that
<http://unstable.elemental.com/mozilla/> will pick them up.

>diff -rub mozilla-030105-unpatched/mailnews/imap/public/nsIMsgImapMailFolder.idl mozilla/mailnews/imap/public/nsIMsgImapMailFolder.idl
>--- mozilla-030105-unpatched/mailnews/imap/public/nsIMsgImapMailFolder.idl	2003-01-30 20:02:07.000000000 +0100
>+++ mozilla/mailnews/imap/public/nsIMsgImapMailFolder.idl	2003-01-30 19:36:25.000000000 +0100
>@@ -52,6 +52,9 @@
>     void setFolderTypeDescription(in wstring folderTypeDescription);
>     void setFolderPermissions(in wstring permissions);
>     void serverDoesntSupportACL();
>+    void showQuotaData(in boolean showData);
>+    void setQuotaStatus(in wstring folderQuotaStatus);
>+    void setQuotaData(in ACString quotaroot, in unsigned long usedKB, in unsigned long maxKB);
> };

Please comment the new methods you've added as mentioned above.  Also, new IDL
methods should use AString, not wstring.

Comment 31

[Lorenzo Colitti]

Attachment: patch v1.02 (ui)

Patch addressing dmose's comments and synced to current CVS.

Comment 32

[Lorenzo Colitti]

Attachment: patch v1.03 (ui)

Same as previous UI patch, plus it fixes the access keys, which previously
didn't work for any of the tabs in the properties dialog box.

Comment 33

[Lorenzo Colitti]

Attachment: patch v1.03 (backend)

Backend patch addressing dmose's comments and synced to current cvs.

dmose: the string concatenation in nsImapProtocol::GetQuotaDataIfSupported
comes from IRC sessions with Biesi and Timeless and is black magic to me. I
hope it's what you meant. :-)

Comment 34

[Lorenzo Colitti]

Comment on attachment 118227 [details] [diff] [review]
patch v1.03 (backend)

I think this is ready to go. Seeking reviews.

Comment 35

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 118227 [details] [diff] [review]
patch v1.03 (backend)

looks great! sr=dmose

Comment 36

[Lorenzo Colitti]

Attachment: patch v1.04 (ui)

Updated UI patch, changes access keys to upper case.

Comment 37

[Lorenzo Colitti]

Comment on attachment 118227 [details] [diff] [review]
patch v1.03 (backend)

Carrying over bienvenu's r=

Comment 38

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 118233 [details] [diff] [review]
patch v1.04 (ui)

sr=dmose

Comment 39

[Lorenzo Colitti]

Attachment: patch v1.05 (ui)

Updated UI patch, eliminates a redundant "hidden=false" (the default) in
folderProps.xul.

Comment 40

[Lorenzo Colitti]

Comment on attachment 118240 [details] [diff] [review]
patch v1.05 (ui)

Carrying forward dmose's sr=

Comment 41

[[e-mail]]

Comment on attachment 118240 [details] [diff] [review]
patch v1.05 (ui)

You should use the hidden property in preference to the hidden attribute. The
value on a progressmeter doesn't need the %, so you can simplify the percentage
calculation (and use ? : as well). You have put for= on labels without access
keys, and for unfocusable controls.

And personally I don't like tabs to have access keys, but there you go.

Comment 42

[Lorenzo Colitti]

Attachment: patch v1.06 (ui)

Updated UI patch addressing Neil's comments.

Comment 43

[[e-mail]]

Comment on attachment 118287 [details] [diff] [review]
patch v1.06 (ui)

quotaStatusLabel.hidden = (showData ? true : false);
folderQuotaData.hidden = (showData ? false : true);

These should just be showData and !showData. Fix them and r=neil

Oh, and you can restore the tab access keys if you find someone who wants them.

Comment 44

[Lorenzo Colitti]

Attachment: patch v1.07 (ui)

Updated UI patch addressing Neil's last comment.

The access keys are in the same state as they were before the patch: declared
in the .properties file, but not used in the .xul file. So they don't appear or
work. I left them out because all the other tabbed dialogs I looked at don't
have access keys on the tabs (e.g. address book card dialog, composer insert
image dialog).

Comment 45

[Dan Mosedale (:dmosedale, :dmose)]

jglick OKed the UI changes in private email to me yesterday.  Unfortunately, I
didn't have time to get it checked in before the tree closed.  I'll check it in
once the tree opens for beta.

Comment 46

[Dan Mosedale (:dmosedale, :dmose)]

Checked in; thanks Lorenzo!

Comment 47

[(not reading, please use [e-mail] instead)]

I just tried this out on my trunk debug bits, and it looks good.

(bienvenu sends his compliments as well)

thanks to dmose, bienvenu, neil and jglick for working with lorenzo on this.

a couple potential spin off bugs:

1) from bienvenu "it didn't work the first time, but it worked after that" (I
didn't notice that problem.  david, can you elaborate?)
2) the plain progress meter doesn't look great under classic skin (it looks much
better under modern.  I think it could use a 1 pixel black border)
3) we should be displaying in MB if the number is bigger than 1 MB
4) when offline, should we displaying the "folder is not open message:  Quota
information is not available because the folder is not open", or something
indicating that you are offline?

gchan isn't the right person to verify.  setting QA contact to esther.

note to esther, this just landed (early 1.4 beta)

Comment 48

[(not reading, please use [e-mail] instead)]

> 2) the plain progress meter doesn't look great under classic skin (it looks much
> better under modern.  I think it could use a 1 pixel black border)

the progress meeting in classic just sort of looks this way, if not surrounded
by a box with a border, it's hard to tell what you are looking at.

I'll take this, and the other issues, to spin off bugs, unless someone beats me
to it.

Comment 49

[David :Bienvenu]

sure, the first time I tried it, I got the folder is not open message even
though the folder was selected/open.

Comment 50

[(not reading, please use [e-mail] instead)]

here's something I saw:

1) start up, load inbox.
2) right click on another folder (Foo)
3) get the "Quota information is not available because the folder is not open"
message

are quota's per folder, or per server, or does it depend?  (I'd need to check
the rfc)

if per server, we could fix the scenario I describe easily.
if per folder, should checking quote establish a connection?

we should still add a new message, for the offline scenario.

Comment 51

[Lorenzo Colitti]

Hey, it's great to see this in the trunk!
I'd like to thank you all for the help (and dmose for his patience as well :-)).

Regarding comment #47: I'm not an expert on UI issues, and I don't use classic,
so that probably slipped throught the net. As for offline, I couldn't find an
easy way to find out if the user was offline or not, so the code falls back to
saying "the folder is not open".

Regarding comment #49: maybe bienvenu opened the folder properties while the
folder was being opened? Quota information is loaded when you open the
properties dialog, not when you select the quota tab.

Comment 52

[Lorenzo Colitti]

Regarding comment #50: unfortunately that can't easily be fixed, because we
don't know in advance what quota root the folder is under without asking the
server. 

Quotas are neither per folder nor per server: each folder under quota is under
one or more "quota roots" each of which specifies one or more resource limits
(e.g. storage or number of messages). We currently treat quotas as per-folder,
even though usually the server is set up to have just one quota root (and just
one storage limit) for each user.

This means that the numbers reported for different folders may be out of sync
(for example, if you add messages to a folder, and click properties on another
folder without selecting it first, the numbers you get don't reflect the
change). A better approach might be to keep a list of quota roots in the server
object and just link to them from the folders.

Perhaps the best approach would be to have an "update" button in the quota
properties tab which would ask the server for up to date information. If the
user clicks on it, we are justified in opening a new connection if necessary. I
didn't do that because I didn't know how to implement it properly (how do I run
an IMAP command from javascript in the properties dialog?). Now that the initial
implementation is in, I can try this approach, if someone gives me pointers on
how to do it.

Comment 53

[(not reading, please use [e-mail] instead)]

> As for offline, I couldn't find an
> easy way to find out if the user was offline or not

var ioService =
Components.classes["@mozilla.org/network/io-service;1"].getService(Components.interfaces.nsIIOService);
if (ioService.offline)
{
 ...
}