Closed Bug 1408170 Opened 3 years ago Closed 3 years ago

stack-overflow in [@ mozilla::HTMLEditRules::WillDeleteSelection]

Categories

(Core :: DOM: Editor, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- unaffected
firefox58 --- fixed

People

(Reporter: tsmith, Assigned: ehsan)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, testcase)

Attachments

(3 files)

Attached file test_case.html
The stack is this repeated... mozilla::HTMLEditRules::WillDeleteSelection(mozilla::dom::Selection*, short, short, bool*, bool*) /builds/worker/workspace/build/src/editor/libeditor/HTMLEditRules.cpp:2321:16
Flags: in-testsuite?
Attached file prefs.js
INFO: Last good revision: a2bc0214fbcaf6104fe2ae8d2f4843997ea283de
INFO: First bad revision: e22de36807c6229b3710382f0094ffe5e31eaa26
INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a2bc0214fbcaf6104fe2ae8d2f4843997ea283de&tochange=e22de36807c6229b3710382f0094ffe5e31eaa26
Ehsan isn't accepting NI requests, so maybe Masayuki can take a look.
Blocks: 1407305
Has Regression Range: --- → yes
Flags: needinfo?(masayuki)
Version: unspecified → Trunk
I have a fix.  I need to figure out what set of prefs the test case depends on...
Assignee: nobody → ehsan
Flags: needinfo?(masayuki)
Hmm, the test doesn't depend on a pref, but doesn't reproduce the crash when loaded from https for some reason.  At any rate, when loaded as a crashtest it does reproduce the crash which is good from a regression testing perspective.
Keywords: regression
Comment on attachment 8918117 [details] [diff] [review]
Set child correctly in HTMLEditRules::GetPromotedPoint()

>@@ -5568,17 +5568,17 @@ HTMLEditRules::GetPromotedPoint(RulesEndpoint aWhere,
>     // from us, and that are enclosed in the same block.
>     nsCOMPtr<nsINode> priorNode =
>       htmlEditor->GetPriorHTMLNode(node, offset, child, true);
> 
>     while (priorNode && priorNode->GetParentNode() &&
>            !htmlEditor->IsVisibleBRElement(priorNode) &&
>            !IsBlockNode(*priorNode)) {
>       offset = priorNode->GetParentNode()->IndexOf(priorNode);
>-      child = node;
>+      child = priorNode;

Oh, it really mismatches with the offset. Sorry for not catching it at review.

>diff --git a/editor/libeditor/crashtests/1408170.html b/editor/libeditor/crashtests/1408170.html
>@@ -0,0 +1,20 @@
>+<script>
>+function jsfuzzer() {
>+  try { document.execCommand("insertUnorderedList", false); } catch(e) { }
>+  try { document.execCommand("delete", false); } catch(e) { }
>+}
>+function eventhandler1() {
>+  try { window.getSelection().collapse(htmlvar00001,1); } catch(e) { }

Oh, don't we need to do |document.getElementById("htmlvar0001")|?

>+}
>+function eventhandler2() {
>+  try { htmlvar00002.appendChild(htmlvar00001); } catch(e) { }

And same here for htmlvar0002 and htmlvar0001?
Attachment #8918117 - Flags: review?(masayuki) → review+
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #7)
> >diff --git a/editor/libeditor/crashtests/1408170.html b/editor/libeditor/crashtests/1408170.html
> >@@ -0,0 +1,20 @@
> >+<script>
> >+function jsfuzzer() {
> >+  try { document.execCommand("insertUnorderedList", false); } catch(e) { }
> >+  try { document.execCommand("delete", false); } catch(e) { }
> >+}
> >+function eventhandler1() {
> >+  try { window.getSelection().collapse(htmlvar00001,1); } catch(e) { }
> 
> Oh, don't we need to do |document.getElementById("htmlvar0001")|?
> 
> >+}
> >+function eventhandler2() {
> >+  try { htmlvar00002.appendChild(htmlvar00001); } catch(e) { }
> 
> And same here for htmlvar0002 and htmlvar0001?

These will just throw.  This is a fuzzer generated test case, so not everything in the test makes sense.  :-)  In this case, the bug was triggered by the execCommand("delete") call...
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4baaea004689
Set child correctly in HTMLEditRules::GetPromotedPoint(); r=masayuki
https://hg.mozilla.org/mozilla-central/rev/4baaea004689
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
See Also: → 1411345
You need to log in before you can comment on or make changes to this bug.