regression: cancelling printing a page crashes - Trunk [@ nsDeviceContextWin::GetDeviceContextFor]

VERIFIED FIXED

Status

()

Core
Printing: Output
--
critical
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: Olivier Cahagne, Assigned: rods (gone))

Tracking

({crash, helpwanted, topcrash})

Trunk
x86
All
crash, helpwanted, topcrash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: PDT, crash signature, URL)

Attachments

(2 attachments)

(Reporter)

Description

16 years ago
Build ID: 2001092208 on Win2k.

Steps to reproduce:
 - load a page (whatever page), for me 'about:' and 'http://www.mozilla.org/'
trigger the bug,
 - Click on the 'Print' icon,
 - Cancel the print by clicking on 'Cancel' (Annuler in french, see attachment),
 - Mozilla crashes.

Expected behaviour: should not crash.
Talkback ID: TB35771679M.
Related bug: bug 93509 (which might be reopened ?).

Related details: I have a HP Laserjet but I don't think you even need a printer
as you could save it to file.
OS -> All as people on #mozillazine could reproduce the crash with recent build
on Linux.
(Reporter)

Comment 1

16 years ago
Created attachment 50467 [details]
Screenshot of printing window, where I hit Cancel ('Annuler')
win2k stack trace (build 20010922)

nsDeviceContextWin::GetDeviceContextFor(nsDeviceContextWin * const 0x04cdea08, 
nsIDeviceContextSpec * 0x00000000, nsIDeviceContext * & 0x04d770e8) line 703 + 3 
bytes
DocumentViewerImpl::Print(DocumentViewerImpl * const 0x03c33f48, int 0, _iobuf * 
0x00000000, nsIPrintListener * 0x00000000) line 4395 + 69 bytes
GlobalWindowImpl::Print(GlobalWindowImpl * const 0x04cdee94) line 1872 + 29 
bytes
XPTC_InvokeByIndex(nsISupports * 0x04cdee94, unsigned int 69, unsigned int 0, 
nsXPTCVariant * 0x0012d5e0) line 139
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode 
CALL_METHOD) line 1952 + 42 bytes
XPC_WN_CallMethod(JSContext * 0x04af89e0, JSObject * 0x04c55758, unsigned int 0, 
long * 0x04e4b0a8, long * 0x0012d818) line 1254 + 14 bytes
js_Invoke(JSContext * 0x04af89e0, unsigned int 0, unsigned int 0) line 807 + 23 
bytes
js_Interpret(JSContext * 0x04af89e0, long * 0x0012e5bc) line 2719 + 15 bytes
js_Invoke(JSContext * 0x04af89e0, unsigned int 1, unsigned int 2) line 824 + 13 
bytes
js_InternalInvoke(JSContext * 0x04af89e0, JSObject * 0x03c6fb78, long 63372160, 
unsigned int 0, unsigned int 1, long * 0x0012e79c, long * 0x0012e6e4) line 899 + 
20 bytes
JS_CallFunctionValue(JSContext * 0x04af89e0, JSObject * 0x03c6fb78, long 
63372160, unsigned int 1, long * 0x0012e79c, long * 0x0012e6e4) line 3362 + 31 
bytes
nsJSContext::CallEventHandler(nsJSContext * const 0x04af84d8, void * 0x03c6fb78, 
void * 0x03c6fb80, unsigned int 1, void * 0x0012e79c, int * 0x0012e798, int 0) 
line 960 + 33 bytes
nsJSEventListener::HandleEvent(nsJSEventListener * const 0x04b9dca8, nsIDOMEvent 
* 0x04d75304) line 139 + 74 bytes
nsEventListenerManager::HandleEventSubType(nsListenerStruct * 0x04b3a440, 
nsIDOMEvent * 0x04d75304, nsIDOMEventTarget * 0x04b3a2a8, unsigned int 8, 
unsigned int 7) line 1197 + 20 bytes
nsEventListenerManager::HandleEvent(nsEventListenerManager * const 0x04b9dc40, 
nsIPresContext * 0x04af3808, nsEvent * 0x0012f2c4, nsIDOMEvent * * 0x0012f170, 
nsIDOMEventTarget * 0x04b3a2a8, unsigned int 7, nsEventStatus * 0x0012f310) line 
2187 + 36 bytes
nsXULElement::HandleDOMEvent(nsXULElement * const 0x04b3a2a0, nsIPresContext * 
0x04af3808, nsEvent * 0x0012f2c4, nsIDOMEvent * * 0x0012f170, unsigned int 1, 
nsEventStatus * 0x0012f310) line 3711
PresShell::HandleDOMEventWithTarget(PresShell * const 0x04b085b8, nsIContent * 
0x04b3a2a0, nsEvent * 0x0012f2c4, nsEventStatus * 0x0012f310) line 5755 + 39 
bytes
nsMenuFrame::Execute() line 1537
nsMenuFrame::HandleEvent(nsMenuFrame * const 0x04d03488, nsIPresContext * 
0x04af3808, nsGUIEvent * 0x0012f768, nsEventStatus * 0x0012f65c) line 459
PresShell::HandleEventInternal(nsEvent * 0x0012f768, nsIView * 0x04d064a0, 
unsigned int 1, nsEventStatus * 0x0012f65c) line 5723 + 41 bytes
PresShell::HandleEvent(PresShell * const 0x04b085bc, nsIView * 0x04d064a0, 
nsGUIEvent * 0x0012f768, nsEventStatus * 0x0012f65c, int 0, int & 1) line 5633 + 
25 bytes
nsView::HandleEvent(nsView * const 0x04d064a0, nsGUIEvent * 0x0012f768, unsigned 
int 8, nsEventStatus * 0x0012f65c, int 0, int & 1) line 377
nsView::HandleEvent(nsView * const 0x04cf8d90, nsGUIEvent * 0x0012f768, unsigned 
int 8, nsEventStatus * 0x0012f65c, int 0, int & 1) line 350
nsView::HandleEvent(nsView * const 0x04d58910, nsGUIEvent * 0x0012f768, unsigned 
int 8, nsEventStatus * 0x0012f65c, int 0, int & 1) line 350
nsView::HandleEvent(nsView * const 0x04b08130, nsGUIEvent * 0x0012f768, unsigned 
int 28, nsEventStatus * 0x0012f65c, int 1, int & 1) line 350
nsViewManager::DispatchEvent(nsViewManager * const 0x04b08008, nsGUIEvent * 
0x0012f768, nsEventStatus * 0x0012f65c) line 2062
HandleEvent(nsGUIEvent * 0x0012f768) line 68
nsWindow::DispatchEvent(nsWindow * const 0x04cf8e4c, nsGUIEvent * 0x0012f768, 
nsEventStatus & nsEventStatus_eIgnore) line 728 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f768) line 749
nsWindow::DispatchMouseEvent(unsigned int 301, nsPoint * 0x00000000) line 4262 + 
21 bytes
ChildWindow::DispatchMouseEvent(unsigned int 301, nsPoint * 0x00000000) line 
4514
nsWindow::ProcessMessage(unsigned int 514, unsigned int 0, long 11665489, long * 
0x0012fb94) line 3221 + 24 bytes
nsWindow::WindowProc(HWND__ * 0x002c0434, unsigned int 514, unsigned int 0, long 
11665489) line 996 + 27 bytes
USER32! 77e02e98()
USER32! 77e030e0()
USER32! 77e05824()
nsAppShellService::Run(nsAppShellService * const 0x00f85280) line 442
main1(int 2, char * * 0x003577d0, nsISupports * 0x00000000) line 1278 + 32 bytes
main(int 2, char * * 0x003577d0) line 1606 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e87d08()
Keywords: crash

