Crash [@nsImageMap::IsAncestor] when removing comment in this document

VERIFIED FIXED

Status

()

--
critical
VERIFIED FIXED
13 years ago
7 years ago

People

(Reporter: martijn.martijn, Assigned: mats)

Tracking

({crash, testcase})

Trunk
x86
All
crash, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature, URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

13 years ago
To reproduce:
- follow the url
- Open DOM Inspector
- Remove the first comment you see in the document
Result -> crash.

I'll attach a testcase.
(Reporter)

Comment 1

13 years ago
Created attachment 189536 [details]
testcase

Click on the button to trigger the crash.

This also crashes Mozilla1.7.

From talkback ID: TB7536067X

nsImageMap::IsAncestorOf 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsImageMap.cpp,
line 962]
XPTC_InvokeByIndex 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp,
line 102]
XPCWrappedNative::CallMethod 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2119]
XPC_WN_CallMethod 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1348]
js_Invoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line
1173]
js_Interpret 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line
3464]
js_Invoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line
1193]
js_InternalInvoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line
1270]
JS_CallFunctionValue 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line
3920]
nsJSContext::CallEventHandler 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1400]
nsJSEventListener::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/dom/src/events/nsJSEventListener.cpp,
line 184]
nsEventListenerManager::HandleEventSubType 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1580]
nsEventListenerManager::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1681]
nsGenericElement::HandleDOMEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/base/src/nsGenericElement.cpp,
line 2126]
nsHTMLButtonElement::HandleDOMEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/html/content/src/nsHTMLButtonElement.cpp,
line 335]
PresShell::HandleEventInternal 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 6357]
PresShell::HandleEventWithTarget 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 6255]
nsEventStateManager::CheckForAndDispatchClick 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/events/src/nsEventStateManager.cpp,
line 2980]
nsEventStateManager::PostHandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/events/src/nsEventStateManager.cpp,
line 1960]
PresShell::HandleEventInternal 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 6428]
PresShell::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 6193]
nsViewManager::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/view/src/nsViewManager.cpp,
line 2503]
nsViewManager::DispatchEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/view/src/nsViewManager.cpp,
line 2230]
HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/view/src/nsView.cpp,
line 174]
nsWindow::DispatchEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1171]
nsWindow::DispatchMouseEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 5791]
ChildWindow::DispatchMouseEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 6037]
nsWindow::WindowProc 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1348]
USER32.dll + 0x27b17 (0x77d37b17)
USER32.dll + 0x2cdce (0x77d3cdce)
USER32.dll + 0x4435 (0x77d14435)
USER32.dll + 0x9611 (0x77d19611)
nsAppStartup::Run 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/toolkit/components/startup/src/nsAppStartup.cpp,
line 146]
main 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/browser/app/nsBrowserApp.cpp,
line 61]
kernel32.dll + 0x1eb69 (0x77e5eb69)
(Assignee)

Comment 2

13 years ago
|aContainer| is null for the #comment node (aChild) here:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/base/src/nsDocument.cpp&rev=3.561&root=/cvsroot&mark=2231-2246#2230

and the image map is a document observer so we end up here:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/generic/nsImageMap.cpp&rev=3.110&root=/cvsroot&mark=1008,959,973#956

and we crash on |aContent->GetParent()| in |nsImageMap::IsAncestorOf|

I'm not sure if having |aContainer| == null is OK to begin with, but if it is,
a simple null check will do...

Boris?
Severity: normal → critical
Keywords: crash
OS: Windows XP → All
(Assignee)

Comment 3

13 years ago
Created attachment 189549 [details] [diff] [review]
Patch rev. 1

like so?
Comment on attachment 189549 [details] [diff] [review]
Patch rev. 1

aContainer will be null any time a child of the document is removed or added
(like here).  So this code does need to deal.

That said, how about using nsContentUtils::IsDescendantOf instead of
reimplementing it (I know, this code predates the existence of that method...)?
 Note that you'd still need a null-check before calling that method.
Though note that nsContentUtils::IsDescendantOf(x, x) will return true; this
code may want to compare aContent to mMap first.
(Assignee)

Updated

13 years ago
Assignee: nobody → mats.palmgren
(Assignee)

Comment 6

13 years ago
Created attachment 189961 [details] [diff] [review]
Patch rev. 2
Attachment #189549 - Attachment is obsolete: true
Attachment #189961 - Flags: superreview?(bzbarsky)
Attachment #189961 - Flags: review?(bzbarsky)
(Reporter)

Comment 7

13 years ago
Shouldn't you also remove the IsAncestorOf function here?
http://lxr.mozilla.org/seamonkey/source/layout/generic/nsImageMap.h#119
(Assignee)

Comment 8

13 years ago
Created attachment 189965 [details] [diff] [review]
Patch rev. 3

Oops, thanks!
Attachment #189961 - Attachment is obsolete: true
Attachment #189965 - Flags: superreview?(bzbarsky)
Attachment #189965 - Flags: review?(bzbarsky)
(Assignee)

Updated

13 years ago
Attachment #189961 - Flags: superreview?(bzbarsky)
Attachment #189961 - Flags: review?(bzbarsky)
Comment on attachment 189965 [details] [diff] [review]
Patch rev. 3

Looks great!
Attachment #189965 - Flags: superreview?(bzbarsky)
Attachment #189965 - Flags: superreview+
Attachment #189965 - Flags: review?(bzbarsky)
Attachment #189965 - Flags: review+
(Assignee)

Comment 10

13 years ago
Comment on attachment 189965 [details] [diff] [review]
Patch rev. 3

Simple null-check to prevent crash
Attachment #189965 - Flags: approval1.8b4?

Updated

13 years ago
Attachment #189965 - Flags: approval1.8b4? → approval1.8b4+
(Assignee)

Comment 11

13 years ago
Checked in to trunk 2005-07-23 18:21 PDT

-> FIXED
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Verified FIXED using build 2005-07-24-06 on Windows XP SeaMonkey trunk with
testcase: https://bugzilla.mozilla.org/attachment.cgi?id=189536.  No more crashing.
Status: RESOLVED → VERIFIED
Crash Signature: [@nsImageMap::IsAncestor]
You need to log in before you can comment on or make changes to this bug.