Closed
Bug 636199
Opened 13 years ago
Closed 13 years ago
Port |Bug 195702 - attachment size should be visible in compose window| (and bug 634620)
Categories
(SeaMonkey :: MailNews: Composition, enhancement)
SeaMonkey
MailNews: Composition
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.1b3
People
(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)
References
Details
Attachments
(1 file)
8.60 KB,
patch
|
mnyromyr
:
review+
mnyromyr
:
superreview+
|
Details | Diff | Splinter Review |
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.
Attachment #514531 -
Flags: review?(mnyromyr)
Comment 1•13 years ago
|
||
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
Attachment #514531 -
Flags: superreview+
Attachment #514531 -
Flags: review?(mnyromyr)
Attachment #514531 -
Flags: review+
Assignee | ||
Comment 2•13 years ago
|
||
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)
Attachment #514531 -
Attachment description: patch → patch [Checkin: comment 2]
Assignee | ||
Updated•13 years ago
|
No longer blocks: TB2SM
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b3
Updated•13 years ago
|
Assignee: nobody → jh
Whiteboard: [TB2SM]
You need to log in
before you can comment on or make changes to this bug.
Description
•