[FIX]"ASSERTION: Non-row-group primary frame list child of an nsTableFrame?" with <xul:menubar>

RESOLVED FIXED in mozilla1.9alpha8

Status

()

defect
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: jruderman, Assigned: bzbarsky)

Tracking

(Blocks 2 bugs, 4 keywords)

Trunk
mozilla1.9alpha8
x86
macOS
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.8.1.x +
wanted1.8.0.x +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Posted file testcase
###!!! 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".
Boris this is from your review comment https://bugzilla.mozilla.org/show_bug.cgi?id=311661#c16

Do you still think we need this?
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.
Flags: blocking1.9?
Flags: blocking1.8.1.5?
Flags: blocking1.8.0.13?
Boris, should we fix this or must we fix it? We are trying to triage this bug as blocking/non-blocking.
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.

What bernd said.  This can definitely be used to cause a crash, and probably an exploitable one (esp. on trunk).
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.
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: blocking1.8.1.5?
Flags: blocking1.8.0.13?
Posted patch Proposed fixSplinter Review
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #270352 - Flags: superreview?(roc)
Attachment #270352 - Flags: review?
Attachment #270352 - Flags: review? → review?(bernd_mozilla)
Summary: "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?" with <xul:menubar>
Target Milestone: --- → mozilla1.9beta1
Attachment #270352 - Flags: review?(bernd_mozilla) → review+
Comment on attachment 270352 [details] [diff] [review]
Proposed fix

Remove the comment
// keep this in sync  with IsSpecialContent
in ConstructXULFrame too.
Attachment #270352 - Flags: superreview?(roc) → superreview+
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
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.
Attachment #270760 - Flags: approval1.8.1.5?
Attachment #270760 - Flags: approval1.8.0.13?
Comment on attachment 270760 [details] [diff] [review]
patch merged to comments

approved for 1.8.1.5 and 1.8.0.13, a=dveditz
Attachment #270760 - Flags: approval1.8.1.5?
Attachment #270760 - Flags: approval1.8.1.5+
Attachment #270760 - Flags: approval1.8.0.13?
Attachment #270760 - Flags: approval1.8.0.13+
Checked in on both branches.
Er, backed out of the 1.8 branch.  It's closed.  :(
Keywords: fixed1.8.1.5
Relanded.
Keywords: fixed1.8.1.5
Flags: blocking1.9?
Depends on: 368573
Flags: in-testsuite?
Crashtest checked in.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.