Steps to reproduce: Load the testcase in a (trunk) debug build. Result: ###!!! ASSERTION: Should not be called: 'Error', file /Users/admin/trunk/mozilla/layout/xul/base/src/grid/nsGridLayout2.cpp, line 302

299 NS_IMETHODIMP 300 nsGridLayout2::GetRowCount(PRInt32& aRowCount) 301 { 302 NS_ERROR("Should not be called"); 303 return NS_OK; 304 } Not assigning to an out-param and returning NS_OK seems like an odd thing to do...

Btw, the assertion happens about 8 times when I load the testcase.

Andreas, are you interested in fixing this bug?

I can have a look at it. Even if I know my way around those files now, debugging takes a bit more understanding of the code than just a read-through deCOM does. I'll fix the return stuff in bug 363879. Is NS_ERROR() best to have when a function is forced by an interface but should not be used in that class, or should I change to some other macro?

The assert is now at line 81 in nsGridLayout2.h (http://landfill.mozilla.org/mxr-test/seamonkey/source/layout/xul/base/src/grid/nsGridLayout2.h#81). The call is from GetGrid() in nsGridRowLayout.cpp (http://landfill.mozilla.org/mxr-test/seamonkey/source/layout/xul/base/src/grid/nsGridRowLayout.cpp#148). The code iterates the children and QI to nsIGridPart, not expecting a GridLayout. There are a few more such places in the grid code, but I'm not sure it is possible to trigger those.

Avoid grids and get rid of an unused rv at the same time.

Comment on attachment 249963 [details] [diff] [review] Avoid calling the assertion IMHO this call is not unexpected, because we already allow <grid><rows><box> and I don't see why we can't allow <grid><rows><grid>. Instead I would have thought that nsIGridPart::GetRowCount should return 1 and only nsGridRowGroupLayout needs to override it.

> Instead I would have thought that nsIGridPart::GetRowCount should return 1 and > only nsGridRowGroupLayout needs to override it. Better. Lets do that.

Comment on attachment 250654 [details] [diff] [review] Default rowcount to 1 row Nice diffstats ;-)

I checked in the patch (2007-01-07 21:58).

Crashtest checked in.