549106 - When printing a message with scale 100%, printed inline previews of attached big images aren't scaled to fit, causing useless, partial printout and waste pages (and with scale "Shrink to fit", message body text becomes unreadable)

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[Thomas D. (:thomas8)]

STR

1 print received msg with image file attachment, e.g. attachment.gif, high resolution pic
2 look at printout

Actual result
- inline image preview is force-printed with the msg (bug 387237)
- image printout isn't scaled down to fit full image (as in msg preview)
- prints only a fraction of the image, other parts are skipped
-> useless and incomplete printout, can't recognize image, lots of cluttered & wasted pages

Expected result:
(- implement bug 387237 for a start)
- scale image printout so that full image fits on page (at least width should fit, as in preview)
- useful image printout that is human-readable and paper-saving
- people who want their attachment printed in full resolution should use native applications to do so

Comment 1

[Thomas D. (:thomas8)]

Reported against Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.8) Gecko/20100223 Shredder/3.0.3pre

Comment 2

[Thomas D. (:thomas8)]

I wonder why we are showing the correct thing in message preview (legible text in normal size AND nicely-shrunk-to-fit image previews), but when it comes to printing, we fail, and I wonder how hard it would be to print what we already show in msg preview.

Please cc others who might know about the code!

-----

Imho this bug is actually one of TB's shame bugs, where we fail for the most basic tasks, like printing a message with attached big images (say high-resolution photos) correctly out of the box - it is not possible at the moment.

Short of disabling "View attachments inline", which is default setting and not easily found, user can only choose between Pest and Cholera:

In Print Preview,

Scale: 100% -> This will print the message body text in normal size, but doesn't shrink the image to fit, so you just get some useless oversized corner of your image and the rest is never printed anywhere. That's ugly and wasting paper for nothing.

Scale: Shrink to fit -> This shrinks your big image to fit on one page, but now unfortunately you need a magnifying glass to read your message body text which has become illegible small print (we have other bugs in this corner).

Comment 3

[Magnus Melin [:mkmelin]]

shrinktofit is probably relevant 
http://mxr.mozilla.org/comm-central/search?string=shrinktofit&find=mail&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central

either the attribute is not set for the printing case, or the css rules aren't there for it

Comment 4

[Thomas D. (:thomas8)]

