713383 - Directly viewed images should be printed with a transparent background on the body

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Andreas Jung]

Since bug 376997 was fixed images are printed centered on the page.
This is a huge waste of space on the printout.

Additionally, if you have "Print Background (colors & images)" (in Page Setup) enabled, it prints the whole page in a dark gray color, which is a huge waste of ink / toner. On the other hand, this setting is a waste of ink / toner anyway, so maybe it doesn't matter?

I think the CSS rules added in bug 376997 probably should not apply for print.

Comment 1

[Daniel Holbert [:dholbert]]

Confirmed (image prints centered, w/ gray background if "Print Background Color" checked), in:
   Mozilla/5.0 (X11; Linux x86_64; rv:12.0a1) Gecko/20111223 Firefox/12.0a1

> I think the CSS rules added in bug 376997 probably should not apply for print.

I very much agree.  This should be simple with CSS Media Queries:
 https://developer.mozilla.org/en/CSS/Media_queries
perhaps like:
  @media screen { ... }
or
  @media not print { ... }

Comment 2

[Daniel Holbert [:dholbert]]

Setting "tracking-firefox11?", since that'd be the first release affected by this bug.

Comment 3

[Josh Matthews [:jdm]]

http://mxr.mozilla.org/mozilla-central/source/layout/style/TopLevelImageDocument.css is the file that needs changing. Look at the patch from bug 367997 to see what changes were made previously.

Anybody who has a better knowledge of CSS and these matters in general will probably be a more effective mentor than me, but I'm willing to take that role for the time being.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Andreas Jung from comment #0)
> Since bug 376997 was fixed images are printed centered on the page.
> This is a huge waste of space on the printout.

I'm not sure why printing in the center of the page is a waste of space compared to the top-left. Can you please describe this more?

> Additionally, if you have "Print Background (colors & images)" (in Page
> Setup) enabled, it prints the whole page in a dark gray color, which is a
> huge waste of ink / toner.

Yeah, I agree that this should be fixed.

Comment 5

[Josh Matthews [:jdm]]

