311526 - [FIX]Backgrounds should be opaque even when Print Backgrounds is off

Status: RESOLVED FIXED

(Core :: Printing: Output, defect, P3)

Description

[Daniel Pope]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4

Turning off the "Print Backgrounds (Colors & Images)" option* doesn't work very
well in CSS-heavy sites.

The intention of this option is to prevent printing large blocks of colour (to
conserve ink) and to increase the legibility of the text.

Because CSS positioning encourages designers to overlap elements, backgrounds
are relied upon because they serve to obscure whatever lies beneath. This is not
generally true of table-based layouts.

Turning off backgrounds currently makes backgrounds transparent. This means that
the text which was above the background can clash with elements (text or images)
which were covered by the background. It also means that potentially large
sections of replaced images can show through.

A better solution would be to make all backgrounds opaque and white.

* The prefs.js option is print.printer_<name>.print_bgcolor/bgimages

Reproducible: Always

Steps to Reproduce:
1. Turn off "Print Backgrounds (Colors & Images)" in Page Setup
2. Visit http://sinks.thetopedge.com/
3. Print or Print Preview

Actual Results:  
The top navigation bar title is illegible above the top banner.

Expected Results:  
Set all (non-transparent) backgrounds to white (opaque) rather than to
transparent when the "Print Backgrounds" option is set to false.

Comment 1

[Daniel Pope]

Actually, I realise that the behaviour is as expected for background colors.

It should also happen for background images.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: This should fix the background image case

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 218641 [details] [diff] [review]
This should fix the background image case

r=dbaron, except, for the rule node changes:

 * if that bit needs to correspond to whether the pointer is null, then you need to set it in the opposite case as well, so, given that, why not move all the setting/clearing of that bit to after that if-else chain, and just do:
if (bg->mBackgroundImage)
  bg->mBackgroundFlags &= ~NS_STYLE_BG_IMAGE_NONE;
else
  bg->mBackgroundFlags |= NS_STYLE_BG_IMAGE_NONE;

But I'm still not sure why we have a bit to tell us if a pointer is null; we could just check the pointer.  But that could probably be a followup patch.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Checked in the CSSRendering part of the patch.  I don't think we can get rid of the flag without making the check for a background image not be inline; see revision 3.72 of nsStyleStruct.h.  Let me know what you think; I can easily do that.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Leaving the flag is fine; I just prefer set/clear based on a single null-check at the end if that's what's needed.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Yeah, do that

Comment 7

[Boris Zbarsky [:bzbarsky]]

Checked in the followup patch.