544984 - Improve print format of attachment file names, esp. remove content-type and content-encoding

Status: RESOLVED FIXED

(MailNews Core :: Printing, enhancement)

Description

[Thomas D. (:thomas8)]

This bug is about improving the visual presentation and content of the list of attachment file names below the msg text when printing out a message. Currently it's somewhat archaic. Instead, it should be modern, visually attractive, and simple. Ideally, it should also be customizable. Here's some first practical ideas towards that goal.

STR
- print a msg with several attachments and look at the list of attachment file names below msg text

Actual result

The current format and content of attachment list is 
- ugly (visual table frames, untidy impression)
- providing superfluous information (content-type, content-encoding: these are completely useless for the average user. Even for the advanced user, there's not much point in having them, and iirc they can even be faked).
- wasting space -> bug 443634, and this bug will also help to that end
- sometimes making the whole printout unreadable -> bug 544950

Expected result
1) remove visual table frames (old-fashioned layout)
2) remove extra separator lines with duplicated file names
3) add visual indication for attachment list, e.g.
3a) a single separator line between msg text and attachments
3b) and/or a headline titled "Attachments"
4) unify bold print application
4a) remove attachment file name bold print (icons are better indicator, see 6a)
4b) (optionally) bold print for "attachments" title (similar to other headers)
5) remove useless information (content-type, content-encoding). This will also save space, both horizontally and vertically
6) include helpful information
6a) 16px attachment icon on the left
6b) (optionally) file size on the right

So altogether, a basic version of better attachments list printout could look like this:

[Msg header]

[Msg text]

--*Attachments:*------------------------------------------------
[W] A Word document.doc        23 kb
[X] An Excel document.xls     400 kb
[P] A PDF document.pdf     20.000 kb  

...where [W], [X], and [P] are the respective system icons for that document type, as we already show them in attachment pane - why not print them in a wysiwyg manner?

Comment 1

[Ludovic Hirlimann [:Usul]]

How's this different from 387237 ?

Comment 2

[Thomas D. (:thomas8)]