(In reply to Jared Wein [:jwein and :jaws] (Vacation December 23-January 3) from comment #4)
> (In reply to Andreas Jung from comment #0)
> > Since bug 376997 was fixed images are printed centered on the page.
> > This is a huge waste of space on the printout.
> 
> I'm not sure why printing in the center of the page is a waste of space
> compared to the top-left. Can you please describe this more?

Consider if you are just going to cut the image out of the paper on which it is printed.

Comment 6

[Justin Dolske [:Dolske]]

(In reply to Josh Matthews [:jdm] from comment #5)

> Consider if you are just going to cut the image out of the paper on which it
> is printed.

Well, there's still a margin around it (since basically all printers are unable to print edge-to-edge). I'd be curious to see a sampling from various image edit/viewer apps, to see what they do.

As jared noted, we _clearly_ need to not print with a background color. Let's keep this bug focused on doing that, especially since it's tagged as [good first bug].

Comment 7

[Andreas Jung]

(In reply to Justin Dolske [:Dolske] from comment #6)
> (In reply to Josh Matthews [:jdm] from comment #5)
> 
> > Consider if you are just going to cut the image out of the paper on which it
> > is printed.
> 
> Well, there's still a margin around it

That's true, but the new behavior is still bad in (at least) two cases:
1) I print a diagram etc. and want to make handwritten notes on the printout.
In this case one larger empty area at the bottom of the page is usually more useful than two small areas above and below.
2) Let's assume I want to print a photo on expensive paper (OK, Firefox isn't really a photo printing program, but it is the fastest solution when printing from the internet and not caring if the width of the printed photo is 18cm or 18.783cm ). Previously, I could easily print two photos on one page by printing one photo, turning the page by 180° and printing the second photo.
Now this isn't possible anymore and I either have to use another program or waste a lot of paper.

> I'd be curious to see a sampling from various
> image edit/viewer apps, to see what they do.

Windows Picture and Fax Viewer (Win XP): Photo is printed (with margin; only predefined formats) in the top left corner.

Adobe Photoshop CS2: When using shrink to fit (afaik this option assumes a printer that can print without a margin) => centered on the page, no margin; when choosing the size manually => users choice, centered or custom position, custom margins.

Internet Explorer 8: With margin in the top left corner.

> As jared noted, we _clearly_ need to not print with a background color.
> Let's keep this bug focused on doing that, especially since it's tagged as
> [good first bug].

I don't think the patch is more complicated when fixing both issues (since it's the same CSS from bug 376997) (actually writing two patches for two bugs is probably unnecessary work). But if you want the discussion for both issues separated then that's fine by me.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Josh Matthews [:jdm] from comment #3) 
> Anybody who has a better knowledge of CSS and these matters in general will
> probably be a more effective mentor than me, but I'm willing to take that
> role for the time being.

I'll take the mentor flag from you! :)

As mentioned above, let's stick with this bug just removing the background color from the print stylesheet. The discussions around the positioning of the image can be brought up in a different bug if someone files it.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Diogo: Would you like to work on this bug?

We'll need to update /layout/style/TopLevelImageDocument.css to have media queries in the stylesheet like dholbert described in comment #1:

 @media not print { ... }

We want the background-color to only apply when |not print|.

Comment 10

[Diogo Golovanevsky Monteiro [:diogogmt]]

(In reply to Jared Wein [:jwein and :jaws] from comment #9)
> Diogo: Would you like to work on this bug?
> 
> We'll need to update /layout/style/TopLevelImageDocument.css to have media
> queries in the stylesheet like dholbert described in comment #1:
> 
>  @media not print { ... }
> 
> We want the background-color to only apply when |not print|.

If nobody is currently working on this bug I would like to give it a try.

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Diogo Golovanevsky Monteiro from comment #10)
> If nobody is currently working on this bug I would like to give it a try.

Thanks Diogo!

Comment 12

[Diogo Golovanevsky Monteiro [:diogogmt]]

Attachment: Changed css to print image with a transparent background

By default, when printing an image, the background is always transparent.
There is an option on the Print menu, under Options Tab, Print Backgrounds section to print the image with a background colour. However, even when checking the option, the image still prints with a transparent background.
It would be nice to have a real time view of the background of the image, so when checking/unchecking the option, be possible to visualize how the final print would look like.
Anyway, I added some changes to set the background of the body to white whenever the image is being printed.

Comment 13

[Daniel Holbert [:dholbert]]

It looks like you're moving the existing styles to be either "@media screen,print", "@media screen", or "@media print"

That leaves out a bunch of other media types, though -- see:
 https://developer.mozilla.org/en/CSS/Media_queries#Pseudo-BNF_%28for_those_of_you_that_like_that_kind_of_thing%29
for a list of other media types.

There are other media types like "projection", "handheld", and "tv" ( which I'd think we want to behave like 'screen') as well as "braille","aural","tty" (which I don't think we need to consider for the purposes of this bug, since a pure image doesn't really have any braille / aural / tty-appropriate presentation)

So anyway -- I suspect we shouldn't explicitly mention "screen" at all in this patch  (despite the fact that I mentioned it in comment 1 -- sorry if that caused confusion. :)).  Rather, I think we want to separate the styles into "applies everywhere", "applies everywhere except print", and "applies only for print" categories.

Comment 14

[Diogo Golovanevsky Monteiro [:diogogmt]]

Attachment: v2

Instead of having two different rules, one for print and another for screen. There is only one new rule: @media not print. So the background will have a greyish color every time the media type is not print, and on all the other media types the background will be greyish as default.

Comment 15

[Justin Dolske [:Dolske]]

I think this should actually just add an explicit |@media print { }| rule to set the background to white. That way it's clear that the color is being set differently in both cases, instead of implicitly relying on the color being white (which may not be correct, I suspect themes / prefs can change that).

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Justin Dolske [:Dolske] from comment #15)
> I think this should actually just add an explicit |@media print { }| rule to
> set the background to white. That way it's clear that the color is being set
> differently in both cases, instead of implicitly relying on the color being
> white (which may not be correct, I suspect themes / prefs can change that).

But wouldn't it be better to respect themes/prefs? I'm also not sure about choosing white as the background color. If we needed to pick a color for print stylesheets, I would rather that we choose |transparent|.

Comment 17

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 590534 [details] [diff] [review]
v2

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

::: layout/style/TopLevelImageDocument.css
@@ +46,2 @@
>  body {
> +	margin: 0;

Nit: There should be two spaces at the beginning of the line but there is a tab character here instead. You might need to change a preference in your text editor to use spaces instead of tabs.

Comment 18

[Diogo Golovanevsky Monteiro [:diogogmt]]

Attachment: Format fixed

Comment 19

[Justin Dolske [:Dolske]]

(In reply to Jared Wein [:jaws] from comment #16)

> But wouldn't it be better to respect themes/prefs? I'm also not sure about
> choosing white as the background color.

No. A theme (or pref) setting the color for the background of a tab doesn't imply users want to print things with that background too. Vastly more likely is someone just didn't consider the implications of that for printing -- like we've done here! [And, frankly, I'd say that printing images with a background page color is not an interesting thing to support.]

> If we needed to pick a color for
> print stylesheets, I would rather that we choose |transparent|.

Sure, I doubt it matters though, since it's going to be composited onto white anyway for CMYK output. (At least, I don't know of any printers that have white ink or can cut away the background paper. ;-)

I suppose "printing" to a PDF might be interesting to consider, not sure what "transparent" will do, nor what the expected result should be if it's different from white.

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 590572 [details] [diff] [review]
Format fixed

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

::: layout/style/TopLevelImageDocument.css
@@ -42,3 @@
>    margin: 0;
>  }
>  

As Justin said in an earlier comment, let's just add the following rule here:

@media print {
 body {
  background-color: white;
 }
}

Comment 21

[Diogo Golovanevsky Monteiro [:diogogmt]]

Attachment: Added rule for media print

Comment 22

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 591404 [details] [diff] [review]
Added rule for media print

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

Thanks Diogo! This looks good to me. I've forward the review on to Justin. I hope that we can get this in for Firefox 12 :)

Comment 23

[Jared Wein [:jaws] (please needinfo? me)]

review ping?

Comment 24

[Dão Gottwald [:dao]]

Comment on attachment 591404 [details] [diff] [review]
Added rule for media print

For the present purpose of getting image printing back to the last known good state, I don't think reverting the centering needs further discussion and a separate bug. The change was unintentional and people complain about it for maybe not very strong but legitimate reasons. I also don't grok the discussion about customized color prefs. It seems like that actually belongs in a different bug, as it wouldn't be a regression but a bug affecting 1) released versions of Firefox and 2) not only image printing, so a different fix would be needed.

For this bug, please just wrap all the stuff that unintentionally affected printing in "@media not print".

Comment 25

[Jared Wein [:jaws] (please needinfo? me)]

Diogo: Sorry for all the back-and-forth on this bug. Are you OK with doing what Dao has suggested?

Comment 26

[Diogo Golovanevsky Monteiro [:diogogmt]]

(In reply to Jared Wein [:jaws] from comment #25)
> Diogo: Sorry for all the back-and-forth on this bug. Are you OK with doing
> what Dao has suggested?

No problem at all, I'm just glad to help.
I'll submit a patch with the fixes later today

Comment 27

[Diogo Golovanevsky Monteiro [:diogogmt]]

Attachment: Removed css rules for @media print

Comment 28

[Dão Gottwald [:dao]]

Comment on attachment 592747 [details] [diff] [review]
Removed css rules for @media print

Thanks Diogo!

Comment 29

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/e1d850a8ee2e

Thanks Diogo!

Comment 30

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/e1d850a8ee2e

Comment 31

[Alex Keybl [:akeybl]]

Now that this has baked for a few days on m-c, do we feel the patch is low risk enough to uplift to Aurora 12 and Beta 11?

Comment 32

[Justin Dolske [:Dolske]]

Yeah, this low-risk and should be safe for Aurora/Beta.

Comment 33

[Dão Gottwald [:dao]]

We'll want bug 724516 as well.

Comment 34

[Dão Gottwald [:dao]]

Comment on attachment 592747 [details] [diff] [review]
Removed css rules for @media print

[Approval Request Comment]
Regression caused by (bug #): bug 376997
User impact if declined: see comment 0
Testing completed (on m-c, etc.): manual on m-c
Risk to taking this patch (and alternatives if risky): no risk as long as bug 724516 is approved along with this :)

Comment 35

[Alex Keybl [:akeybl]]

Comment on attachment 592747 [details] [diff] [review]
Removed css rules for @media print

[Triage Comment]
Approving for Aurora 12 and Beta 11 along with bug 724516 in support of printing images. Since this is landing fairly late, adding the qawanted keyword to get some extra testing around printing this cycle.

Comment 36

[u279076]

Litmus test cases:
 * Aurora: https://litmus.mozilla.org/show_test.cgi?id=48274
 * Beta: https://litmus.mozilla.org/show_test.cgi?id=48275

Please let me know if these are sufficient.
Also, if there is a minimized testcase I can use, please point me to it.

Thanks

Comment 37

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-aurora/rev/57e02511df21
http://hg.mozilla.org/releases/mozilla-beta/rev/4796b7056532

Comment 38

[Paul Silaghi, QA [:pauly]]

I've verified this on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a2) Gecko/20120220 Firefox/12.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0a2) Gecko/20120220 Firefox/12.0a2
Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux i686; rv:12.0a2) Gecko/20120221 Firefox/12.0a2

Images are printed with transparent background on the top left corner of the paper, both with "Print Background (colors & images)" option checked/unchecked. Is this correct? What purpose would have the "print background" option then?

Comment 39

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Paul Silaghi [QA] from comment #38) 
> Images are printed with transparent background on the top left corner of the
> paper, both with "Print Background (colors & images)" option
> checked/unchecked. Is this correct? What purpose would have the "print
> background" option then?

Yeah that's correct. The "print background" option is good for webpages that have relevant information/design contained within background images.

Comment 40

[Paul Silaghi, QA [:pauly]]

Based on comment 39 marking the bug as verified fixed.