(In reply to Magnus Melin from comment #3)
> shrinktofit is probably relevant 
> http://mxr.mozilla.org/comm-central/
> search?string=shrinktofit&find=mail&findi=&filter=^[^\0]*%24&hitlimit=&tree=c
> omm-central
> 
> either the attribute is not set for the printing case, or the css rules
> aren't there for it

Shrinktofit is another can of worms which creates unreadable smallprint when shrinking a msg with text and big inline previews, but this bug is about printing at 100% size which will print just a corner of inline previews of attached big images instead of shrinking *only the images* to fit (as in message reader). So I don't think shrinktofit matters for this bug, or does it?

Comment 5

[Thomas D. (:thomas8)]

(In reply to Thomas D. from comment #4)
> (In reply to Magnus Melin from comment #3)
> > shrinktofit is probably relevant 

Pls scrap my last comment, I didn't look at the link provided by Magnus close enough. So yes, comment 3 seems to point in the right direction.

Comment 6

[Thomas D. (:thomas8)]

(In reply to Magnus Melin from comment #3)
> shrinktofit is probably relevant 
> http://mxr.mozilla.org/comm-central/
> search?string=shrinktofit&find=mail&findi=&filter=^[^\0]*%24&hitlimit=&tree=c
> omm-central
> 
> either the attribute is not set for the printing case, or the css rules
> aren't there for it

Richard, could you check if the CSS rules are available?

Comment 7

[Richard Marti (:Paenglab)]

Attachment: shrinktofit.patch

The style isn't applied because in print the overflowing attribute (https://dxr.mozilla.org/comm-central/search?q=Scale+any+overflowing+images%2C+exclude+http+content+-path%3Aobj-x86&redirect=false) isn't set and our rule wont apply.

Either we need to add code that the overflowing attribute is set in print too or simpler we add a special print rule without the need of the [overflowing].

Comment 8

[Thomas D. (:thomas8)]

Comment on attachment 8886765 [details] [diff] [review]
shrinktofit.patch

Review of attachment 8886765 [details] [diff] [review]:
-----------------------------------------------------------------

Wow! Superfast response time, thanks.

Hmmm. This looks right, but on my local install this doesn't change the behaviour for me in any way, still as described in comment 0. Did you test with a big picture (high-resultion photograph) attached to the message? Does this patch work for you?

Comment 9

[Thomas D. (:thomas8)]

I tried with saved draft, is messagebody.css applicable to that?

Comment 10

[Thomas D. (:thomas8)]

Btw we also want to ensure this looks right on print preview, which is what I tried first, but also no change in behaviour there.

Comment 11

[Thomas D. (:thomas8)]

Tested on Windows 10, TB 53.0a1 (2017-01-04) (64-bit), my local flat TB install from January this year...

Comment 12

[Richard Marti (:Paenglab)]

For me this works, only tested in print preview, with attached images. The image has always the page width when it's wider than the page. When I scale the preview down then the image is also scaled down correctly.

You are sure it's not an image which is embedded in a HTML page? My test is a plain text message with a attached image.

Comment 13

[Richard Marti (:Paenglab)]

Comment on attachment 8886765 [details] [diff] [review]
shrinktofit.patch

Let's see what Jörg says to this patch. There is no need to print, only check the print preview.

Comment 14

[Thomas D. (:thomas8)]

Comment on attachment 8886765 [details] [diff] [review]
shrinktofit.patch

Review of attachment 8886765 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, my mistake: Accidentally copied + characters over from patch into css. Now it works! And fixes both parts of the problem, "Shrink to fit" no longer creates microscopic message text font sizes! That's great, thanks a lot Richard! (r+ on your review request). :)

Tested in Safe Mode, with flat install updated to 56.0a1 (2017-07-15) (64-bit); as a caveat for others with flat installs, note that updating will not update your flat install, but just come with a new omni.ja which then gets preference over your flat files until you flatten it out again and remove or rename omni.ja.

I'll be very happy to see this shame bug go away!
Great teamwork, and again, we could have done this much earlier...

For perfection, and saving paper, perhaps we can fix the following nit here:
Space between inline image header (--- image.name ---) and its image is bigger than between message text and image header.
This is odd because header and image belong together, and we shouldn't waste whitespace for printouts:

> Hello world
>                                <- 1 blank line here (sometimes more?)
> ---- Image.jpg --------------
>                                <- 2 blank lines here
>                                <-
>+---------------------------+
>| (Image)                   +
>+---------------------------+
>                               <- 2 blank lines here
>                               <-
>---- Attachments: ------------
>                               <- 1 blank line here
> Image.jpg               2 Mb

Personally, I wouldn't have more than 1 line for any of these whitespace areas.
Between captions and their content, maybe even 1/2 blank line would do.
As a user, I'll certainly not like it if TB's over-generous print layout causes extra almost empty pages to be printed for no reason.

One more nit (maybe followup bug):

1) Is it possible to keep each image header together with its image? It's very odd and confusing to have image header on first page, then the respective image on next page... And now that we're scaling images better, this shouldn't be a problem...

Comment 15

[Thomas D. (:thomas8)]

(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #14)
> Personally, I wouldn't have more than 1 line for any of these whitespace
> areas.
> Between captions and their content, maybe even 1/2 blank line would do.
> As a user, I'll certainly not like it if TB's over-generous print layout
> causes extra almost empty pages to be printed for no reason.

-> That's existing bug 699000, maybe it can be fixed?

Comment 16

[Richard Marti (:Paenglab)]

https://hg.mozilla.org/comm-central/rev/ca430d5ac1be6b6418066639b7d59f7cd77ce9b0

Comment 17

[Richard Marti (:Paenglab)]

Comment on attachment 8886765 [details] [diff] [review]
shrinktofit.patch

This is a simple patch which could enhance the usability also for TB 52.

Comment 18

[Thomas D. (:thomas8)]

Attachment: A big image for testing - beach-01.jpg

Comment 19

[Thomas D. (:thomas8)]

Attachment: Testcase1.eml: plaintext message with big image attached

Test message: plaintext with big image attached

Comment 20

[Jorg K (CEST = GMT+2)]

Someone stole my review :-(

As for beta approval: I don't know we're doing another TB 55 beta since beta 2 has just been done today.

Comment 21

[Thomas D. (:thomas8)]

Attachment: Printout1.pdf: (before patch, scaled 100%) Test msg - plaintext with big image attached

FTR: To better understand the significance of this bug, and the radical improvement of printing UX, here's the garbage which we used to print before the patch, with printout scaled to 100%:
- Message printout bloated from 1 page to 3 pages
- Partial, huge image printout; cut both sides (datalossy!)
- last page with attachment list, almost empty

Comment 22

[Thomas D. (:thomas8)]

Attachment: Printout2.pdf: (before patch, Shrink-to-fit) Test msg - plaintext with big image attached.pdf

Worse, this is how the printout of Testcase1.eml looked before the patch, now with *Shrink-to-fit* scaling: Main message text shrinked to unreadable microprint.

Comment 23

[Thomas D. (:thomas8)]

Attachment: Printout3.pdf: (after patch, 100% or Shrink-to-fit) Test msg - plaintext with big image attached

Finally, this is how the printout of Testcase1.eml looks *after* the patch: Everything nice and neatly on a single page. Printing tamed.

Comment 25

[Jorg K (CEST = GMT+2)]

Comment on attachment 8886765 [details] [diff] [review]
shrinktofit.patch

Not doing another TB 55 beta.

Comment 26

[Jorg K (CEST = GMT+2)]

TB 52.3 ESR:
https://hg.mozilla.org/releases/comm-esr52/rev/72188657faa8