Closed Bug 264914 Opened 20 years ago Closed 20 years ago

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


(Core :: Layout, defect, P1)






(Reporter: bzbarsky, Assigned: bzbarsky)




(6 files, 3 obsolete files)

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.
Priority: -- → P1
Target Milestone: --- → mozilla1.8alpha5
Attached file Testcase to not break
Attached patch Proposed patch (obsolete) — Splinter Review
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).
Attached file Another testcase (float inside button) (obsolete) —
Attachment #163601 - Attachment is obsolete: true
Attached patch Slightly better (obsolete) — Splinter Review
Slightly simpler code, with kinks worked out a bit.
Attachment #163598 - Attachment is obsolete: true
Attached patch Tested and allSplinter Review
Attachment #163618 - Attachment is obsolete: true
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
Attachment #163647 - Flags: superreview?(roc)
Attachment #163647 - Flags: review?(roc)
Oh, I should mention that this passes the regression tests just fine.
Summary: Refactor frame constructor code for adding kids to child lists → [FIX]Refactor frame constructor code for adding kids to child lists
Comment on attachment 163647 [details] [diff] [review]
Tested and all

This looks fantastic. Great work.
Attachment #163647 - Flags: superreview?(roc)
Attachment #163647 - Flags: superreview+
Attachment #163647 - Flags: review?(roc)
Attachment #163647 - Flags: review+
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".
Closed: 20 years ago
Resolution: --- → FIXED
Depends on: 348729
Product: Core → Core Graveyard
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.