Closed Bug 403965 Opened 12 years ago Closed 12 years ago

Crash [@ nsCOMPtr<nsIEditorIMESupport>::nsCOMPtr<nsIEditorIMESupport>] with contenteditable, onbeforecopy and xul


(Core :: DOM: Editor, defect, critical)

Windows XP
Not set





(Reporter: martijn.martijn, Assigned: smaug)



(Keywords: crash, regression, testcase, Whiteboard: [sg:critical?] post 1.8-branch)

Crash Data


(2 files)

Attached file testcase
See testcase, which crashes current trunk build on load.
It doesn't crash in a 2007-11-13 build, but does crash in a 2007-11-14 build:
so I think a regression from bug 207531.
0  	@0x0  	
1 	nsCOMPtr<nsIEditorIMESupport>::nsCOMPtr<nsIEditorIMESupport>(nsQueryInterface) 	nsCOMPtr.h:645
2 	nsTextEditorFocusListener::Focus(nsIDOMEvent*) 	mozilla/editor/libeditor/text/nsEditorEventListeners.cpp:1167
3 	nsEventListenerManager::HandleEvent(nsPresContext*, nsEvent*, nsIDOMEvent**, nsISupports*, unsigned int, nsEventStatus*) 	mozilla/content/events/src/nsEventListenerManager.cpp:1208
4 	nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, unsigned int) 	mozilla/content/events/src/nsEventDispatcher.cpp:206
5 	nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor&, unsigned int, nsDispatchingCallback*) 	mozilla/content/events/src/nsEventDispatcher.cpp:264
6 	nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*) 	mozilla/content/events/src/nsEventDispatcher.cpp:479
7 	nsEventStateManager::PreHandleEvent(nsPresContext*, nsEvent*, nsIFrame*, nsEventStatus*, nsIView*) 	mozilla/content/events/src/nsEventStateManager.cpp:1021
8 	PresShell::HandleEventInternal(nsEvent*, nsIView*, nsEventStatus*) 	mozilla/layout/base/nsPresShell.cpp:5782
9 	PresShell::HandleEvent(nsIView*, nsGUIEvent*, nsEventStatus*) 	mozilla/layout/base/nsPresShell.cpp:5593
10 	nsViewManager::HandleEvent(nsView*, nsPoint, nsGUIEvent*, int) 	mozilla/view/src/nsViewManager.cpp:1296

The stacktrace also contains PresShell::HandleEventInternal, maybe you're interested in that, Olli?
Group: security
nsIEditor* mEditor is bad. Either we must keep editor alive while
handling an event or change mEditor to be nsCOMPtr or nsWeakPtr.
Assignee: nobody → Olli.Pettay
Attached patch proposed patchSplinter Review
Actually, keeping editor alive should be enough. 
That is done in other methods already.
Attachment #289219 - Flags: superreview?(peterv)
Attachment #289219 - Flags: review?(peterv)
Attachment #289219 - Flags: superreview?(peterv)
Attachment #289219 - Flags: superreview+
Attachment #289219 - Flags: review?(peterv)
Attachment #289219 - Flags: review+
Comment on attachment 289219 [details] [diff] [review]
proposed patch

Although it might be nicer to add a nsCOMPtr<nsIEditor> or to nullcheck imeEditor too. If we ever have an mEditor that doesn't implement nsIEditorIMESupport the crash will be back, no?
No, this crash won't be back. imeEditor is then null because  QI gets executed in a safe time.
Attachment #289219 - Flags: approval1.9?
Attachment #289219 - Flags: approval1.9? → approval1.9+
Closed: 12 years ago
Resolution: --- → FIXED
But mEditor is used before imeEditor is null-checked.
Sure, but the crash happened in the end of the mEditor block, when trying to QI dead mEditor to imeEditor.
Flags: in-testsuite?
Flags: wanted1.8.1.x-
Whiteboard: [sg:critical] post 1.8-branch
Whiteboard: [sg:critical] post 1.8-branch → [sg:critical?] post 1.8-branch
Group: security
crash test landed
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ nsCOMPtr<nsIEditorIMESupport>::nsCOMPtr<nsIEditorIMESupport>]
You need to log in before you can comment on or make changes to this bug.