Closed Bug 1457862 Opened Last year Closed Last year

Crash in mozilla::EditorBase::GetChildOffset

Categories

(Core :: DOM: Core & HTML, defect, P1, critical)

Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 --- fixed

People

(Reporter: marcia, Assigned: bzbarsky)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-7378c60d-ae08-4521-81d4-11aa00180427.
=============================================================

Seen while looking at nightly crash stats - small volume Windows crash that started using 20180427102245: https://bit.ly/2vXfyxH

Looks as if some code was touched in Bug 1455674. ni on :bz

Top 10 frames of crashing thread:

0 xul.dll mozilla::EditorBase::GetChildOffset editor/libeditor/EditorBase.cpp:3376
1 xul.dll mozilla::HTMLEditor::GetCellContext editor/libeditor/HTMLTableEditor.cpp:2843
2 xul.dll mozilla::HTMLEditor::InsertTableCell editor/libeditor/HTMLTableEditor.cpp:178
3 xul.dll mozilla::HTMLEditor::InsertTableColumn editor/libeditor/HTMLTableEditor.cpp:437
4 xul.dll mozilla::HTMLEditor::DoInlineTableEditingAction editor/libeditor/HTMLInlineTableEditor.cpp:143
5 xul.dll mozilla::HTMLEditorEventListener::MouseClick editor/libeditor/HTMLEditorEventListener.cpp:202
6 xul.dll mozilla::EditorEventListener::HandleEvent editor/libeditor/EditorEventListener.cpp:458
7 xul.dll mozilla::EventListenerManager::HandleEventSubType dom/events/EventListenerManager.cpp:1121
8 xul.dll mozilla::EventListenerManager::HandleEventInternal dom/events/EventListenerManager.cpp:1288
9 xul.dll mozilla::EventTargetChainItem::HandleEventTargetChain dom/events/EventDispatcher.cpp:528

=============================================================
Flags: needinfo?(bzbarsky)
Any ideas on how to write a test for this?
Attachment #8972100 - Flags: review?(masayuki)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Marcia, thank you for filing this!

Looks like a null deref of aParent when doing aParent->GetFirstChild() in EditorBase::GetChildOffset.  The problem is this code in GetCellContext:


    nsCOMPtr<nsINode> cellParent = cell->GetParentNode();

    // Cell has to have a parent, so fail if not found
    NS_ENSURE_TRUE(cellParent, NS_ERROR_FAILURE);

    cellParent.forget(aCellParent);

    if (aCellOffset) {
      *aCellOffset = GetChildOffset(cell, cellParent);

    }

which nulls out cellParent (via the forget() call) after the null-check.  This was indeed changed in bug 1455674; it was OK before.
Blocks: 1455674
Flags: needinfo?(bzbarsky)
Priority: -- → P1
Comment on attachment 8972100 [details] [diff] [review]
Don't forget cellParent until we're done using it in HTMLEditor::GetCellContext

Folks in Japan are on holiday.


The old code did
-    *aCellParent = cellParent->AsDOMNode();
-    NS_ADDREF(*aCellParent);
so the change looks reasonable.
Attachment #8972100 - Flags: review?(masayuki) → review+
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0fcc5bcaf24
Don't forget cellParent until we're done using it in HTMLEditor::GetCellContext.  r=masayuki
https://hg.mozilla.org/mozilla-central/rev/e0fcc5bcaf24
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.