{ib}Frame construction inconsistencies between ContentAppended() and ConstructInline()

RESOLVED WORKSFORME

Status

()

Core
Layout
P3
normal
RESOLVED WORKSFORME
16 years ago
4 years ago

People

(Reporter: kinmoz, Assigned: kinmoz)

Tracking

({topembed-})

Trunk
mozilla1.5beta
topembed-
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

16 years ago
I'm filing this bug to notate some of the inconsistencies I was seeing during 
frame construction when loading documents with inlines that contain blocks and 
floater/positioned elements, while debugging bug 106489. My hope is that some of 
the issues can be spun off as separate bugs, and this bug used as a tracking bug 
and perhaps for some discussion of the big picture of how it's all *supposed* to 
work.

I'm still fumbling my way around layout and frame construction, so some of the 
details I put down here may be off or just plain wrong, so please correct me 
where necessary.

The short version of what I was seeing is that during document loading, 
depending on when content appended notifications happened, frames were being 
added to different containers, and floaters/positioned frames were pointing at 
different parents, depending on whether they were created directly via 
ContentAppended() or ConstructInline(). IMO, we should be consistently placing 
frames in containers and parenting them correctly whether they arecreated one by 
one during several content appended notifications, or in one shot with a single 
notification.

To reliably recreate some of the situations mentioned below, I added the 
following code in nsHTMLContentSink.cpp which allowed me to tweak when 
notifications were fired off while in the debugger:


     PRBool
     HTMLContentSink::IsTimeToNotify()
     {
    +//**** KDEBUG ****
    +static int kdebug = 0;
    +static int kdebug_val = PR_FALSE;
    +if (kdebug)
    +  return kdebug_val;
    +//**** KDEBUG ****
    +

A setting of kdebug=1 and kdebug_val=1 would allow an inline to be created 
during one notification, and then it's children to be created during a different 
call to ContentAppened(). While a setting of kdebug=1 and kdebug_val=0 would 
allow the inline and all it's children to be created in one shot during a call 
to ConstructInline().

--------------------
-- ContentAppended()
--------------------

ContentAppended() gets fired when one or more children have been added to a 
particular content node. It gets the frame for the content node, then iterates 
over all the new child nodes and creates frames for them and then adds these 
frames to the content node's frame.

If the content node is an inline and it's "special" (see ConstructInline() notes 
below for what is meant by "special") the newly created frames, floaters and 
positioned elements are added to the inline-block frame instead.

--------------------
-- ConstructInline()
--------------------

ConstructInline() creates frames for the children, of an inline, that exist at 
the time it is called. This method gets triggered as a result of the document 
being loaded in chunks, and the content sink calling the ContentInserted() and 
ContentAppended() notification methods periodically, so some of the inline's 
children might not have been read in and created 
yet.

It's important to note that at this point, all of the child frames that were 
created are parented (have their parent pointers set to) the inline frame.

ConstructInline() then tries to separate the frame child list into 3 lists:

  1. List containing all inline children before the first block child.
  2. List containing the first and last block child, and every child in
     between them.
  3. List containing the inline children following the last block child.

If list 1 is not empty, the frames in the list are made children of the inline 
frame. No reparenting necessary.

If list 2 is not empty, a block frame (inline-block) is created to hold all of 
the frames in the list, and all of the frames are then reparented to point at 
the block frame. Both the inline frame and the block frame have their state 
marked as special (NS_FRAME_IS_SPECIAL), and then the block frame is stored as a 
frame property (nsLayoutAtoms::inlineFrameAnnotation) on the inline frame.

If list 3 is not empty, a new inline frame is created to hold all of the frames 
in the list, and all of the frames are then reparented to point at the inline 
frame. The frame is then marked as special and stored as an 
nsLayoutAtoms::inlineFrameAnnotation frame property on the block frame created 
above.

-------------------------------------
-- Inconsistancies/Problems/Questions
-------------------------------------

