Closed
Bug 164384
Opened 22 years ago
Closed 22 years ago
Print Preview does not close
Categories
(Core :: Print Preview, defect)
Tracking
()
VERIFIED
FIXED
mozilla1.2alpha
People
(Reporter: sam, Assigned: rods)
References
Details
(Keywords: regression)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
rods
:
review+
kinmoz
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.1b) Gecko/20020823 MultiZilla/v1.1.20 Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.1b) Gecko/20020823 MultiZilla/v1.1.20 After using Print Preview to print pages, it will not close when pressing the 'Close' button. Each time I must then clos out the browser and re-start it. Reproducible: Always Steps to Reproduce: 1.Select Print Preview 2.Print a page 3.Press the Close button Actual Results: The Close button is unresponsive. The browser must be closed and re-started. Expected Results: Closed the Print Preview and returned to normal browsing. This only started with some of the most recent nightly builds and did not happen to me earlier this week (week of 8-18-02).
Comment 1•22 years ago
|
||
confirmed with windows98 trunk build 2002-08-26-04-trunk
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 3•22 years ago
|
||
This also happens on OS/2 and Win NT. The fix for bug 139905 checked in on 08/21/2002 by rods caused this problem.
OS: Windows 98 → All
Comment 5•22 years ago
|
||
Why is this assigned to sgehani and not rods? Drivers will want this fixed for 1.2alpha. /be
Maybe we shouldn't release mPrintEngine when we in printpreview since we need it to return to GalleyPresentation by press close button.
Assignee | ||
Comment 7•22 years ago
|
||
taking
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.2beta
Assignee | ||
Comment 9•22 years ago
|
||
Comment on attachment 98111 [details] [diff] [review] purpose patch r=rods for pete's patch (thank you)
Attachment #98111 -
Flags: review+
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Target Milestone: mozilla1.2beta → mozilla1.2alpha
Comment 11•22 years ago
|
||
Do we really understand the refcounting behaviour here? mPrintEngine->Destroy(); - NS_RELEASE(mPrintEngine); + if (!GetIsPrintPreview()) { + NS_RELEASE(mPrintEngine); + } Why would we not want to release something that has just been destroyed? And why does the failure to do a release prevent the window from being closed?
Assignee | ||
Comment 13•22 years ago
|
||
Actually we do understand the ref count behavior. The DV holds a strong ref to the PrintEngine (PE) and the PE holds a weak ref to DV. Other services and objects in the system may hold a strong pointer to either the PE or an object that it has a strong ref to. In the DV it is necessary to call "Destroy" before the release of the PE because it needs a chance to cleanup its internal objects (like the PrintData object). By cleaning up its internal data object release the PE. Then the DV can release the PE. It is never assumed that when the DV releases the PE that it will get destroyed at that moment. Typically it gets detroyed when the PrintProgress finishes and goes away. I have tested this with the debugger and verified that the PE is getting detroyed when printing, when PP, and when printing from PP. In summary, Pete's patch was correct except that the Printing data was being cleaned up when in PP, this patch makes it so the printing data gets cleaned up.
Attachment #98111 -
Attachment is obsolete: true
Comment 14•22 years ago
|
||
kin, pete, can one of you review? who can sr? time is short and we need something asap.
Comment 15•22 years ago
|
||
Comment on attachment 98210 [details] [diff] [review] patch v2 >+ if (!mPrintEngine->GetIsPrintPreview()) { >+ mPrintEngine->Destroy(); >+ NS_RELEASE(mPrintEngine); >+ } else { >+ mPrintEngine->DestroyPrintingData(); >+ } Maybe we can avoid using "!" op by exchange code in if(){} and else{}. r=pete
Attachment #98210 -
Flags: review+
Assignee | ||
Comment 17•22 years ago
|
||
Comment on attachment 98443 [details] [diff] [review] patch with pete's suggestion bringing pete's r= forward r=pete
Attachment #98443 -
Flags: review+
Attachment #98443 -
Flags: superreview+
Comment 18•22 years ago
|
||
Comment on attachment 98443 [details] [diff] [review] patch with pete's suggestion sr=kin@netscape.com
Comment 19•22 years ago
|
||
Comment on attachment 98443 [details] [diff] [review] patch with pete's suggestion a=asa (on behalf of drivers) for checkin to 1.2a
Attachment #98443 -
Flags: approval+
Assignee | ||
Comment 20•22 years ago
|
||
fixed
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•