Closed
Bug 370872
Opened 18 years ago
Closed 17 years ago
29826174px wide page with colspan and marquee
Categories
(Core :: Layout: Tables, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 363858
People
(Reporter: jruderman, Unassigned)
References
Details
(Keywords: assertion, testcase)
Attachments
(3 files)
Steps to reproduce:
1. Load the testcase.
Result: The page's width is 29826174px (about 2^32/144) and a bunch of width-related assertion failures occur.
Expected: There should be no horizontal scrollbar, and no assertion failures.
Comment 1•18 years ago
|
||
Works in 2006-12-07, fails in 2006-12-08, so probably a regression from bug 300030.
Blocks: reflow-refactor
Updated•18 years ago
|
Flags: blocking1.9?
Comment 2•18 years ago
|
||
So I think the basic problem is that there are three fundamental intrinsic widths rather than two. There are really two variants of pref width, one (the wider one) that is allowed to become infinite, and the other (the narrower one) that does not include expansion based on reverse computation of percentages. Shrink-wrapping uses the wider one, but table cell containment and XUL pref width use the narrower one. That said, there are advantages to using the wider one whenever possible since it means that things with percentage widths will fit.
Right now the only place where we distinguish between the two is BasicTableLayoutStrategy, so that we can use the wider one for shrink-wrapping of tables. But we might need to do the same thing for other places where we do percent-expansion.
If we need to do that, I think I want to fix what I sort of consider a design mistake with the reflow branch, and combine GetMinWidth and GetPrefWidth into a single GetIntrinsicWidth function that takes a parameter for which intrinsic width to get. It shouldn't be too hard to combine the existing implementations -- most places that override one override both. It would also make switching into and out of nsLayoutUtils::IntrinsicForContainer simpler. Then we can add a third enum value and make the necessary distinctions based on that enum value. I suppose this means we should start caching all three values in nsBlockFrame as well, though.
That said, this analysis of the bug might be entirely wrong. I'm not sure why a colspan is necessary. I'd have expected marquees (with their 100% margins internally) to expand tables all the time if the do so at all. So I'm clearly missing something.
Comment 3•18 years ago
|
||
This is also giving a very wide page. This also regressed during the reflow branch landing. I minimised it from a nested marquee testcase.
Comment 4•18 years ago
|
||
Related to bug 370866?
Updated•18 years ago
|
Flags: blocking1.9? → blocking1.9+
Updated•18 years ago
|
OS: Mac OS X → All
Comment 5•17 years ago
|
||
The initial testcase is fixed (no assertions, no longer really really wide) by my patch for bug 367673 (which isn't yet checked in -- awaiting review). That patch basically prevents certain nscoord operations from overflowing nscoord_MAX.
Having said that, the Trunk rendering using that patch is still different from Branch rendering -- patched Trunk makes the table take up the whole width of the window, whereas Branch makes the table very skinny. So there might still be something else broken; I'm not sure.
Depends on: 367673
Comment 6•17 years ago
|
||
I did some analysis on the second testcase, and it looks like we're getting a width of nscoord_MAX via this chain of calls:
#0 AddPercents (aType=nsLayoutUtils::PREF_WIDTH, aCurrent=2820, aPercent=1)
nsLayoutUtils.cpp:1249
#1 nsLayoutUtils::IntrinsicForContainer (aRenderingContext=0x8c228c0, aFrame=0x8ac4ab0, aType=nsLayoutUtils::PREF_WIDTH)
nsLayoutUtils.cpp:1548
#2 nsBlockFrame::GetPrefWidth (this=0x8ac4904, aRenderingContext=0x8c228c0)
nsBlockFrame.cpp:729
#3 nsFrame::RefreshSizeCache (this=0x8ac4904, aState=@0xbfab6d44)
nsFrame.cpp:5712
#4 nsFrame::GetMinSize (this=0x8ac4904, aState=@0xbfab6d44)
nsFrame.cpp:5829
#5 nsSprocketLayout::GetMinSize (this=0x84c3510, aBox=0x8ac47bc, aState=@0xbfab6d44, aSize=@0xbfab6d64)
nsSprocketLayout.cpp:1393
#6 nsBoxFrame::GetMinSize (this=0x8ac47bc, aBoxLayoutState=@0xbfab6d44)
nsBoxFrame.cpp:862
#7 nsBoxFrame::GetMinWidth (this=0x8ac47bc, aRenderingContext=0x8c228c0)
nsBoxFrame.cpp:630
...
#20 nsBlockFrame::Reflow (this=0x8ac462c, aPresContext=0x8915480, aMetrics=@0xbfab7de8, aReflowState=@0xbfab7d14, aStatus=@0xbfab7efc)
nsBlockFrame.cpp:921
Stack level #20 is a reflow of the body block.
The nsBoxFrame "this" at levels #7 and #6 is the outer -moz-box div. The nsBlockFrame "this" at #4, #3, and #2 is the intermediate div between the two -moz-box divs. The aFrame at #1 is the inner -moz-box div.
At level #0, nsLayoutUtils.cpp:1249, we hit this clause:
1248 if (aPercent >= 1.0f)
1249 result = nscoord_MAX;
This gets returned up through stack level #2, setting the intermediate div's "mPrefWidth" to nscoord_MAX.
We then pass the nscoord_MAX up to level #3, setting metrics->mBlockPrefSize.width to nscoord_MAX.
Then we call BoxReflow at nsFrame.cpp:5717, passing in nscoord_MAX as "aWidth", This is incorporated into parentSize.width which is used to initialize parentReflowState at nsFrame.cpp:6032, and becomes "availableWidth" in that reflowState.
Finally, when parentReflowState calls Init() in its constructor, we detect that availableWidth == NS_UNCONSTRAINEDSIZE, and the first assertion fails:
ASSERTION: shouldn't use unconstrained widths anymore: 'availableWidth != NS_UNCONSTRAINEDSIZE', file /scratch/work/builds/trunk.07-09-07.12-28/mozilla/layout/generic/nsHTMLReflowState.cpp, line 302
Comment 7•17 years ago
|
||
This third testcase has the same sorts of assertions as testcase 2 (unconstrained withs in nsHTMLReflowState), and it has a similar root cause: IntrinsicForContainer calling AddPercents() with aPercent >= 1, and getting nscoord_MAX returned.
Here's the backtrace for where nscoord_MAX width gets introduced on testcase3:
#0 AddPercents (aType=nsLayoutUtils::PREF_WIDTH, aCurrent=480, aPercent=2)
at nsLayoutUtils.cpp:1249
#1 nsLayoutUtils::IntrinsicForContainer (aRenderingContext=0x8c808b0, aFrame=0x9067f28, aType=nsLayoutUtils::PREF_WIDTH)
at nsLayoutUtils.cpp:1548
#2 nsBlockFrame::GetPrefWidth (this=0x90658a4, aRenderingContext=0x8c808b0)
at nsBlockFrame.cpp:729
#3 nsHTMLScrollFrame::GetPrefWidth (this=0x90657e0, aRenderingContext=0x8c808b0)
at nsGfxScrollFrame.cpp:674
#4 nsComboboxControlFrame::GetPrefWidth (this=0x906562c, aRenderingContext=0x8c808b0)
at nsComboboxControlFrame.cpp:598
#5 nsComboboxControlFrame::GetMinWidth (this=0x906562c, aRenderingContext=0x8c808b0)
at nsComboboxControlFrame.cpp:571
#6 nsFrame::ShrinkWidthToFit (this=0x906562c, aRenderingContext=0x8c808b0, aWidthInCB=24180)
at nsFrame.cpp:3088
#7 nsContainerFrame::ComputeAutoSize (this=0x906562c, aRenderingContext=0x8c808b0, aCBSize=@0xbfc10780, aAvailableWidth=25680, aMargin=@0xbfc10778, aBorder=@0xbfc10770, aPadding=@0xbfc10768, aShrinkWrap=1)
at nsContainerFrame.cpp:673
#8 nsFrame::ComputeSize (this=0x906562c, aRenderingContext=0x8c808b0, aCBSize=@0xbfc10824, aAvailableWidth=25680, aMargin=@0xbfc1081c, aBorder=@0xbfc10814, aPadding=@0xbfc1080c, aShrinkWrap=1)
at nsFrame.cpp:2966
#9 nsHTMLReflowState::InitConstraints (this=0xbfc108f8, aPresContext=0x8e386f0, aContainingBlockWidth=25680, aContainingBlockHeight=1073741824, aBorder=0x0, aPadding=0x0)
at nsHTMLReflowState.cpp:1820
#10 nsHTMLReflowState::Init (this=0xbfc108f8, aPresContext=0x8e386f0, aContainingBlockWidth=-1, aContainingBlockHeight=-1, aBorder=0x0, aPadding=0x0)
at nsHTMLReflowState.cpp:315
#11 nsHTMLReflowState (this=0xbfc108f8, aPresContext=0x8e386f0, aParentReflowState=@0xbfc11664, aFrame=0x906562c, aAvailableSpace=@0xbfc109e4, aContainingBlockWidth=-1, aContainingBlockHeight=-1, aInit=1)
at nsHTMLReflowState.cpp:180
#12 nsLineLayout::ReflowFrame (this=0xbfc10bfc, aFrame=0x906562c, aReflowStatus=@0xbfc10af4, aMetrics=0x0, aPushedFrame=@0xbfc10af0)
at nsLineLayout.cpp:812
...
(Note: This testcase is reduced from bug 393656, but I thought I'd post it here since it's more related to this bug's assertions + issues.)
Comment 8•17 years ago
|
||
Just checked in the fix for bug 367673, which fixes first testcase, but not the second and third.
Comment 9•17 years ago
|
||
See also bug 363858.
Comment 10•17 years ago
|
||
Resolving as dupe of bug 363858, because testcases 2 and 3 are based on that bug's issue (padding causing exponential growth). They aren't related to this bug's title ("colspan and marquee") or original problem.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Updated•17 years ago
|
Flags: blocking1.9+
Updated•17 years ago
|
Flags: blocking1.9+
You need to log in
before you can comment on or make changes to this bug.
Description
•