Bugzilla@Mozilla – Bug 413048
Crash [@ nsFrameList::SortByContentOrder] with -moz-column, float
Last modified: 2008-09-23 21:31:47 PDT
Summon comment box
Created attachment 297903 [details] testcase (crashes Firefox when loaded) 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.
Created attachment 297904 [details] stack trace
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.
Created attachment 330909 [details] [review] fix 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.
>-== 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.
Created attachment 330973 [details] [review] fix v2 Updated with that
Comment on attachment 330973 [details] [review] fix v2 r+sr=dbaron
Pushed 5d09fb3015d1.
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] [review] fix v2 This should fix a number of bugs on branch.
Comment on attachment 330973 [details] [review] fix v2 Approved for 1.9.0.2. Please land in CVS. a=ss
Checked into 1.9.0.
doesnt look like 1.8.0 is affected. if i am mistaken, please flip wanted or blocking flag.