264914 - [FIX]Refactor frame constructor code for adding kids to child lists

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

Right now, various places in the frame constructor have to explicitly add the
new kids to the containing block's child list.  If they mess this up, things
like bug 263846 happen.

In my opinion, what should happen is that when a containing block is popped, any
pending kids are added to its child list by the frame constructor state.  When
pushing a containing block, code should specify whether it's ok to call
SetInitialChildList() (if the containing block is newly created, basically), or
whether InsertOutOfFlow() should be used.

The goal is to be able to push a containing block and then forget about it,
basically, and have the frame constructor state do the right thing.

Marking dependant on bug 263406 because this touches the same code, so I can't
really work on this till that patch is checked in.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase to not break

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase that's currently broken

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase showing rendering inconsistency with floats in {ib} splits

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

This really shouldn't change behavior, with the exception of the one place
where absolute/fixed items were not being handled (there was an XXX comment to
that effect).

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Another testcase (float inside button)

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Float inside button for real

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Abs pos kid inside rel pos button

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Slightly better

Slightly simpler code, with kinks worked out a bit.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Tested and all

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 163647 [details] [diff] [review]
Tested and all

roc, could you review?	Basic summary of changes:

1)  Store the frame list we're working with and a pointer to the frame
constructor state in the save state.

2)  Automatically process frame insertions when the save state is destroyed or
when the frame constructor state is destroyed.

3)  Remove some unused code in ConstructHTMLFrame, set things to be
float/absolute containing blocks when they are.  This fixes the rel pos button
testcase attached to this bug.

4)  Fix nsButtonControlFrame to properly handle insertion of out-of-flow kids,
since it can have them

5)  Minor cleanup like not passing aPresContext to PushAbsoluteContainingBlock
and so forth.

I should probably file a separate bug on the XXX comment I added to
AdjustOutOfFlowParentPtrs....

Comment 11

[Boris Zbarsky [:bzbarsky]]

Oh, I should mention that this passes the regression tests just fine.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 163647 [details] [diff] [review]
Tested and all

This looks fantastic. Great work.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Patch checked in; no performance impact, thankfully.  I've checked in the
testcase this patch fixed into the regression tests, filed bug 266971 on floats
in {ib} splits, and filed bug 266968 on the "testcase that's currently broken".