Last Comment Bug 636199 - Port |Bug 195702 - attachment size should be visible in compose window| (and bug 634620)
: Port |Bug 195702 - attachment size should be visible in compose window| (and ...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Composition (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: seamonkey2.1b3
Assigned To: Jens Hatlak (:InvisibleSmiley)
:
Mentors:
: 636185 (view as bug list)
Depends on: 195702 634620
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-23 10:25 PST by Jens Hatlak (:InvisibleSmiley)
Modified: 2012-06-21 12:57 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch [Checkin: comment 2] (8.60 KB, patch)
2011-02-23 10:25 PST, Jens Hatlak (:InvisibleSmiley)
mnyromyr: review+
mnyromyr: superreview+
Details | Diff | Splinter Review

Description Jens Hatlak (:InvisibleSmiley) 2011-02-23 10:25:28 PST
Created attachment 514531 [details] [diff] [review]
patch [Checkin: comment 2]

Bug 26517 added the attachment size to the attachments list of existing messages. This is for adding the same for new messages (composition).

Note: This does not port |Bug 623346 - Show Total File Size of all attachments|, which I'd like to keep for another porting bug. Bug 623346 is already listed as a TB2SM dependency.

NB: It was hard to resist the temptation of reformatting everything. Too bad I'm not allowed to do that.
Comment 1 Karsten Düsterloh 2011-03-09 10:46:47 PST
Comment on attachment 514531 [details] [diff] [review]
patch [Checkin: comment 2]

>       for (let i = 0; i < dataListLength; i++)
>       {
>         var item = dataList[i].first;
>         var prettyName;
>+        var size;
>         var rawData = item.data;

This is a bit weird (not your fault) - 'var' variables are always function-global in JS…
I'd prefer sub-scope definitions to use 'let' instead of 'var'.

>+            if (pieces.length > 2)
>+              size = parseInt(pieces[2]);

If - whyever - pieces[2] contains junk that begins with a number, parseInt will take for real and extract that number, which will usually be wrong. 

>+              if (size != undefined)
>+                attachment.size = size;

Comparing against undefined is plain ugly.

How about:
- initializing size to NaN
- using Number() instead parseInt(), because it'll give NaN for invalid numbers
- checking 'if (!isNaN(size))' finally before using it 

r/moa=me with that
Comment 2 Jens Hatlak (:InvisibleSmiley) 2011-03-09 13:24:09 PST
Comment on attachment 514531 [details] [diff] [review]
patch [Checkin: comment 2]

http://hg.mozilla.org/comm-central/rev/34dc4195f5fd
with comment 1 addressed (complete in-for-loop s/var/let/ as per IRC)
Comment 3 Philip Chee 2012-06-21 12:57:43 PDT
*** Bug 636185 has been marked as a duplicate of this bug. ***

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