70946 - Need to report printing progress

Status: VERIFIED WORKSFORME

(Core :: Printing: Output, defect)

Description

[Kevin McCluskey (gone)]

Need to be able to communicate what page number is currently printing to an
embedded application.

Comment 1

[Stephen Walker]

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

Comment 2

[Kevin McCluskey (gone)]

Reassigning to Rod since he is working on a fix for this.

Comment 3

[rods (gone)]

accepting

Comment 4

[rods (gone)]

Attachment: patch

Comment 5

[rods (gone)]

Attachment: PrintProgressDialog.cpp

Comment 6

[rods (gone)]

Attachment: PrintProgressDialog.h

Comment 7

[Marc Attinasi]

I'm reviewing all but the MFC code now - kinda large...

Comment 8

[Marc Attinasi]

Review Comment (also in the bug):

[nsDocumentViewer.cpp]
* it looks like you added 12 new member variables that are used for printing
only. Can these be popped into a separate structure and a pointer to that
structure used inthe DocumentViewer instead? I hate to add so much bloat to the
DocumentViewer for something that is rarely used. Also note that many (most?) of
these are never initialized in the ctors for the DocumentViewer.

* nsPagePrintTimer: the default delay value is 500 - is this ever used? should
it be a constant, or initialized to a sentinal value instead? It looks like it
HAS to be set by the caller anyway in the Start method, so initialization to a
sentinal value is probably better (like 0 or MAX_PRUINT32)

* nsPagePrintTimer ctor does not initialze all members - mPrintOptions and
mDocViewer are missed.
* typo in the comment on Notify - search for 'donoe'

* nsPagePrintTimer::Stop: should the timer be destroyed after it is stopped?
Also, the dtor calls mTimer->Cancel but it may already be cancelled - NBD but if
you clear it this would be eliminated.

* PrintContent: currently asserts and returns error - comment says may be needed
- plans to test or what?

* SetupToPrintContent: need to check for mPrintDocList being non-null after call
to new nsVoidArray

* StartPagePrintTimer: NS_NewUpdateTimer will return ERROR_NO_MEMORY if it
returns null, so you can condense the error checking. the check for NS_FAILED
just before the mPagePrintTimer->Start is unnecessary. Also, don't you want to
return the result instead of NS_OK?

[nsSimplePageSequence.cpp]
* nsSimplePageSequenceFrame methods should check/ASSERT the out-params pointers
(StartPrint does but none of the others do).

* nsSimplePageSequenceFrame::StartPrint: check / ASSERT that dc, presShell, vm
ar not null. When a failure-rv is handled anyway, it should be reset to NS_OK or
you will end up returning an error when you actually handled it internally
(mostly around the GetLocalizedString calls).

* PrintNextPage: check/assert dc, presShell, vm

I did NOT review the emebedding files or the gfx files.

Comment 9

[rods (gone)]

Chak has given me the r=chak for the mfc changes - rods

Comment 10

[rods (gone)]

fixed

Comment 11

[sujay]

I see printing progress on Mac, but not on Windows.

Comment 12

[Kevin McCluskey (gone)]

Using MfcEmbed with todays build on WINNT it shows a print progress dialog when
printing www.cnn.com and www.yahoo.com.

Marking WORKSFORME. 

Comment 13

[sujay]

okay I guess it works if you have MfcEMbed on . I filed a new
bug to wire it up to mozilla.