[FIX]ASSERTION: unconstrained widths no longer supported: 'aContainingBlockWidth != NS_UNCONSTRAINEDSIZE' [@ nsLayoutUtils::ComputeWidthValue]

VERIFIED FIXED in mozilla1.9.2a1

Status

()

Core
Layout: Tables
VERIFIED FIXED
11 years ago
7 years ago

People

(Reporter: mats, Assigned: bz)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla1.9.2a1
assertion, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

11 years ago
Created attachment 256326 [details]
Testcase #1

STEPS TO REPRODUCE
1. load testcase

ACTUAL RESULT
###!!! ASSERTION: unconstrained widths no longer supported: 'aContainingBlockWidth != NS_UNCONSTRAINEDSIZE', file nsLayoutUtils.cpp, line 1427

PLATFORMS AND BUILDS TESTED
Bug occurs in Firefox trunk debug build on Linux
OS: Linux → All
Created attachment 361911 [details]
Stack on OS X
Summary: ###!!! ASSERTION: unconstrained widths no longer supported: 'aContainingBlockWidth != NS_UNCONSTRAINEDSIZE', file nsLayoutUtils.cpp, line 1427 → ASSERTION: unconstrained widths no longer supported: 'aContainingBlockWidth != NS_UNCONSTRAINEDSIZE' [@ nsLayoutUtils::ComputeWidthValue]
Hmm.  So outer table frames return false for IsContainingBlock().  So the mCBReflowState for the inner table ends up being the positioned inline.  But the out of flow bit is set on the _outer_ table, so the reflow state frame type for the inner is NOT absolutely positioned.