(In reply to comment #1)
> How's this different from 387237 ?

Bug 387237 is about excluding inline attachment content previews from printing (attachments that are displayed inline, like html, images etc.). It's not about the list of attachment file names at the end of the printed msg.

This bug 544984 is only about brushing up the visual format of the list of attachment files that is printed at the end of the msg. It's not about printing inline attachments.

This bug proposes one specific solution for similar bug 443634. As explained in bug 443634 comment 3, bug 443634 could be duplicated against this one, but not vice versa.

Comment 3

[Jim Porter (:squib)]

I might end up working on this after I finish up bug 559559. We'll see.

Comment 4

[Jim Porter (:squib)]

Taking this. I'll try to upload a patch in a few days.

Comment 5

[Jim Porter (:squib)]

Attachment: Improve print formatting

This simplifies the print formatting to name + size. I didn't add icons because I couldn't figure out how to do it.

Comment 6

[Jim Porter (:squib)]

Attachment: What it looks like

Here's what it looks like.

clarkbw, any input/suggestions?

Comment 7

[Jonathan Protzenko [:protz]]

(In reply to comment #5)
> Created attachment 487070 [details] [diff] [review]
> Improve print formatting
> 
> This simplifies the print formatting to name + size. I didn't add icons because
> I couldn't figure out how to do it.

You can do this using the gecko moz-icon protocol:

"moz-icon://" + att.name + "?size=" + 16 + "&contentType=" + att.contentType

Beware, it doesn't always work well, but it's better than nothing and I think it's the way it's done in other places as well.

Comment 8

[Jim Porter (:squib)]

I recall trying that and it not working when I printed. Maybe there are some other subtleties to it.

Comment 9

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 487072 [details]
What it looks like

That looks a lot better.

Here's a few design suggestions, I did a quick mockup in HTML:
http://clarkbw.net/designs/tb-print-email/

I'm not sure where the margins/spacing is coming from but I would lessen them to only 1em in from the attachments header text.

With interfaces we'd usually us an alternating background color to give some structure to the list which helps people relate the name to the size.  However I was thinking that wouldn't be as good for printers which would have to print a long background color so I went with a thin border separator.  I did both in the mockup to show the difference.

Some detail notes: The last one doesn't have a bottom border and a single attachment doesn't have a bottom border either (I used a redundant CSS rule for that).  The Attachments header can remain as is, I didn't copy the coloring exactly.

Also I think it would make sense for here and perhaps the other places we show this text to use "color: GrayText" as the size is less important data than the name.

ui-r+ so far, let me know if the suggestions are difficult or awkward for some reason.  Thanks!

Comment 10

[Jim Porter (:squib)]

Attachment: Updated display

When printing (at least to .ps and .pdf), gray text ends up being rendered in black. I suppose this is intentional to make things easier to read, but it does mean that GrayText won't work for printing.

Eventually, I'd like a layout more like this for attachments elsewhere, instead of the "attachment.txt (1.2 KB)" that happens currently. One thing at a time, though. :)

In the attached image, I fixed up the margins and added horizontal lines between files. Note also that the "Attachments:" header is kinda screwy; the horizontal line is going through the word. This also happens elsewhere, and I think it's a bug in libmime.

Comment 11

[Jim Porter (:squib)]

(In reply to comment #10)
> Note also that the "Attachments:" header is kinda screwy; the
> horizontal line is going through the word. This also happens elsewhere, and I
> think it's a bug in libmime.

Actually, this might be a bug in printing.

Comment 12

[Jim Porter (:squib)]

Attachment: Updated patch

Here's the patch for the above changes.

Comment 13

[Ludovic Hirlimann [:Usul]]

(In reply to comment #12)
> Created attachment 488597 [details] [diff] [review]
> Updated patch
> 
> Here's the patch for the above changes.

Any reasons you didn't ask review ?

Comment 14

[Jim Porter (:squib)]

Comment on attachment 488597 [details] [diff] [review]
Updated patch

(In reply to comment #13)
> Any reasons you didn't ask review ?

Originally, I was hoping to get attachment icons to work, but I have no idea what the issue is there, so I guess I'll flag this for review.

Comment 15

[Jim Porter (:squib)]

Comment on attachment 488536 [details]
Updated display

Just to be on the safe side: clarkbw, does this look close enough to your proposal?

Comment 16

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 488536 [details]
Updated display

looks good, I think GrayText is the best choice we have available there for border color.

Comment 17

[David :Bienvenu]

Comment on attachment 488597 [details] [diff] [review]
Updated patch

Thx very much for the patch. I'll give this a try (sorry for the delay), but in the meantime, can you attach a patch where you don't change all the braces? I.e., put them back on their own line, which is the prevailing style in this file.

Comment 18

[Jim Porter (:squib)]

Attachment: Use Allman braces

Ok, here's a version with Allman braces.

Comment 19

[David :Bienvenu]

Comment on attachment 496636 [details] [diff] [review]
Use Allman braces

missed on brace:

        if (!mHeaderSink) { // if the url is not overriding the header sink, then just get the one from the msg window

r=me with that nit fixed, thx for the patch!

Comment 20

[Jim Porter (:squib)]

Attachment: Fix a brace [Checkin: comment 22]

This fixes the above comment, pulling r+ forward.

Does this need sr or can I tag it with checkin-needed?

Comment 21

[Ludovic Hirlimann [:Usul]]

(In reply to comment #20)
> Created attachment 496901 [details] [diff] [review]
> Fix a brace
> 
> This fixes the above comment, pulling r+ forward.
> 
> Does this need sr or can I tag it with checkin-needed?

Doesn't look like you are changing an API so checking-needed is the way to go.

Comment 22

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 496901 [details] [diff] [review]
Fix a brace [Checkin: comment 22]

http://hg.mozilla.org/comm-central/rev/9704a3548bdb

Comment 23

[Jens Hatlak (:InvisibleSmiley)]

Pushed; leaving bug resolution to you.

Comment 24

[Jim Porter (:squib)]

Resolving this. Maybe we want a Litmus test for this?

Comment 25

[Philip Chee]

Did anyone get reviews for the /suite changes?

Comment 26

[Jim Porter (:squib)]

Oops, it doesn't look like it.

Comment 27

[Philip Chee]

> Oops, it doesn't look like it.
Neil says over IRC that we keep this stuff in sync with Thunderbird anyway so it should be OK. No further action needed. Thanks for including the suite fix in your patch as well.

Comment 28

[Ludovic Hirlimann [:Usul]]

(In reply to comment #24)
> Resolving this. Maybe we want a Litmus test for this?

https://litmus.mozilla.org/show_test.cgi?id=15027 Ain't sure if I should details things more or not in the TC.

Comment 29

[Jim Porter (:squib)]

(In reply to comment #28)
> (In reply to comment #24)
> > Resolving this. Maybe we want a Litmus test for this?
> 
> https://litmus.mozilla.org/show_test.cgi?id=15027 Ain't sure if I should
> details things more or not in the TC.

Hmm. "Print preview" should be sufficient for this. I guess you could say that the attachments list should be at the end and have one line per attachment with the size on the right, but then again a picture says a thousand words.

Comment 30

[Thomas D. (:thomas8)]

As the reporter of this bug, may I send a big thank you to all who contributed to the fix, especially Jim! With just a handful of people like Jim, so many neglected little corners of TB could be polished to return the badly-needed shine to the bird!

Jim, did you use a two-colum table for the attachment list?
If yes, does it have fixed width (hopefully yes)?
iow, could someone test the fix for this bug with attachments whose file names are longer than a single line and check if bug 544950 still occurs?
Otherwise, bug 544950 might have been fixed by this bug.