- If an inline frame is created with a call to ConstructInline() and it is 
marked as "special", and a new inline-block and a new inline frame was created, 
subsequent calls to ContentAppended() will create new child frames with the new 
inline as parent. Does WipeContainingBlock() re-trigger a call to 
ConstructInline() to re-distribute the inline frame's children across 
the inline, inline-block and new inline frame for that case or is it possible 
that we are going to be adding block frames under the new inline frame?

----

- It's possible for placeholders to be in list 2, so there is a need in some 
cases, for floaters and perhaps absolute positioned elements, to also have their 
out-of-flow frames reparented to the newly created inline-block frame (see Bug 
106489) to avoid problems during incremental reflows due to an incorrect 
ReflowCommand path. The short story here, is that an incorrect reflow command 
path can cause an incremental reflow command to be mapped to some other type of 
reflow that in some cases does absolutely nothing.

- Floaters whose placeholders were in list 2, are still being placed in the 
inline's parent block floater list. Shouldn't they be placed in the 
inline-block's floater list? If the floater is created inside ContentAppended(), 
it does end up in the inline-block's floater list. This seems inconsistent.

Here's an example that can be used to create the 2 situations above:

    <html>
    <body>
      <font>
        <table></table>
        <table align="left"><input type="text"></table>
        <table></table>
    </body>
    </html>

----

- If the inline frame has the style property style="position: relative;", any 
absolute positioned out-of-flow child frames whose placeholders are in list 2, 
are parented to the inline frame, and are placed in the inline frame's absolute 
list, yet the placeholder is parented to the inline-block. This situation seems 
to trigger the pre-condition in nsHTMLReflowState::InitAbsoluteConstraints() to 
fail during reflow:

  NS_PRECONDITION(containingBlockHeight != NS_AUTOHEIGHT,
                  "containing block height must be constrained");

and as a result, things don't render at all. If this absolute positioned frame 
had been created directly in ContentAppeneded(), the out-of-flow frame would've 
been parented to the inline-block and placed on the inline-block's absolute 
list, and things would render fine.

Here's an example that creates the situation above:

    <html>
    <body>
    <font style="position: relative;">
      <table></table>
      <table border="1" style="position: absolute; top: 0; left: 0; bottom: 
auto; right: auto;">
        <tr><td><input type="text"></td></tr>
      </table>
      <table></table>
    </body>
    </html>
To clarify terminology, this deals with blocks-within-inlines, not
inline-blocks, which are entirely different.  (We should probably change all
those {ib} markers in bugzilla to {bi}.)
Summary: Inconsistencies ContentAppended() and ConstructInline() → {ib}Inconsistencies ContentAppended() and ConstructInline()
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Priority: -- → P3
Summary: {ib}Inconsistencies ContentAppended() and ConstructInline() → {ib}Frame construction inconsistencies between ContentAppended() and ConstructInline()
Target Milestone: --- → mozilla1.0.1

Comment 2

16 years ago
I see this assertion also on the layout regression tests:

http://lxr.mozilla.org/seamonkey/source/layout/html/tests/block/bugs/48653.html

Updated

16 years ago
Blocks: 152015
(Assignee)

Updated

16 years ago
Target Milestone: mozilla1.0.1 → mozilla1.1beta

Updated

16 years ago
Target Milestone: mozilla1.1beta → mozilla1.2alpha

Comment 3

16 years ago
batch: adding topembed per Gecko2 document
http://rocknroll.mcom.com/users/marek/publish/Gecko/Gecko2Tasks.html
Keywords: topembed

Updated

16 years ago
Keywords: topembed → topembed-
(Assignee)

Updated

16 years ago
Target Milestone: mozilla1.2alpha → mozilla1.3alpha

Updated

16 years ago
Blocks: 176349
(Assignee)

Updated

15 years ago
Target Milestone: mozilla1.3alpha → mozilla1.3beta
(Assignee)

Updated

15 years ago
Target Milestone: mozilla1.3beta → mozilla1.5alpha
(Assignee)

Updated

15 years ago
Target Milestone: mozilla1.5alpha → mozilla1.5beta
I suspect this is not really a problem anymore... The testcase in comment 2 definitely doesn't assert.

Comment 5

10 years ago
wfm
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.