Comment 3

16 years ago
Reporter, could you please post a stack trace from a
Linux/Solaris/AIX/HP-UX/etc. build, please ?

Comment 4

16 years ago
*** Bug 101299 has been marked as a duplicate of this bug. ***

Comment 5

16 years ago
confirmed on Win NT, build 2001092308

Talkback incident ID TB35797417Z

Comment 6

16 years ago
This bug is a silly one:
nsDeviceContextSpecFactoryWin::CreateDeviceContextSpec() returns NS_OK even when
the print dialog was _canceled_ or something else went wrong there.
(This is partially my "fault" because I fixed error checking in bug 97907 and
assumed that every platform properly returns an error in these cases... but the
braindead code in gfx/src/windows/ doesn't do that... ouch...).

Solution:
If "BOOL result = ::PrintDlg(&prntdlg);" returns |FALSE|
nsDeviceContextSpecFactoryWin::CreateDeviceContextSpec() should return
NS_ERROR_ABORT instead of NS_OK (patch in bug 101289 will fix the anoying error
dialog for this case).

Unfortunately I do not have a Win32 build machine, I can't help much... ;-(
Keywords: helpwanted

Comment 7

16 years ago
Created attachment 50512 [details] [diff] [review]
(Theoretical) fix for 2001-09-23-08-trunk (I don't have a Win32 build machine to test it)

Comment 8

16 years ago
OK... filed a patch which _should_ fix this... however I cannot test it due lack
of a matching build machine.

Comment 9

16 years ago
CC:'ing some printing staff, AFAIK dcone is on vacation ...

Can anyone please test attachment 50512 [details] [diff] [review] on a Win32 box, please ?

Comment 10

16 years ago
I tried the patch under Win98  and ended in an infinite (more then 50 times)
loop of the following assertion
###!!! ASSERTION: nsFont name wasn't lowercase: 'name.Equals(aOther.name)', file
 C:\MOZ_SOUR\MOZILLA\mozilla\gfx\src\nsFont.cpp, line 56

Comment 11

16 years ago
Bernd Mielke wrote:
> I tried the patch under Win98  and ended in an infinite (more then 50 times)
> loop of the following assertion
> ###!!! ASSERTION: nsFont name wasn't lowercase: 'name.Equals(aOther.name)', 
> file
> C:\MOZ_SOUR\MOZILLA\mozilla\gfx\src\nsFont.cpp, line 56

No no no... this is caused by patch for bug 97299 ("Store font names in
lowercase to avoid conversions"). The patch was backed-out few hours ago.
IMHO totally unrelated to this one...

Comment 12

16 years ago
The patch works, win98 I just updated my tree.

Comment 13

16 years ago
Adding topcrash keyword and Trunk [@ nsDeviceContextWin::GetDeviceContextFor] to
summary...this is a topcrasher on the MozillaTrunk:

nsDeviceContextWin::GetDeviceContextFor   13 
BBID range: 35720805 - 35789477
Min/Max Seconds since last crash: 9 - 10561
Min/Max Runtime: 23 - 93989
Crash data range: 2001-09-21 to 2001-09-23
Build ID range: 2001092114 to 2001092310

Stack Trace: 

	 nsDeviceContextWin::GetDeviceContextFor
[d:\builds\seamonkey\mozilla\gfx\src\windows\nsDeviceContextWin.cpp  line 705]
	 DocumentViewerImpl::Print
[d:\builds\seamonkey\mozilla\content\base\src\nsDocumentViewer.cpp  line 4397]
	 GlobalWindowImpl::Print
[d:\builds\seamonkey\mozilla\dom\src\base\nsGlobalWindow.cpp  line 1873]
	 XPTC_InvokeByIndex
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp 
line 139]
	 XPCWrappedNative::CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp  line 1954]
	 XPC_WN_CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednativejsops.cpp 
line 1255]
	 js_Invoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 809]
	 js_Interpret
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 2720]
	 js_Invoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 825]
	 js_InternalInvoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 900]
	 JS_CallFunctionValue
