Closed Bug 1332876 Opened 3 years ago Closed 3 years ago

Crash in mozilla::HTMLEditRules::BeforeEdit

Categories

(Core :: Editor, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox52 --- fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: mats, Assigned: mats)

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(3 files)

Attached file 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))
...
Attached patch fixSplinter Review
Attachment #8829181 - Flags: review?(ehsan)
Crash Signature: [@ mozilla::HTMLEditRules::BeforeEdit] → [@ mozilla::HTMLEditRules::BeforeEdit] [@ nsHTMLEditRules::BeforeEdit ] [@ nsCOMPtr_base::assign_with_AddRef | mozilla::HTMLEditRules::BeforeEdit ]
Comment on attachment 8829181 [details] [diff] [review]
fix

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

I don't own editor/ any more.  :-)
Attachment #8829181 - Flags: review?(ehsan) → review?(masayuki)
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.
Attachment #8829181 - Flags: review?(masayuki) → review+
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
Attachment #8829181 - Flags: approval-mozilla-beta?
Attachment #8829181 - Flags: approval-mozilla-aurora?
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/6c16eca3cc95
https://hg.mozilla.org/mozilla-central/rev/952d534e1347
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment on attachment 8829181 [details] [diff] [review]
fix

crash fix for aurora53, beta52
Attachment #8829181 - Flags: approval-mozilla-beta?
Attachment #8829181 - Flags: approval-mozilla-beta+
Attachment #8829181 - Flags: approval-mozilla-aurora?
Attachment #8829181 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.