96870 - [PRINT BACKGROUND]Properly handle backgrounds/text when we print.

Status: VERIFIED FIXED

(Core :: Printing: Output, defect)

Description

[sujay]

using 8/24 build of netscape on windows

1) launch netscape
2) open attachment
3 [details] [diff] [review]) print

the output is a dark rectangle with white lines. should look like  what it
appears in
the browser.

Comment 1

[sujay]

Attachment: banner attachment

Comment 2

[dcone (gone)]

what happens here is.. when text is printing out a color.. and its a light 
color, the code automatically darkens the color, this was to address the bug 
when a light color is printed out on a dark background.. and that background 
does not get printed.  Those lines.. are the underline.. and are supposed to be 
white.  So..even though this looks like a bug.. its doing what it supposed to do 
because the table does print that background, but the text is being lightened.  
The question then becomes how to handle this.  I am gonna change the title of 
this bug to .. properly handle backgrounds when we print, and put all these 
issue  in this bug.

Comment 3

[dcone (gone)]

*** Bug 23146 has been marked as a duplicate of this bug. ***

Comment 4

[Joe Wells]

I've added a nicer example of why it is important to print background images at
this URL:

  http://www.cee.hw.ac.uk/~jbw/background-image-example/index.xml

This is like my first example (a comment on bug 23146) which was at:

  http://www.cee.hw.ac.uk/~jbw/background-image-example/

Both examples show how to depict XML element trees.  Both of them rely on
background images to make sure the pieces that make up the lines of the tree are
the right length.  If the background images are not printed, then the printed
version will lose a lot of the meaning.

Comment 5

[dcone (gone)]

*** Bug 62344 has been marked as a duplicate of this bug. ***

Comment 6

[Markus Hübner]

There are some of the major site on the internet involved in this bug
for example http://sportsillustrated.cnn.com 

Comment 7

[Mike Kaply [:mkaply]]

I think there are two issues here.

The sportsillustrated.cnn.com page in particular prints on 4.x.

And incidentally gets different behaviors based on using PCL or PS on Windows.

The issue here has to do with what gets printed first. If you print white text 
and then put the background over it, you only get the background when you print.

If you put the text over the background, usually the printer is smart enough to 
get things right.

Comment 8

[Kevin McCluskey (gone)]

Rod added an option to the PrintOptions object which allows the background to be
printed with the default set to NOT print the background. However, there isn't
any UI for yet.

CC'ing Rod

Comment 9

[Kevin McCluskey (gone)]

*** Bug 97845 has been marked as a duplicate of this bug. ***

Comment 10

[Hervé]

> Rod added an option to the PrintOptions object which allows 
> the background to be printed with the default set to NOT 

ok but i read at
http://lxr.mozilla.org/seamonkey/source/gfx/src/nsPrintOptionsImpl.cpp#76

76 // There are currently NOT supported
...
81 //const char kPrintBackgrounds[]   = "print.print_backgrounds";

does it mean that for now we must recompile the lizard in order to test this
background printing feature ?

Comment 11

[dcone (gone)]

*** Bug 110282 has been marked as a duplicate of this bug. ***

Comment 12

[dcone (gone)]

*** Bug 103389 has been marked as a duplicate of this bug. ***

Comment 13

[sujay]

*** Bug 87366 has been marked as a duplicate of this bug. ***

Comment 14

[Kevin McCluskey (gone)]

Added keywords edt094 and topembed because edt094,topembed bug was marked as dup
of this bug.

Don: Please verify all dup's are fixed when you fix this bug.

Comment 15

[Christine Hoffman]

Retested on 11_14_22_0.9.4ec. Remains a bug - using original testcase, output is
a dark rectangle with white lines

Comment 16

[dcone (gone)]

Since backgrounds will be turned off for tables.. this is no longer a topembed.  
The bug for turning backgrounds off is 111953.. which will take care of this 
issue for the embedding project.

Comment 17

[Marek Z. Jeziorek]

per comment above adding - to edt0.9.4 keyword

Comment 18

[dcone (gone)]

Attachment: patch so backgrounds will now work for the printing.

Patch to generate backgrounds for printing and print preview.  This wont work
right away because all backgrounds are currently turned off for printing, but
this will work for print preview.

Comment 19

[Marc Attinasi]

Comment on attachment 59578 [details] [diff] [review]
patch so backgrounds will now work for the printing.

Leak: GetStyleContext will addref the parentContext, so youhave to use a COMPtr
or NS_RELEASE it.

Nit: LoadBackGround is a bad name, as the backgroudn is not actually loaded,
the background struct is simply fetched and cached.  If you do choose to keep
the name, please change the 'G' to 'g' :) 

Fix the leak and possibly the nit, and sr=attinasi

Comment 20

