Closed Bug 307242 Opened 19 years ago Closed 17 years ago

ASSERTION: bad argument: '(aRight > mLeft) && (aRight < mRight)', @ nsSpaceManager::BandRect::SplitHorizontally

Categories

(Core :: Layout: Floats, defect, P5)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9beta2

People

(Reporter: bc, Assigned: roc)

References

()

Details

(Keywords: assertion, memory-leak, testcase, Whiteboard: [dbaron-1.9:Rm])

Attachments

(3 files, 3 obsolete files)

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()
(In reply to comment #0) This stack is probably entirely bogus. I will try to get a good one later today.
Summary: ASSERTION: bad argument: '(aRight > mLeft) && (aRight < mRight)', @ nsSpaceManager::BandRect::SplitHorizontally → ASSERTION: bad argument: '(aRight > mLeft) && (aRight < mRight)', @ nsSpaceManager::BandRect::SplitHorizontally
(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
Version: 1.8 Branch → Trunk
Minimal testcase would help. :(
Keywords: qawanted
Is there an actual observable layout bug in the page in question? This assertion was added pretty late in 1.5.
(In reply to comment #4) Not that I can see.
Attached file testcase
This testcase still asserts in my debug build.
Keywords: qawantedtestcase
Keywords: assertion
I see the assertion using Martijn's testcase.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.9a2?
Should this assertion have an "or equal to"?
Flags: blocking1.9a2? → blocking1.9-
Whiteboard: [wanted-1.9]
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.
Component: Layout → Layout: Floats
QA Contact: layout → layout.floats
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.
Depends on: 191448
WFM with the URL and with Martijn's testcase.
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.
Fwiw, it is all wfm now, using a 5 day old debug build, testing the url, the url in comment 12 and the testcase.
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.
Attachment #277773 - Flags: review?(roc)
(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.
See Bug 387201 and the regression caused by exactly this change, bug 391412.
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).
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.
Attachment #277773 - Flags: review?(roc)
Yeah, you want something like Attachment 273676 [details] [diff], except for the fact that it breaks nsSpaceManager::YMost, and therefore bug 391412.
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.
Attached patch Patch (obsolete) — Splinter Review
Here's the fix I outlined earlier. I think it fixes the BandRect leak from http://wiki.mozilla.org/LeakingPages as well.
Assignee: nobody → sharparrow1
Status: NEW → ASSIGNED
Attachment #279786 - Flags: review?(roc)
+ 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.
Yes, you're right... this is why PR_MAX sucks. I'll post a fixed version.
Attached patch Patch v2 (obsolete) — Splinter Review
Attachment #279786 - Attachment is obsolete: true
Attachment #279827 - Flags: review?(roc)
Attachment #279786 - Flags: review?(roc)
Comment on attachment 279827 [details] [diff] [review] Patch v2 Approving because this fixes common assertions and also because it might fix a leak
Attachment #279827 - Flags: superreview+
Attachment #279827 - Flags: review?(roc)
Attachment #279827 - Flags: review+
Attachment #279827 - Flags: approval1.9?
Attachment #279827 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M9
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla1.9 M9 → ---
Reassigning to roc (as per his request) and re-requesting blocking1.9? due to the leaks this patch might fix.
Assignee: sharparrow1 → roc
Status: REOPENED → NEW
Flags: blocking1.9- → blocking1.9?
Keywords: mlk
Flags: blocking1.9? → blocking1.9+
Whiteboard: [wanted-1.9] → [wanted-1.9][dbaron-1.9:Rm]
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.
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.
Attachment #279827 - Flags: approval1.9+
Attached patch patch v3Splinter Review
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.
Attachment #277773 - Attachment is obsolete: true
Attachment #279827 - Attachment is obsolete: true
Attachment #286086 - Flags: superreview+
Attachment #286086 - Flags: review?(sharparrow1)
Whiteboard: [wanted-1.9][dbaron-1.9:Rm] → [needs review][dbaron-1.9:Rm]
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)
OS: Windows XP → All
Hardware: PC → All
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.
Attached file testcase 2
Reduced from layout/reftests/bugs/387201-3.html.
Comment on attachment 286086 [details] [diff] [review] patch v3 This looks right.
Attachment #286086 - Flags: review?(sharparrow1) → review+
Thanks Eli.
Whiteboard: [needs review][dbaron-1.9:Rm] → [needs landing][dbaron-1.9:Rm]
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
Status: NEW → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing][dbaron-1.9:Rm] → [dbaron-1.9:Rm]
Target Milestone: --- → mozilla1.9 M10
Need to make a crashtest assertion test here.
Flags: in-testsuite?
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: