Closed Bug 164781 Opened 22 years ago Closed 22 years ago

Context-menus crashes in tabs (with mozdev plugins) [@ nsHTMLImageElement::SetSrcInner] Trunk N701 M130A

Categories

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

x86
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: greer, Unassigned)

References

Details

(Keywords: crash, topcrash-)

Crash Data

Attachments

(4 files, 1 obsolete file)

Users are commenting on crashes that happen when right-clicking/closing tabs.
Many of the comments refer to the use of RadialContext (mozdev.org) or Optimoz
pie-menu plugin. I haven't been able to reproduce this one yet, but those
plugins may be the source of most of these crashes.

Here's the stack that Talkback is showing:

nsHTMLImageElement::SetSrcInner
[c:/builds/seamonkey/mozilla/content/html/content/src/nsHTMLImageElement.cpp
line 834]
nsHTMLImageElement::SetSrc
[c:/builds/seamonkey/mozilla/content/html/content/src/nsHTMLImageElement.cpp
line 937]
XPTC_InvokeByIndex
[c:/builds/seamonkey/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp
line 106]
XPCWrappedNative::CallMethod
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp line 1996]
XPC_WN_GetterSetter
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp line
1291]
js_Invoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c line 840]
js_InternalInvoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c line 931]
js_SetProperty [c:/builds/seamonkey/mozilla/js/src/jsobj.c line 2617]
js_Interpret [c:/builds/seamonkey/mozilla/js/src/jsinterp.c line 2634]
js_Invoke [c:/builds/seamonkey/mozilla/js/src/jsinterp.c line 856]
nsXPCWrappedJSClass::CallMethod
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp line 1195]
nsXPCWrappedJS::CallMethod
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappedjs.cpp line 430]
PrepareAndDispatch
[c:/builds/seamonkey/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp
line 117]
SharedStub
[c:/builds/seamonkey/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp
line 139]
nsEventListenerManager::HandleEventSubType
[c:/builds/seamonkey/mozilla/content/events/src/nsEventListenerManager.cpp line
1275]
nsEventListenerManager::HandleEvent
[c:/builds/seamonkey/mozilla/content/events/src/nsEventListenerManager.cpp line
1441]
nsXULElement::HandleDOMEvent
[c:/builds/seamonkey/mozilla/content/xul/content/src/nsXULElement.cpp line 3453]
nsXULElement::HandleDOMEvent
[c:/builds/seamonkey/mozilla/content/xul/content/src/nsXULElement.cpp line 3434]
nsXULElement::HandleDOMEvent
[c:/builds/seamonkey/mozilla/content/xul/content/src/nsXULElement.cpp line 3434]
nsXULElement::HandleDOMEvent
[c:/builds/seamonkey/mozilla/content/xul/content/src/nsXULElement.cpp line 3434]
nsXULElement::HandleDOMEvent
[c:/builds/seamonkey/mozilla/content/xul/content/src/nsXULElement.cpp line 3434]
PresShell::HandleEventInternal
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp line 6113]
PresShell::HandleEvent
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp line 6031]
nsViewManager::HandleEvent
[c:/builds/seamonkey/mozilla/view/src/nsViewManager.cpp line 2098]
nsView::HandleEvent [c:/builds/seamonkey/mozilla/view/src/nsView.cpp line 306]
nsViewManager::DispatchEvent
[c:/builds/seamonkey/mozilla/view/src/nsViewManager.cpp line 1909]
HandleEvent [c:/builds/seamonkey/mozilla/view/src/nsView.cpp line 83]
nsWindow::DispatchEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp line 1038]
nsWindow::DispatchWindowEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp line 1055]
nsWindow::DispatchMouseEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp line 5127]
ChildWindow::DispatchMouseEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp line 5382]
nsWindow::ProcessMessage
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp line 3834]
nsWindow::WindowProc
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp line 1304]
USER32.dll + 0x2a290 (0x77e3a290)
USER32.dll + 0x45b1 (0x77e145b1)
USER32.dll + 0x5b1d (0x77e15b1d)
nsAppShellService::Run
[c:/builds/seamonkey/mozilla/xpfe/appshell/src/nsAppShellService.cpp line 452]
main1 [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp line 1534]
main [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp line 1881]
WinMain [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp line 1899]
WinMainCRTStartup()
KERNEL32.DLL + 0x1ca90 (0x77e9ca90)
Source File:
c:/builds/seamonkey/mozilla/content/html/content/src/nsHTMLImageElement.cpp line
: 834
Keywords: crash, qawanted, topcrash
Attached file CrashReporter Log
Seeing a very similar crash with Optimoz mouse gestures installed on Mac OS X
10.2, Mozilla 1.1 final.
ok, let's try to get my patch reviewed. fwiw I filed bug 165360 because other
stuff in the function I patched looked strange to some people.

