1332876 - Crash in mozilla::HTMLEditRules::BeforeEdit

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Mats Palmgren (inactive)]

Attachment: CRASH testcase

This bug was filed from the Socorro interface and is 
report bp-91244312-5b4d-4a98-a9f2-e0fff2170117.
=============================================================

I stumbled into this crash while writing some tests for bug 795418.

Stack:
mozilla::HTMLEditRules::BeforeEdit(EditAction, short)
mozilla::HTMLEditor::StartOperation(EditAction, short)
mozilla::AutoRules::AutoRules(mozilla::EditorBase*, EditAction, short)
mozilla::TextEditor::DeleteSelection(short, short)
mozilla::EditorBase::HandleKeyPressEvent(nsIDOMKeyEvent*)
mozilla::HTMLEditor::HandleKeyPressEvent(nsIDOMKeyEvent*)
mozilla::EditorEventListener::KeyPress(nsIDOMKeyEvent*)
mozilla::EditorEventListener::HandleEvent(nsIDOMEvent*)


In a DEBUG build, you get a fatal assertion instead:

Assertion failure: mRawPtr != nullptr (You can't dereference a NULL RefPtr with operator->().), at dist/include/mozilla/RefPtr.h:314
#01: RefPtr<mozilla::dom::Selection>::operator->() const (dist/include/mozilla/RefPtr.h:313 (discriminator 10))
#02: mozilla::HTMLEditRules::BeforeEdit(EditAction, short) (editor/libeditor/HTMLEditRules.cpp:325 (discriminator 1))
...

Comment 1

[Mats Palmgren (inactive)]

Attachment: fix

Comment 2

[Mats Palmgren (inactive)]

Attachment: test

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7ba3f1d44a1aa6cd33eb27c379c8b31304c0cb71

Comment 3

[(no longer active)]

Comment on attachment 8829181 [details] [diff] [review]
fix

Review of attachment 8829181 [details] [diff] [review]:
-----------------------------------------------------------------

I don't own editor/ any more.  :-)

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8829181 [details] [diff] [review]
fix

>     // Get selection
>     RefPtr<Selection> selection = htmlEditor->GetSelection();
> 
>     // Get the selection location
>-    if (!selection->RangeCount()) {
>+    if (!selection || !selection->RangeCount()) {

nit: I like |if (NS_WARN_IF(!selection) || !selection->RangeCount()) {| better because it's really unespected case.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c16eca3cc95
Bail out of HTMLEditRules::BeforeEdit if we have no Selection.  r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/952d534e1347
test.

Comment 6

[Mats Palmgren (inactive)]

Comment on attachment 8829181 [details] [diff] [review]
fix

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: crash
[Is this code covered by automated tests?]:yes
[Has the fix been verified in Nightly?]:yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:no
[Why is the change risky/not risky?]:just adding a trivial null-pointer check
[String changes made/needed]:none

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6c16eca3cc95
https://hg.mozilla.org/mozilla-central/rev/952d534e1347

Comment 8

[Julien Cristau [:jcristau]]

Comment on attachment 8829181 [details] [diff] [review]
fix

crash fix for aurora53, beta52

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d6e53354c034

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c0345ca8cb3b

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/cf1c3ab4b7aa

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f7b5ec13731e