768621 - show the size of a mail folder on disk somewhere in the Thunderbird UI

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, enhancement)

Description

[:aceman]

Show the size of a mail folder on disk somewhere in the Thunderbird UI, my proposal is for folder -> Properties... -> General information.

I understand this information was intentionally removed from the folder pane. But there should at least be some hidden way to show the size to the user. Even Outlook has this possibility.

Maybe this is applicable to Seamonkey, even though it still has the size in the folder pane.

Comment 1

[:aceman]

Attachment: WIP patch

This is a first stab at this. I am not sure if the values should be in a textbox, but that allows them to be selectable (copyable) by mouse. Is there a way to make a label selectable?

(Of course I will add the strings into the locale dtd, this patch is just to get UI discussion.)

Comment 2

[Mark Banner (:standard8)]

A nicer (though more complex) way would be to integrate Mail Summaries into Thunderbird...

Comment 3

[Jim Porter (:squib)]

aceman, you should probably be using nsIMessenger.formatFileSize to display the size. That'll give you the most sensible units instead of just using KB.

(In reply to Mark Banner (:standard8) from comment #2)
> A nicer (though more complex) way would be to integrate Mail Summaries into
> Thunderbird...

One day... :) (Though Mail Summaries doesn't show all folders, and I don't expect it ever will.)

Comment 4

[:aceman]

Thanks. But I am not sure I want the unit to change per folder. There are enough complaints about similar feature of Extra folder columns at https://addons.mozilla.org/sk/thunderbird/addon/extra-folder-columns/reviews/ .

Comment 5

[Jim Porter (:squib)]

(In reply to :aceman from comment #4)
> Thanks. But I am not sure I want the unit to change per folder. There are
> enough complaints about similar feature of Extra folder columns at
> https://addons.mozilla.org/sk/thunderbird/addon/extra-folder-columns/reviews/
> .

Since this is the properties dialog, which should only show one folder at a time, the criticisms there shouldn't apply. Besides that, we use variable units for pretty much every other file size in Thunderbird, so we should be consistent. Add-ons are, of course, free to change the behavior.

Comment 6

[:aceman]

OK, anything is better than nothing :)

How can I retrieve the messenger object from inside the folderProps.js file?

Comment 7

[Jim Porter (:squib)]

You'll want:

let gMessenger = Cc["@mozilla.org/messenger;1"].
                   createInstance(Ci.nsIMessenger);

Comment 8

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 636875 [details] [diff] [review]
WIP patch

In general, I like the idea, but I've got a few suggestions before I give it the ui-r+…

I think we should tighten up the space between the size and the "KB", and get that second phrase to line up with the end of the text box above it.  So it would end up looking something like http://dl.dropbox.com/u/2301433/Screenshots/diskSize.png

As for making the text selectable, I guess that would be a good idea, but I'm not sure how many people would use the information, and what they would use it for.  Having to type out "17 MB" doesn't seem that terrible to me, and perhaps if it's actually useful information, we might want to put it in Troubleshooting Information…

Thanks,
Blake.

Comment 9

[:aceman]

Attachment: patch v2

Thanks, I think I addressed all issues.

Comment 10

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 637206 [details] [diff] [review]
patch v2

Much better.  Not quite there, as you can see at the right of http://dl.dropbox.com/u/2301433/Screenshots/BetterDiskSize.png but I trust you can fix it without further meddling by me.  ;)  So ui-r=me.

Thanks,
Blake.

Comment 11

[David :Bienvenu]

just a note that maildir doesn't implement folder size (cuz it's a bit more complicated that just fstat'ing a mailbox file). I think the maildir impl would probably just sum up the message sizes in the .msf file instead of fstating all the message files.

Comment 12

[:aceman]

Bwinton, I can't actually fix that easily as there is a <textbox> with hidden borders that has a size = length("2.9 MB") in you case. That uses some average font width and is always larger than the value it contains. It is visible in all the other textboxes of type="number" (with the 2 small arrows). They are larger than the max value they take by 1 digit.

