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

RESOLVED FIXED in Thunderbird 3.3a2

Status

MailNews Core
Printing
--
enhancement
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: Thomas D. (currently busy elsewhere; needinfo?me), Assigned: squib)

Tracking

(Blocks: 2 bugs)

Trunk
Thunderbird 3.3a2
Dependency tree / graph
Bug Flags:
in-litmus +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 4 obsolete attachments)

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?
How's this different from 387237 ?
Component: General → Printing
Product: Thunderbird → MailNews Core
QA Contact: general → printing
(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.
Blocks: 443634
(Assignee)

Comment 3

7 years ago
I might end up working on this after I finish up bug 559559. We'll see.
(Assignee)

Comment 4

7 years ago
Taking this. I'll try to upload a patch in a few days.
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
(Assignee)

Updated

7 years ago
Depends on: 559559
(Assignee)

Comment 5

7 years ago
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.
(Assignee)

Comment 6

7 years ago
Created attachment 487072 [details]
What it looks like

Here's what it looks like.

clarkbw, any input/suggestions?
Attachment #487072 - Flags: ui-review?(clarkbw)
(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.
(Assignee)

Comment 8

7 years ago
I recall trying that and it not working when I printed. Maybe there are some other subtleties to it.
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!
Attachment #487072 - Flags: ui-review?(clarkbw) → ui-review+
(Assignee)

Comment 10

7 years ago
Created attachment 488536 [details]
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.
Attachment #487072 - Attachment is obsolete: true
(Assignee)

Comment 11

7 years ago
(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.
(Assignee)

Comment 12

7 years ago
Created attachment 488597 [details] [diff] [review]
Updated patch

Here's the patch for the above changes.
Attachment #487070 - Attachment is obsolete: true
(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 ?
(Assignee)

Comment 14

7 years ago
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.
Attachment #488597 - Flags: review?(bienvenu)
(Assignee)

Comment 15

7 years ago
Comment on attachment 488536 [details]
Updated display

Just to be on the safe side: clarkbw, does this look close enough to your proposal?
Attachment #488536 - Flags: ui-review?(clarkbw)
Comment on attachment 488536 [details]
Updated display

looks good, I think GrayText is the best choice we have available there for border color.
Attachment #488536 - Flags: ui-review?(clarkbw) → ui-review+

Comment 17

7 years ago
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.
(Assignee)

Comment 18

7 years ago
Created attachment 496636 [details] [diff] [review]
Use Allman braces

Ok, here's a version with Allman braces.
Attachment #488597 - Attachment is obsolete: true
Attachment #496636 - Flags: review?(bienvenu)
Attachment #488597 - Flags: review?(bienvenu)

Comment 19

7 years ago
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!
Attachment #496636 - Flags: review?(bienvenu) → review+
(Assignee)

Comment 20

7 years ago
Created attachment 496901 [details] [diff] [review]
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?
Attachment #496636 - Attachment is obsolete: true
Attachment #496901 - Flags: review+
(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.
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
Comment on attachment 496901 [details] [diff] [review]
Fix a brace [Checkin: comment 22]

http://hg.mozilla.org/comm-central/rev/9704a3548bdb
Attachment #496901 - Attachment description: Fix a brace → Fix a brace [Checkin: comment 22]
Pushed; leaving bug resolution to you.
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 3.3a2
(Assignee)

Comment 24

7 years ago
Resolving this. Maybe we want a Litmus test for this?
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 25

7 years ago
Did anyone get reviews for the /suite changes?
Flags: in-litmus?(ludovic)
(Assignee)

Comment 26

7 years ago
Oops, it doesn't look like it.

Comment 27

7 years ago
> 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.
(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.
Flags: in-litmus?(ludovic) → in-litmus+
(Assignee)

Comment 29

7 years ago
(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.
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.
Blocks: 544950
Blocks: 666840
(Assignee)

Updated

5 years ago
No longer depends on: 559559
(Assignee)

Updated

5 years ago
Depends on: 559559
You need to log in before you can comment on or make changes to this bug.