Closed Bug 759748 Opened 12 years ago Closed 12 years ago

crash in nsHTMLEditRules::WillDoAction


(Core :: DOM: Editor, defect)

15 Branch
Not set



Tracking Status
firefox15 + verified


(Reporter: scoobidiver, Assigned: ayg)



(Keywords: crash, regression, reproducible)

Crash Data


(1 file)

It first appeared in 15.0a1/20120527. The regression range is:
It's likely a regression from bug 748307.

Signature 	nsHTMLEditRules::WillDoAction(nsTypedSelection*, nsRulesInfo*, bool*, bool*) More Reports Search
UUID	cf244791-46d1-46fa-916d-cad542120529
Date Processed	2012-05-29 20:47:16
Uptime	146
Install Age	1.1 hours since version was first installed.
Install Time	2012-05-29 19:43:31
Product	Firefox
Version	15.0a1
Build ID	20120529030518
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7601 Service Pack 1
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 37 stepping 2
Crash Address	0x10
App Notes 	
AdapterVendorID: 0x1002, AdapterDeviceID: 0x68e0, AdapterSubsysID: 033d1025, AdapterDriverVersion: 8.951.0.0
D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ 
EMCheckCompatibility	True	
Total Virtual Memory	4294836224
Available Virtual Memory	3685588992
System Memory Use Percentage	47
Available Page File	6080536576
Available Physical Memory	2182213632

Frame 	Module 	Signature 	Source
0 	xul.dll 	nsHTMLEditRules::WillDoAction 	editor/libeditor/html/nsHTMLEditRules.cpp:540
1 	xul.dll 	nsPlaintextEditor::InsertText 	editor/libeditor/text/nsPlaintextEditor.cpp:791
2 	xul.dll 	nsPlaintextEditor::TypedText 	editor/libeditor/text/nsPlaintextEditor.cpp:403
3 	xul.dll 	nsHTMLEditor::TypedText 	editor/libeditor/html/nsHTMLEditor.cpp:1241
4 	xul.dll 	nsHTMLEditor::HandleKeyPressEvent 	editor/libeditor/html/nsHTMLEditor.cpp:692
5 	xul.dll 	nsEditorEventListener::KeyPress 	editor/libeditor/base/nsEditorEventListener.cpp:458
6 	xul.dll 	nsEditorEventListener::HandleEvent 	editor/libeditor/base/nsEditorEventListener.cpp:295
7 	xul.dll 	nsEventListenerManager::HandleEventInternal 	content/events/src/nsEventListenerManager.cpp:868
8 	xul.dll 	nsEventTargetChainItem::HandleEventTargetChain 	content/events/src/nsEventDispatcher.cpp:340
9 	xul.dll 	nsEventTargetChainItem::HandleEventTargetChain 	content/events/src/nsEventDispatcher.cpp:372
10 	xul.dll 	nsEventDispatcher::Dispatch 	content/events/src/nsEventDispatcher.cpp:643
11 	xul.dll 	PresShell::HandleEventInternal 	layout/base/nsPresShell.cpp:6362
12 	xul.dll 	PresShell::HandleEvent 	layout/base/nsPresShell.cpp:5977
13 	xul.dll 	PresShell::HandleEvent 	layout/base/nsPresShell.cpp:5630
14 	xul.dll 	nsViewManager::DispatchEvent 	view/src/nsViewManager.cpp:865
15 	xul.dll 	AttachedHandleEvent 	view/src/nsView.cpp:158
16 	xul.dll 	nsWindow::DispatchEvent 	widget/windows/nsWindow.cpp:3468
17 	xul.dll 	nsWindow::DispatchWindowEvent 	widget/windows/nsWindow.cpp:3494
18 	xul.dll 	nsWindow::DispatchKeyEvent 	widget/windows/nsWindow.cpp:3526
19 	xul.dll 	nsWindow::OnChar 	widget/windows/nsWindow.cpp:6647
20 	xul.dll 	nsWindow::OnKeyDown 	widget/windows/nsWindow.cpp:6366
21 	xul.dll 	nsWindow::ProcessKeyDownMessage 	widget/windows/nsWindow.cpp:5597
22 	xul.dll 	nsWindow::ProcessMessage 	widget/windows/nsWindow.cpp:4737
23 	xul.dll 	nsWindow::WindowProcInternal 	widget/windows/nsWindow.cpp:4289
24 	xul.dll 	CallWindowProcCrashProtected 	xpcom/base/nsCrashOnException.cpp:32
25 	xul.dll 	nsWindow::WindowProc 	widget/windows/nsWindow.cpp:4231
26 	user32.dll 	InternalCallWinProc 	

More reports at:*%2C+nsRulesInfo*%2C+bool*%2C+bool*%29
In principle probably yes -- someone might have called removeAllRanges() or such, or the user might have just not interacted with the page.  That was stupid of me.  :(

This reminds me of an old proposal I had -- I think it would be great if we specced a Selection to always have exactly one range.  So initially it would be collapsed at (document, 0), and removing the last range would re-collapse it there.  That would save authors plenty of bugs too!

I'll write a patch ASAP.
Steps to reproduce:

1) Load this page:

data:text/html,<!doctype html>
<div contenteditable>abc</div>
<script>setInterval(function() {
}, 1000)</script>

2) Click in the editable region

3) Wait for the cursor to disappear

4) Hit a key

Patch in a few minutes.
Keywords: reproducible
Attached patch Patch v1Splinter Review
This fixes the crash.  It doesn't even require user interaction; document.execCommand("inserttext", false, "a") does the trick.  The testcase also caught an assertion being hit in increaseFontSize/decreaseFontSize, so I fixed that too -- nothing was checking the return code of GetStartNodeAndOffset.

I don't particularly like listing all the commands like this, since they're prone to get out of date, but I guess it works.

ExecCommand() should really check if the command is enabled, and return false without executing it if so.  That would have prevented this crash, at least the execCommand()-triggered variant.
Assignee: nobody → ayg
Attachment #628664 - Flags: review?(ehsan)
Depends on: 760052
Attachment #628664 - Flags: review?(ehsan) → review+

All the Android XUL opt tests were red in my try push, but I'm guessing that's just Android being Android and nothing to do with my change.  I guess we'll find out!
Flags: in-testsuite+
OS: Windows 7 → All
Crash Signature: [@ nsHTMLEditRules::WillDoAction(nsTypedSelection*, nsRulesInfo*, bool*, bool*)] → [@ nsHTMLEditRules::WillDoAction(nsTypedSelection*, nsRulesInfo*, bool*, bool*)] [@ nsHTMLEditRules::WillDoAction]
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
Depends on: 760583
No crash loading the STR in comment 3.
Verified fixed on FF 15b3 on Win 7/64, Ubuntu 12.04 and Mac OS X 10.6.
You need to log in before you can comment on or make changes to this bug.