84537 - During printing we need to prevent the currently printing page from being changed until the document is spooled to the printer

Status: VERIFIED FIXED

(Core :: Printing: Output, defect)

Description

[Kevin McCluskey (gone)]

If you print in rapid succession Mozilla will crash because it has not completed
the spooling of the document to the printer. We should prevent a new document
from being printed until the entire document has been spooled out or the user
has cancelled printing.

Comment 1

[Kevin McCluskey (gone)]

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

Comment 2

[Kevin McCluskey (gone)]

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

Comment 3

[Kevin McCluskey (gone)]

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

Comment 4

[dcone (gone)]

Attachment: Patch to limit printing to one job at a time.

Comment 5

[dcone (gone)]

Attachment: Correct patch

Comment 6

[Marc Attinasi]

Some comments on the patch: can the global be made a member of the
DocumentViewerImpl class instead? Can multiple document viewers print at the
same time or not? If not, then a static is in order...

Also, in the Print method is there a chance for re-entrancy? I ask because the
way you are setting the gCurrentlyPrinting flag seems sub-optimal. I would think
you would jsut set it to TRUE when the printing is really happening, instead of
seeding it with true and then setting it to false if an error occurs (tested in
many places).

Comment 7

[Kevin McCluskey (gone)]

Your setting gCurrentlyPrinting to PR_FALSE in the destructor for
DocumentViewerImpl.

I would think that it would be possible to destroy a document other than the one
that is printing which would cause gCurrentPrinting to become PR_FALSE.
(Example: closing a secondary browser window just after initiating the print).
If this happens you could be in the middle of printing with gCurrentlyPrinting
set to PR_FALSE.

In the ::Print method you only check the return value before calling
soundInterface->Beep();

I think you should also check for (soundInterface != nsnull)

I agree with Marc's comment about setting gCurrentlyPrinting to false in
Multiple places. If you guaratee that you set it to true when you are actually
going to print and back to false if the printing has completed or has been
aborted then you don't need to set it back to PR_FALSE for the error conditions.

Comment 8

[dcone (gone)]

Attachment: New Patch

Comment 9

[dcone (gone)]

This new patch takes the setting of the global out of the Destructor.  Also 
checks for (soundInterface != nsnull).

I did not change the ::print method.. because it can be re-entered.  A classic 
example is if the print dialog is up.. and the user hits print again, ::print 
will be called twice.. with both halted inside of print.. with print dialogs up.
I commented that code better to explain this.

Comment 10

[rods (gone)]

As long as you have tested it. r=rods

Comment 11

[Marc Attinasi]

sr=attinasi - thanks Don.

Comment 12

[Christopher Blizzard (:blizzard)]

a=blizzard on behalf of drivers for the trunk

Comment 13

[dcone (gone)]

fixed

Comment 14

[sujay]

verified.