Can I align the contents of the textbox to right?

Comment 13

[Blake Winton (:bwinton) (:☕️)]

I think it's possible.  Or just replace the whole thing with a right-aligned label, since I don't think we need to let people copy/paste that text…

Comment 14

[:aceman]

Attachment: patch v3

Comment 15

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 637266 [details] [diff] [review]
patch v3

Yep, looks good.  ui-r=me.

Later,
Blake.

Comment 16

[Ian Neal]

Comment on attachment 637266 [details] [diff] [review]
patch v3

>+++ b/mailnews/base/content/folderProps.js
>+  // set folder sizes
>+  let numberOfMsgsElem = document.getElementById("numberOfMessages");
>+  let numberOfMsgs = gMsgFolder.getTotalMessages(false);
>+  numberOfMsgsElem.value = numberOfMsgs < 0 ? "???" : numberOfMsgs;
>+
>+  let sizeOnDiskElem = document.getElementById("sizeOnDisk");
>+  let sizeOnDisk;
>+  try {
>+    sizeOnDisk = Components.classes["@mozilla.org/messenger;1"]
>+                           .createInstance(Components.interfaces.nsIMessenger)
>+                           .formatFileSize(gMsgFolder.sizeOnDisk, true);
>+  } catch (e) {
>+    sizeOnDisk = "???";
>+  }
>+  sizeOnDiskElem.value = sizeOnDisk;
Nit: You could inline sizeOnDiskElem

>+++ b/mailnews/base/content/folderProps.xul
>+  onload="folderPropsOnLoad();" style="width: 36em;"
Nit: Shouldn't widths be measured in ch rather than em?

r=me with those addressed / fixed.

Comment 17

[:aceman]

No idea about the "ch". It is not my code and I can't answer that. Maybe bwinton knows about these theme guidelines.

Comment 18

[:aceman]

Attachment: patch v4

Thanks, changed to '56ch' (seemed about the same width at my font) and inlined both element variables (those were remnants from previous versions where they were needed).

Comment 19

[Jim Porter (:squib)]

Comment on attachment 643554 [details] [diff] [review]
patch v4

Review of attachment 643554 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good! r=me with the comments below addressed.

::: mailnews/base/content/folderProps.js
@@ +265,5 @@
>    document.getElementById("retention.useDefault").checked = retentionSettings.useServerDefaults;
>  
> +  // set folder sizes
> +  let numberOfMsgs = gMsgFolder.getTotalMessages(false);
> +  document.getElementById("numberOfMessages").value = numberOfMsgs < 0 ? "???" : numberOfMsgs;

This string should be localized; I'd prefer "unknown" instead.

@@ +273,5 @@
> +    sizeOnDisk = Components.classes["@mozilla.org/messenger;1"]
> +                           .createInstance(Components.interfaces.nsIMessenger)
> +                           .formatFileSize(gMsgFolder.sizeOnDisk, true);
> +  } catch (e) {
> +    sizeOnDisk = "???";

Ditto here; it's more important to localize this one, since newsgroups don't report a size.

Comment 20

[David :Bienvenu]

just an FYI - the size on disk attribute isn't yet supported for the maildir pluggable store. My suggestion for its implementation would be to simply sum up the message size of the headers in the .msf file, rather than statting all the .eml files.

Comment 21

[:aceman]

I don't think I am able to do what you suggest, David. Will maildir return the "unknown" reply (or exception) in the current state? If yes, I think it would be enough here.
If maildir has unimplemented .sizeOnDisk then it should be fixed there, not in this bug.

Comment 22

[:aceman]

Attachment: patch v5

Would this work? I use entities as I didn't want to put it in some .properties file where it would be disconnected from the added strings in folderProps.dtd. As the translation depends heavily on the strings after which they come in the dialog. That is also why I added a string for each "unknown" (the size one may be translated differently than the number one).

Comment 23

[Jim Porter (:squib)]

Comment on attachment 647247 [details] [diff] [review]
patch v5

Review of attachment 647247 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the following changes (note that I didn't run Thunderbird with this latest version, but it's a minor change). You may want to ping bwinton to see if he prefers "unknown" vs. "Unknown" for the strings. I (not strongly) prefer "Unknown".

::: mailnews/base/content/folderProps.js
@@ +275,5 @@
> +                           .createInstance(Components.interfaces.nsIMessenger)
> +                           .formatFileSize(gMsgFolder.sizeOnDisk, true);
> +  } catch (e) { }
> +  if (sizeOnDisk)
> +    document.getElementById("sizeOnDisk").value = sizeOnDisk;

