126799 - [ps] Print preview spews errors to terminal window

Status: VERIFIED FIXED

(Core :: Print Preview, defect, P4)

Description

[Boris Zbarsky [:bzbarsky]]

BUILD:  Linux 2002-02-20-08

When I open print preview, I get the following the the terminal window I started
Mozilla from:

lpr: lp: lp: unknown printer

The same happens when closing the page setup window, trying to change zoom.

Comment 1

[Boris Zbarsky [:bzbarsky]]

ccing rods in case this is his printsettings checkin....

Comment 2

[Samir Gehani]

Is this coming from the backend or does the frontend is needs to initialize
something?  I'll leave it assigned to me for now in case it's a frontend issue.

Comment 3

[Boris Zbarsky [:bzbarsky]]

The more I think about it and look at the code, the more I suspect that the
PrintSettings changes are responsible

Comment 4

[Boris Zbarsky [:bzbarsky]]

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

Comment 5

[Boris Zbarsky [:bzbarsky]]

OK, I think this is a back-end bug.  I'm not sure whether rods or dcone should
have this...

Here is what happens:

1)  DocumentViewerImpl::PrintPreview calls
    nsDeviceContextGTK::GetDeviceContextFor
2)  nsDeviceContextGTK::GetDeviceContextFor calls nsDeviceContextPS::SetSpec
    (I'm not using XPrint, obviously).
3)  nsDeviceContextPS::SetSpec calls nsPostScriptObj::Init
4)  nsPostScriptObj::Init unconditionally calls popen() on the "print command"
    (which is bogus in my case, hence the errors).  It also opens a temp file
    and starts writing postscript to it (via nsPostScriptObj::begin_document)

It seems incredibly wrong to me that we're opening pipes, writing to files, and
so forth in print _preview_.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Oh, and we get the SIGPIPE signals while writing to the temp file, of course,
which is why bug 126929 is a dup of this.

Comment 7

[Roland Mainz]

Taking...

Comment 8

[Boris Zbarsky [:bzbarsky]]

As a further comment, these SIGPIPE signals happen while we have a menu up, so
mozilla has mouse focus grabbed.  This means that debugging mozilla's print
preview under Unix involves weird games with X displays and making sure that gdb
and Mozilla are running on different displays and the like.

Comment 9

[Roland Mainz]

Attachment: Patch for 2002-03-06-08-trunk

Comment 10

[Roland Mainz]

Changes:
- No more unwanted empty print jobs when visiting print preview
- No more uncomplete print jobs when user aborts the print job (e.g. I
implemented nsDeviceContextPS::AbortDocument())
- PostScript module now uses nsCOMPtr where possible. Death to all leaks!
- Added PR_LOG()-based logging. It's on even for release builds to do debugging
on the customer side on demand (fix for bug 126799)
- PostScript module now uses 24bit as device depth. This should avoid problems
with printing SVG and other stuff in the future
- Minor other cleanup (removed unused vars, functions, TABs and misc. other
cleanup)

Comment 11

[Boris Zbarsky [:bzbarsky]]

wow... that patch has a _lot_ of stuff in it that's totally irrelevant to this
bug.  

If I read the crumbs that seem relevant correctly, we will not always open some
random temp file and write some garbage to it (even in print preview).  Then if
we're printing for real we'll write the actual data to the file and then print
that file using the print command. Is that right?

Comment 12

[Boris Zbarsky [:bzbarsky]]

er, s/we will not always/we will now always/

Comment 13

[Roland Mainz]

Boris Zbarsky wrote:
> wow... that patch has a _lot_ of stuff in it that's totally irrelevant to this
> bug.

It isn't "irrelevant". I simply to not want to wait a month or longer to get
five or more patches "in" to get rid of all junk (incl. "random" leaks, that's
why I moved all code over to use nsCOMPtr instead of the manual refcounting
(Xlib and Xprint gfx code does the same since eternity)), that's all. I'd like
to fix all open major regressions before filing the first patches for bug 119491
("Cleanup global vars in PostScript and Xprint modules") ...

> If I read the crumbs that seem relevant correctly, we will now always open 
> some random temp file and write some garbage to it (even in print preview).  
> Then if we're printing for real we'll write the actual data to the file and 
> then print that file using the print command. Is that right?

Uhm uhm, not really.
We create the temp. file always. Either data will be rendered with that device,
storing these data in the temp. file or we do no rendering on the device.
If we use the device for rendering either nsIDeviceContext::EndDocument() may be
called (which starts the spooler application and feed it with the collected
data) or nsIDeviceContext::AbortDocument() may be called (which discards the
collected data) or we delete the device object in other ways.

Using the temp. file as storage is IMHO the cleanest way to handle all cases
incl. AbortDocument().

Additionally we now have a "blueprint" to get bug 6810 ("UNIX printing should
set the correct title (e.g. -T option)") implemented in a clean way...

The only side-effect is that we spool the file twice (we "spool" into the temp.
file and the spooler command usually does spool itself when being fed from
stdin; we could avoid that by using tmpnam() and pass the name of the spool file
to the spooler application - but that would require to change the print command
prefs, too - and I prefer tmpfile() because it _always_ (even if we crash)
cleans-up after closing the last reference to the file (e.g. no dead junk files
we would have to cleanup...)).

Comment 14

[Roland Mainz]

Attachment: New patch for 2002-03-06-08-trunk per rod's comments

Comment 15

[Roland Mainz]

I removed the fix for bug 129655 ("[ps] PostScript module device depth is
1bit")  for now per dcone's request...

Comment 16

[Roland Mainz]

Attachment: New patch for 2002-03-06-08-trunk per bz's review comments

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 73420 [details] [diff] [review]
New patch for 2002-03-06-08-trunk per bz's review comments

Looks good to me.  Nice cleanup, fixes memory leaks, and definitely
fixes the issues I was seeing.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 73420 [details] [diff] [review]
New patch for 2002-03-06-08-trunk per bz's review comments

r=bzbarsky, but make sure to get an OK from rods or dcone on this

Comment 19

[rods (gone)]

Comment on attachment 73420 [details] [diff] [review]
New patch for 2002-03-06-08-trunk per bz's review comments

Please take out where you are setting the depth to mDepth instead of 24 and we
will make that a separate bug that we can track just that change.

Comment 20

[Roland Mainz]

Attachment: New patch for 2002-03-11-08-trunk

Changes:
- I removed the requested change for the depth
- Integrated the OpenVMS-only patch from bug 129836 ("impossible to use an
alternate print command") into this one to avoid patch conflicts

Comment 21

[Roland Mainz]

Attachment: New patch for 2002-03-11-08-trunk

Changes:
- Fixed PR_LOG() output to print the returned depth(=24) instead of the value
of |mDepth| ...

Comment 22

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 73805 [details] [diff] [review]
New patch for 2002-03-11-08-trunk

r=bzbarsky, with those minor changes

Comment 23

[rods (gone)]

Comment on attachment 73794 [details] [diff] [review]
New patch for 2002-03-11-08-trunk

r=rods

Comment 24

[Marc Attinasi]

Comment on attachment 73805 [details] [diff] [review]
New patch for 2002-03-11-08-trunk

rs=attinasi

Comment 25

[Asa Dotzler [:asa]]

Comment on attachment 73805 [details] [diff] [review]
New patch for 2002-03-11-08-trunk

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 26

[Roland Mainz]

Fix checked in, marking bug as FIXED...

Comment 27

[sujay]

bz/roland, please verify...

Comment 28

[Boris Zbarsky [:bzbarsky]]

verified that mozilla now clutters up my console less.