[d:\builds\seamonkey\mozilla\js\src\jsapi.c  line 3364]
	 nsJSContext::CallEventHandler
[d:\builds\seamonkey\mozilla\dom\src\base\nsJSEnvironment.cpp  line 963]
	 nsJSEventListener::HandleEvent
[d:\builds\seamonkey\mozilla\dom\src\events\nsJSEventListener.cpp  line 140]
	 nsEventListenerManager::HandleEventSubType
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp  line
1198]
	 nsEventListenerManager::HandleEvent
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp  line
1368]
	 nsGenericElement::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\base\src\nsGenericElement.cpp  line 1805]
	 nsGenericHTMLElement::HandleDOMEventForAnchors
[d:\builds\seamonkey\mozilla\content\html\content\src\nsGenericHTMLElement.cpp 
line 1254]
	 nsHTMLAnchorElement::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\html\content\src\nsHTMLAnchorElement.cpp 
line 400]
	 nsGenericDOMDataNode::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\base\src\nsGenericDOMDataNode.cpp  line 706]
	 nsDOMDocumentType::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\base\src\nsDOMDocumentType.cpp  line 227]
	 PresShell::HandleEventInternal
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5710]
	 PresShell::HandleEventWithTarget
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5680]
	 nsEventStateManager::CheckForAndDispatchClick
[d:\builds\seamonkey\mozilla\content\events\src\nsEventStateManager.cpp  line 2487]
	 nsEventStateManager::PostHandleEvent
