115227 - [FIX]Need to Abort print obj and Printing on network printer sends single byte printjob

Status: VERIFIED FIXED

(Core :: Printing: Output, defect)

Description

[Jörg Skottke]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:0.9.6+) Gecko/20011212
BuildID:    2001121219

When printing to a remote printer  (Xerox Docuprint N24 with built-in
printserver in this case) the printer reports two submitted jobs. The first is
always of size 1 byte, the second job contains the data.

Reproducible: Always
Steps to Reproduce:
1. Print a page to a network printer
2. Watch out for the messages from "Network Messaging"
3.

Actual Results:  two jobs are reported from the network printer's printserver

Expected Results:  There should only be one printjob.

The printer is defined in CONFIG.SYS:
SET LPR_SERVER=xm1sof
SET LPR_PRINTER=xm1sof

Output from Network Messaging (Job 1, german):

Von:   SPOOLER auf \\SO-DOMAIN
An:     JS104017
Grund:   ** DRUCKBENACHRICHTIGUNG **
Datum:   14.12.01 11:25

Druckauftrag 142 ist beendet auf Xerox xm1sof Raum 4.30(129.157.138.23:XM1SOF).
Der Auftrag wurde in Xerox xm1sof Raum 4.30 am 14.12.01 11:25 eingereiht.
Die Größe des Auftrags ist 1 Bytes.

Output from Network Messaging (Job 2, german):

Von:   SPOOLER auf \\SO-DOMAIN
An:     JS104017
Grund:   ** DRUCKBENACHRICHTIGUNG **
Datum:   14.12.01 11:25

Druckauftrag 144 ist beendet auf Xerox xm1sof Raum 4.30(129.157.138.23:XM1SOF).
Der Auftrag wurde in Xerox xm1sof Raum 4.30 am 14.12.01 11:25 eingereiht.
Die Größe des Auftrags ist 1810245 Bytes.

Comment 1

[Jessica Blanco]

Attachment: Removed call to BeginDocument from nsDeviceContextOS2.cpp

The reason there are 2 jobs is because BeginDocument is getting called twice on
OS/2. Every OS along with OS/2 calls BeginDocument with the title in
SetupToPrintContent in nsDocumentViewer.cpp. OS/2 needs to call BeginDocument
sooner than or fonts won't look right when printing. To fix the font problem, a
call to BeginDocument was put into nsDeviceContextOS2.cpp. With this patch, I'm
taking out that call to BeginDocument in nsDeviceContextOS2.cpp.

Comment 2

[Jessica Blanco]

Attachment: Call BeginDocument sooner in nsDocumentViewer.cpp

With this patch, BeginDocument now gets called right after GetDeviceContextFor
in nsDocumentViewer.cpp. This is sooner than when it was getting called. With
these 2 patches BeginDocument will only get called once on OS/2 so there won't
be 2 print jobs, and fonts will continue working with printing on OS/2. I
tested this change on Windows and it didn't seem to affect printing. Rod, is
this okay to move where BeginDocument gets called in nsDocumentViewer.cpp?

Comment 3

[rods (gone)]

I understand what you are trying to do, but this patch isn't quite right.

What call does the BeginDocument need to hapen before?

Comment 4

[Jessica Blanco]

By moving the piece of code around, it looks like BeginDocument needs 
to be called before ReflowDocList gets called in the 
SetupToPrintContent function. Could we just move the code up in the 
function? Also noticed print preview has tons of assertions and 
crashes the browser most of the time. Is there a bug open for that?

Comment 5

[rods (gone)]

I think it would be bad to call BeginDocument before we reflow anything, because
that is assuming that no matter what happens we are printing the document.

What I don't understand is what is happening between the
nsDeviceContextOS2::Init and the BeginDocument now that is causing the fonts to
be be messed up. 

Reflow makes calls that probably causes fonts to be realized, is that the probably?

Comment 6

[Jessica Blanco]

This comment is made in nsDeviceContextOS2.cpp before BeginDocument is called(at 
the end of the Init function):

// We need to begin a document now, because the client is entitled at
// this point to do stuff like create fonts, which required the PS to
// be associated with a DC which has been DEVESC_STARTDOC'd.

There are functions being called that need to have BeginDocument called before 
them. What if we did call BeginDocument early, and if there's an error, we can 
AbortDoc.

Comment 7

[Scott MacGregor]

sr=mscott of the mail print engine change. 

Comment 8

[rods (gone)]

Attachment: patch

This fixes the following things:
1) It moves the BeginDocument to a earlier place in the call change
2) Fixes the negitive page range display problem 
3) Improves performance by reducing the delay on pages not in the page range,
   i.e. before if you printed the 30th page out of 31 you had to wait 15 secs 
   to get to the 30th page to print it.
4) Added AbortDocument, so now when Print jobs are cancelled we call
   AbortDocument and remove them from the print spooler
5) Fixed a couple of minor issue with Canceling and cancelling mail printings

Comment 9

[dcone (gone)]

Comment on attachment 67314 [details] [diff] [review]
patch

r=dcone

Comment 10

[Jessica Blanco]

Attachment: Removed BeginDocument call & added AbortDocument function to nsDeviceContextOS2.h/cpp

Removed the extra call to BeginDocument and added the AbortDocument function to
nsDeviceContextOS2.cpp/h.

Comment 11

[Jessica Blanco]

Attachment: updated nsDeviceContextOS2.cpp/h to fix other printing errors

This patch contains the AbortDocument function and will only call BeginDocument
once on OS/2. This also fixes bug 118652 because now there's no extra sheets at
the beginning and end of each print job. By calling BeginDocument once, it
fixes the extra sheet popping out at the beginning so I added the fix for extra
sheets popping out at the end.

Comment 12

[Jessica Blanco]

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

Comment 13

[Kevin McCluskey (gone)]

Marking nsbeta1+.
Changing OS to All.
Setting severity to normal from trivial.

Comment 14

[Kevin McCluskey (gone)]

Updated summary

Comment 15

[rods (gone)]

Attachment: new patch

Includes OS/2 patch and this now also fixes Bug 123181, selection was not
printing correctly. 

This fixes everything descrribed in comment #8 and Bug 123181, selection was
not printing correctly. The problem with selection was that the clip was not
being set corrctly.

Comment 16

[Marc Attinasi]

Comment on attachment 68878 [details] [diff] [review]
new patch

I'd prefer to SR patches for single bugs rather than for several bugs - it is
too hard to tell what fixes what bug, and there are not enough comments in the
bug or code to make it clear.  Anyway, for this bug it sounds like the OS/2
patch has been tested and works.  I'll assume you tested the other bug
(selection printing on Linux?) adequately too. sr=attinasi

Comment 17

[dcone (gone)]

Comment on attachment 68878 [details] [diff] [review]
new patch

r=dcone

Comment 18

[rods (gone)]

fixed

Comment 19

[sujay]

Jorg/Erik/Roland, can one of you please verify this bug and mark it 
verified-fixed? 

It should be working for you now...thanks!

Comment 20

[sujay]

marking verified per comments from Erik: 
Have tested it.  Works OK on Mozilla build 2002022008