Jesse Ruderman Reporter
	Description • 18 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

	neil@parkwaycc.co.uk
	Comment 2 • 18 years ago

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.

	neil@parkwaycc.co.uk
	Comment 3 • 18 years ago

If we change the style rule in xul.css to
tree > treechildren:first-child
would that resolve the problem?

	Jesse Ruderman Reporter
	Comment 4 • 17 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

	Jesse Ruderman Reporter
	Comment 5 • 15 years ago

How bad are these assertions?

	neil@parkwaycc.co.uk
	Comment 6 • 15 years ago

(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.

	Timothy Nikkel (:tnikkel)
	Comment 7 • 15 years ago

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.

	Timothy Nikkel (:tnikkel)
	Comment 8 • 15 years ago

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.

	Timothy Nikkel (:tnikkel)
	Comment 9 • 15 years ago

http://hg.mozilla.org/mozilla-central/rev/58fd8a926bf5

	Timothy Nikkel (:tnikkel)
	Comment 10 • 15 years ago

Backed out due to bug 524293.

	Timothy Nikkel (:tnikkel)
	Comment 11 • 15 years ago

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.

	Ryan VanderMeulen [:RyanVM]
	Comment 12 • 9 years ago

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).

	Ryan VanderMeulen [:RyanVM]
	Comment 13 • 9 years ago

Comment 12 spun off to bug 1217984 at Jesse's request. I'll close this out as WFM when I push the crashtest.

	Pulsebot
	Comment 14 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef8c5b8c435e