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

RESOLVED WORKSFORME

Status

()

defect
RESOLVED WORKSFORME
13 years ago
4 years ago

People

(Reporter: jruderman, Unassigned)

Tracking

(Blocks 1 bug, {assertion, testcase})

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

843 bytes, application/vnd.mozilla.xul+xml
Details
811 bytes, patch
neil
: review+
enndeakin
: review+
Details | Diff | Splinter Review
Reporter

Description

13 years ago
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
Reporter

Comment 1

13 years ago
Posted 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?
Reporter

Comment 4

12 years ago
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

Updated

11 years ago
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: xptoolkit.trees → xptoolkit.widgets
Reporter

Comment 5

10 years ago
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.
Posted 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)

Updated

10 years ago
Attachment #407199 - Flags: review?(neil) → review+
Attachment #407199 - Flags: review?(enndeakin)
Comment on attachment 407199 [details] [diff] [review]
patch

I just happened now to see the message at the top of xul.css about needing review from the other Neil.

Updated

10 years ago
Attachment #407199 - Flags: review?(enndeakin) → review+
http://hg.mozilla.org/mozilla-central/rev/58fd8a926bf5
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Depends on: 524293
Backed out due to bug 524293.
Status: RESOLVED → REOPENED
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.
Status: REOPENED → RESOLVED
Closed: 10 years ago4 years ago
Flags: in-testsuite+
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.