Filed on behalf of timeless: <timeless> dom inspecting a message compose thing the addressing widget i had dug pretty deeply into it and was walkin around w/ my keyboard i couldn't see the name of the node i walked onto when i crashed :( Stack from talkback ID 23673921: nsInspectorCSSUtils::IsRuleNodeRoot [c:/builds/seamonkey/mozilla/content/html/style/src/nsInspectorCSSUtils.cpp, line 87] inDOMUtils::GetCSSStyleRules [c:/builds/seamonkey/mozilla/extensions/inspector/base/src/inDOMUtils.cpp, line 186] XPTC_InvokeByIndex [c:/builds/seamonkey/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp, line 102] XPCWrappedNative::CallMethod [c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2019] XPC_WN_CallMethod [c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 1270] js_Invoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 845] js_Interpret [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 2861] js_Invoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 861] js_Interpret [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 2475] js_Invoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 861] js_InternalInvoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 936] js_InternalGetOrSet [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 979] js_SetProperty [c:/builds/seamonkey/mozilla/js/src/jsobj.c, line 2754] js_Interpret [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 2703] js_Invoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 861] js_InternalInvoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 936] js_InternalGetOrSet [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 979] js_SetProperty [c:/builds/seamonkey/mozilla/js/src/jsobj.c, line 2754] js_Interpret [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 2703] js_Invoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 861] js_InternalInvoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 936] JS_CallFunctionValue [c:/builds/seamonkey/mozilla/js/src/jsapi.c, line 3540] nsJSContext::CallEventHandler [c:/builds/seamonkey/mozilla/dom/src/base/nsJSEnvironment.cpp, line 1222] nsJSEventListener::HandleEvent [c:/builds/seamonkey/mozilla/dom/src/events/nsJSEventListener.cpp, line 183] nsEventListenerManager::HandleEventSubType [c:/builds/seamonkey/mozilla/content/events/src/nsEventListenerManager.cpp, line 1423] nsEventListenerManager::HandleEvent [c:/builds/seamonkey/mozilla/content/events/src/nsEventListenerManager.cpp, line 1500] nsXULElement::HandleDOMEvent [c:/builds/seamonkey/mozilla/content/xul/content/src/nsXULElement.cpp, line 3245] nsTreeSelection::FireOnSelectHandler [c:/builds/seamonkey/mozilla/layout/xul/base/src/tree/src/nsTreeSelection.cpp, line 763] nsTreeSelection::SelectCallback [c:/builds/seamonkey/mozilla/layout/xul/base/src/tree/src/nsTreeSelection.cpp, line 345] nsTimerImpl::Fire [c:/builds/seamonkey/mozilla/xpcom/threads/nsTimerImpl.cpp, line 383] nsAppShell::Run [c:/builds/seamonkey/mozilla/widget/src/windows/nsAppShell.cpp, line 142] nsAppShellService::Run [c:/builds/seamonkey/mozilla/xpfe/appshell/src/nsAppShellService.cpp, line 484] main1 [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1307] main [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1673] WinMain [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1695] WinMainCRTStartup() KERNEL32.DLL + 0x7903 (0x77e87903)

DOM Inspector path (from the browser session formerly labelled crashed) listitem < xul:listboxbody < xul:listrows < listbox < vbox < hbox < xul:hbox < toolbar < xul:vbox < toolbox < window chrome://messenger/content/messengercompose/messengercompose.xul

note i own some blame for this code :(, bug 153821 buildid: 2003091604

Um.. Why is GetRuleNodeForContent returning null? Is the content not in a document or something? Perhaps this code should throw (and warn) if GetRuleNodeForContent throws and caller should check the return value or something?

that would be proposal 2. i'll do whatever people agree on. i don't have a functional inspector in my cvs builds so i need to fix that before i expiriment, and i'm still using the crashed/patched/running session so i can't try again. it's possible that the autocomplete widgets went away, i'm not sure. i could certainly make the c++ caller check, but unfortunately i'm not certain what the js caller was (silly release builds don't have DumpJSStack).

Comment on attachment 131633 [details] [diff] [review] proposal 1 This is wallpapering over a real issue somewhere.. I'd rather we fix the real problem.

Here's what I think is happening. inDOMUtils.cpp::GetCSSStyleRules, line 173, has a for-loop: for (PRBool isRoot; mCSSUtils->IsRuleNodeRoot(ruleNode, &isRoot), !isRoot; mCSSUtils->GetRuleNodeParent(ruleNode, &ruleNode)) Now, IsRuleNodeRoot returns a simple Boolean value (true or false) asking if the node is apparently a root node. The root of what, I don't exactly know (I'm not that strong at DOM Level 2 CSS). IsRuleNodeRoot calls on the passed node's IsRoot() method, which exists in precisely one place in all of Mozilla's source code: http://lxr.mozilla.org/seamonkey/source/content/base/src/nsDocumentEncoder.cpp#1653 Hazarding a guess based on that function, it looks like it was written for HTML and not for XUL... that most XUL nodes passed to it would return false. As the IsRoot returns false, the Boolean at inDOMUtils line 174 is apparently evaluated as a not-value of what it gets, thus a true. So the loop goes on. The next line of inDOMUtils calls for the parent node of the node just evaluated. Sooner or later, as you go up towards the document node from any DOM node, you'll pass the document element node, then the document node, and find yourself getting a null value here. Run the null through IsRuleNodeRoot, which calls upon aNode->IsRoot() ==> crash. Am I close?

sold

Excuse me? :) Figuring out what's broken and figuring out how to fix it are not the same thing. I'm not completely sure of the first to begin with... helpwanted: Neither timeless nor I know what should be done to fix this bug. timeless told me, based on my analysis, that the code is "basically broken". At the very least, I need some guidance.