[dcone (gone)]

Attachment: new patch with fixes

Comment 21

[dcone (gone)]

*** Bug 111772 has been marked as a duplicate of this bug. ***

Comment 22

[rods (gone)]

talked about a few changes, r=rods

Comment 23

[dcone (gone)]

Attachment: Patch with changes suggested by rods

Comment 24

[Marc Attinasi]

Comment on attachment 59738 [details] [diff] [review]
Patch with changes suggested by rods

sr=attinasi - still

Comment 25

[dcone (gone)]

Attachment: Patch for finding out if the background should be printed.

This patch is for the PresContext, and the nsCSSRender'er will use that boolean
to see whether the background should be printed or not.  The PrintContext and
PrintPreview context will set this value to the settings in the
nsIPrintSettings object.  This is an easier way to communicate this setting
that getting the print services each time the background is painted.

Comment 26

[dcone (gone)]

*** Bug 114297 has been marked as a duplicate of this bug. ***

Comment 27

[Marc Attinasi]

Comment on attachment 61131 [details] [diff] [review]
Patch for finding out if the background should be printed.

Can you use a PRPackedBool instead of a PRBool in the nsPResContext? Also,
virtual functions cannot be inlined so there is no sense making
GetBackgroundDraw and SetBackgroundDraw inline.

sr=attinasi, assuming you check out my comments.

Comment 28

[Peter Lubczynski]

Comment on attachment 61131 [details] [diff] [review]
Patch for finding out if the background should be printed.

r=peterl

Comment 29

[dcone (gone)]

Backgrounds now us the nsPresContext to determine if the background is painted 
or not.  The only thing left to do is get the printoptions to set the varible 
for printing the background.  The color of the text needs to be update when that 
happens.  

Comment 30

[dcone (gone)]

There is a routine in nsCSSRendering called TransformColor(), this routine needs 
to know if the background is being printed or not before it changes color.  When 
that is finished.. this bug will be closed.  Bug 79477 was the bug that changed 
font color for printing.

Comment 31

[R.K.Aa.]

see http://bugzilla.mozilla.org/show_bug.cgi?id=113789#c4
this doesn't build for me either...same output

Comment 32

[R.K.Aa.]

Build terminates like mentioned for andred and diego (from IRC) and myself
Only common denominator i found is that we use optimize=O2 or higher.
Compilers vary: gcc 2.96-85, gcc 2.95.4, gcc 2.95.4-pre

Comment 33

[dcone (gone)]

What exactly are you sayin.. I dont understand why you are posting that info 
here?  The tinderbox builds fine for all platforms with this patch.

Comment 34

[André Dahlqvist]

Which is exactly the reason why we think it is an optimization issue. There are
several of us that see this compile failure, and all of use use -O2 or higher
optimization level. As far as I know, tinderboxes doesn't.

Comment 35

[Diego Biurrun]

I am seeing that failure also and bug 53486 (Default linux MOZ_OPTIMIZE_FLAG is
-0 !!) has 0.9.8 as target milestone, so this is not a future issue, it will
block a bug that is targetted for the next milestone.  Please build optimized
and see for yourself.  I use --enable-optimize="-O4 -finline
-fno-omit-frame-pointer -march=k6 -mcpu=k6".  After backing out the changes as
pointed out by rkaa my tree compiled without problems.

Comment 36

[André Dahlqvist]

I just built a -O1 build and it still failed at the same spot. diego is now
building a non-optimized build to see if that works.

Comment 37

[Diego Biurrun]

My build failed using the following mozconfig

# ac_add_options --enable-optimize="-O4 -finline -fno-omit-frame-pointer
-march=k6 -mcpu=k6"
ac_add_options --enable-crypto
ac_add_options --disable-debug
ac_add_options --disable-dtd-debug
ac_add_options --disable-jsd
ac_add_options --disable-ldap
ac_add_options --disable-xprint
ac_add_options --disable-logging
ac_add_options --disable-mailnews
ac_add_options --disable-tests
ac_add_options --disable-bidi
ac_add_options --disable-accessibility

What do you other guys use?  Maybe we can narrow it down..  My system is Debian
testing, gcc 2.95.4, kernel 2.4.16 FWIW.

Comment 38

[André Dahlqvist]

Attachment: Here is my ~/.mozconfig

This ~/.mozconfig fails, but also if I change optimize to -O1 or leave it out
altogether. Could the cause be some other common thing between diego, rkaa and
my .mozconfig, like --disable-bidi or --disable-dtd-debug?

Comment 39

[R.K.Aa.]

Attachment: rkaa .mozconfig

Comment 40

[Diego Biurrun]

I have spun off bug 114957 on the build failures and assigned it to you, dcone,
I hope you don't mind.

Comment 41

[dcone (gone)]