So ComputeContainingBlockRectangle ends up using the computed width of the containing block reflow state, which is unconstrained (since it's a reflow state for an inline frame).

David, is the fact that a table outer frame returns false from IsContainingBlock() correct?  It seems a bit odd to me, but maybe we needed it back when percentage width on the inner table needed to "not see" it somehow?
It sounds like the positioned inline is in fact the correct containing block to use, but its reflow state doesn't have a useful width in it.
The positioned inline is certainly the correct containing block for the outer frame.  Not sure about the inner frame.  In any case, there's something else going on weirdly here; trying to sort out what.
I'm pretty sure we don't want the outer frame being a containing block.
So one option is to just add a check to ComputeContainingBlockRectangle to look for a table inner and if so decide what to do based on the mFrameType of the parent reflow state.

If we really want mCBReflowState to be the inline, that's the way to go, since if we make the inner table reflow state's frame type TYPE_ABSOLUTE we'll end up with the parent (the outer frame) as the mCBReflowState.
Why is this specific to tables?  Doesn't it happen for other absolutely positioned blocks inside relatively positioned inlines?
No, because those blocks have the NS_FRAME_OUT_OF_FLOW bit set, hence get mFrameType set to TYPE_ABSOLUTE in the reflow state, and if that's set we use the mCBReflowState->frame->GetRect() for the containing block if the mCBReflowState->frame is inline.

The specific thing with tables is that the NS_FRAME_OUT_OF_FLOW is on the outer frame but the abs pos sizing needs to happen on the inner frame, at least if the inner frame is what has the inline as its containing block.
(In reply to comment #8)
> No, because those blocks have the NS_FRAME_OUT_OF_FLOW bit set, hence get
> mFrameType set to TYPE_ABSOLUTE in the reflow state, and if that's set we use
> the mCBReflowState->frame->GetRect() for the containing block if the
> mCBReflowState->frame is inline.

We should probably also follow that codepath for inner table frames whose outer is absolutely positioned (although I'm not sure if we're guaranteed to have a reflow state for the outer).
Created attachment 362258 [details] [diff] [review]
Perhaps like this?

We fail that reftest without the patch... but should we be passing it?

Also, using "left: 0; right: 0" for the table doesn't work, since auto-width tables always shrink-wrap.  Not sure whether that's correct.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #362258 - Flags: superreview?(dbaron)
Attachment #362258 - Flags: review?(dbaron)
Summary: ASSERTION: unconstrained widths no longer supported: 'aContainingBlockWidth != NS_UNCONSTRAINEDSIZE' [@ nsLayoutUtils::ComputeWidthValue] → [FIX]ASSERTION: unconstrained widths no longer supported: 'aContainingBlockWidth != NS_UNCONSTRAINEDSIZE' [@ nsLayoutUtils::ComputeWidthValue]
Comment on attachment 362258 [details] [diff] [review]
Perhaps like this?

>@@ -293,19 +293,21 @@ void nsHTMLReflowState::InitCBReflowStat
>     mCBReflowState = nsnull;
>     return;
>   }
> 
>   if (parentReflowState->frame->IsContainingBlock() ||
>       // Absolutely positioned frames should always be kids of the frames that
>       // determine their containing block
>       (NS_FRAME_GET_TYPE(mFrameType) == NS_CSS_FRAME_TYPE_ABSOLUTE)) {
>-    // a block inside a table cell needs to use the table cell
>+    // a block inside a table cell needs to use the table cell, and an
>+    // inner table needs to use the parent of the outer table.

s/an inner table/an absolutely positioned inner table/.

You probably want to assert here that the outer table is not IsContainingBlock(), and leave a comment next to the assertion that if it ever is, we'd need to use mCBReflowState in the non-absolute cases.

>     if (parentReflowState->parentReflowState &&
>-        IS_TABLE_CELL(parentReflowState->parentReflowState->frame->GetType())) {
>+        (IS_TABLE_CELL(parentReflowState->parentReflowState->frame->GetType()) ||
>+         frame->GetType() == nsGkAtoms::tableFrame)) {
>       mCBReflowState = parentReflowState->parentReflowState;
>     } else {
>       mCBReflowState = parentReflowState;
>     }


> void
> nsHTMLReflowState::InitFrameType()

Did you go through users of mFrameType to make sure they're ok with this change?  It looks like they're all in nsHTMLReflowState and nsLineLayout (and actually all the nsLineLayout use is write-only... I'll post a patch to remove it).  I'm a little worried about the InitAbsoluteConstraints vs. ComputeSize branch in nsHTMLReflowState::InitConstraints; I wonder if we'll pick up any undesired changes there.
> Did you go through users of mFrameType

I did now.  We're basically switching from TYPE_BLOCK to TYPE_ABSOLUTE for inner table frames.  The only places that treat the two differently are:

1)  InitCBReflowState, which I fixed.
2)  ComputeContainingBlockRectangle, which is what was causing the asserts.
3)  The InitConstraints code you mention.

Hence my question about the desired behavior of the testcase...  If that reftest is supposed to be passing, we should be going through InitAbsoluteConstraints, I think.  If left/right/etc should not affect the sizing of the table, we should take the old codepath.  If they should, then we need to take the new one.  Hence the testcase and my question about it.

I did check that we don't double-count the offsets or anything like that...
Attachment #362258 - Flags: superreview?(dbaron)
Attachment #362258 - Flags: superreview+
Attachment #362258 - Flags: review?(dbaron)
Attachment #362258 - Flags: review+
Comment on attachment 362258 [details] [diff] [review]
Perhaps like this?

OK, r+sr=dbaron.

I'm not sure why you would think that maybe we shouldn't be passing that testcase, but maybe I'm missing something.
We fail the testcase if I use right:0 instead of width:100% (because the table always shrinkwraps when computing auto widths).  I wasn't sure whether the two cases should render identically or not, basically....

Or put another way, the interaction of outer-display-is-block and inner-display-is-table behavior is pretty wacky.  :(
Pushed http://hg.mozilla.org/mozilla-central/rev/c6a5e02177b1
Status: ASSIGNED → RESOLVED
Last Resolved: 9 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.2a1pre) Gecko/20090224 Minefield/3.2a1pre
Status: RESOLVED → VERIFIED
Hardware: x86 → All
Target Milestone: --- → mozilla1.9.2a1

Updated

7 years ago
Depends on: 635706
You need to log in before you can comment on or make changes to this bug.