[d:\builds\seamonkey\mozilla\content\events\src\nsEventStateManager.cpp  line 1573]
	 PresShell::HandleEventInternal
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5731]
	 PresShell::HandleEvent
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5635]
	 nsView::HandleEvent
[d:\builds\seamonkey\mozilla\view\src\nsView.cpp  line 377]
	 nsView::HandleEvent
[d:\builds\seamonkey\mozilla\view\src\nsView.cpp  line 350]
	 nsView::HandleEvent
[d:\builds\seamonkey\mozilla\view\src\nsView.cpp  line 350]
	 nsViewManager::DispatchEvent
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp  line 2062]
	 HandleEvent
[d:\builds\seamonkey\mozilla\view\src\nsView.cpp  line 68]
	 nsWindow::DispatchEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 732]
	 nsWindow::DispatchWindowEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 749]
	 nsWindow::DispatchMouseEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 4264]
	 ChildWindow::DispatchMouseEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 4514]
	 nsWindow::ProcessMessage
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 3251]
	 nsWindow::WindowProc
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 997]
	 KERNEL32.DLL + 0x3613 (0xbff63613)
	 KERNEL32.DLL + 0x248f7 (0xbff848f7)
	 0x00688b5e
	 0x00058f64
	 0x04000000
 
 	Source File :
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/gfx/src/windows/nsDeviceContextWin.cpp
line : 705
     (35777245)	URL: http://www.mozilla.org/
(35777245)
Comments: bug 101221
     (35772044)	Comments: print cancel crash bug 101221
     (35771985)	URL: http://www.mozilla.org/
(35771985)
Comments: bug 101221
     (35771865)	Comments: menu -> file -> print  cancel dialog   crash
     (35771679)	URL: http://www.mozilla.org/
(35771679)
Comments: Cancelling printing
     (35771627)	Comments: GKGFXWIN.DLL print cancel
     (35771600)	URL: http://www.mozilla.org/
(35771600)
Comments: loading about:   trying to Print and cancelling
     (35771577)	URL: http://www.mozilla.org/
(35771530)
URL: http://www.mozilla.org/
(35771530)
Comments: Cancelling printing crashes...
     (35720805)	Comments: Press 'Cancel' button to close print dialog

Roland:  This particular stack signature is only happening on Win32...do we know
if this is also happening on Linux?
Keywords: topcrash
Summary: regression: cancelling printing a page crashes → regression: cancelling printing a page crashes - Trunk [@ nsDeviceContextWin::GetDeviceContextFor]

Comment 14

16 years ago
> Roland:  This particular stack signature is only happening on Win32...do we 
> know if this is also happening on Linux?

I doubt it crashes on Linux. I am running Solaris and it works here perfectly
(except bug 101289 - but that does not crash... it's only silly... :-)
And I am monitoring all new bug reports... no reports from any Unix/Linux
platforms about crashes yet.

This bug is caused by broken error checking in Windows-only code... that's
all...

Comment 15

16 years ago
Can anyone give an r=/sr= for that patch here, please ?
Keywords: patch, review

Comment 16

16 years ago
While the fix prevents the crash ... it has the annoying side affect of popping 
up an error dialog when there shouldn't be one.

Why is it considered an error if I decide to hit the cancel button?

You should probably keep the windows CreateDeviceContextSpec() method the way it 
is, return NS_OK, and roll back some of the changes you made 
to DocumentViewerImpl::Print() ... in particular put back the (nsnull != 
devspec) check:

    if (NS_SUCCEEDED(rv) && nsnull != devspec)

after the call to factory->CreateDeviceContextSpec() and restore the else clause 
at the bottom of the method that stops the print timer and ends printing to the 
way it was, I believe that has to execute regardless of the error returned ... 
you can still add a check for errors after that code executes to throw up a 
dialog if there really was an error.

Does mac crash too? I didn't see anywhere where they were returning 
NS_ERROR_ABORT.

Updated

16 years ago
Attachment #50512 - Flags: needs-work+

Comment 17

16 years ago
By the way, I forgot to mention, the other reason why we shouldn't throw a 
dialog up when canceling is because it puts up a dialog you can't dismiss in the 
viewer test app which Layout guys use for testing layout and printing.

Comment 18

16 years ago
Comment on attachment 50512 [details] [diff] [review]
(Theoretical) fix for 2001-09-23-08-trunk (I don't have a Win32 build machine to test it)

kin wrote:
> While the fix prevents the crash ... it has the annoying side affect of 
> popping up an error dialog when there shouldn't be one.
[etc. etc. etc.]

kin... I already have a fix for that issue. 
That's bug 101289 ('"Canceling" the print dialog fires-up a print error dialog') and it includes a working fix.
AFAIK no need to delay this one... :-)
Attachment #50512 - Flags: needs-work+

Comment 19

16 years ago
What about the mac?

Comment 20

16 years ago
> Does mac crash too? 

Looking at the source... ... no... it looks that it correctly uses the error
codes.

> I didn't see anywhere where they were returning 
> NS_ERROR_ABORT.

I see... it looks that it returns NS_ERROR_FAILURE. We need a Mac expert to
plug-in a one-line fix for that in the right location (somewhere in
nsDeviceContextSpecMac::Init()).

Comment 21

16 years ago
Mac code simply needs a 
-- snip --
theResult = NS_ERROR_ABORT;
-- snip --
in the right place... that's all. Patch in bug 101289 will then suppress the
"you pressed the 'cancel' button"-alert ... :-)