*** Bug 67681 has been marked as a duplicate of this bug. ***

Comment 42

[Kevin McCluskey (gone)]

Marking nsbeta1+. Backgrounds need to be conditionally printed based on the
page-setup dialog for both print-preview and printing.

Comment 43

[dcone (gone)]

*** Bug 123704 has been marked as a duplicate of this bug. ***

Comment 44

[Kevin McCluskey (gone)]

Marking topembed+

Comment 45

[Josh Kelley]

Here are some sites i believe to be example of this bug:

d-n-e.com
www.puma.com
www.undergear.com

Comment 46

[dcone (gone)]

Attachment: Patch to implement background images and color for printing.

This patch does a few things.
1.) Fixes the problem that the background for printing was not being captured.
2.) Adds background image and color booleans for printing.
3.) Fixes how the Paintbackground is called.  Now has a boolean to use the
printsetings or not.  This keeps the elements that use backgrounds for special
purposes from breaking. (radio buttons, scroll bars).
4.) Hooks up the options to the print settings dialog.

Comment 47

[rods (gone)]

Change the items we talked about and r=rods

Comment 48

[Marc Attinasi]

Comment on attachment 69306 [details] [diff] [review]
Patch to implement background images and color for printing.

tab probs:
+  PRPackedBool 				mDrawImageBackground;
+  PRPackedBool 				mDrawColorBackground;

Please remove the comment part of this:
+    if ( (frameType == nsLayoutAtoms::pageContentFrame)/* ||
+	     (frameType == nsLayoutAtoms::pageFrame) */){

otherwise, looks good. sr=attinasi

Comment 49

[dcone (gone)]

Attachment: new patch.. with cosmetic fixes.

This patch also adds in capablity to that if the backgrounds are being
printed.. the text will not be darkened.  If the backgrounds are not being
printed.. then the text can be darkened if it is to light.

Comment 50

[Marc Attinasi]

Comment on attachment 69528 [details] [diff] [review]
new patch.. with cosmetic fixes.  

Please comment the part about canLightenColor, and make it an inline function
since you call it three times. sr=attinasi

Comment 51

[dcone (gone)]

Attachment: Fixed patch.. now a routine called candarken is used.

Marc's comments.. and I changed name to CanDarken()..

Comment 52

[rods (gone)]

r=rods

Comment 53

[dcone (gone)]

fixed.

Comment 54

[Aleksey Nogin]

The patch for nsTextFrame.cpp does something weird with canDarkenColor. When
isPaginated is false, an uninitialized PRBool will be passed to SetColor!

Comment 55

[dcone (gone)]

*** Bug 126278 has been marked as a duplicate of this bug. ***

Comment 56

[rbs]

>Additional Comment #54 From Aleksey Nogin 2002-02-15 10:56 -------
>
>The patch for nsTextFrame.cpp does something weird with canDarkenColor. When
>isPaginated is false, an uninitialized PRBool will be passed to SetColor!

That's right. I just spent time chasing a non-existing bug, until I traced to 
the fact that the color set in the rendering context wasn't the color in the 
style context, and discovered that it was due to this uninitialized variable -- 
and was not due to something in the style system. I made a fixup patch.

Comment 57

[rbs]

Attachment: initialize variables

Comment 58

[rbs]

Left-over was checked-in.

Comment 59

[dcone (gone)]

I had a bug on this (122996).. complete with patch... thanks for the fix, but 
next time.. get and r and sr.. and I wont redo..that patch. Dont get me wrong.. 
I do appreciate it.. but imagine my surpise when I found that was already in 
there.  Yes... I should have commented in here.. but I was tracking with that 
other bug. Anyway.. good work..next time can you request a review.. and sr 
before you do that.  Thanks

Comment 60

[dcone (gone)]

125795 is the bug that tracked this.. not 122996.

Comment 61

[rbs]

I commented here for notification. Unfortunately, by that time, it had already 
eaten two days of my Mozilla hacking time, trying all sort voodoos in hunting 
an elusive bug elsewhere in the system following the simple left-over (ah, two 
days of hunting, it wasn't that simple...). I carried the earlier r/sr forward 
on the trivial fixup since there were no mention of the action that was going on 
in the other bug, and comment #54 from Aleksey Nogin 2002-02-15 with a REOPEN 
seemed to have been ignored. Next time I might not bother too.

Comment 62

[sujay]

ok verified on 2/20 trunk build on windows...

everyone, if you can verify your DUPS to thsi bug and test it out.

Make sure you go into File | Page Setup first and enable Background
image first though.

thanks.

marking verified, if anyone finds a DUP that does not work, then
REOPEN just that DUP....

Comment 63

[Michael Lefevre]

*** Bug 127629 has been marked as a duplicate of this bug. ***