Closed Bug 413048 Opened 15 years ago Closed 15 years ago

Crash [@ nsFrameList::SortByContentOrder] with -moz-column, float


(Core :: Layout, defect, P3)






(Reporter: jruderman, Assigned: roc)



(4 keywords, Whiteboard: [sg:critical?])

Crash Data


(3 files, 1 obsolete file)

Loading this testcase in a Mac trunk debug build triggers:

###!!! ASSERTION: unexpected flow: 'mFrames.ContainsFrame(nextInFlow)', file /Users/jruderman/trunk/mozilla/layout/generic/nsInlineFrame.cpp, line 470

###!!! ASSERTION: StealFrame failure: 'NS_SUCCEEDED(rv)', file /Users/jruderman/trunk/mozilla/layout/generic/nsContainerFrame.cpp, line 1092

Crash in nsIFrame::GetNextSibling, called by nsFrameList::SortByContentOrder, dereferencing 0xddddddfd.
Flags: blocking1.9?
Priority: -- → P1
Whiteboard: [sg:critical?]
Attached file stack trace
Flags: blocking1.9? → blocking1.9+
Priority: P1 → P3
Flags: wanted1.9.0.x+
Flags: blocking1.9-
Flags: tracking1.9+
This is very nasty. Basically, inline frames have no protection against pulling a placeholder continuation into the same container as its prev-in-flow. Blocks have that protection in nsBlockFrame::HandleOverflowPlaceholdersForPulledFrame ... but I just now realized that that's broken for the case where aFrame is a span; we descend into the span and traverse its children, but later we assume that the frames we're looking at have a block frame as parent...

I wonder if it would be easier to just remove float continuations as we're planning to do.
I think we should just disable float breaking in columns. It's too fragile to fix up with any reasonable amount of effort. We should reform layout (at least by making placeholders not have continuation frames) and then revisit the issue.
Attached patch fix (obsolete) — Splinter Review
This disables float breaking in columns by reflowing them with unrestricted height.

The change to nsBlockReflowState::AddFloat is the scary bit. We need this change because we don't want to mark the placeholder for a below-current-line float as incomplete, that triggers this crash. As far as I can tell, this code is only needed to stop the teardown of next-in-flows, so we only need the GetNextInFlow check --- which, conveniently, will now never be true for floats in columns.

The test 386147-1.html depends on floats breaking across columns so I just disabled it.
Assignee: nobody → roc
Attachment #330909 - Flags: superreview?(dbaron)
Attachment #330909 - Flags: review?(dbaron)
>-== 386147-1.html 386147-1-ref.html
>+fails == 386147-1.html 386147-1-ref.html # bug 413048

Please reference a new bug instead of this one.  I like being able to grep reftest.list for bug references and make sure they are all still open bugs.
Sounds reasonable. I filed bug 447660 for that.
Attached patch fix v2Splinter Review
Updated with that
Attachment #330909 - Attachment is obsolete: true
Attachment #330973 - Flags: superreview?(dbaron)
Attachment #330973 - Flags: review?(dbaron)
Attachment #330909 - Flags: superreview?(dbaron)
Attachment #330909 - Flags: review?(dbaron)
Blocks: 426040
Blocks: 425981
Comment on attachment 330973 [details] [diff] [review]
fix v2

Attachment #330973 - Flags: superreview?(dbaron)
Attachment #330973 - Flags: superreview+
Attachment #330973 - Flags: review?(dbaron)
Attachment #330973 - Flags: review+
Pushed 5d09fb3015d1.
Closed: 15 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/20080816113032 Minefield/3.1a2pre
Comment on attachment 330973 [details] [diff] [review]
fix v2

This should fix a number of bugs on branch.
Attachment #330973 - Flags: approval1.9.0.2?
Comment on attachment 330973 [details] [diff] [review]
fix v2

Approved for Please land in CVS. a=ss
Attachment #330973 - Flags: approval1.9.0.2? → approval1.9.0.2+
doesnt look like 1.8.0 is affected. if i am mistaken, please flip wanted or blocking flag.
Flags: wanted1.8.0.x-
Group: core-security
Crash Signature: [@ nsFrameList::SortByContentOrder]
You need to log in before you can comment on or make changes to this bug.