Closed Bug 736881 Opened 8 years ago Closed 8 years ago

Improve calculation of attached message size


(Thunderbird :: Message Reader UI, defect)

Not set


(Not tracked)

Thunderbird 14.0


(Reporter: squib, Assigned: squib)




(1 file, 2 obsolete files)

Attached patch Improve attached message sizes (obsolete) — Splinter Review
Currently, there are two problems with the sizes of attached messages:

1) the size of the headers isn't included
2) for the total size, subparts of the attached message are counted twice

This patch fixes both issues, with tests.
Attachment #606999 - Flags: review?(dbienvenu)
Comment on attachment 606999 [details] [diff] [review]
Improve attached message sizes

To be safe, this could use UI review. Aside from comment 0, the only significant thing I should mention is that when attached emails have attachments, the "total size" in the attachment pane isn't the sum of all sizes in the attachment list, since that would cause the inner attachment to be counted twice.

I think this is probably the best way to handle this, unless we want to hide inner attachments in the list entirely. I don't know if I want to do that though, since the current way makes it easier to get at those attachments.
Attachment #606999 - Flags: ui-review?(bwinton)
Assignee: nobody → squibblyflabbetydoo
Attached patch Fix size counting (obsolete) — Splinter Review
Whoops, it turns out I over-corrected for libmime's weirdness with newlines. This should now produce consistent results for the size of attached messages, no matter whether they're displayed inline or not.
Attachment #606999 - Attachment is obsolete: true
Attachment #606999 - Flags: ui-review?(bwinton)
Attachment #606999 - Flags: review?(dbienvenu)
Attachment #607081 - Flags: ui-review?(bwinton)
Attachment #607081 - Flags: review?(dbienvenu)
Comment on attachment 607081 [details] [diff] [review]
Fix size counting

When I saved a .eml attachment, the file on disk was slightly smaller (11.7 KB vs 11.8 in our UI).  The .eml file uses 8 bit encoding, so I wouldn't expect encoding to be an issue. This is on windows, so the saved file should have the same crlf endings.

Anyway, this patch does work as advertised.
Attachment #607081 - Flags: review?(dbienvenu) → review+
The thing I used to compare the file sizes to make sure they're correct (or, as correct as libmime gets, anyway) is to toggle "Display Attachments Inline" on and off. The sizes for the .eml should be the same in both cases. You can get the exact size with the DOM inspector by clicking on the attachment in the attachment list, navigating up to the <attachmentitem>, then looking at the "attachment" member of the JS object.

I'm not entirely surprised that the reported size is slightly different from the size you'd get when saving, since I know libmime does some weird things with trailing newlines, among other things.
Blocks: 736798
Attached patch Fix bitrotSplinter Review
Attachment #607081 - Attachment is obsolete: true
Attachment #607081 - Flags: ui-review?(bwinton)
Attachment #613218 - Flags: ui-review?(bwinton)
Attachment #613218 - Flags: review+
Comment on attachment 613218 [details] [diff] [review]
Fix bitrot

Yeah, sure, making the number better makes sense to me.  (This is more of a ui-rs, but ui-r=me.)
Attachment #613218 - Flags: ui-review?(bwinton) → ui-review+
Checked in:
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 14.0
You need to log in before you can comment on or make changes to this bug.