350456 - [reflow branch] Crash with display: table-row-group in xul

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Martijn Wargers (dead)]

See upcoming testcase, which crash in the latest reflow branch build.
Basically, the testcase consists of this:
<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
<box style="display: table-row-group;">
</box>
</window>

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Comment 2

[Martijn Wargers (dead)]

It's just not table-row-group, table-row and table are also crashing. Probably every table display type is crashing on the reflow branch.

Comment 3

[Boris Zbarsky [:bzbarsky]]

The basic problem is that there is no parent reflow state for the outer table, and therefore there is no mCBReflowState for the inner table (since the outer is not a block).  This causes us to assert and then crash in InitConstraints.

This item from http://wiki.mozilla.org/Gecko:Reflow_Refactoring is probably relevant:

  "Sort out the behavior of nsHTMLReflowState::InitConstraints when
   parentReflowState is null. This happens for reflow roots and for frames
   being reflowed as a child of a box (via nsFrame::BoxReflow). The current
   behavior seems wrong."

Should we just be wrapping any non-box kids of a box in a block?

Comment 4

[Boris Zbarsky [:bzbarsky]]

*** Bug 351627 has been marked as a duplicate of this bug. ***

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #3)
> Should we just be wrapping any non-box kids of a box in a block?

Yes.  That would solve a huge number of problems, including bug 321402 / bug 320502, but it's not trivial to do.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Another nice way to fix this would be to propagate containing block width/height down the reflow state chain and get rid of mCBReflowState.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I suspect the only reason this particular testcase regressed on the reflow branch is that I'm now breaking the reflow state chain when crossing into and out of box land.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment 7 is right.

Just propagating the things we want out of mCBReflowState should be pretty straightforward, I think.  I'll take a shot at it.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Er, not as straightforward as I thought.  We do a bunch of walking starting with mCBReflowState, and have that loop in nsHTMLReflowState::InitResizeFlags adding resize bits.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think it would need to be replaced by a width, a height, and a frame pointer.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I said I'd fix bug 321402 for the alpha, so this should just get fixed by that...

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fixed on reflow branch.

Checking in nsFrame.cpp;
/cvsroot/mozilla/layout/generic/nsFrame.cpp,v  <--  nsFrame.cpp
new revision: 3.679.2.10; previous revision: 3.679.2.9
done

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Except that fix introduced some off-by-padding/border/margin problems...

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

OK, fixed again.

Checking in nsFrame.cpp;
/cvsroot/mozilla/layout/generic/nsFrame.cpp,v  <--  nsFrame.cpp
new revision: 3.679.2.12; previous revision: 3.679.2.11
done