Bob Clary [:bc] (inactive) Reporter
	Description • 19 years ago

Found in a Firefox 1.5 build from 2005-09-05 while scanning abc.com

###!!! ASSERTION: bad argument: '(aRight > mLeft) && (aRight < mRight)', file
c:/work/mozilla/builds/ff/1.5/mozilla/layout/generic/nsSpaceManager.cpp, line 1350

nsSpaceManager::BandRect::SplitHorizontally(int 5955) line 1350
nsSpaceManager::AddRectToBand(nsSpaceManager::BandRect * 0x03490b50,
nsSpaceManager::BandRect * 0x03491228) line 651 + 15 bytes
nsSpaceManager::InsertBandRect(nsSpaceManager::BandRect * 0x03491228) line 779
nsSpaceManager::AddRectRegion(nsIFrame * 0x03488fe0, const nsRect & {...}) line 842
nsBlockReflowState::FlowAndPlaceFloat(nsFloatCache * 0x034910d0, int *
0x0012774c, unsigned int & 0, int 0) line 989 + 22 bytes
nsBlockReflowState::AddFloat(nsLineLayout & {...}, nsPlaceholderFrame *
0x03489034, int 0, unsigned int & 0) line 626 + 24 bytes
nsLineLayout::AddFloat(nsPlaceholderFrame * 0x03489034, unsigned int & 0) line 261
nsLineLayout::ReflowFrame(nsIFrame * 0x03489034, unsigned int & 0,
nsHTMLReflowMetrics * 0x00000000, int & 0) line 1016 + 22 bytes
nsBlockFrame::ReflowInlineFrame(nsBlockReflowState & {...}, nsLineLayout &
{...}, nsLineList_iterator {...}, nsIFrame * 0x03489034, unsigned char *
0x00127a0b) line 4000 + 22 bytes
nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState & {...}, nsLineLayout &
{...}, nsLineList_iterator {...}, int * 0x00127db4, unsigned char * 0x00127aff,
int 1, int 0) line 3839 + 32 bytes
nsBlockFrame::ReflowInlineFrames(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x00127db4, int 0, int 1) line 3712 + 46 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x00127db4, int 0) line 2681
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x03486994, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {...}, int 0,
nsCollapsingMargin & {...}, int 0, int 1, nsMargin & {...}, nsHTMLReflowState &
{...}, unsigned int & 0) line 588 + 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x00128a00) line 3427 + 66 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x00128a00, int 0) line 2588 + 27 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x03486834, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {...}, int 1,
nsCollapsingMargin & {...}, int 0, int 1, nsMargin & {...}, nsHTMLReflowState &
{...}, unsigned int & 0) line 588 + 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x0012964c) line 3427 + 66 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x0012964c, int 0) line 2588 + 27 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x0347c96c, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {...}, int 1,
nsCollapsingMargin & {...}, int 0, int 0, nsMargin & {...}, nsHTMLReflowState &
{...}, unsigned int & 0) line 588 + 42 bytes
nsBlockFrame::ReflowFloat(nsBlockReflowState & {...}, nsPlaceholderFrame *
0x0347c9c0, nsFloatCache * 0x0348da90, unsigned int & 0) line 5899 + 54 bytes
nsBlockReflowState::FlowAndPlaceFloat(nsFloatCache * 0x0348da90, int *
0x0012a110, unsigned int & 0, int 0) line 841
nsBlockReflowState::AddFloat(nsLineLayout & {...}, nsPlaceholderFrame *
0x0347c9c0, int 0, unsigned int & 0) line 626 + 24 bytes
nsLineLayout::AddFloat(nsPlaceholderFrame * 0x0347c9c0, unsigned int & 0) line 261
nsLineLayout::ReflowFrame(nsIFrame * 0x0347c9c0, unsigned int & 0,
nsHTMLReflowMetrics * 0x00000000, int & 0) line 1016 + 22 bytes
nsBlockFrame::ReflowInlineFrame(nsBlockReflowState & {...}, nsLineLayout &
{...}, nsLineList_iterator {...}, nsIFrame * 0x0347c9c0, unsigned char *
0x0012a3cf) line 4000 + 22 bytes
nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState & {...}, nsLineLayout &
{...}, nsLineList_iterator {...}, int * 0x0012a778, unsigned char * 0x0012a4c3,
int 1, int 1) line 3839 + 32 bytes
nsBlockFrame::ReflowInlineFrames(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x0012a778, int 1, int 1) line 3712 + 46 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x0012a778, int 1) line 2681
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x03428264, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {...}, int 1,
nsCollapsingMargin & {...}, int 0, int 1, nsMargin & {...}, nsHTMLReflowState &
{...}, unsigned int & 0) line 605 + 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x0012b3c4) line 3427 + 66 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x0012b3c4, int 1) line 2588 + 27 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x03428074, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x03428074, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 15, int 0,
unsigned int 0, unsigned int & 0) line 904 + 31 bytes
nsTableCellFrame::Reflow(nsTableCellFrame * const 0x03428000, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 858
nsContainerFrame::ReflowChild(nsIFrame * 0x03428000, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 105, int 0,
unsigned int 0, unsigned int & 0) line 904 + 31 bytes
nsTableRowFrame::IR_TargetIsChild(nsTableRowFrame * const 0x0346e268,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState
& {...}, nsTableFrame & {...}, unsigned int & 0, nsIFrame * 0x03428000) line
1226 + 45 bytes
nsTableRowFrame::IncrementalReflow(nsTableRowFrame * const 0x0346e268,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState
& {...}, nsTableFrame & {...}, unsigned int & 0) line 1111 + 46 bytes
nsTableRowFrame::Reflow(nsTableRowFrame * const 0x0346e268, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 1409 + 35 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x0346e268, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0,
unsigned int 0, unsigned int & 0) line 904 + 31 bytes
nsTableRowGroupFrame::IR_TargetIsChild(nsTableRowGroupFrame * const 0x033f7754,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, nsRowGroupReflowState &
{...}, unsigned int & 0, nsIFrame * 0x0346e268) line 1640 + 45 bytes
nsTableRowGroupFrame::IncrementalReflow(nsTableRowGroupFrame * const 0x033f7754,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, nsRowGroupReflowState &
{...}, unsigned int & 0) line 1324 + 42 bytes
nsTableRowGroupFrame::Reflow(nsTableRowGroupFrame * const 0x033f7754,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState
& {...}, unsigned int & 0) line 1230 + 31 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x033f7754, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0,
unsigned int 0, unsigned int & 0) line 904 + 31 bytes
nsTableFrame::IR_TargetIsChild(nsTableFrame * const 0x0346df34,
nsTableReflowState & {...}, unsigned int & 0, nsIFrame * 0x033f7754) line 2932 +
50 bytes
nsTableFrame::IncrementalReflow(nsTableFrame * const 0x0346df34, const
nsHTMLReflowState & {...}, unsigned int & 0) line 2669 + 34 bytes
nsTableFrame::Reflow(nsTableFrame * const 0x0346df34, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 1925 + 23 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x0346df34, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0,
unsigned int 3, unsigned int & 0) line 904 + 31 bytes
nsTableOuterFrame::OuterReflowChild(nsTableOuterFrame * const 0x0346ddd0,
nsPresContext * 0x0325c090, nsIFrame * 0x0346df34, const nsHTMLReflowState &
{...}, nsHTMLReflowMetrics & {...}, int 11400, nsSize & {...}, nsMargin & {...},
nsMargin & {...}, nsMargin & {...}, nsReflowReason eReflowReason_Incremental,
unsigned int & 0, int * 0x0012c6ac) line 1314 + 47 bytes
nsTableOuterFrame::IR_InnerTableReflow(nsTableOuterFrame * const 0x0346ddd0,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState
& {...}, unsigned int & 0) line 1662 + 81 bytes
nsTableOuterFrame::IR_TargetIsInnerTableFrame(nsTableOuterFrame * const
0x0346ddd0, nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, const
nsHTMLReflowState & {...}, unsigned int & 0) line 1426 + 31 bytes
nsTableOuterFrame::IR_TargetIsChild(nsTableOuterFrame * const 0x0346ddd0,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState
& {...}, unsigned int & 0, nsIFrame * 0x0346df34) line 1399 + 31 bytes
nsTableOuterFrame::IncrementalReflow(nsTableOuterFrame * const 0x0346ddd0,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState
& {...}, unsigned int & 0) line 1379 + 42 bytes
nsTableOuterFrame::Reflow(nsTableOuterFrame * const 0x0346ddd0, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 1925 + 31 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {...}, int 0,
nsCollapsingMargin & {...}, int 0, int 1, nsMargin & {...}, nsHTMLReflowState &
{...}, unsigned int & 0) line 605 + 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x0012d240) line 3427 + 66 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x0012d240, int 1) line 2588 + 27 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x0346db74, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {...}, int 0,
nsCollapsingMargin & {...}, int 0, int 1, nsMargin & {...}, nsHTMLReflowState &
{...}, unsigned int & 0) line 605 + 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x0012de8c) line 3427 + 66 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x0012de8c, int 1) line 2588 + 27 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x0346b584, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {...}, int 1,
nsCollapsingMargin & {...}, int 0, int 1, nsMargin & {...}, nsHTMLReflowState &
{...}, unsigned int & 0) line 605 + 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x0012ead8) line 3427 + 66 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x0012ead8, int 1) line 2588 + 27 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x0346b400, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x0346b400, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0,
unsigned int 0, unsigned int & 0) line 904 + 31 bytes
CanvasFrame::Reflow(CanvasFrame * const 0x033f717c, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 522
nsContainerFrame::ReflowChild(nsIFrame * 0x033f717c, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0,
unsigned int 3, unsigned int & 0) line 904 + 31 bytes
nsHTMLScrollFrame::ReflowScrolledFrame(const ScrollReflowState & {...}, int 0,
nsHTMLReflowMetrics * 0x0012f3b0, int 1) line 512 + 54 bytes
nsHTMLScrollFrame::ReflowContents(ScrollReflowState * 0x0012f558, const
nsHTMLReflowMetrics & {...}) line 567 + 25 bytes
nsHTMLScrollFrame::Reflow(nsHTMLScrollFrame * const 0x033f72c8, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 759 + 16 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x033f72c8, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0,
unsigned int 0, unsigned int & 0) line 904 + 31 bytes
ViewportFrame::Reflow(ViewportFrame * const 0x033f70e8, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 239 + 43 bytes
IncrementalReflow::Dispatch(nsPresContext * 0x0325c090, nsHTMLReflowMetrics &
{...}, const nsSize & {...}, nsIRenderingContext & {...}) line 912
PresShell::ProcessReflowCommands(int 1) line 6874
ReflowEvent::HandleEvent() line 6700
HandlePLEvent(ReflowEvent * 0x03476778) line 6717
PL_HandleEvent(PLEvent * 0x03476778) line 688 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00f39460) line 623 + 9 bytes
_md_TimerProc(HWND__ * 0x001504d0, unsigned int 275, unsigned int 0, unsigned
long 89559940) line 1013 + 9 bytes
USER32! 77d48734()
USER32! 77d49857()
USER32! 77d49791()
USER32! 77d48a10()
nsAppShell::Run(nsAppShell * const 0x00f51588) line 135
nsAppStartup::Run(nsAppStartup * const 0x00f514e8) line 145 + 26 bytes
XRE_main(int 3, char * * 0x003f6f60, const nsXREAppData * 0x0042201c kAppData)
line 2322 + 35 bytes
main(int 3, char * * 0x003f6f60) line 61 + 18 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 7c816d4f()

	Bob Clary [:bc] (inactive) Reporter
	Comment 1 • 19 years ago

(In reply to comment #0)
This stack is probably entirely bogus. I will try to get a good one later today.

	Bob Clary [:bc] (inactive) Reporter
	Comment 2 • 19 years ago

(In reply to comment #1)

The previous stack is actually good and reproducible in Trunk as well as 1.8
branch. In this case aRight == 0x2472, mLeft == 0x2472, mRight == 0x2b89

	Boris Zbarsky [:bzbarsky]
	Comment 3 • 19 years ago

Minimal testcase would help.  :(

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 4 • 19 years ago

Is there an actual observable layout bug in the page in question? This assertion was added pretty late in 1.5.

	Bob Clary [:bc] (inactive) Reporter
	Comment 5 • 19 years ago

(In reply to comment #4)

Not that I can see.

	Martijn Wargers (dead)
	Comment 6 • 19 years ago

This testcase still asserts in my debug build.

	Jesse Ruderman
	Comment 7 • 19 years ago

I see the assertion using Martijn's testcase.

	David Baron :dbaron:
	Comment 8 • 18 years ago

Should this assertion have an "or equal to"?

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 9 • 18 years ago

No. When equality holds, we're splitting a BandRect in a way that creates an empty BandRect, in which case we shouldn't be splitting at all.

	Alex Vincent [:WeirdAl]
	Comment 10 • 18 years ago

FWIW, nsSpaceManager::AddRectRegion (nsSpaceManager.cpp#833) has the first indication of a failure.  A breakpoint set for:

bandRect->mLeft == bandRect->mRight

immediately after bandRect is created triggers under the attached testcase.

CNN loves this assertion failure, by the way.

	Jesse Ruderman
	Comment 11 • 17 years ago

WFM with the URL and with Martijn's testcase.

	Robert Sayre
	Comment 12 • 17 years ago

I can trigger this assertion by visiting http://dailymotion.com, with the same situation as described in Comment #2.

XPCOM_MEM_LEAK_LOG also claims we're leaking BandRect objects on that page, but there are no remaining references.

	Martijn Wargers (dead)
	Comment 13 • 17 years ago

Fwiw, it is all wfm now, using a 5 day old debug build, testing the url, the url in comment 12 and the testcase.

	Robert Sayre
	Comment 14 • 17 years ago

This patch fixes the assertions and leaks for me, but perhaps we're supposed to insert zero-width rects that have a height.

I'll also update my build to check that I'm not missing a patch.

	Robert Sayre
	Comment 15 • 17 years ago

(In reply to comment #14)
> 
> I'll also update my build to check that I'm not missing a patch.

I can still reproduce with an up-to-date build.

	Eli Friedman
	Comment 16 • 17 years ago

See Bug 387201 and the regression caused by exactly this change, bug 391412.

	Eli Friedman
	Comment 17 • 17 years ago

I just looked at this a bit more; this change specifically breaks nsSpaceManager::YMost.  That looks to be fixable, though; I think all that is needed is to change it to do something more like ClearFloats (possibly just calling "ClearFloats(nscoord_MIN, NS_STYLE_CLEAR_BOTH)" would work, although I'm not sure what sort of perf impact that would have).

	Robert Sayre
	Comment 18 • 17 years ago

Comment on attachment 277773 [details] [diff] [review]
exit early for zero-width rects as well

Well, this patch is no good. It crashes on cnn.com.

	Eli Friedman
	Comment 19 • 17 years ago

Yeah, you want something like Attachment 273676 [details] [diff], except for the fact that it breaks nsSpaceManager::YMost, and therefore bug 391412.

	Bob Clary [:bc] (inactive) Reporter
	Comment 20 • 17 years ago

The asserts are not reproducible for me in winxp/linux with the url and testcase in this bug, but are still reproducible with the testcase in bug 387214 for both winxp/linux. Since this assertion was #3 in a recent top site test run with over 3000 occurrences, it is definitely not fixed. I suggest duping forward to 387214.

	Eli Friedman
	Comment 21 • 17 years ago

Here's the fix I outlined earlier.  I think it fixes the BandRect leak from http://wiki.mozilla.org/LeakingPages as well.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 23 • 17 years ago

+      autoHeight = PR_MAX(autoHeight,
+                          aReflowState.mSpaceManager->ClearFloats(nscoord_MIN, NS_STYLE_CLEAR_LEFT_AND_RIGHT));

I think this can result in two calls to ClearFloats.

	Eli Friedman
	Comment 24 • 17 years ago

Yes, you're right... this is why PR_MAX sucks.  I'll post a fixed version.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 26 • 17 years ago

Comment on attachment 279827 [details] [diff] [review]
Patch v2

Approving because this fixes common assertions and also because it might fix a leak

	Reed Loden [:reed]
	Comment 27 • 17 years ago

Checked-in for elif since he's awol.

Checking in layout/generic/nsBlockFrame.cpp;
/cvsroot/mozilla/layout/generic/nsBlockFrame.cpp,v  <--  nsBlockFrame.cpp
new revision: 3.872; previous revision: 3.871
done
Checking in layout/generic/nsSpaceManager.cpp;
/cvsroot/mozilla/layout/generic/nsSpaceManager.cpp,v  <--  nsSpaceManager.cpp
new revision: 3.86; previous revision: 3.85
done

	Reed Loden [:reed]
	Comment 28 • 17 years ago

Backed out due to this patch causing a reftest failure for the test in bug 218473.

REFTEST UNEXPECTED FAIL: file:///builds/slave/trunk_osx/mozilla/layout/reftests/bugs/218473-1.html

	Reed Loden [:reed]
	Comment 29 • 17 years ago

Reassigning to roc (as per his request) and re-requesting blocking1.9? due to the leaks this patch might fix.

	Mats Palmgren (inactive)
	Comment 30 • 17 years ago

FWIW, I don't see the assertion for the link in the URL field (on Linux).
I do see it loading this URL though:
http://www.alistapart.com/d/multicolumnlayouts/3ColLiquidWithContent.html
The "Patch v2" does NOT fix it.

	Damon Sicore (:damons)
	Comment 31 • 17 years ago

Comment on attachment 279827 [details] [diff] [review]
Patch v2

Resetting approval flags on bugs that have not been checked in by Oct 22, 11:59PM PDT.  Please re-request approval if needed.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 32 • 17 years ago

Simple problem ... mSpaceManager->ClearFloats was returning a value relative to the block's top content edge, but autoHeight is relative to the block's top border edge, so the padding wasn't being included in the computed height in the 218473-1.html testcase.

The fix is easy, just go through nsBlockReflowState::ClearFloats, which adds the padding-top in for us. We do need to pass autoHeight instead of nscoord_MIN as the default result, but that's OK, it won't affect anything either way.

I'l see if I can get Eli to review this change.

	Jeremy Baron
	Comment 33 • 17 years ago

I get this assert @ the same function when visiting https://bugs.launchpad.net/ubuntu/+source/mozilla-thunderbird/+bug/128288 in my own debug build from 2007110101.  I don't assert at all in the testcase with that build.  Jesse doesn't assert either on that testcase. (he's also using a recent trunk debug)

	Boris Zbarsky [:bzbarsky]
	Comment 34 • 17 years ago

Please attach something (not necessarily minimal) to this bug that reproduces that issue.  Relying on an external URI to not go away is a recipe for disaster.

	Jesse Ruderman
	Comment 35 • 17 years ago

Reduced from layout/reftests/bugs/387201-3.html.

	Eli Friedman
	Comment 36 • 17 years ago

Comment on attachment 286086 [details] [diff] [review]
patch v3

This looks right.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 37 • 17 years ago

Thanks Eli.

	Reed Loden [:reed]
	Comment 38 • 17 years ago

Checking in layout/generic/nsBlockFrame.cpp;
/cvsroot/mozilla/layout/generic/nsBlockFrame.cpp,v  <--  nsBlockFrame.cpp
new revision: 3.884; previous revision: 3.883
done
Checking in layout/generic/nsSpaceManager.cpp;
/cvsroot/mozilla/layout/generic/nsSpaceManager.cpp,v  <--  nsSpaceManager.cpp
new revision: 3.89; previous revision: 3.88
done

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 39 • 17 years ago

Need to make a crashtest assertion test here.

	Carsten Book [:Tomcat]
	Comment 40 • 17 years ago

verified fixed using debug build Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007120520 Minefield/3.0b2pre and the testcases from this bug 

no assertions on testcases -> Verified