Last Comment Bug 759748 - crash in nsHTMLEditRules::WillDoAction
: crash in nsHTMLEditRules::WillDoAction
: crash, regression, reproducible
Product: Core
Classification: Components
Component: Editor (show other bugs)
: 15 Branch
: All All
: -- critical (vote)
: mozilla15
Assigned To: :Aryeh Gregor (away until August 15)
Depends on: 760052 760583
Blocks: 748307
  Show dependency treegraph
Reported: 2012-05-30 07:32 PDT by Scoobidiver (away)
Modified: 2012-08-07 01:33 PDT (History)
4 users (show)
ayg: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (3.97 KB, patch)
2012-05-31 03:50 PDT, :Aryeh Gregor (away until August 15)
ehsan: review+
Details | Diff | Review

Description Scoobidiver (away) 2012-05-30 07:32:32 PDT
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
Comment 1 :Ehsan Akhgari (busy, don't ask for review please) 2012-05-30 11:28:36 PDT
Hmm, Aryeh, can |range| be null here? <>
Comment 2 :Aryeh Gregor (away until August 15) 2012-05-31 02:43:59 PDT
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.
Comment 3 :Aryeh Gregor (away until August 15) 2012-05-31 03:06:09 PDT
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.
Comment 4 :Aryeh Gregor (away until August 15) 2012-05-31 03:50:52 PDT
Created attachment 628664 [details] [diff] [review]
Patch v1

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.
Comment 5 :Aryeh Gregor (away until August 15) 2012-05-31 11:24:56 PDT

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!
Comment 6 Ed Morley [:emorley] 2012-06-01 08:38:56 PDT
Comment 7 Paul Silaghi, QA [:pauly] 2012-08-07 01:33:28 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.