Closed Bug 366203 Opened 17 years ago Closed 8 years ago

"ASSERTION: row count changed unexpectedly" with multiple <treechildren> and removing one


(Core :: XUL, defect)

Not set





(Reporter: jruderman, Unassigned)



(Keywords: assertion, testcase)


(2 files)

843 bytes, application/vnd.mozilla.xul+xml
811 bytes, patch
: review+
: review+
Details | Diff | Splinter Review
Loading this testcase triggers:

###!!! ASSERTION: row count changed unexpectedly: 'mRowCount == rowCount', file /Users/admin/trunk/mozilla/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp, line 2633
Attached file testcase
What seems to be happening here is that each <treechildren> element has its own tree body frame, but only the first one can receive row change notifications.
If we change the style rule in xul.css to
tree > treechildren:first-child
would that resolve the problem?
Now loading the assertion gives me lots of stuff like:

###!!! ASSERTION: bad index: 'aIndex >= 0 && aIndex < mRows.Count()', file /Users/jruderman/trunk/mozilla/layout/xul/base/src/tree/src/nsTreeContentView.cpp, line 284

###!!! ASSERTION: bad row: 'aRow >= 0 && aRow < mRows.Count()', file /Users/jruderman/trunk/mozilla/layout/xul/base/src/tree/src/nsTreeContentView.cpp, line 244
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: xptoolkit.trees → xptoolkit.widgets
How bad are these assertions?
(In reply to comment #5)
> How bad are these assertions?
The #0 one warns that the tree may not paint what you're expecting it to.
The #4 ones aren't really bad; they're usually just a programming error, like trying to access the (n+1)th row of an n-row tree, so maybe they should be converted from NS_PRECONDITION + if to NS_ENSURE_TRUE instead. However this is often linked with #0 because if the frame's count is out of sync with the view then it may access an invalid row while trying to paint it.
Attached patch patchSplinter Review
Just do what Neil suggested in comment 3. Well, actually first-of-type, not first-child. This fixes the assertions and makes the tree do what you would expect: it uses the next treechildren when the first is removed.
Assignee: Jan.Varga → tnikkel
Attachment #407199 - Flags: review?(neil)
Attachment #407199 - Flags: review?(neil) → review+
Attachment #407199 - Flags: review?(enndeakin)
Comment on attachment 407199 [details] [diff] [review]

I just happened now to see the message at the top of xul.css about needing review from the other Neil.
Attachment #407199 - Flags: review?(enndeakin) → review+
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Depends on: 524293
Backed out due to bug 524293.
Flags: in-testsuite+
Resolution: FIXED → ---
Target Milestone: mozilla1.9.3a1 → ---
OS: Mac OS X → All
Hardware: PowerPC → All
I think autocomplete was failing (bug 524293) because it attaches the treechildren element using a binding, and the first-of-type selector doesn't work on anonymous nodes.
Assignee: tnikkel → nobody
The assertion this was filed for was downgraded to a warning in bug 326033. On Android, the testcase still hits the warning and also asserts:

WARNING: row count changed unexpectedly: 'mRowCount == rowCount', file /builds/slave/try-and-api-11-d-0000000000000/build/src/layout/xul/tree/nsTreeBodyFrame.cpp, line 2878
[731] ###!!! ASSERTION: bad index: 'aIndex >= 0 && aIndex < int32_t(mRows.Length())', file /builds/slave/try-and-api-11-d-0000000000000/build/src/layout/xul/tree/nsTreeContentView.cpp, line 224
[731] ###!!! ASSERTION: bad row: 'aRow >= 0 && aRow < int32_t(mRows.Length())', file /builds/slave/try-and-api-11-d-0000000000000/build/src/layout/xul/tree/nsTreeContentView.cpp, line 193

It's otherwise green on desktop platforms (and I'm told that an NS_WARN_IF_FALSE would show up in a desktop test log if hit).
Comment 12 spun off to bug 1217984 at Jesse's request. I'll close this out as WFM when I push the crashtest.
Closed: 14 years ago8 years ago
Flags: in-testsuite+
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.