Closed Bug 1267757 Opened 4 years ago Closed 9 months ago

AVR:NULL f54.7f7 @ firefox.exe!xul.dll!nsHTMLEditRules::GetNodesForOperation

Categories

(Core :: DOM: Editor, defect)

45 Branch
defect
Not set
critical

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox48 - ---
firefox49 - ---
firefox50 - ---
firefox58 --- affected
firefox59 --- ?

People

(Reporter: abbGZcvu_bugzilla.mozilla.org, Assigned: masayuki)

References

()

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(1 file)

267 bytes, text/html
Details
Attached file Repro
<html id=h><script>
  onload = function() {
    d=document;
    d.replaceChild(b,h);
    d.designMode="on";
    d.addEventListener("DOMSubtreeModified",function(){d.normalize()});
    d.execCommand("JustifyCenter");
  };
</script>
<body id=b> x</body></html>
Source:
https://dxr.mozilla.org/mozilla-central/source/editor/libeditor/nsHTMLEditRules.cpp#5828
Release build uses older source than what is online, so the bug is reported at a lower line number. I therefore can't pinpoint the issue more closely.
Component: DOM: Core & HTML → Editor
MXR has source code for all of our current releases: http://mxr.mozilla.org/
Crash Signature: [@ nsHTMLEditRules::GetNodesForOperation ]
That's better; it's reported to crash on this line.
https://mxr.mozilla.org/mozilla-central/source/editor/libeditor/nsHTMLEditRules.cpp#5793

5767 ///////////////////////////////////////////////////////////////////////////////
5768 // GetNodesForOperation: Run through the ranges in the array and construct a
5769 //                       new array of nodes to be acted on.
5770 //
5771 nsresult
5772 nsHTMLEditRules::GetNodesForOperation(nsTArray<RefPtr<nsRange>>& aArrayOfRanges,
5773                                       nsTArray<OwningNonNull<nsINode>>& aOutArrayOfNodes,
5774                                       EditAction aOperationType,
5775                                       TouchContent aTouchContent)
5776 {
5777   NS_ENSURE_STATE(mHTMLEditor);
5778   nsCOMPtr<nsIEditor> kungFuDeathGrip(mHTMLEditor);
5779 
5780   int32_t rangeCount = aArrayOfRanges.Length();
5781   nsresult res = NS_OK;
5782 
5783   if (aTouchContent == TouchContent::yes) {
5784     // Split text nodes. This is necessary, since GetPromotedPoint() may return a
5785     // range ending in a text node in case where part of a pre-formatted
5786     // elements needs to be moved.
5787     for (int32_t i = 0; i < rangeCount; i++) {
5788       RefPtr<nsRange> r = aArrayOfRanges[i];
5789       nsCOMPtr<nsIContent> endParent = do_QueryInterface(r->GetEndParent());
5790       if (!mHTMLEditor->IsTextNode(endParent)) {
5791         continue;
5792       }
5793       nsCOMPtr<nsIDOMText> textNode = do_QueryInterface(endParent);

Hope this helps.
testcase crashes opt builds up to beta  and reproduced with bughunter and general opt builds 

https://crash-stats.mozilla.com/report/index/e08f388d-de08-4da8-ae7e-0396b2160804

Masayuki, can you take a look ?
Flags: needinfo?(masayuki)
Hmm, I don't have much time, but looks like that we just need a nullptr check before somewhere. So, I'll try... (Or, it's probably faster if somebody write a patch and give the patch r+ from me...)
Assignee: nobody → masayuki
Flags: needinfo?(masayuki)
The volume is tiny, not tracking
Crash Signature: [@ nsHTMLEditRules::GetNodesForOperation ] → [@ nsHTMLEditRules::GetNodesForOperation ] [@ mozilla::HTMLEditRules::GetNodesForOperation]
See Also: → 1345015
Flags: in-testsuite?

I don't reproduce this crash anymore with the testcase. We've made editor module solider (and also keep working on it) so that this must have already been fixed.

Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.