Comment 22

16 years ago
> if (NS_SUCCEEDED(rv) && nsnull != devspec)
> after the call to factory->CreateDeviceContextSpec()

I don't like this code because it usually hides bugs. Think about (currently)
broken Win32 code which returns NS_OK in our case... which makes it later
impossible to get the cause of the error (and we don't call the cleanup code
which usually leads to other hard-to-hunt-down issues).
Thats why I removed that thing and cleaned-up error checking (unfortunately I
did not check what happens if someone selects the CANCEL button of the native
Win32 print dialog... ;-(( ).
I prefer to kill the real sources of a bug instead trying to hide it with
workarounds...

> and restore the else 
> clause at the bottom of the method that stops the print timer and ends 
> printing to the way it was, I believe that has to execute regardless of the 
> error returned ... 

No... that code must only be executed if an error occured. Otherwise havoc will
occur which ends in random coredumps... ;-(

Comment 23

16 years ago
Comment on attachment 50512 [details] [diff] [review]
(Theoretical) fix for 2001-09-23-08-trunk (I don't have a Win32 build machine to test it)

Ok, so that's an sr=kin on the win32 patch,
your patch for bug 101289, covers gtk/xlib, so
just get someone to help you verify/fix mac so
all 3 platforms are in sync with their error
throwing/handling.
Attachment #50512 - Flags: superreview+

Comment 24

16 years ago
> Ok, so that's an sr=kin on the win32 patch,

thanks!
sujay/jepatel/rods/etc. ... can anyone give me a r= for that patch and check it
in, please (I don't have CVS write access) ?

> your patch for bug 101289, covers gtk/xlib, so
> just get someone to help you verify/fix mac so
> all 3 platforms are in sync with their error
> throwing/handling.

Any idea who can help ?
(Assignee)

Comment 25

16 years ago
*** Bug 101514 has been marked as a duplicate of this bug. ***

Comment 26

16 years ago
Comment on attachment 50512 [details] [diff] [review]
(Theoretical) fix for 2001-09-23-08-trunk (I don't have a Win32 build machine to test it)

r=karnaze
Attachment #50512 - Flags: review+

Comment 27

16 years ago
Adding nsbranch+. Please don't close this bug until it is decided if it will 
make it into the m0.9.4 branch.
Keywords: nsbranch
Whiteboard: nsbranch+

Updated

16 years ago
Keywords: nsbranch → nsbranch+
Whiteboard: nsbranch+
Reassigning to Rod.
Assignee: dcone → rods

Comment 29

16 years ago
adding PDT for review
Whiteboard: PDT
(Assignee)

Comment 30

16 years ago
The 9.4 branch doesn't suffer from this unless the other changes get checked in.
Status: NEW → ASSIGNED
*** Bug 101693 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 32

16 years ago
Checked in on tip

it wouldn't hurt to check this in on the branch, its low risk

Comment 33

16 years ago
Note to PDT: This isn't a problem on the 0.9.4 branch because the print error
dialog changes that caused this regression/crash are only checked into the TRUNK
as rod said above.
Marking nsbranch- since it does not affect the branch.
Keywords: nsbranch+ → nsbranch-
Closing this bug as fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Reporter)

Comment 36

16 years ago
*** Bug 101901 has been marked as a duplicate of this bug. ***

Comment 37

16 years ago
verified on 10/25 trunk.
Status: RESOLVED → VERIFIED
Crash Signature: [@ nsDeviceContextWin::GetDeviceContextFor]
You need to log in before you can comment on or make changes to this bug.