Steps to reproduce: 1. Load the testcase. A nightly crashes [@ nsRuleNode::GetStyleData] dereferencing zero, but my debug build crashes [@ nsCachedStyleData::GetStyleData] dereferencing a random address, so [sg:critical]. The debug build also complains: ###!!! ASSERTION: not implemented: '!IsBorderCollapse(aParentFrame)', file /Users/admin/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 1848
short german description of this bug: Mitgegangen, mitgehangen. I guess one needs to implement http://lxr.mozilla.org/seamonkey/source/layout/tables/nsTableCellFrame.cpp#1133
Once a table is border collapse the cellmap changes and is is expected that the td correspond to nsBCTableCellFrame rather than to nsTableCellFrame. So the assert is right to the point. However it does not help against really determined people.
bernd, what if it is the reverse, i.e., the table has border collapse, and the cell is nsTableCellFrame. Will that crash too? In order words, is nsBCTableCellFrame stronger against the mismatch? The reason I am asking is because I could then change mtable in mathml.css to be border collapse, and change nsMathMLmtdFrame to just inherit from nsBCTableCellFrame across the board. If this is a no-no too, then we can simply fix this bug with: if (IsBorderCollapse(aParentFrame)) return nsBCTableCellFrame() [i.e., those determined people get the default non-MathML frames and don't crash]
>the table has border collapse, and the cell is nsTableCellFrame This should NEVER happen. I can't imagine that one (Jesse and Martijn are not included in this) can construct a case where this happens. ::IsBorderCollapse protects against it. >I am asking is because I could then change mtable in mathml.css to be border >collapse, You don't want make border collapse the default for mtables. You really don't want this. Trust me. >those determined people get the default non-MathML frames and don't crash Probably a better option but I am not sure that the mathml code does not rely on mrow to have only mtd children.
> Probably a better option but I am not sure that the mathml code does not rely > on [mrow] (you meant mtr) to have only mtd children. There is only one spot with a static cast to nsTableCellFrame*: http://lxr.mozilla.org/mozilla/source/layout/mathml/base/src/nsMathMLmtableFrame.cpp#206 Shoudn't be a problem, as nsBCTableCellFrame is a subclass of nsTableCellFrame, right? If not, I can add a test for frametype == ::tableCell (rather than ::BCtableCell), and exit early from that function. (It is of little consequence to exit early from that function.)
I think that will work.
Created attachment 240588 [details] [diff] [review] patch
Comment on attachment 240588 [details] [diff] [review] patch I should have mentioned that this patch effectively fixes the crash.
Comment on attachment 240588 [details] [diff] [review] patch looks good to me, at least if MathTableRowFrames can live with a non MathML child.
Comment on attachment 240588 [details] [diff] [review] patch This could probably use a little comment explaining what we're doing....
Checked in with these comments added next to that code: // <mtable> is border separate in mathml.css and the MathML code doesn't implement // border collapse. For those users who style <mtable> with border collapse, // give them the default non-MathML table frames that understand border collpase. // This won't break us because MathML table frames are all subclasses of the default // table code, and so we can freely mix <mtable> with <mtr> or <tr>, <mtd> or <td>. // What will happen is just that non-MathML frames won't understand MathML attributes // and will therefore miss the special handling that the MathML code does.
rbs, is that patch acceptable for the branches?
Yes, it is pretty safe for the branch.
OK. If you feel ok landing it there, please request 126.96.36.199 approval?
Comment on attachment 240588 [details] [diff] [review] patch Safe patch for a crasher.
Comment on attachment 240588 [details] [diff] [review] patch a=mconnor on behalf of drivers for 188.8.131.52 and 184.108.40.206 checkin
v.fixed on 1.8.0 and 1.8.1 branches with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:220.127.116.11) Gecko/20061206 Firefox/18.104.22.168 and Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:22.214.171.124) Gecko/20061204 Firefox/126.96.36.199 No crash with testcase.
Crashtest checked in.