Last Comment Bug 779736 - FormatFileSize fails for sizes >999.5 GB
: FormatFileSize fails for sizes >999.5 GB
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Attachments (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: Thunderbird 17.0
Assigned To: Jim Porter (:squib)
:
Mentors:
Depends on:
Blocks: 668756
  Show dependency treegraph
 
Reported: 2012-08-02 00:15 PDT by Jim Porter (:squib)
Modified: 2012-08-18 01:04 PDT (History)
2 users (show)
squibblyflabbetydoo: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Fix formatFileSize (5.46 KB, patch)
2012-08-02 01:18 PDT, Jim Porter (:squib)
standard8: review+
Details | Diff | Review

Description Jim Porter (:squib) 2012-08-02 00:15:28 PDT
It turns out I had an off-by-one error in FormatFileSize. We should fix this. I have a patch, but haven't written a test yet. I will (hopefully) do that in the next couple of days.

(This isn't really the right component, but it's the closest there is in MailNews Core.)
Comment 1 Jim Porter (:squib) 2012-08-02 01:18:51 PDT
Created attachment 648246 [details] [diff] [review]
Fix formatFileSize

Here's a fix, with unit tests. I also fixed a bug where formatFileSize(0, true) returned "0.1 KB". It now returns "0 KB".
Comment 2 :aceman 2012-08-02 02:01:21 PDT
Kudos for the size==0 case ;)
Comment 3 Mark Banner (:standard8) 2012-08-17 03:59:49 PDT
Comment on attachment 648246 [details] [diff] [review]
Fix formatFileSize

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

r=me with the comments fixed.

::: mailnews/base/test/unit/test_formatFileSize.js
@@ +2,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +/*
> + * Tests for nsIMessenger methods.

Whilst true, I think this should just reference formatFileSize, or you need to rename the file...

@@ +9,5 @@
> +load("../../../../mailnews/resources/logHelper.js");
> +load("../../../../mailnews/resources/asyncTestUtils.js");
> +
> +const gStringBundle = Cc["@mozilla.org/intl/stringbundle;1"]
> +                        .getService(Ci.nsIStringBundleService)

Please use Services.strings
Comment 4 Jim Porter (:squib) 2012-08-18 01:04:16 PDT
Checked in: https://hg.mozilla.org/comm-central/rev/0ce4d789f7ec

Note You need to log in before you can comment on or make changes to this bug.