We can make the safe assumption that _most_ of the cellmaps are in the right order, I think. That should allow us to do a lot better here. The testcase is in bug 313295. See bug 313295 comment 35 for details.

+ // XXXbz we just cast |frame| to nsTableRowGroupFrame*. Why are + // we bothering to call mTableFrame.GetRowGroup()? NEVER EVER miss the GetRowGroup() call, sometimes scrollframes wrap around the rowgroup frame and then it will certainly crash

> NEVER EVER miss the GetRowGroup() call, sometimes scrollframes wrap around The point is that the code looks like: nsIFrame* frame = (nsTableRowGroupFrame*)mRowGroups.ElementAt(mRowGroupIndex); if (!frame) ABORT1(PR_FALSE); mRowGroup = mTableFrame.GetRowGroupFrame(frame); if (!mRowGroup) ABORT1(PR_FALSE); If mRowGroups.ElementAt(mRowGroupIndex) returns a scrollframe, then we're dead as soon as we cast it to nsTableRowGroupFrame* and proceed to call virtual methods on it (which is what GetRowGroupFrame does). Having the cast here be a cast to nsIFrame* would make a lot more sense to me. What would make even more sense is having nsTableFrame::OrderRowGroups fill an nsTPtrArray<nsIFrame> (and having that as a member in the iterator). I'd be happy to do that in a separate bug.

Fixed.