Closed
Bug 66934
Opened 24 years ago
Closed 24 years ago
window.print() from javascript as page loads causes crash
Categories
(Core :: DOM: Navigation, defect)
Tracking
()
RESOLVED
FIXED
mozilla0.9.1
People
(Reporter: mack, Assigned: adamlock)
References
()
Details
(Keywords: crash)
Attachments
(6 files)
(deleted),
text/html
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
Articles at thestreet.com have a 'print this page' link which points to a printer-friendly version. The printer friendly page contains javascript which calls windows.print() immediately. If you click <ok> on the print dialog, mozilla crashes. However, if you click <cancle> you can successfully print using the print button. At the time the dialog box appears, neither the page title nor contents have been rendered. Possibly trying to print before other steps are complete? I'll attach a test case as well.
Reporter | ||
Comment 1•24 years ago
|
||
![]() |
||
Comment 2•24 years ago
|
||
Crashes on Linux too (CVS build from 2001-01-28 evening).
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows NT → All
![]() |
||
Comment 3•24 years ago
|
||
Comment 4•24 years ago
|
||
Rod.. this crashes at the ToNewUnicode() call in nsDocumentViewer.cpp...
Mack, is this working for you now? please mark VERIFIED-FIXED if so..
Reporter | ||
Comment 7•24 years ago
|
||
I still crash on both the original site and the test case using 20010226 nightly.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 8•24 years ago
|
||
setting TFM to mozilla0.9.1 as part of the bug triage
Target Milestone: --- → mozilla0.9.1
Comment 9•24 years ago
|
||
This does not crash in the printing code.. its crashes after the print job is completed. I will send this back to the Javascript people.
Assignee: dcone → rogerl
Status: REOPENED → NEW
Component: Printing → Javascript Engine
QA Contact: sujay → pschwartau
Comment 10•24 years ago
|
||
Stack trace from Mack's testcase, with a Mozilla debug build 2001-03-01. Due to insufficient memory on my Linux box, I was only able to expand the top 18 frames of the stack (details will be attached below) (gdb) bt #0 0x8181f5c in ?? () #1 0x41c26c27 in PresShell::CaptureHistoryState () #2 0x41c99479 in nsCSSFrameConstructor::ConstructDocElementFrame () #3 0x41ca67e4 in nsCSSFrameConstructor::ContentInserted () #4 0x414234eb in StyleSetImpl::ContentInserted () #5 0x41c20cf0 in PresShell::InitialReflow () #6 0x4128c9bf in HTMLContentSink::StartLayout () #7 0x4128ab92 in HTMLContentSink::OpenBody () #8 0x416ab1f4 in CNavDTD::OpenBody () #9 0x416ab9cb in CNavDTD::OpenContainer () #10 0x416a783f in CNavDTD::HandleDefaultStartToken () #11 0x416a8786 in CNavDTD::HandleStartToken () #12 0x416a6a0a in CNavDTD::HandleToken () #13 0x416a5ff4 in CNavDTD::BuildModel () #14 0x416bbdcf in nsParser::BuildModel () #15 0x416bbb6a in nsParser::ResumeParse () #16 0x416bc8be in nsParser::OnDataAvailable () #17 0x40f3f41a in nsDocumentOpenInfo::OnDataAvailable () #18 0x40d57145 in nsHTTPFinalListener::OnDataAvailable () FWIW, here is the entire stack without any expansion: (gdb) bt #0 0x8181f5c in ?? () #1 0x41c26c27 in ?? () from /data/phil/mozilla/dist/bin/components/libgklayout.so #2 0x41c99479 in ?? () from /data/phil/mozilla/dist/bin/components/libgklayout.so #3 0x41ca67e4 in ?? () from /data/phil/mozilla/dist/bin/components/libgklayout.so #4 0x414234eb in ?? () from /data/phil/mozilla/dist/bin/components/libgkcontent.so #5 0x41c20cf0 in ?? () from /data/phil/mozilla/dist/bin/components/libgklayout.so #6 0x4128c9bf in ?? () from /data/phil/mozilla/dist/bin/components/libgkcontent.so #7 0x4128ab92 in ?? () from /data/phil/mozilla/dist/bin/components/libgkcontent.so #8 0x416ab1f4 in ?? () from /data/phil/mozilla/dist/bin/components/libhtmlpars.so #9 0x416ab9cb in ?? () from /data/phil/mozilla/dist/bin/components/libhtmlpars.so #10 0x416a783f in ?? () from /data/phil/mozilla/dist/bin/components/libhtmlpars.so #11 0x416a8786 in ?? () from /data/phil/mozilla/dist/bin/components/libhtmlpars.so #12 0x416a6a0a in ?? () from /data/phil/mozilla/dist/bin/components/libhtmlpars.so #13 0x416a5ff4 in ?? () from /data/phil/mozilla/dist/bin/components/libhtmlpars.so #14 0x416bbdcf in ?? () from /data/phil/mozilla/dist/bin/components/libhtmlpars.so #15 0x416bbb6a in ?? () from /data/phil/mozilla/dist/bin/components/libhtmlpars.so #16 0x416bc8be in ?? () from /data/phil/mozilla/dist/bin/components/libhtmlpars.so #17 0x40f3f41a in ?? () from /data/phil/mozilla/dist/bin/components/liburiloader.so #18 0x40d57145 in ?? () from /data/phil/mozilla/dist/bin/components/libnecko.so #19 0x40d2236d in ?? () from /data/phil/mozilla/dist/bin/components/libnecko.so #20 0x40d18ee4 in ?? () from /data/phil/mozilla/dist/bin/components/libnecko.so #21 0x40d554c1 in ?? () from /data/phil/mozilla/dist/bin/components/libnecko.so #22 0x40cf1284 in ?? () from /data/phil/mozilla/dist/bin/components/libnecko.so #23 0x40cf00c0 in ?? () from /data/phil/mozilla/dist/bin/components/libnecko.so #24 0x40128a2e in ?? () from /data/phil/mozilla/dist/bin/./libxpcom.so #25 0x4012884c in ?? () from /data/phil/mozilla/dist/bin/./libxpcom.so #26 0x4012a779 in ?? () from /data/phil/mozilla/dist/bin/./libxpcom.so #27 0x407e2d64 in ?? () from /data/phil/mozilla/dist/bin/components/libwidget_gtk.so #28 0x407e299f in ?? () from /data/phil/mozilla/dist/bin/components/libwidget_gtk.so #29 0x409aa52a in ?? () from /usr/lib/libglib-1.2.so.0 #30 0x409abbe6 in ?? () from /usr/lib/libglib-1.2.so.0 #31 0x409ac1a1 in ?? () from /usr/lib/libglib-1.2.so.0 #32 0x409ac341 in ?? () from /usr/lib/libglib-1.2.so.0 #33 0x408d6209 in ?? () from /usr/lib/libgtk-1.2.so.0 #34 0x407e345a in ?? () from /data/phil/mozilla/dist/bin/components/libwidget_gtk.so #35 0x405c51f4 in ?? () from /data/phil/mozilla/dist/bin/components/libnsappshell.so #36 0x8056fbb in main1 (argc=1, argv=0xbffffb54, nativeApp=0x0) at nsAppRunner.cpp:1004 #37 0x8057caa in main (argc=1, argv=0xbffffb54) at nsAppRunner.cpp:1295
Comment 11•24 years ago
|
||
Comment 12•24 years ago
|
||
There are no JS Engine issues apparent in the stack trace. Reassigning this to Embedding:Docshell for further triage -
Assignee: rogerl → adamlock
Component: Javascript Engine → Embedding: Docshell
QA Contact: pschwartau → adamlock
Assignee | ||
Comment 13•24 years ago
|
||
Perhaps the printing code is not waiting for loading to complete before proceeding? I imagine all sorts of nasty race conditions could happen if that is the case. Reassigning to printing for further comment.
Assignee: adamlock → dcone
Component: Embedding: Docshell → Printing
QA Contact: adamlock → sujay
Comment 14•24 years ago
|
||
Initial relow is called.. and printing will not start until the URL's have been loaded, we have addressed those issue for thing with large images, or sites that have not been loaded. This actually prints.. and crashes after the print code has executed.
Assignee: dcone → adamlock
Component: Printing → Embedding: Docshell
QA Contact: sujay → adamlock
Assignee | ||
Comment 15•24 years ago
|
||
CC Don and Radha because it's definitely a printing/history issue. This code is a bit confusing, but I will summarize what I think is happening. Firstly the URL in question contains Javascript that invokes window.print() before the page has finished loading, so as soon as content arrives and before the initial reflow nsGlobalWindowImpl::Print is being invoked. The caush of the crash occurs later when DocumentViewerImpl::ReflowPrintObject calls nsPresShell::CaptureHistoryState on a preshell (presumably to feed the layout history into the print preshell). When ReflowPrintObject() goes out of scope, the smart pointer holding the layout state releases its reference and the object is destroyed. This is because preshell holds a weak reference to the object and does not AddRef its own internal copy (in mHistoryState). Later during reflow, CaptureHistoryState is called again and the presshell thinks mHistoryState is valid. When it attempts to use it the crash occurs. I imagine that the solution to this bug would be to make mHistoryState into a smart pointer. I don't understand from the code why nsPresShell only holds a weak pointer when it causes all this refcounting confusion. What would be the effect on session history if a presshell were to manage the lifetime of the layout history for itself? Don, Radha can you shed some light?
Assignee | ||
Comment 16•24 years ago
|
||
Assignee | ||
Comment 17•24 years ago
|
||
The previous patch prevents the patch but might have object ownership issues with the session history. Also note that although it fixed the crash, the page didn't print properly afterwards, just printing a blank single with the correct URL in the header and a footer. This crash is hiding another bug to do with printing a page which is still downloading.
Comment 18•24 years ago
|
||
The page should not print until it is fully loaded, there are listeners that should prevent such things from happening. The first reflow is just to notify printing when the document has been loaded. Then once the document is loaded printing should complete. As far as the history.. is seems the a stong reference is the solution, I am not sure why the page is blank though. There was a bug on that issue about a week or two ago, but that was fixed.
Assignee | ||
Comment 19•24 years ago
|
||
Could it be that the print code is holding onto the wrong presshell in this instance, such as the outgoing one? I noticed one time when it printed a blank page that it title in the header was from the previous URL, even though the header got the right URL.
Comment 20•24 years ago
|
||
It could be.. that the document is being switched out from under the printing. It thinks its printing the old document.. which is loaded.. but then that document is somehow changing.. Kind of the same situation as printing a bunch of urls one after the other.. some document stuff gets messed up.. and can cause a crash.
Comment 21•24 years ago
|
||
The weak ptr for Historystate in PresShell has caused problems in the past. So, the strong reference is a good thing. History will be happy as long as it has a valid reference to the state. cc'ing pollmann who owns the nsILayoutHistoryState.
Assignee | ||
Comment 22•24 years ago
|
||
Does somebody want to take this then or r/a= my diff? Note that this bug just fixes the crash and doesn't attempt to make window.print during loading work instead of printing blank content.
Comment 23•24 years ago
|
||
I will try your patch and see if it affects any of the form value restoration for SH. I think pollmann should review it.
Comment 24•24 years ago
|
||
SH seems to be OK with the attached patch
Comment 25•24 years ago
|
||
The change seems reasonable r=pollmann@netscape.com I talked with Nisheeth about this (he originally wrote the code) and he suggested use of an nsWeakPtr instead of an nsCOMPtr. This should also fix the crash and would be less likely to cause leaks or unintended state preservation. Neither of us could think of anything immediately wrong with using an nsCOMPtr though, so I'm okay giving an r= for either change with slight preference for the nsWeakPtr.
Assignee | ||
Comment 26•24 years ago
|
||
Assignee | ||
Comment 27•24 years ago
|
||
Eric, could you review this latest patch which uses weak references? Thanks
Comment 28•24 years ago
|
||
This looks great, r=pollmann. You did a good thing making the following a bit more straightforward. Now you can move the (now repeated) *aState = historyState, NS_IF_ADDREF(*aState) outside the if / else blocks, and axe the else block entirely! :) ... Index: mozilla/layout/html/base/src/nsPresShell.cpp ... @@ -4150,20 +4151,23 @@ NS_PRECONDITION(nsnull != aState, "null state pointer"); - if (!mHistoryState) { + nsCOMPtr<nsILayoutHistoryState> historyState = do_QueryReferent(mHistoryState); + if (!historyState) { // Create the document state object - rv = NS_NewLayoutHistoryState(aState); // This addrefs + rv = NS_NewLayoutHistoryState(getter_AddRefs(historyState)); // This addrefs if (NS_FAILED(rv)) { *aState = nsnull; return rv; } - mHistoryState = *aState; + *aState = historyState; + NS_IF_ADDREF(*aState); + mHistoryState = getter_AddRefs(NS_GetWeakReference(historyState)); } else { - *aState = mHistoryState; - NS_IF_ADDREF(mHistoryState); + *aState = historyState; + NS_IF_ADDREF(*aState); } // Capture frame state for the entire frame hierarchy
Assignee | ||
Comment 29•24 years ago
|
||
Comment 30•24 years ago
|
||
sr=jst
Assignee | ||
Comment 31•24 years ago
|
||
Thanks everyone, fix is checked in. Don, I've opened bug 80572 to cover the blank page printing issues.
Status: NEW → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•