The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in Thunderbird 17.0

Status

Thunderbird
Mail Window Front End
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: aceman, Assigned: aceman)

Tracking

(Depends on: 2 bugs)

Trunk
Thunderbird 17.0
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [parity-Outlook])

Attachments

(1 attachment, 5 obsolete attachments)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 636875 [details] [diff] [review]
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.)
Attachment #636875 - Flags: ui-review?(bwinton)
Attachment #636875 - Flags: feedback?(iann_bugzilla)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
A nicer (though more complex) way would be to integrate Mail Summaries into Thunderbird...

Comment 3

5 years ago
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.)
(Assignee)

Comment 4

5 years ago
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

5 years ago
(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.
(Assignee)

Comment 6

5 years ago
OK, anything is better than nothing :)

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

Comment 7

5 years ago
You'll want:

let gMessenger = Cc["@mozilla.org/messenger;1"].
                   createInstance(Ci.nsIMessenger);
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.
Attachment #636875 - Flags: ui-review?(bwinton) → ui-review-
(Assignee)

Comment 9

5 years ago
Created attachment 637206 [details] [diff] [review]
patch v2

Thanks, I think I addressed all issues.
Attachment #636875 - Attachment is obsolete: true
Attachment #636875 - Flags: feedback?(iann_bugzilla)
Attachment #637206 - Flags: ui-review?(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.
Attachment #637206 - Flags: ui-review?(bwinton) → ui-review+

Comment 11

5 years ago
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.
(Assignee)

Comment 12

5 years ago
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?
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…
(Assignee)

Comment 14

5 years ago
Created attachment 637266 [details] [diff] [review]
patch v3
Attachment #637206 - Attachment is obsolete: true
Attachment #637266 - Flags: ui-review?(bwinton)
Comment on attachment 637266 [details] [diff] [review]
patch v3

Yep, looks good.  ui-r=me.

Later,
Blake.
Attachment #637266 - Flags: ui-review?(bwinton) → ui-review+
(Assignee)

Updated

5 years ago
Attachment #637266 - Flags: review?(iann_bugzilla)

Comment 16

5 years ago
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.
Attachment #637266 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 17

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

Comment 18

5 years ago
Created attachment 643554 [details] [diff] [review]
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).
Attachment #637266 - Attachment is obsolete: true
Attachment #643554 - Flags: review?(squibblyflabbetydoo)

Comment 19

5 years ago
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.
Attachment #643554 - Flags: review?(squibblyflabbetydoo) → review+

Comment 20

5 years ago
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.
(Assignee)

Comment 21

5 years ago
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.
(Assignee)

Comment 22

5 years ago
Created attachment 647247 [details] [diff] [review]
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).
Attachment #643554 - Attachment is obsolete: true
Attachment #647247 - Flags: review?(squibblyflabbetydoo)
Attachment #647247 - Flags: feedback?(iann_bugzilla)

Comment 23

5 years ago
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) {}
Attachment #647247 - Flags: review?(squibblyflabbetydoo) → review+

Comment 24

5 years ago
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.
Attachment #647247 - Flags: feedback?(iann_bugzilla) → feedback+
(Assignee)

Comment 25

5 years ago
Created attachment 649012 [details] [diff] [review]
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?
Attachment #647247 - Attachment is obsolete: true
Attachment #649012 - Flags: ui-review?(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.
Attachment #649012 - Flags: ui-review?(bwinton) → ui-review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/3c84748b73e9
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 17.0
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 29

5 years ago
Then you should file a new bug for that because you want to change how the backend .sizeOnDisk function works.
(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…
(Assignee)

Comment 31

5 years ago
But correct info is not the job of this bug. It just exposes whatever .sizeOnDisk function returns, with all its current limitations.
(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…
Stefan, please file a new bug
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 786022

Updated

5 years ago
Depends on: 786023
You need to log in before you can comment on or make changes to this bug.