Last Comment Bug 580151 - Crash [@ nsSelectionState::DoTraverse] with textarea, changing root
: Crash [@ nsSelectionState::DoTraverse] with textarea, changing root
Status: RESOLVED FIXED
[sg:critical?] [qa-examined-191] [qa-...
: assertion, crash, testcase
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: x86 Mac OS X
: -- critical (vote)
: mozilla2.0b4
Assigned To: :Ehsan Akhgari (out sick)
:
Mentors:
Depends on:
Blocks: stirdom 343951
  Show dependency treegraph
 
Reported: 2010-07-19 19:54 PDT by Jesse Ruderman
Modified: 2011-06-13 10:01 PDT (History)
5 users (show)
ehsan: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
wanted
.11+
.11-fixed
.14+
.14-fixed


Attachments
testcase (crashes Firefox later, during cycle collection) (655 bytes, text/html)
2010-07-19 19:54 PDT, Jesse Ruderman
no flags Details
Part 1: Fix the first assertion (1.35 KB, patch)
2010-07-20 06:05 PDT, :Ehsan Akhgari (out sick)
roc: review+
Details | Diff | Review
Part 2: Fix the second assertion (1.06 KB, patch)
2010-07-20 06:19 PDT, :Ehsan Akhgari (out sick)
roc: review+
Details | Diff | Review
Part 3: crashtest (1.29 KB, patch)
2010-07-20 06:22 PDT, :Ehsan Akhgari (out sick)
roc: review+
Details | Diff | Review
Rolled up patch (3.25 KB, patch)
2010-08-03 11:44 PDT, :Ehsan Akhgari (out sick)
dbaron: approval2.0+
dveditz: approval1.9.2.11+
dveditz: approval1.9.1.14+
Details | Diff | Review

Description Jesse Ruderman 2010-07-19 19:54:22 PDT
Created attachment 458543 [details]
testcase (crashes Firefox later, during cycle collection)

###!!! ASSERTION: bad action nesting!: 'mActionNesting>0', file editor/libeditor/text/nsTextEditRules.cpp, line 250

###!!! ASSERTION: zero or negative placeholder batch count when ending batch!: 'mPlaceHolderBatch > 0', file editor/libeditor/base/nsEditor.cpp, line 876

Crash [@ nsSelectionState::DoTraverse]
Comment 1 :Ehsan Akhgari (out sick) 2010-07-20 06:05:36 PDT
Created attachment 458641 [details] [diff] [review]
Part 1: Fix the first assertion

The first assertion is caused by the fact that nsTextEditRules::BeforeEdit can be bailing out early if nsEditor::GetSelection fails, and therefore not incrementing mActionNesting as expected.
Comment 2 :Ehsan Akhgari (out sick) 2010-07-20 06:19:09 PDT
Created attachment 458644 [details] [diff] [review]
Part 2: Fix the second assertion

The cause of the second assertion is similar: failing to increment mPlaceHolderBatch because we bail out early if we can't get the selection.  These two patches fix the crash as well (as it was caused by a partially initialized placeholder transaction.)
Comment 3 :Ehsan Akhgari (out sick) 2010-07-20 06:22:17 PDT
Created attachment 458645 [details] [diff] [review]
Part 3: crashtest
Comment 4 Daniel Veditz [:dveditz] 2010-07-21 10:40:18 PDT
Why would this block the branch releases? Seems nice to have on the branches so request approval when you've got reviews, etc.  If we're misunderstanding the importance of this apparently non-security, non-topcrash bug please let us know.
Comment 5 :Ehsan Akhgari (out sick) 2010-08-03 11:44:06 PDT
Created attachment 462449 [details] [diff] [review]
Rolled up patch
Comment 6 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-08-03 17:17:49 PDT
Comment on attachment 462449 [details] [diff] [review]
Rolled up patch

a2.0=dbaron
Comment 9 Daniel Veditz [:dveditz] 2010-08-27 10:29:46 PDT
Comment on attachment 462449 [details] [diff] [review]
Rolled up patch

Approved for 1.9.2.10 and 1.9.1.13, a=dveditz for release-drivers
Comment 11 Daniel Holbert [:dholbert] 2010-08-27 18:24:57 PDT
Looks like this might've caused a new mochitest failure in editor code on 1.9.1:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1282941921.1282944723.14877.gz
OS X 10.5.2 mozilla-1.9.1 test mochitests on 2010/08/27 13:45:21
s: moz2-darwin9-slave55

>44652 ERROR TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/html/tests/test_CF_HTML_clipboard.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - SimpleTest.waitForFocus is not a function
Comment 12 Daniel Holbert [:dholbert] 2010-08-27 18:32:15 PDT
I had aki|buildduty trigger another round of mochitests for that box, to see if the orange from comment 11 sticks.
Comment 13 Daniel Holbert [:dholbert] 2010-08-27 19:02:30 PDT
Actually it looks like that test has been perma-orange on 1.9.1 for a while (probably ever since it landed in bug 572642)

I filed Bug 591544 on it.
Comment 14 Al Billings [:abillings] 2010-09-09 15:36:42 PDT
Running the testcase in comment 0 and comment 3 in my own debug 1.9.2 build (pre-fix) on Windows XP, I don't trigger any crash, even if I sit around for a while. How do we trigger cycle collection to cause this crash?
Comment 15 :Ehsan Akhgari (out sick) 2010-09-09 16:12:26 PDT
Loading the page in a tab, closing the tab and waiting for a while should be enough.
Comment 16 Al Billings [:abillings] 2010-09-22 15:02:50 PDT
Nothing here running a 1.9.2.9pre build in order to see the crash. What operating system are you using?
Comment 17 :Ehsan Akhgari (out sick) 2010-09-22 15:19:10 PDT
Mac.
Comment 18 Al Billings [:abillings] 2010-09-22 15:43:00 PDT
I just tried with 10.6.4, using both testcases, opening them in a new tab, waiting a minute, closing the tab, and then watching. I see no asserts and no crashes in 1.9.2.11pre.
Comment 19 :Ehsan Akhgari (out sick) 2010-09-22 15:53:23 PDT
Hmm, weird.  What about 1.9.1?
Comment 20 Al Billings [:abillings] 2010-09-23 16:19:59 PDT
The same.
Comment 21 :Ehsan Akhgari (out sick) 2010-09-23 16:23:23 PDT
(In reply to comment #20)
> The same.

Then something else is probably in play here...

Note You need to log in before you can comment on or make changes to this bug.