233955 - [FIXr]DOM L3 Core Crash in nsDocument::RenameNode

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Bob Clary [:bc] (inactive)]

Running the DOM 3 L3 Core TS crashes with the following stack

nsDocument::RenameNode(nsDocument * const 0x03a110bc, nsIDOMNode * 0x00000000,
const nsAString & {...}, const nsAString & {...}, nsIDOMNode * * 0x0012dbc0)
line 3522 + 7 bytes
XPTC_InvokeByIndex(nsISupports * 0x03a110bc, unsigned int 28, unsigned int 4,
nsXPTCVariant * 0x0012db90) line 102
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
CALL_METHOD) line 2022 + 42 bytes
XPC_WN_CallMethod(JSContext * 0x039c4e60, JSObject * 0x031a7ee8, unsigned int 3,
long * 0x04129610, long * 0x0012de60) line 1272 + 14 bytes
js_Invoke(JSContext * 0x039c4e60, unsigned int 3, unsigned int 0) line 941 + 23
bytes
js_Interpret(JSContext * 0x039c4e60, long * 0x0012e84c) line 2962 + 15 bytes
js_Execute(JSContext * 0x039c4e60, JSObject * 0x0218e530, JSScript * 0x03fbfbc8,
JSStackFrame * 0x0012f110, unsigned int 32, long * 0x0012e84c) line 1155 + 13 bytes
obj_eval(JSContext * 0x039c4e60, JSObject * 0x03939260, unsigned int 1, long *
0x04129538, long * 0x0012e84c) line 1068 + 27 bytes
js_Invoke(JSContext * 0x039c4e60, unsigned int 1, unsigned int 0) line 941 + 23
bytes
js_Interpret(JSContext * 0x039c4e60, long * 0x0012f180) line 2962 + 15 bytes
js_Invoke(JSContext * 0x039c4e60, unsigned int 1, unsigned int 0) line 958 + 13
bytes
js_Interpret(JSContext * 0x039c4e60, long * 0x0012fac8) line 2962 + 15 bytes
js_Execute(JSContext * 0x039c4e60, JSObject * 0x03939260, JSScript * 0x0402e710,
JSStackFrame * 0x00000000, unsigned int 0, long * 0x0012fac8) line 1155 + 13 bytes
JS_EvaluateUCScriptForPrincipals(JSContext * 0x039c4e60, JSObject * 0x03939260,
JSPrincipals * 0x03a688b4, const unsigned short * 0x03a96ec0, unsigned int 26,
const char * 0x04161608, unsigned int 158, long * 0x0012fac8) line 3543 + 25 bytes
nsJSContext::EvaluateString(const nsAString & {...}, void * 0x03939260,
nsIPrincipal * 0x03a688b0, const char * 0x04161608, unsigned int 158, const char
* 0x00c84430, nsAString & {...}, int * 0x0012fbe0) line 880 + 85 bytes
GlobalWindowImpl::RunTimeout(nsTimeoutImpl * 0x04161590) line 5130 + 114 bytes
GlobalWindowImpl::TimerCallback(nsITimer * 0x04161680, void * 0x04161590) line 5508
nsTimerImpl::Fire() line 382 + 17 bytes
nsTimerManager::FireNextIdleTimer(nsTimerManager * const 0x01feeb48) line 616
nsAppShell::Run(nsAppShell * const 0x00a5c788) line 142
nsAppShellService::Run(nsAppShellService * const 0x00a5c4d8) line 484
main1(int 1, char * * 0x002e2638, nsISupports * 0x009af5d8) line 1291 + 32 bytes
main(int 1, char * * 0x002e2638) line 1678 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e814c7()

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

I guess I own this.

Comment 2

[Bob Clary [:bc] (inactive)]

Attachment: documentrenamenode27.zip

zip file with test case.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 141208 [details] [diff] [review]
Fix

Gotta love that test suite passing null to renameNode.... ;)

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 141208 [details] [diff] [review]
Fix

r+sr=jst

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 141208 [details] [diff] [review]
Fix

>Index: content/base/src/nsDocument.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/content/base/src/nsDocument.cpp,v
>retrieving revision 3.481
>diff -u -p -d -u -8 -r3.481 nsDocument.cpp
>--- content/base/src/nsDocument.cpp	11 Feb 2004 02:22:43 -0000	3.481
>+++ content/base/src/nsDocument.cpp	12 Feb 2004 02:40:21 -0000
>@@ -3513,16 +3513,18 @@ nsDocument::NormalizeDocument()
> }
> 
> NS_IMETHODIMP
> nsDocument::RenameNode(nsIDOMNode *aNode,
>                        const nsAString& namespaceURI,
>                        const nsAString& qualifiedName,
>                        nsIDOMNode **aReturn)
> {
>+  NS_ENSURE_ARG_POINTER(aNode);
>+  

I don't think we want to warn or throw here.

Comment 7

[Boris Zbarsky [:bzbarsky]]

I can see not warning, but I think we absolutely want to throw.  Worst-case, we
should throw NOT_SUPPORTED_ERR, since the arg is not an element or attribute node.

Comment 8

[Bob Clary [:bc] (inactive)]

<http://www.w3.org/TR/2004/PR-DOM-Level-3-Core-20040205/core.html>
NOT_SUPPORTED_ERR seems good.

Comment 9

[Boris Zbarsky [:bzbarsky]]

OK, I can switch to that before landing.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Fix checked in for 1.7b

Comment 11

[Bob Clary [:bc] (inactive)]

verified in my cvs debug build.