I this this would make a lot more sense like so:

  try {
    let sizeOnDisk = Components.classes["@mozilla.org/messenger;1"]
                               .createInstance(Components.interfaces.nsIMessenger)
                               .formatFileSize(gMsgFolder.sizeOnDisk, true);
    document.getElementById("sizeOnDisk").value = sizeOnDisk;
  } catch (e) {}

Comment 24

[Ian Neal]

Comment on attachment 647247 [details] [diff] [review]
patch v5

>+<!-- LOCALIZATION NOTE: When the number of messages can't be determined, this string is displayed as the number -->
>+<!ENTITY numberUnknown.label                     "unknown">
>+<!-- LOCALIZATION NOTE: When the size can't be determined, this string is displayed as the size -->
>+<!ENTITY sizeUnknown.label                       "unknown">
Do we really need two entities for unknown? I'll defer to Blake though.

>+  let sizeOnDisk = null;
>+  try {
>+    sizeOnDisk = Components.classes["@mozilla.org/messenger;1"]
>+                           .createInstance(Components.interfaces.nsIMessenger)
>+                           .formatFileSize(gMsgFolder.sizeOnDisk, true);
>+  } catch (e) { }

>+    document.getElementById("sizeOnDisk").value = sizeOnDisk;
As already suggested move this and the let into the try.

Comment 25

[:aceman]

Attachment: patch v6

The reasoning for "unknown" in lowercase is that is appears after a colon.
The reason for having 2 entities of "unknown" is that in some languages the translation depends on the sentence before the colon. E.g. the "size" and "number" subjects have different gender.
Bwinton, what do you think?

Comment 26

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 649012 [details] [diff] [review]
patch v6

I think I agree with the reasoning behind the lowercase unknown, and having the two different unknowns.  ui-r=me.

Thanks,
Blake.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/3c84748b73e9

Comment 28

[Stefan Plewako [:stef]]

For imap folders without sync/offline support enabled size on disk info seems to be completely wrong (at least for me with Daily 17 pl Mac 10.6.8).

Additionally, since it's labeled as folder size on disc and not all messages size, we should count .msf file size too.

Comment 29

[:aceman]

Then you should file a new bug for that because you want to change how the backend .sizeOnDisk function works.

Comment 30

[Stefan Plewako [:stef]]

(In reply to :aceman from comment #29)
> Then you should file a new bug for that because you want to change how the
> backend .sizeOnDisk function works.

No I don't, I just want the correct info…

Comment 31

[:aceman]

But correct info is not the job of this bug. It just exposes whatever .sizeOnDisk function returns, with all its current limitations.

Comment 32

[Stefan Plewako [:stef]]

(In reply to :aceman from comment #31)
> But correct info is not the job of this bug. It just exposes whatever
> .sizeOnDisk function returns, with all its current limitations.

I read summary of this bug and comment #0 differently - nothing about blindly reusing some function.

Some ways to fix this:
- display true size of the folder on disk (incorporating maybe .msf file size if significant)
- change the label to reflect what is currently displayed
- display total messages size on server and folder size on disk…

Comment 33

[Wayne Mery (:wsmwk)]

Stefan, please file a new bug