as for the macosx stack err.. it looks like stack overflow, perhaps dbradley or
someone can offer ideas, but I think we'll limit this bug to the talkback stack.
Assignee: jaggernaut → timeless
Component: Tabbed Browser → DOM HTML
QA Contact: sairuh → stummala
Comment on attachment 97118 [details] [diff] [review]
talkback fingers a null document

>+      NS_WARNING("mNodeInfo->GetDocument returned null, perhaps the document went away or this is anonymous content? -- bug 164781");

Please break this up into multiple lines no longer then 80 chars.

r=sicking
Attachment #97118 - Flags: review+
I don't see the need for the warning, so there's no warning in this diff, this
also fixes the problem with not returning NS_CheckContentLoadPolicy()'s error
code to the caller, and it cleans up this code a lot, and removes the
let's-nest-if's-to-death mentality from this method :-)
*** Bug 165360 has been marked as a duplicate of this bug. ***
.
Assignee: timeless → jst
Comment on attachment 97207 [details] [diff] [review]
Same fix (plus others) and more cleanup...

r=timeless w/ this change:
<+  nsCOMPtr<nsIDocument> doc = mDocument;
>+  nsCOMPtr<nsIDocument> doc(mDocument);
Attachment #97207 - Flags: review+
IIRC, both forms end up being the same on all but some very old compilers.
(Don't know if we compile on any of those "old" compilers) The compiler is
allowed to optimize out the assignment. I know the constructor form in some
cases can trip up some compilers because in some situations they think its a
function prototype and not a variable declaration.
Comment on attachment 97207 [details] [diff] [review]
Same fix (plus others) and more cleanup...

> -  if (NS_SUCCEEDED(result) && !mDocument) {

The whole point of that function is that we only want to load the image when
srcinner is set and we're _not_ in a document (var foo = new Image()
situation).  Perhaps a more relevant check is that we do not have a frame; not
sure on that.  In either case, doing this in cases when the frame is also doing
an image load seems wrong, no?	At least without some disentangling first (eg
with this change I think images will fire two onload events).

> +  nsAutoString url;

I know you just copied this code, but what is its reason for existing?
Attachment #97207 - Flags: needs-work+
Is this patch gonna get checked in anytime soon?  Looking at the latest Talkback
data, there are only a handful of crashes for Mozilla 1.3 Alpha, Netscape 7.01
and recent MozillaTrunk builds.

Still, if we have a fix ready to go, might as well get it in, right?  I'm going
to make this a topcrash- based on Talkback data and leave it open in case we
want to get the patch checked in.
Keywords: topcrashtopcrash-
updating summary with N701 and M130A for future reference.
Summary: Context-menus crashes in tabs (with mozdev plugins) [@ nsHTMLImageElement::SetSrcInner] Trunk M101RC1 N70PR1 → Context-menus crashes in tabs (with mozdev plugins) [@ nsHTMLImageElement::SetSrcInner] Trunk N701 M130A
> Is this patch gonna get checked in anytime soon? 

Not unless someone fixes it to be correct first...
Mass-reassigning bugs.
Assignee: jst → dom_bugs
This is fixed since bug 83774 landed.  The whole method in question no longer
exists; its replacement (nsImageLoadingContent::ImageURIChanged) properly
handles a null document.

Marking fixed.
Status: NEW → RESOLVED
Closed: 22 years ago
Depends on: 83774
Keywords: qawanted
Resolution: --- → FIXED
Verifying crash at URL in comment 3 is no longer present.
Component: DOM: HTML → DOM: Core & HTML
QA Contact: stummala → general
Crash Signature: [@ nsHTMLImageElement::SetSrcInner]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: