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

VERIFIED FIXED

Status

()

Core
Event Handling
--
critical
VERIFIED FIXED
10 years ago
7 years ago

People

(Reporter: Martijn Wargers (zombie), Assigned: smaug)

Tracking

({crash, regression, testcase})

Trunk
x86
Windows XP
crash, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.2 +

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

(crash signature)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
Created attachment 362020 [details]
testcase

See testcase, which crashes current trunk build.
This seems to have regressed between 2009-02-09 and 2009-02-10:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2009-02-10+04%3A00%3A00&enddate=2009-02-11+06%3A00%3A00

http://crash-stats.mozilla.com/report/index/d79afdbb-2ae9-456f-ada1-e24e82090212?p=1
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:
<html>
<head>
</head>
<body>
<span id="a" contenteditable="true" onfocus="window.frameElement.parentNode.removeChild(window.frameElement)" href="#"></span>
<script>
function mousedown(){
document.getElementById('a').focus();
}
setTimeout(mousedown,100);
</script>
</body>
</html>
(Reporter)

Comment 1

10 years ago
Still happening in today's trunk build.
(Assignee)

Comment 2

10 years ago
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).
(Assignee)

Updated

10 years ago
Assignee: nobody → Olli.Pettay
The crash is from nsIMEStateManager::OnTextStateFocus calling nsPresContext::GetViewManager when the PresShell is null

I'm working on a patch.
(Assignee)

Comment 5

10 years ago
I already assigned this to me and I'm working on a patch too ;)
(Assignee)

Comment 6

10 years ago
This is sort of regression from bug 208190, which added inlined getter methods to
PresContext.
(Assignee)

Comment 7

10 years ago
Created attachment 363949 [details] [diff] [review]
remove nsPresContext::GetViewManager();

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)
(Assignee)

Comment 8

10 years ago
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)
(Assignee)

Comment 9

9 years ago
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+
(Assignee)

Comment 11

9 years ago
(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.
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.
(Assignee)

Comment 12

9 years ago
Created attachment 366800 [details] [diff] [review]
up-to-date
(Assignee)

Comment 13

9 years ago
Created attachment 366811 [details] [diff] [review]
up-to-date
(Assignee)

Updated

9 years ago
Attachment #366800 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Status: NEW → RESOLVED
Last Resolved: 9 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.
Status: RESOLVED → VERIFIED

Updated

9 years ago
Blocks: 483591

Updated

9 years ago
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: http://is.gd/2ydcb - if this bug is not actually fixed on mozilla1.9.2, please remove the keyword. Apologies for the bugspam)
Keywords: fixed1.9.2
status1.9.2: --- → beta1-fixed
Keywords: fixed1.9.2
Crash Signature: [@ nsEventStateManager::PreHandleEvent]
You need to log in before you can comment on or make changes to this bug.