Closed Bug 478245 Opened 15 years ago Closed 14 years ago

Crash [@ nsEventStateManager::PreHandleEvent] with onfocus removing window and contenteditable


(Core :: DOM: UI Events & Focus Handling, defect)

Windows XP
Not set



Tracking Status
status1.9.2 --- beta1-fixed


(Reporter: martijn.martijn, Assigned: smaug)



(Keywords: crash, regression, testcase)

Crash Data


(3 files, 1 obsolete file)

Attached file testcase
See testcase, which crashes current trunk build.
This seems to have regressed between 2009-02-09 and 2009-02-10:
Frame  	Module  	Signature [Expand]  	Source
0 	xul.dll 	xul.dll@0x330e6a 	
1 	xul.dll 	nsEventStateManager::PreHandleEvent 	content/events/src/nsEventStateManager.cpp:1083
2 	xul.dll 	PresShell::HandleEventInternal 	layout/base/nsPresShell.cpp:5917
3 	xul.dll 	PresShell::HandleEvent 	layout/base/nsPresShell.cpp:5721
4 	xul.dll 	nsViewManager::DispatchEvent 	view/src/nsViewManager.cpp:1338
5 	xul.dll 	HandleEvent 	view/src/nsView.cpp:167
6 	xul.dll 	nsWindow::DispatchEvent 	widget/src/windows/nsWindow.cpp:1019
7 	xul.dll 	nsWindow::DispatchWindowEvent 	widget/src/windows/nsWindow.cpp:1039
8 	xul.dll 	nsWindow::DispatchFocus 	widget/src/windows/nsWindow.cpp:6498
9 	xul.dll 	nsWindow::ProcessMessage 	widget/src/windows/nsWindow.cpp:4790
10 	xul.dll 	nsWindow::WindowProc 	widget/src/windows/nsWindow.cpp:1235
11 	user32.dll 	InternalCallWinProc 	
12 	user32.dll 	UserCallWinProcCheckWow 	
13 	user32.dll 	DispatchClientMessage 	
14 	user32.dll 	__fnDWORD 	
15 	ntdll.dll 	KiUserCallbackDispatcher 	
16 	xul.dll 	nsAString_internal::Replace 	
17 	xul.dll 	nsEventStateManager::SendFocusBlur 	content/events/src/nsEventStateManager.cpp:5251
18 	xul.dll 	nsEventStateManager::SetContentState 	content/events/src/nsEventStateManager.cpp:4828
19 	xul.dll 	nsGenericElement::SetFocus 	content/base/src/nsGenericElement.cpp:3069
20 	xul.dll 	nsGenericHTMLElement::SetElementFocus 	content/html/content/src/nsGenericHTMLElement.cpp:2935
21 	xul.dll 	nsHTMLTextAreaElement::Focus 	content/html/content/src/nsHTMLInputElement.cpp:1348
22 	xul.dll 	nsGenericHTMLElementTearoff::Focus 	content/html/content/src/nsGenericHTMLElement.cpp:189
23 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
24 	xul.dll 	XPCWrappedNative::CallMethod 	js/src/xpconnect/src/xpcwrappednative.cpp:2424

Iframe content is:
<span id="a" contenteditable="true" onfocus="window.frameElement.parentNode.removeChild(window.frameElement)" href="#"></span>
function mousedown(){
Still happening in today's trunk build.
Flags: blocking1.9.1?
This is a null pointer crash. Regression from bug 88831.
Blocks: 88831
Well that hasn't landed in 1.9.1 and presumably will not (afaict).
Flags: blocking1.9.2?
Assignee: nobody → Olli.Pettay
The crash is from nsIMEStateManager::OnTextStateFocus calling nsPresContext::GetViewManager when the PresShell is null

I'm working on a patch.
I already assigned this to me and I'm working on a patch too ;)
This is sort of regression from bug 208190, which added inlined getter methods to
Calling nsPresContext::GetViewManager() at random times may crash, since
the relevant nsIPresShell may be null.

When nsPresContext and nsIPresShell merge, we can drop "GetPresShell()".
But before that happens, the broken API should be just removed, IMO.

Similar thing should be done for FrameManager() and StyleSet(), but since
those are pretty layout/ related, bad usage is less likely.
Attachment #363949 - Flags: superreview?(roc)
Attachment #363949 - Flags: review?(roc)
Comment on attachment 363949 [details] [diff] [review]
remove nsPresContext::GetViewManager();

Actually, since David fixed bug 208190.
Attachment #363949 - Flags: superreview?(roc)
Attachment #363949 - Flags: superreview?(dbaron)
Attachment #363949 - Flags: review?(roc)
Attachment #363949 - Flags: review?(dbaron)
This is trunk only, clearing blocking1.9.1?.
Flags: blocking1.9.1?
Comment on attachment 363949 [details] [diff] [review]
remove nsPresContext::GetViewManager();

Probably better not to change the NS_ASSERTION in nsSelection.cpp to NS_ENSURE_STATE.

With that, r+sr=dbaron.
Attachment #363949 - Flags: superreview?(dbaron)
Attachment #363949 - Flags: superreview+
Attachment #363949 - Flags: review?(dbaron)
Attachment #363949 - Flags: review+
(In reply to comment #10)
> (From update of attachment 363949 [details] [diff] [review])
> Probably better not to change the NS_ASSERTION in nsSelection.cpp to
NS_ENSURE_STATE is there to prevent possible (though perhaps not even possible) crash. GetPresShell() may return null, so there should be the null check
unless it is otherwise guaranteed that it can't return null.
Attached patch up-to-date (obsolete) — Splinter Review
Attachment #366800 - Attachment is obsolete: true
Closed: 14 years ago
Resolution: --- → FIXED
Verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090312 Minefield/3.2a1pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090312 Minefield/3.2a1pre. No crash with the testcase.
Blocks: 483591
Flags: blocking1.9.2? → blocking1.9.2+
Mass change: adding fixed1.9.2 keyword

(This bug was identified as a mozilla1.9.2 blocker which was fixed before the mozilla-1.9.2 repository was branched (August 13th, 2009) as per this query: - if this bug is not actually fixed on mozilla1.9.2, please remove the keyword. Apologies for the bugspam)
Keywords: fixed1.9.2
Crash Signature: [@ nsEventStateManager::PreHandleEvent]
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.