Crash [@ nsSelectionState::DoTraverse] with textarea, changing root

RESOLVED FIXED in mozilla2.0b4

Status

()

Core
Editor
--
critical
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: Ehsan)

Tracking

(Blocks: 2 bugs, {assertion, crash, testcase})

Trunk
mozilla2.0b4
x86
Mac OS X
assertion, crash, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 -, status2.0 wanted, blocking1.9.2 .11+, status1.9.2 .11-fixed, blocking1.9.1 .14+, status1.9.1 .14-fixed)

Details

(Whiteboard: [sg:critical?] [qa-examined-191] [qa-examined-192], crash signature)

Attachments

(5 attachments)

(Reporter)

Description

7 years ago
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]
blocking2.0: --- → ?
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.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #458641 - Flags: review?(roc)
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.)
Attachment #458644 - Flags: review?(roc)
Created attachment 458645 [details] [diff] [review]
Part 3: crashtest
Attachment #458645 - Flags: review?(roc)
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
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.
blocking1.9.1: ? → ---
blocking1.9.2: ? → ---
status1.9.1: --- → wanted
status1.9.2: --- → wanted
Attachment #458641 - Flags: review?(roc) → review+
Attachment #458644 - Flags: review?(roc) → review+
Attachment #458645 - Flags: review?(roc) → review+
Created attachment 462449 [details] [diff] [review]
Rolled up patch
Attachment #462449 - Flags: approval2.0?
Comment on attachment 462449 [details] [diff] [review]
Rolled up patch

a2.0=dbaron
Attachment #462449 - Flags: approval2.0? → approval2.0+
blocking2.0: ? → -
status2.0: --- → wanted
http://hg.mozilla.org/mozilla-central/rev/6a75787301c4
http://hg.mozilla.org/mozilla-central/rev/c87965964a17
http://hg.mozilla.org/mozilla-central/rev/440a4bfe2f5f
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b4
Attachment #462449 - Flags: approval1.9.2.10?
Attachment #462449 - Flags: approval1.9.1.13?
crashing in cycle collection, and a smattering of definitely NOT near-null addresses in http://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=startswith&query=nsSelectionState%3A%3ADoTraverse&date=08%2F27%2F2010%2010%3A21%3A14&range_value=1&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=nsSelectionState%3A%3ADoTraverse%28nsCycleCollectionTraversalCallback%26%29

make me think this is possible a security problem.
Group: core-security
blocking1.9.1: --- → .13+
blocking1.9.2: --- → .10+
Whiteboard: [sg:critical?]
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
Attachment #462449 - Flags: approval1.9.2.10?
Attachment #462449 - Flags: approval1.9.2.10+
Attachment #462449 - Flags: approval1.9.1.13?
Attachment #462449 - Flags: approval1.9.1.13+
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a41c2b7f926b
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/595868e7f677
status1.9.1: wanted → .13-fixed
status1.9.2: wanted → .10-fixed
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
I had aki|buildduty trigger another round of mochitests for that box, to see if the orange from comment 11 sticks.
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.
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?
Loading the page in a tab, closing the tab and waiting for a while should be enough.
Nothing here running a 1.9.2.9pre build in order to see the crash. What operating system are you using?
Whiteboard: [sg:critical?] → [sg:critical?] [qa-examined-191] [qa-examined-192]
Mac.
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.
Hmm, weird.  What about 1.9.1?
The same.
(In reply to comment #20)
> The same.

Then something else is probably in play here...
Group: core-security
Crash Signature: [@ nsSelectionState::DoTraverse]
You need to log in before you can comment on or make changes to this bug.