"ASSERTION: Disagreement about whether it's a block or not" with table-as-root, float, absolute

RESOLVED FIXED

Status

()

Core
Layout
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: Jesse Ruderman, Assigned: bz)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
x86
Mac OS X
assertion, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

11 years ago
Created attachment 266321 [details]
testcase

The testcase triggers two assertions on Mac trunk:

###!!! ASSERTION: wrong kind of child frame: 'aIsBlock == f->GetStyleDisplay()->IsBlockLevel()', file /Users/jruderman/trunk/mozilla/layout/generic/nsLineBox.cpp, line 71

###!!! ASSERTION: Disagreement about whether it's a block or not: 'fromLine->IsBlock() == fromLine->mFirstChild->GetStyleDisplay()->IsBlockLevel()', file /Users/jruderman/trunk/mozilla/layout/generic/nsBlockFrame.cpp, line 2278

Also, the red "absolute" block has mysterious space on the right side, as if it's trying to make room for the "float" block.
I'm not sure those assertions are valid, given the code in nsLineLayout::TreatFrameAsBlock that claims that positioned/floated stuff is not a block...  Because for a table root we don't have the right containing blocks around, frames that have positioned or floated style can end up in-flow.

Now I don't know _why_ those clauses are there in TreatFrameAsBlock.  dbaron, roc, any idea?
Presumably the intent was that nsBlockFrame::AddFrames doesn't put placeholder frames on a block line... but that's not necessary since placeholders shouldn't have these styles. This code dates back to the dawn of time so we can probably just remove it.
Created attachment 267931 [details] [diff] [review]
Sounds good
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #267931 - Flags: superreview?(dbaron)
Attachment #267931 - Flags: review?(dbaron)
It looks like most of this landed as part of the patch for bug 383551.  I'd still like to land the assert, I think.
Created attachment 270366 [details] [diff] [review]
Just the assert
Attachment #267931 - Attachment is obsolete: true
Attachment #270366 - Flags: review?(dbaron)
Attachment #267931 - Flags: superreview?(dbaron)
Attachment #267931 - Flags: review?(dbaron)
Comment on attachment 270366 [details] [diff] [review]
Just the assert

r+sr=dbaron.  Sorry for the delay.
Attachment #270366 - Flags: superreview+
Attachment #270366 - Flags: review?(dbaron)
Attachment #270366 - Flags: review+
Checked in the assertion.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Reporter)

Comment 8

10 years ago
Testcase checked in as a crashtest.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.