Closed Bug 6421 Opened 25 years ago Closed 25 years ago

M5: JavaScript-invoked DOM window event handler crash AND CODE FIX

Categories

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

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: mle, Assigned: joki)

Details

(Whiteboard: no talkback report submitted)

Bug causes crashes in setting certain window event handlers. Code fix included below, with a couple comments inline. in nsGlobalWindow.cpp nsresult GlobalWindowImpl::GetListenerManager(nsIEventListenerManager **aInstancePtrResult) { if (nsnull != mListenerManager) { return mListenerManager->QueryInterface(kIEventListenerManagerIID, (void**) aInstancePtrResult);; } //This is gonna get ugly. Can't use NS_NewEventListenerManager because of a circular link problem. nsIDOMEventCapturer *mDoc = nsnull; /* mle added ^^^^^^^^^^ */ if (nsnull != mDocument && NS_OK == mDocument->QueryInterface(kIDOMEventCapturerIID, (void**)&mDoc)) { if (NS_OK == mDoc->GetNewListenerManager(aInstancePtrResult)) { mListenerManager = *aInstancePtrResult; NS_ADDREF(mListenerManager); NS_RELEASE(mDoc); return NS_OK; } } NS_IF_RELEASE(mDoc); /* mle because ^^^^^^^^^^^^^^^^^^^^ if mDocument is null mDoc is never set */ return NS_ERROR_FAILURE; }
Assignee: vidur → joki
mle, thanks for your contribution. Please provide the necessary steps to reproduce the described failure. It would allow us to verify the code fix actually fixes it. If needed, please refer to http://www.mozilla.org/quality/bug-writing-guidelines.html
Yes, I know that I should submit a testcase with the bug report and in almost all cases I do. But here it is really difficult because the bug was detected running on data which is not valid input to Gecko without our module extension, SGML, and it could be very time consuming to create a Gecko-valid testcase. Nonetheless it is a bug which can cause Gecko to crash. Frankly, in this case, for a simple uninitalized variable where the code is as obviously wrong as 1+1=3 it doesn't seem worth it. Please let me know if you disagree. I am happy to everything I can to help. I do have experience and understanding of software QA so I perfectly understand why you ask for a test case.
Whiteboard: no talkback report submitted
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Thanks for the fix. I went to put it in and found that danm had already fixed the bug. He did it slightly differently by moving the NS_IF_RELEASE(mDoc) up into the if statement so it won't get hit unless mDoc is set. Anyway, its fixed now.
Target Milestone: M7
Status: RESOLVED → VERIFIED
verified by developer
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.