Main mail window should show the quota usage in the status bar, if the user is on quota on the (IMAP) server. Otherwise user won't notice his usage (and can't adapt his usage patters to the permitted storage) until it's too late. I am implementing this.

Screenshot at <https://bugzilla.mozilla.org/attachment.cgi?id=171088>. I decided against a general-purpose progressmeter with label (bug 278159), for now.

Spec: It shows the quota as progressmeter in the lower right corner of the window, in the staus bar, next to the "Total" msg count. Progressmeter is small, to save screen real estate. Shows value in percent as label (with shadow, so that it's visible independent of background - bar or remainder). If you click on it, the folder properties dialog opens with the Quota tab, as a discovery feature for people to find out what it means at all. The display will be updated together with the Unread/Total msg count, which happens only when you change the folder (it seems). An additional lag is introduced by the IMAP backend code, so you may have to switch folders a few times for it to update.

Oh, and you can supress the display of the progressmeter, if the quota is below a certain threshold, to not encourage users to use up their quota. There are also thresholds when to make the bar yellow and red. (Defaults are 20, 80 and 95 percent, respectively.) If there's no quota information from the server, no progressmeter is shown either.

Comment on attachment 171104 [details] [diff] [review] Fix, v1 I don't have access to a server that supports quotas... >+pref("mailnews.quota.mainwindow_threshold.show", 20); >+pref("mailnews.quota.mainwindow_threshold.warning", 80); >+pref("mailnews.quota.mainwindow_threshold.red", 95); Perhaps "critical" rather than "red"... >+var UpdateStatusMessageCountsCache; This variable name is a bit meaningless... perhaps gQuotaFeedback or something? > function UpdateStatusMessageCounts(folder) > { >+ // get element references and prefs > var unreadElement = GetUnreadCountElement(); > var totalElement = GetTotalCountElement(); >- if(folder && unreadElement && totalElement) >+ if(!(folder && unreadElement && totalElement)) >+ return; >+ if (typeof(UpdateStatusMessageCountsCache) != "object") Why not call an init function from CreateMailWindowGlobals? >+ var cache = UpdateStatusMessageCountsCache; I don't think you gain a lot by making a local reference. >+ var imapFolderSink = folder >+ .QueryInterface(Components.interfaces.nsIImapMailFolderSink); >+ if (imapFolderSink && >+ imapFolderSink.folderQuotaDataIsValid) QueryInterface does not return null on error. Use instanceof or verify the server type first instead. Additionally you need to hide the quota panel when switching to account central (presumably in ShowAccountCentral) or to non-IMAP folders. >+ cache.quotaMeter.setAttribute("value", percent); >+ // do not use value property, because that is inprecise (3%) >+ // for optimization that we don't need here "imprecise". Maybe we can get rid of that 'optimization'. >+ var label = gMessengerBundle.getFormattedString("percent", [percent]); nsMsgStatusFeedback.showProgress just appends a % sign (but check with l10n folks) >-<tabbox> >+<tabbox id="folderPropTabBox"> You could remove the id from the <tabpanels>. >+/** >+ @param msgFolder if null: selected >+ @param tabID initial tab >+ */ I can't see this comment being helpful, sorry >+ tabID:tabID?tabID:"", name:name}); The ?: clause is unnecessary, because the dialog doesn't care about nulls. >+#quotaMeter { >+ max-width: 4em; >+} The min-width set by progressmeter.css should override this, but setting min-width here would probably work. >+ color: white; Modern theme style is to use #FFFFFF. >+ color: white; Classic theme should always use a system colour.

(In reply to comment #4) >nsMsgStatusFeedback.showProgress just appends a % sign Although sendProgress and printProgress do support i18n

(In reply to comment #4) > (From update of attachment 171104 [details] [diff] [review] [edit]) > I don't have access to a server that supports quotas... I think I could supply you with an account/pw. > Perhaps "critical" rather than "red"... Right. I couldn't remember the word. > >+var UpdateStatusMessageCountsCache; > This variable name is a bit meaningless... perhaps gQuotaFeedback or something? Well, the variable is only for this function, thus the name of the function in there. (I forgot the prefix "g"). > > function UpdateStatusMessageCounts(folder) > > { > >+ // get element references and prefs > > var unreadElement = GetUnreadCountElement(); > > var totalElement = GetTotalCountElement(); > >- if(folder && unreadElement && totalElement) > >+ if(!(folder && unreadElement && totalElement)) > >+ return; > >+ if (typeof(UpdateStatusMessageCountsCache) != "object") > Why not call an init function from CreateMailWindowGlobals? I wanted to keep things local, in one place. Maybe I'll change it, though. > >+ var cache = UpdateStatusMessageCountsCache; > I don't think you gain a lot by making a local reference. The idea is not speed, but code readability. > >+ var imapFolderSink = folder > >+ .QueryInterface(Components.interfaces.nsIImapMailFolderSink); > >+ if (imapFolderSink && > >+ imapFolderSink.folderQuotaDataIsValid) > QueryInterface does not return null on error. Uh. I thought so. What *does* it return on error? > Use instanceof hmpf, that's crap. > Additionally you need to hide the quota panel when > switching to account central (presumably in ShowAccountCentral) or to non-IMAP > folders. hm, true. > "imprecise" OK. > You could remove the id from the <tabpanels>. OK. > >+/** > >+ @param msgFolder if null: selected > >+ @param tabID initial tab > >+ */ > I can't see this comment being helpful, sorry I should have said "initial tab. if null: first tab." It was important to specify that the parameters are optional and what happens, if they are missing, because all current callers call without arguments. > >+ tabID:tabID?tabID:"", name:name}); > The ?: clause is unnecessary, because the dialog doesn't care about nulls. Good catch, I saw that after I submitted the patch. What does document.getElementById(null) do? throw? return null? document.getElementById("nonexistingID") returns null, right? > >+#quotaMeter { > >+ max-width: 4em; > >+} > The min-width set by progressmeter.css should override this, but setting > min-width here would probably work. I don't understand. This works fine for me, and the rule does have an effect (it makes the difference between the normal size and the mini size you can see on the screenshot). |width| didn't work because of the rules in progressmeter.css. > >+ color: white; > Modern theme style is to use #FFFFFF. Gack. And I have to use the same uglyness? OK.... > >+ color: white; > Classic theme should always use a system colour. Which one?

Or, you could just implement nsIMsgImapFolderProps in your extension and call nsImapMailFolder::FillInFolderProps(nsIMsgImapFolderProps *aFolderProps) then you don't have to touch any core code.

Feels like somebody dragged the carpet from under my feet just when I'm done. :( *sigh* I think the attributes were just fine, exactly as they should be, only in the wrong interface (BTW: Nothing in nsIImapMailFolderSink tells me that I should not use that). > nsImapMailFolder::FillInFolderProps I saw that interface and intentionally didn't use it, because it's ugly: push from backend to frontend, and even worse with backend having knowledge of UI, knowing which exact fields will be needed. I guess I have to re-add the getters to nsIMsgImapMailFolder. I prefer single attributes (cleaner, much nicer from JS), but oh well, I can implement a single 3-value getter. (The pushing of all these unnecessary values using 7 functions in nsIMsgImapFolderProps, would surely be far worse than getting 3 attributes as I do here.) Will do that next.

Did so. I followed bienvenu's advice in bug 209545 comment 9 as far as I understood it. --- (In reply to comment #4) > QueryInterface does not return null on error. Use instanceof fixed > Additionally you need to hide the quota panel when > switching to account central (presumably in ShowAccountCentral) or to non-IMAP > folders. fixed > >+#quotaMeter { > >+ max-width: 4em; > >+} > The min-width set by progressmeter.css should override this, but setting > min-width here would probably work. This comment didn't apply to Mozilla back then, but does apply now to current toolkit/. Fixed.

Ported Fix v1 to Thunderbird 2.0 branch. Readded interface to get quota as discussed above. Also fixed review comments. Requesting review, please.

Screenshots of Thunderbird statusbar with no quota, 40% (normal) and 80% (warning). critical is the same as warning, just red instead of orange. Also a screenshot of the folder properties dialog's quota pane. This opens when you click on the quota progressmeter. I have not modified the layout of this dialog, the screenshot is for illustration purposes only. The changes for Seamonkey should be identical. I plan to do them when I passed review, to avoid work duplication.

Comment on attachment 238718 [details] [diff] [review] Fix, v2, Thunderbird cool, generally looks OK. a few nits: nsIMsgImapMailFolder.idl when you change an interface, you need to generate a new uuid. I don't quite understand the three vars for holding the quota information: + var c = new Object(); gUpdateStatusQuote_Cache and var cache = gUpdateStatusQuota_Cache; Do we really need all three, or can we just use one : gUpdateStatusQuota_Cache? I haven't had a chance to try this patch out - I'll try it out as soon as I can.

> cool, generally looks OK. a few nits: Great! > when you change an interface, you need to generate a new uuid. Sure, I didn't know that. > Do we really need all three [variables], or can we just use one : > gUpdateStatusQuota_Cache? c and cache are just aliases, because gUpdateStatusQuota_Cache is so ugly and long, and I need it a lot. I thought it was clear but I guess not. Should I make a comment or do you have a better idea to achieve the same? > I haven't had a chance to try this patch out > I'll try it out as soon as I can. Thanks.

Comment on attachment 238718 [details] [diff] [review] Fix, v2, Thunderbird you could have separate global vars - gQuotaLabel, gQuataMeter, etc. instead of having a top level object. The way the code is now is just needlessly obscure...or just use gUpdateStatusQuota...

7 global vars would be a bit much. I'll rename it "gQuotaUICache" or so, remove the redundant "quota" from the members, and use it directly.

Fixed above comments. I assume I need to fix the UUID only in the IDL, I haven't found any other instance. Also fixed hiding the meter in one edgecase and moved the class for more flexible theming. If you need a test IMAP account with quota, tell me and I can supply you with one.

Thx for doing this, Ben! It seems to basically work but there are a few issues that I think should be addressed. 1. Clicking on a top-level account (e.g., local folders) doesn't remove the quota widget. 2. I think having a tooltip for the quota widget is a must - otherwise, users will have no idea what it is. The tooltip should say something about IMAP quota usage. Maybe it should mention the server, but I don't think it should mention the folder (see item 3) 3. I have a server which has an account-based quota, not a per-folder quota (which is probably by far the most common setup). When I go look at the quota info in the quota tab, I see the same information for each folder. But the quota widget seems to vary a bit based on which folder is selected. Certain folders seem to lag behind a bit. I don't know why that would be... At first I thought the quota widget should be to the right of the status/progress bar, so the status/progress bar wouldn't move, but now I kinda think it should be next to the folder totals, the way you have it now.

4 - the first time I select a folder in the current run of TB, it doesn't show the quota information. If I click away and then back, it shows it. That was confusing the heck out of me, but now I think I see the pattern.

Comment on attachment 239099 [details] [diff] [review] Fix, v3 if you put these in mailnews.js, +pref("mail.quota.mainwindow_threshold.show", 20); +pref("mail.quota.mainwindow_threshold.warning", 80); +pref("mail.quota.mainwindow_threshold.critical", 95); + you won't need the catch part of this code: + try { + gQuotaUICache.showTreshold = gPrefBranch.getIntPref(kBranch + "show"); + gQuotaUICache.warningTreshold = gPrefBranch.getIntPref(kBranch + "warning"); + gQuotaUICache.criticalTreshold = gPrefBranch.getIntPref(kBranch + "critical"); + } catch (e) { + gQuotaUICache.showTreshold = 0; + gQuotaUICache.warningTreshold = 80; + gQuotaUICache.criticalTreshold = 95; because you'll always get a value.

can you try this and roll it into your patch if it works? You might want to remove the old call, though I'm not sure...

- Incorporated bienvenu's fix for problem 4. (Thanks a lot!) - Fixed problem 1 by adding a call to ChangeFolderURI, isServer case. - Changed the default prefs and removed try/catch per comment 19. - Tried to add tooltip (problem 2). It doesn't show up, I don't know why, String / property content is OK. - No idea about problem 3, I have never seen that.

I found the tooltip problem(s): 1. caps was wrong, tooltipText is correct for the property. 2. It needs to be on the label, not the meter, then it shows up even when next to the text. This is most likely because of the <stack>. Replacing the existing tooltip line with the following makes it work: gQuotaUICache.label.tooltipText = tooltip; Also, the string should be (it wrongly had percent signs): quotaTooltip=IMAP quota: %S KB used of %S KB total. Click for details. Not attaching a new patch just yet.

> Not attaching a new patch just yet. Actually, I do, to make it easier for bienvenu.

great, thx, I will try to look at this tomorrow...

Comment on attachment 239739 [details] [diff] [review] Fix, v5 lastest patch seems to be working well for me. Thx, Ben!

I've landed the backend imap changes for this on the trunk and branch.

You'd earn major brownie points with me in the quota UI was hidden until you start to get say within 75% (or some other numbe) of your quota. The status bar is prime real estate and I like to keep in uncluttered as much as possible.

> You'd earn major brownie points with me in the quota UI was hidden until you > start to get say within 75% (or some other numbe) of your quota. Exactly that is already implemented :) See comment 3 (and start of comment 4). I'll collect the brownie points at next occasion ;-P

I think we'd much prefer that we not show the quota meter at all until the user gets to 75% instead of 20%, by default.

Well, that's a policy and easily adjusted. But I think that this would make it relatively useless. If you're at 75%, it's almost "too late", you're almost full and will probably whine to your admin to get more space, or have to delete half of your mails. When you start to see it at a lower value, you can adjust your usage according to the quota. I think a user at 40% quota will care about how much he can still use and if he needs to change his patterns. That's why I created the 4 states: - No bar when there's no problem: no quota or less than 20%. If you have a gigabyte, you won't care when you have 150 MB used after a year. If you see only 3%, because admins gave a lot of freedom, you are encouraged to use up more space, which would drive admins nuts. - If you are at 40%, you probably do want to know about it. Esp. if it's after 6 months, because you only have 20 MB of quota (which is quite common). That's the normal case, which will show the bar normally. - If you are at 80%, you should take action. That's when the bar gets yellow - warning. - If you are at 95%, you must takre immediate action or will lose mails. That's when the bar gets red - criticial. So, I think the current settings are sensible. You could increate the "show" value to 40%, which would show the bar less often. If you put it at 75%, the feature would be reduced to the warning function only, and would not allow people adapt to the quota early on.

To emphasise: I think that 20 MB quota and a user who doesn't understand the implications of *copy* to trash and the necessity to "compress folders" is quite common. If she sees she's at 78%, and decides to delete half of her mails, e.g. everything from last year and before, she'll get problems, going over quota just during the delete action, and get very confused. That's why she probably shouldn't even come close to 75% quota, if she has e.g. 100 MB quota, and that's what the main point of this feature is: stopping her from reaching 80% quota in the first place. That also means admins can give more quota. Anyways, your decision.

What are we going to do now?

Comment on attachment 239739 [details] [diff] [review] Fix, v5 In the interest of makin g a decision so we can get this fix in, David and I talked this over and we'd like it set at 75%. If we decide down the line that this is too late, we can adjust it later. Thank you for fixing this Ben!

Hmm I was going to nominate this patch for thunderbid 2 approval, but can't seem to find the flag for that.

Moving bug to Thunderbird product, maybe that gives me the flag (grr)

Ben, can you check this into the trunk? Then ping me in a couple days and if it still looks good on the trunk, I'll approve it for the branch.

Comment on attachment 239739 [details] [diff] [review] Fix, v5 (Sorry for the delay.) Checked in on trunk. Thanks a lot to bienvenu for finding that one bug and the detailed and quick review. Will let bake on trunk, then re-request approval for TB2. QA: You need to actually reach 75% quota usage to see anything, or reduce the threshold in prefs (search for "quota"). Also try switching folders back and forth.

Comment on attachment 239739 [details] [diff] [review] Fix, v5 Ben, I think we are ready to move this to the branch now. But I'd like to see the patch in Bug 360591 land with it.

did this miss 2.0? It has a string change, which means it's too late, unless we make a very special exception

your right David, I thought this had already gone into the branch. Sadly we can't take it anymore unless we take out the string change part.

Comment on attachment 239739 [details] [diff] [review] Fix, v5 Rats! I also thought this was long done. The only localization change is: +# for quota in main window (commandglue.js) +percent=%S%% +quotaTooltip=IMAP quota: %S KB used of %S KB total. Click for details. The "%" sign could be hardcoded, or does any language need to translate that? The tooltip could be removed. The meaning of the progress bar could still be kind-of discoverable, because clicking on it goes to the Quota tab of the Folder properties. (You usually can't click on status bar items, though.) I'd adapt the patch and do the checkin. What do you think?

I'd rather have this feature with the % hard coded (I don't think locales would be changing that anyway) and without a tooltip than not have this at all on the branch. Let's go that route.

OK. It's suboptimal, to introduce a feature without tooltip, but if that's the only way... I'll work on it ASAP (fetching branch tree now). Do you want fries with that? (comment 38)

heh, yes I'd like to land 360591 with it too, I like french fries. If you still want the tooltip, another possibility is to write JS that sets the tooltip that doesn't assume the tooltip is in the string bundle. i.e. var tooltipString = get the tooltip string if (tooltipString) set the tooltip string on the quote box That way if a localization doesn't have the translated tooltip, things still work. cc'ing axel to make sure that's kosher. No tooltip at all is probably easiest and safest though depending.

OK, if that is a reasonable approach, i.e. some important translations would have the string, I'd like to go that route. (I like french fries, too. And dual core for building.)

On a general note on the patch, I don't think that changing hwaara's real name in some of the files is a good idea. Check your encodings? I don't think that we should go into 2.0 with optional strings, nor am I convinced that quota display is a big enough thing to break string freeze. Another route in addition to having this non-localizable would be a patch checking in the English strings for all locales and post to .l10n. That way, l10n folks can at least fix that for 2.0.0.3 or 4 or whatever the first minor update would be.

I agree with Axel, we don't want to break the string freeze for this, hence the idea of an optional string. Given Axel's comments about that too, let's just not have the tooltip on the branch Ben.

Hi Ben, any updates on a string free patch for this? Would be great to get this in before the beta 2 deadline tonight.

Yes, I adapted the code and tested it, it works and is ready for checkin. I held back, though, because I didn't have time for the fries, yet.

Comment on attachment 251537 [details] [diff] [review] Fix, v5, but without locale strings, for 2.0 branch cool. I can land the fries part later today if you don't have time. Approving this patch for the branch so you can land it. Thanks Ben!

Comment on attachment 239739 [details] [diff] [review] Fix, v5 (clearing obsolete flag) OK, great. I'll check in once my tree is updated and rebuilt and retested.

Checked in to branch. Thanks to all! :)

thanks again Ben.

verified using version 2 beta 2 (20070116). Although my box wasn't at quote, I tweaked the about:config pref to show the progress meter, and it looks great. Thanks for the feature Ben.

I could really use *thousands separators* for the amount used and total amount in the tooltip (especially for huge quotas, like gmail): 1,000,000,000 kibi used of 2,000,000,000 kibi total. Better yet, switch to MiBi or GiBi when the numbers get so large as to make those units more sensible.

FYI, 2 GB = 2000000KB

(eh, I mean 2097152, and yes, I know about the kiwi discussion.)