crash in nsHTMLEditRules::WillDoAction

VERIFIED FIXED in Firefox 15

Status

()

Core
Editor
--
critical
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: Scoobidiver (away), Assigned: ayg)

Tracking

({crash, regression, reproducible})

15 Branch
mozilla15
crash, regression, reproducible
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox15+ verified)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
It first appeared in 15.0a1/20120527. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e4574b46f0ba&tochange=133aa3a2ef0a
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 Reason	EXCEPTION_ACCESS_VIOLATION_READ
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:
https://crash-stats.mozilla.com/report/list?signature=nsHTMLEditRules%3A%3AWillDoAction%28nsTypedSelection*%2C+nsRulesInfo*%2C+bool*%2C+bool*%29
Hmm, Aryeh, can |range| be null here? <http://hg.mozilla.org/mozilla-central/annotate/79262a88881d/editor/libeditor/html/nsHTMLEditRules.cpp#l540>
tracking-firefox15: --- → +
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() {
  getSelection().removeAllRanges()
}, 1000)</script>

2) Click in the editable region

3) Wait for the cursor to disappear

4) Hit a key

Patch in a few minutes.
(Reporter)

Updated

5 years ago
Keywords: reproducible
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.

https://tbpl.mozilla.org/?tree=Try&rev=f76bc311ecf3
Assignee: nobody → ayg
Status: NEW → ASSIGNED
Attachment #628664 - Flags: review?(ehsan)
Depends on: 760052
Attachment #628664 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/550e64dd5656

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

Updated

5 years ago
Crash Signature: [@ nsHTMLEditRules::WillDoAction(nsTypedSelection*, nsRulesInfo*, bool*, bool*)] → [@ nsHTMLEditRules::WillDoAction(nsTypedSelection*, nsRulesInfo*, bool*, bool*)] [@ nsHTMLEditRules::WillDoAction]

Comment 6

5 years ago
https://hg.mozilla.org/mozilla-central/rev/550e64dd5656
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15

Updated

5 years ago
Depends on: 760583
(Reporter)

Updated

5 years ago
status-firefox15: --- → fixed
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.
Status: RESOLVED → VERIFIED
status-firefox15: fixed → verified
You need to log in before you can comment on or make changes to this bug.