nsDocumentEncoder.cpp is a red herring. Do not trust "Identifier Search" in LXR to give you all results. nsRuleNode::IsRoot method is what is being called. That is http://lxr.mozilla.org/seamonkey/source/content/shared/public/nsRuleNode.h#530 The problem does not appear to be with the for loop (though I agree that it should check IsRuleNodeRoot's return value, and that IsRuleNodeRoot should null-check and throw). -> dbaron

Comment on attachment 131633 [details] [diff] [review] proposal 1 > mCSSUtils->GetRuleNodeForContent(content, &ruleNode); >+ NS_ASSERTION(ruleNode, "Bug 219514 Crash in DOM Inspector"); >+ if (!ruleNode) return NS_OK; although we probably should check the return value here and do NS_ENSURE_SUCCESS(rv,rv);

Next time I have a crash doing this, I'll post a reproducible testcase and get a better stack from gdb. (I should've done that when I got the crash.)

#0 0x405a36e6 in nanosleep () from /lib/libc.so.6 #1 0x0000001c in ?? () #2 0x08072086 in ah_crap_handler(int) (signum=11) at nsSigHandlers.cpp:135 #3 0x4194955e in nsProfileLock::FatalSignalHandler(int) (signo=11) at nsProfileLock.cpp:195 #4 0x4012ac2d in __pthread_sighandler () from /lib/libpthread.so.0 #5 0x4051ed58 in __libc_sigaction () from /lib/libc.so.6 #6 0x414477a8 in nsInspectorCSSUtils::IsRuleNodeRoot(nsRuleNode*, int*) (this=0x9225220, aNode=0x0, aIsRoot=0xbfffc740) at nsInspectorCSSUtils.cpp:86 #7 0x4278321b in inDOMUtils::GetCSSStyleRules(nsIDOMElement*, nsISupportsArray**) (this=0x910e410, aElement=0x934e05c, _retval=0xbfffc9a0) at inDOMUtils.cpp:173 #8 0x4082a6b1 in XPTC_InvokeByIndex () at xptcinvoke_gcc_x86_unix.cpp:69 #9 0x4092d88a in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (ccx=@0xbfffca60, mode=CALL_METHOD) at xpcwrappednative.cpp:2016 #10 0x40937b2d in XPC_WN_CallMethod(JSContext*, JSObject*, unsigned, long*, long*) (cx=0x8c8cb48, obj=0x9163ac0, argc=1, argv=0x93a9518, vp=0xbfffcc00) at xpcwrappednativejsops.cpp:1269 #11 0x4005d6cd in js_Invoke (cx=0x8c8cb48, argc=1, flags=0) at jsinterp.c:912 #12 0x4006b7da in js_Interpret (cx=0x8c8cb48, result=0xbfffd16c) at jsinterp.c:2933 #13 0x4005d74b in js_Invoke (cx=0x8c8cb48, argc=1, flags=1) at jsinterp.c:929 #14 0x40067181 in js_Interpret (cx=0x8c8cb48, result=0xbfffd70c) at jsinterp.c:2547 #15 0x4005d74b in js_Invoke (cx=0x8c8cb48, argc=1, flags=2) at jsinterp.c:929 #16 0x4005dac1 in js_InternalInvoke (cx=0x8c8cb48, obj=0x91638f0, fval=153586416, flags=0, argc=1, argv=0xbfffdd40, rval=0xbfffdd40) at jsinterp.c:1006 #17 0x4005dd55 in js_InternalGetOrSet (cx=0x8c8cb48, obj=0x91638f0, id=136805488, fval=153586416, mode=JSACC_WRITE, argc=1, argv=0xbfffdd40, rval=0xbfffdd40) at jsinterp.c:1049 #18 0x4007f8ba in js_SetProperty (cx=0x8c8cb48, obj=0x91638f0, id=136805488, vp=0xbfffdd40) at jsobj.c:2748 #19 0x40069fc0 in js_Interpret (cx=0x8c8cb48, result=0xbfffde8c) at jsinterp.c:2774 #20 0x4005d74b in js_Invoke (cx=0x8c8cb48, argc=1, flags=2) at jsinterp.c:929 This happens when I do something stupid: (1) Load a document (2) Inspect it using CTRL+SHIFT+I (3) Unload the document (4) Select an element in DOM Inspector in the left-hand viewer (DOM Nodes) (5) Change the right-hand viewer to CSS Style Rules ==> crash. In this testcase, it's another symptom of bug 111411.

WeirdAl: the code in comment 7 walks the ruletree. The "root" is the root of that tree. There is no DOM or DOM CSS involved. The document encoder has nothing to do with anything here. The whole point is that if we hit something with no parent it's a root and we should break out. The style system code is totally correct, so I don't know why dbaron got stuck with this bug. I bet the issue is that http://lxr.mozilla.org/seamonkey/source/content/html/style/src/nsInspectorCSSUtils.cpp#174 throws for content not in a document or something along those lines.... dbaron is right -- the caller of GetRuleNodeForContent should check for an error rv and propagate it or handle it or something.

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

bz, does this relate to bug 240767?

Yeah, probably like a duplicate. *** This bug has been marked as a duplicate of 240767 ***