Last Comment Bug 385880 - [FIX]"ASSERTION: Non-row-group primary frame list child of an nsTableFrame?" with <xul:menubar>
: [FIX]"ASSERTION: Non-row-group primary frame list child of an nsTableFrame?" ...
: assertion, fixed1.8.0.13, fixed1.8.1.5, testcase
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla1.9alpha8
Assigned To: Boris Zbarsky [:bz]
Depends on: 368573
Blocks: randomclasses 344486 367706
  Show dependency treegraph
Reported: 2007-06-25 23:49 PDT by Jesse Ruderman
Modified: 2007-12-16 22:09 PST (History)
5 users (show)
dveditz: wanted1.8.1.x+
dveditz: wanted1.8.0.x+
jruderman: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (206 bytes, application/xhtml+xml)
2007-06-25 23:49 PDT, Jesse Ruderman
no flags Details
Proposed fix (1.16 KB, patch)
2007-06-29 11:55 PDT, Boris Zbarsky [:bz]
bernd_mozilla: review+
roc: superreview+
Details | Diff | Splinter Review
patch merged to comments (1.19 KB, patch)
2007-07-03 11:49 PDT, Boris Zbarsky [:bz]
dveditz: approval1.8.1.5+
dveditz: approval1.8.0.13+
Details | Diff | Splinter Review

Description Jesse Ruderman 2007-06-25 23:49:20 PDT
Created attachment 269809 [details]

###!!! ASSERTION: Non-row-group primary frame list child of an nsTableFrame?  How come?: 'Not Reached', file nsTableFrame.cpp, line 2723

This assertion was added in bug 367706, "OrderRowGroups should be typesafe".
Comment 2 Bernd 2007-06-26 12:35:52 PDT
Boris this is from your review comment

Do you still think we need this?
Comment 3 Boris Zbarsky [:bz] 2007-06-26 18:12:18 PDT
Er... on non-OSX, menubar is special.  On OSX, sometimes it creates no frame... and the rest of the time it's special.  So the code is definitely wrong as-is.  It's probably fine to always treat it as special.  The other option is for the code here should do the same checks as the XUL frame construction code does, for OSX.
Comment 4 juan becerra [:juanb] 2007-06-27 10:49:39 PDT
Boris, should we fix this or must we fix it? We are trying to triage this bug as blocking/non-blocking.
Comment 5 Bernd 2007-06-27 11:48:18 PDT
From my experience I would say every violation of IsSpecialContent leads to a crash, this assumes people like Jesse who are really determined to find them. The crashes might not be exploitable but they are crashes.

Comment 6 Boris Zbarsky [:bz] 2007-06-27 15:55:53 PDT
What bernd said.  This can definitely be used to cause a crash, and probably an exploitable one (esp. on trunk).
Comment 7 Daniel Veditz [:dveditz] 2007-06-28 11:13:18 PDT
Sounds like we'd like a branch fix, but given the lack of trunk progress (no assignee, for one) it's probably not realistic to actually "block" on it.
Comment 8 Boris Zbarsky [:bz] 2007-06-29 11:55:19 PDT
Created attachment 270352 [details] [diff] [review]
Proposed fix
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-01 16:44:17 PDT
Comment on attachment 270352 [details] [diff] [review]
Proposed fix

Remove the comment
// keep this in sync  with IsSpecialContent
in ConstructXULFrame too.
Comment 10 Boris Zbarsky [:bz] 2007-07-03 11:49:19 PDT
Created attachment 270760 [details] [diff] [review]
patch merged to comments
Comment 11 Boris Zbarsky [:bz] 2007-07-03 11:49:44 PDT
Checked in.
Comment 12 Boris Zbarsky [:bz] 2007-07-03 11:50:45 PDT
Comment on attachment 270760 [details] [diff] [review]
patch merged to comments

I think we should get this in on branches too.  It's a simple and safe fix for a broken frame tree that could lead to nasty crashes.
Comment 13 Daniel Veditz [:dveditz] 2007-07-06 15:21:26 PDT
Comment on attachment 270760 [details] [diff] [review]
patch merged to comments

approved for and, a=dveditz
Comment 14 Boris Zbarsky [:bz] 2007-07-06 17:56:59 PDT
Checked in on both branches.
Comment 15 Boris Zbarsky [:bz] 2007-07-06 18:01:57 PDT
Er, backed out of the 1.8 branch.  It's closed.  :(
Comment 16 Boris Zbarsky [:bz] 2007-07-06 18:20:50 PDT
Comment 17 Jesse Ruderman 2007-12-16 22:09:21 PST
Crashtest checked in.

Note You need to log in before you can comment on or make changes to this bug.