Closed Bug 209694 Opened 21 years ago Closed 20 years ago

[MARGIN-C]clear margin merged with bottom margin on empty last child

Categories

(Core :: Layout: Block and Inline, defect, P1)

x86
All
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: forum, Assigned: roc)

References

()

Details

(Keywords: testcase, Whiteboard: (py8ieh: take testcase and make a real one))

Attachments

(5 files, 3 obsolete files)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030531 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030531 http://www.marilouonline.com/videos.html there should be a margin between the 2 <div class="videoPanel">. Displayed correctly in IE and Opera Reproducible: Always Steps to Reproduce: 1.Open the link 2.Check for a margin between the 2 div's forementionned Actual Results: there's no margin at all between the 2 divs Expected Results: check it out in IE or Opera for expected results
Attached file Testcase
Confirming bug, 2003-06-12-22 trunk Linux.
Assignee: general → float
Status: UNCONFIRMED → NEW
Component: Browser-General → Layout: Floats
Ever confirmed: true
Keywords: testcase
OS: Windows XP → All
QA Contact: general → ian
We're probably collapsing the margin out the top.
Assignee: float → block-and-inline
Component: Layout: Floats → Layout: Block & Inline
Summary: margin-bottom is being ignored if no padding is declared → [MARGIN-C]margin-bottom is being ignored if no padding is declared
Summary: [MARGIN-C]margin-bottom is being ignored if no padding is declared → [MARGIN-C]margin-bottom ignored when last child empty and cleared
Attached file Testcase #2
I don't think it's a margin collapsing problem. The problem is that an element that has "clear:both" does not have any effect if its height is zero and there is no other content following it. Even though it's height is zero it should still be positioned below the float and thus make the parent container high enough to "wrap" the floater. (It works for <br clear=all>) This testcase is a bit clearer I hope.
The behavior here is actually arguably correct per CSS2, since clear works by increasing margin, and yet the margins in question are adjacent and should thus collapse. That said, I'm not sure it's the only possible interpretation, and I'm not sure if it's a good thing even if it is.
Summary: [MARGIN-C]margin-bottom ignored when last child empty and cleared → [MARGIN-C]clear margin merged with bottom margin on empty last child
Whiteboard: [csswg]
(And, FWIW, my comment 3 didn't make any sense at all, since there was non-empty content.)
(This bug is really a case where the requirements of the CSS spec are impossible to meet simultaneously.)
Actually, I realized that the CSS spec isn't contradictory. I'd always translated the section in 10.6.3 which says: If it has block-level children, the height is the distance between the top border-edge of the topmost block-level child box and the bottom border-edge of the bottommost block-level child box. [...] [ http://www.w3.org/TR/2003/WD-CSS21-20030128/visudet.html#q19 ] into the concept that child/parent collapsed margins always end up outside the parent. However, this need not be the case when margins collapse through an empty block, so the spec isn't contradictory, and this testcase should do what you think it ought to do.
(And, combining that with a new sentence likely to be in CSS 2.1 would actually make bug 44242 invalid.)
Priority: -- → P1
This is IMHO a dupe of bug 45830.
Blocks: 224057
*** Bug 226487 has been marked as a duplicate of this bug. ***
Blocks: 204831
We need to fix this because an empty DIV with a 'clear:both' is the easiest way to get a float container to grow to the size of the float, which is what IE does by default, so this bug blocks people modifying their layouts to be standards-compliant.
Assignee: core.layout.block-and-inline → roc
So given the markup: <div style="background-color:yellow;"> <img style="float:left" src="http://mozilla.org/images/hack.gif"> <div style="clear:both"></div> </div> the yellow DIV gets zero height because the delta-Y induced by the clear becomes part of the margin, which collapses all the way through the content. So if that's wrong, what's supposed to happen here?
See: http://www.w3.org/TR/CSS21/box.html#collapsing-margins The margin of the yellow box can't collapse with the margin of the empty DIV because "The top margin of an in-flow block-level element is adjoining to its first in-flow block-level child's top margin if the element has no top border, no top padding, *and the child has no clearance*". This then doesn't collapse with the bottom margin of the yellow div because "When an element's own margins collapse, and that element has had clearance applied to it, its top margin collapses with the adjoining margins of subsequent siblings but that resulting margin does not collapse with the bottom margin of the parent block". So the yellow box in this example is exactly the height of the float, and has zero margins before and after. Please check the CSS2.1 rules, they have been significantly updated and clarified since CSS2 and are now pretty explicit.
Does "has clearance" mean that the space required by the 'clear' property is non-zero? If so, then what about the following testcase: <div style="float:left; height:100px;"></div> <div id='parent' style="margin-top:50px;"> <div id='child' style="margin-top:80px; clear:both;"></div> </div> What should the result be? It seems to me that the logical result would put child's top adjacent to the bottom of the float. But that would require 'partial collapse'. If we're restricted to the margins either collapsing or not, then I don't see a solution consistent with the CSS 2.1 rules.
See: http://www.w3.org/TR/CSS21/visuren.html#clearance Computing the clearance of an element on which 'clear' is set is done by first determining the hypothetical position of the element's top border edge within its parent block. This position is determined after the top margin of the element has been collapsed with previous adjacent margins (including the top margin of the parent block). If the element's top border edge has not passed the relevant floats, then its clearance is set to the amount necessary to place the border edge of the block even with the bottom outer edge of the lowest float that must be cleared. | <div style="float:left; height:100px;"></div> | <div id='parent' style="margin-top:50px;"> | <div id='child' style="margin-top:80px; clear:both;"></div> | </div> What this looks like depends on the parent element (and in particular, on whether it is the root element or not). Assuming the markup above is contained in a normal in-flow block-level element X with a non-zero border and zero padding, then the rendering would be determined as follows: At the top left of the X box's borders, a zero width float of height 100px is placed. The two next elements ('parent' and 'child') are then collapsed together to find the hypothetical position of 'child' to see if it needs to be cleared. The hypothetical position is determined as being 80px from the inner top of X. (This is the collapsing of 50px, 80px, 0px, and 0px, the four hypothetically adjacent margins at that point.) The hypothetical top border edge position hasn't cleared the float in this example, so clearance is introduced. Do the collapsing with that in mind. The 'parent' box is not adjacent to the 'child' box's top margin, since it has clearance. So its top border edge is 50px from the top of X. The 'child' box is next. Its top margin collapses with its bottom margin and the parent's bottom margin, but nothing else. Its clearance is 50px (the distance from the parent's top border edge) - 80px (the resulting collapsed top margin of the 'child' box). The clearance is thus -30px. The clearance conceptually goes before the top margin of the element, and is then followed by the margin (which is 80px). The top border edge of the 'child' box is thus -30px+80px from the 'parent' box's top border edge, which places it exactly 100px from the top of X (which is exactly where it was supposed to be -- 'clear' means "but the top border edge even with the bottom of the float"). The bottom margin of the 'parent' box is part of the collapsing above, and thus the bottom of the 'parent' box is even with the 'child' box. This is, at least, my reading (and for that matter, writing) of the spec. Is there something in that which seems inconsistent? There shouldn't be... :-)
I get it. I hadn't considered *negative* clearance values. It would be nice if the spec mentioned this possibility in the paragraph "If the element's top border edge has not passed the relevant floats, then its clearance is set to the amount necessary to place the border edge of the block even with the bottom outer edge of the lowest float that must be cleared." I hope you realize that this is phenomenally complex, even for authors, and even more so for implementors.
(In reply to comment #17) > I get it. I hadn't considered *negative* clearance values. The definition says "Clearance is introduced as spacing above the margin-top of an element. It is used to push the element vertically (typically downward), past the float." -- note the "typically downward" bit. That was the result of an argument in the working group regarding whether or not to explicitly mention that it could be negative, if I recall correctly. > I hope you realize that this is phenomenally complex, even for authors, and > even more so for implementors. Yes, it is. The working group attempted to make CSS2.1 a lot clearer than CSS2 and CSS1 were on this issue. Unfortunately the basic model was pretty well established and all we could do is explain how on earth it was actually supposed to work. If I could go back to 1995 and help the development of CSS, I would, but unfortunately it's mostly too late now. :-( (Havint said that, I _can_ help reduce the complexity of new specs I'm involved with, especially those of www.whatwg.org. If you see anything in those specs that is too complex for your liking, let us know whatwg@whatwg.org and I'll do my best to make it easier.)
I remember reading that text and not being able to imagine how 'clear' could push the element "up past the float", and thus ignoring it. I'll have to think about how this is going to work. I suspect we're going to have to move the float clearing logic from nsBlockFrame::ReflowBlockFrame into nsBlockReflowContext::ComputeCollapsedTopMargin. ComputeCollapsedTopMargin will have to follow the leading blocks down the frame tree, computing the clearance (if any) of each block encountered, and then recalculate the collapsed margin if we encountered a block with clearance. We'll need to store state in the lines, probably an mHasClearance bit, to record that a block had clearance and it will have to apply its own top margin and clearance.
Can you clarify exactly what "its top margin collapses with the adjoining margins of subsequent siblings but that resulting margin does not collapse with the bottom margin of the parent block" means? <div> <div style="clear:both; margin-top:100px"></div>
Can you clarify exactly what "its top margin collapses with the adjoining margins of subsequent siblings but that resulting margin does not collapse with the bottom margin of the parent block" means? <div id="float" style="float:left; height:100px"></div> <div id="outer" style="margin-bottom:100px;"> <div id="clear" style="clear:both; margin-top:100px"></div> <div id="sib" style="margin-bottom:100px;"></div> </div> <div id="end" style="border:1px solid red"></div> My impression is that there will be a 200px margin between the bottom of "float" and the border-box of "end", because the margin-top of "clear" collapses with the margin-bottom of "sib" but not the margin-bottom of "outer". But what if we change the border-bottom of "sib" to 150px? <div id="float" style="float:left; height:100px"></div> <div id="outer" style="margin-bottom:100px;"> <div id="clear" style="clear:both; margin-top:100px"></div> <div id="sib" style="margin-bottom:150px;"></div> </div> <div id="end" style="border:1px solid red"></div> Is it true that we now have a 250px margin between the bottom of "float" and the border-box of "end" because exactly the same reasoning applies as in the first case? Then what about this case? <div id="float" style="float:left; height:100px"></div> <div id="outer" style="margin-bottom:100px;"> <div id="clear" style="clear:both; margin-top:0.0001px"></div> <div id="sib" style="margin-bottom:100px;"></div> </div> <div id="end" style="border:1px solid red"></div> Do we still see a 200.0001px margin? It seems strange for the existence of a nonzero margin-top on "clear" to 'poison' the collapsing margin and prevent a potentially distant sibling from collapsing its bottom margin with its parent.
Also, what if "clear"'s margin-top is zero? Can it still be said to collapse with adjoining margins and thus prevent later siblings from collapsing with the parent? The testcase would be <div id="float" style="float:left; height:100px"></div> <div id="outer" style="margin-bottom:100px;"> <div id="clear" style="clear:both;"></div> <div id="sib" style="margin-bottom:100px;"></div> </div> <div id="end" style="border:1px solid red"></div> Is the margin now only 100px or still 200px between between the bottom of "float" and the border-box of "end"?
Attached patch work in progress (obsolete) — Splinter Review
I'm quite certain this doesn't work, because I've only just finished typing it in, but it shows the approach I'm trying to take.
(BTW, I can attempt to understand what the spec says about clearance, if you need any help, but I really have no clue from memory, because it's been through so many iterations and proposals. We spent a good part of the evening on the last day of the Oslo meeting last year ironing out all the messy issues, during which time I repeatedly regretted giving in (on the second day of the meeting) on my alternative (simpler and more consistent with prior specs, but somewhat non-intuitive) proposal for how the interaction of clearing and margin collapsing should work. Not that I remember what that was either...)
Ian's doing a great job so far :-)
> Can you clarify exactly what "its top margin collapses with the > adjoining margins of subsequent siblings but that resulting margin > does not collapse with the bottom margin of the parent block" means? The definition of "adjoining margins" is given in 8.3.1 after the bulleted list (each case that could be considered adjoining is listed, no other cases are adjoining). To answer your question with reference to your example: > <div id="float" style="float:left; height:100px"></div> > <div id="outer" style="margin-bottom:100px;"> > <div id="clear" style="clear:both; margin-top:100px"></div> > <div id="sib" style="margin-bottom:100px;"></div> > </div> > <div id="end" style="border:1px solid red"></div> (Again assuming a box X around the whole thing that is not the root element and that has a solid border and zero padding.) The float is at the top left of X's content area. The 'clear' box is self-collapsing and so we use the special rules for finding the top of self-collapsing blocks (that's the bullet point starting with "If the top and bottom margins of a box are adjoining, then..."). Per these rules, we collapse the top margin of the 'outer' block (0) and the top margin of the 'clear' block (100px) together to find the top border edge of the 'clear' block. (Remember, this is still just hypothetical, we're trying to work out whether the 'clear' box needs any clearance.) This tells us that the hypothetical top of the 'clear' box is 100px down from the top of X's content area, which means it is past the float and that we don't need any clearance, so the 'clear' box has no clearance and six margins are adjacent (0, 100px, 0, 0, 100px, 100px respectively). Thus the total distance from the top of 'outer' to the bottom of 'outer' is 100px and the red line is immediately below the float, and the height of 'outer' is zero. (This matters -- it means if it had any background, it wouldn't be visible.) The height of X in this example is thus 101px (100px + 1px for the red border) Read that again until you understand it because if it doesn't make any sense the next bit will just make matters worse. Once you're sure you understand it: Note that if we had the _exact_ same markup but changed the margin-top of the 'clear' box to one less pixels -- 99px -- that the result would be quite different: The hypothetical position of the box would now not be enough to push the top border edge of the 'clear' box past the float (collapse 0 and 99px and you get 99px, less than the float's 100px) and so the box would need clearance, in this case of exactly 1px. If the 'clear' box has clearance, that means it doesn't collapse with the 'outer' box, whose top border edge would therefore be at the top of X. Thus the top border edge of the self-collapsing 'clear' box would be defined as being 1px+99px (clearance and top margin respectively) below the top of the parent 'outer' box and thus 100px below the top of X. This position would be relevant if 'clear' contained any floats of its own, or was a containing block for any positioned elements. Then, the margins of the self-collapsing box and its siblings would collapse together -- 99px, 0, 0, 100px, collapsing to 100px (note that the margins collapse straight through the top border edge of self-collapsing blocks -- their top border edge position is defined as not taking into account later margins, but doesn't stop the adjacent margins from collapsing anyway), meaning that the bottom of 'outer' is 1px+100px (that pesky clearance, plus the collapsed margin) below the top of 'outer'. So the height of 'outer' is 101px. A decrease in the margin of the 'clear' box by 1px increased the height of the 'outer' box from 0 to 101px, making its background visible in the process. The bottom margin of 'outer' is then added on top of that -- it doesn't collapse with the margins that collapsed with the self-collapsing block that had clearance -- and thus the red line is 201px below the top of X, and 101px below the float. The height of X in this example would thus be 202px (201px + 1px for the red border). This discontinuity is visible in this example which uses similar (although not identical) markup: http://www.hixie.ch/tests/adhoc/css/box/float/clear/004-demo.html ...if you implemented it right. > <div id="float" style="float:left; height:100px"></div> > <div id="outer" style="margin-bottom:100px;"> > <div id="clear" style="clear:both; margin-top:100px"></div> > <div id="sib" style="margin-bottom:150px;"></div> > </div> > <div id="end" style="border:1px solid red"></div> (Still assuming the wrapping X block.) Here the 'clear' box still gets past the float on the strength of its own margin and so the initial description above still applies, except that the resulting collapsed margin is 150px instead of 100px. All the margins still collapse fine, because there is no clearance in this example. If 'clear' had only had 99px of top margin, then it would have been similar to the second example above, with the resulting distance between the top of X and the red line being 251px (1px clearance, 150px margin collapsed around the 'clear' and 'sib' boxes, and 100px bottom margin from 'outer'), and thus 151px from the bottom of the float. > <div id="float" style="float:left; height:100px"></div> > <div id="outer" style="margin-bottom:100px;"> > <div id="clear" style="clear:both; margin-top:0.0001px"></div> > <div id="sib" style="margin-bottom:100px;"></div> > </div> > <div id="end" style="border:1px solid red"></div> It doesn't make any difference if the margin is 0.0001px or 0, 0 margins still fully take part in the collapsing margin model. In this example, the hypothetical top border edge of the 'clear' block would be 0.0001px from the top of the ever-assumed wrapper X, which if clearly not enough to clear the float, and thus clearance of 99.9999px is introduced. The margins of 'clear' and 'sib' then collapse (to 100px), and are added after the clearance, meaning that 'outer' has height 199.9999px, and the distance from the bottom of the float to the red line is 199.9999px (distance from top of X to the bottom of 'outer') + 100px (bottom margin of 'outer') - 100px (height of 'float') which is 199.9999px. > ... prevent a potentially distant sibling from collapsing its bottom > margin with its parent. The sibling can't be "distant" if it is collapsing its margins with the 'clear' block. Margins only collapse when they are adjacent. > Also, what if "clear"'s margin-top is zero? Can it still be said to > collapse with adjoining margins and thus prevent later siblings from > collapsing with the parent? Yes, zero margins take part in collapsing just like any others. The key here is not whether there is a margin, but whether the float's hypothetical (i.e., pre-clearance) top border edge position is beyond the float or not (where "beyond" is defined as ""). > The testcase would be > > <div id="float" style="float:left; height:100px"></div> > <div id="outer" style="margin-bottom:100px;"> > <div id="clear" style="clear:both;"></div> > <div id="sib" style="margin-bottom:100px;"></div> > </div> > <div id="end" style="border:1px solid red"></div> Clearance here is 100px, height of 'outer' is 200px starting at the top of X, height of X is 301px (including the 1px red line). There are a number of margin collapsing testcases (although few that involve 'clear', sadly) on my site, at: http://www.hixie.ch/tests/adhoc/css/box/block/margin-collapse/
> (where "beyond" is defined as "") Oops, forgot to paste the definition. I meant to say: a block has cleared its floats if its top border edge is below the bottom of the floats. So if you have a one pixel float, and the top of the box with 'clear' is on the second vertical pixel, it has cleared the float: _____________________ [FLOAT] _________ ' | | CLEARED | | | ' '
Thanks. My examples weren't quite what I wanted because they didn't have clearance, but you guessed what I meant and it's all pretty clear now. Thanks.
Well, one more question. When computing the clearance we're to assume that the cleared block's top margin collapses with the margin of its parent or previous sibling. What if the cleared block's first child has a top margin, should we collapse with that too when computing clearance? Consider, under the usual assumptions: <div id="float" style="float:left; height:100px"></div> <div id="clear" style="clear:both;"> <div id="child" style="margin-top:200px;"></div> </div> <div id="end" style="border:1px solid red"></div> Does "clear" have clearance or not? The current patch assumes that it does, because we don't consider the child's top margin when computing the clearance, but reading the spec I'm beginning to think this is incorrect. A gigantic problem with the current patch is that ComputeCollapsedTopMargin isn't considering the floats whose placeholders occur between the start of the top-margin-root block and the descendant with clearance when it calculates whether clearance is needed. But hacking ComputeCollapsedTopMargin to reflow and place those floats would be a lot of code and really slow, not to mention that we'd have to push and and pop state since this would only be a speculative reflow. However, if we must compute the correct top margin value before reflowing the top-margin-root block, then I see no other choice. It might be possible to move away from this design but I don't yet see what an alternative would look like. I realize that I let something go past that I should have nailed down, in your comment #16. You appear to interpret this text, "If the element's top border edge has not passed the relevant floats, then its clearance is set to the amount necessary to place the border edge of the block even with the bottom outer edge of the lowest float that must be cleared.", to mean that that this computation takes place assuming a layout where the element's top margin DID NOT collapse with its parent's. So the hypothetical position is used only to determine *whether* there is clearance, but not the value of the clearance. Right? Then I have another question about an example, to test if my understanding is correct. <div id="outer" style="border:0.000001px solid red; float:left"> <div id="interior" style="margin-top:100px;"> <div id="float" style="float:left; height:100px"></div> <div id="clear" style="clear:left; margin-top:150px;"></div> </div> </div> We start by assuming that the top margins of "clear" and "interior" collapse. The top border-edges of "interior", "float" and "clear" would then be 150px below the top of "outer". So "clear" has clearance. Now we assume that the top margins of "clear" and "interior" do not collapse. The top edges of "interior" and "float" are now 100px below the top of "outer", and the top edge of "clear" is 200px below the top of "outer". Is that correct? If so, then this example confirms that we need to reflow floats at different places for each element whose clearance we're testing. Think about this example where the floats get laid out quite differently in the hypothetical vs the actual cases. <div id="outer" style="border:0.000001px solid red; float:left; width:1000px"> <div id="right" style="float:right; width:900px; height:150px;"></div> <div id="interior" style="margin-top:100px;"> <div id="float1" style="float:left; height:100px; width:100px;"></div> <div id="float2" style="float:left; height:100px; width:100px;"></div> <div id="clear" style="clear:left; margin-top:150px;"></div> </div> </div> Hmm, I wonder if a good way to implement all this would be to use our ComputeCollapsedTopMargin approach and just reflow everything assuming that no blocks have clearance, and then when we find that a block needs clearance we back out to the nearest block whose margin collapse was affected and re-reflow it. That approach might not actually be too bad in performance and complexity.
> Well, one more question. When computing the clearance we're to assume that the > cleared block's top margin collapses with the margin of its parent or previous > sibling. What if the cleared block's first child has a top margin, should we > collapse with that too when computing clearance? Yes. In the hypothetical case, the element (and any subsequent elements) don't have clearance, and so margins collapse well. (Some of the previous siblings might have clearance already; those, per rules in 8.3.1, have various restrictions on where they collapse.) > <div id="float" style="float:left; height:100px"></div> > <div id="clear" style="clear:both;"> > <div id="child" style="margin-top:200px;"></div> > </div> > <div id="end" style="border:1px solid red"></div> > > Does "clear" have clearance or not? The hypothetical case collapses all the margins togother and finds that 'clear' has a top margin of 200px. This is enough to clear the float so there is no clearance. > So the hypothetical position is used only to determine *whether* there is > clearance, but not the value of the clearance. Right? That is correct. The hypothetical position decides whether there is clearance. If there is, then that affects the rules in 8.3.1 -- so when you do the "real" collapsing, different things actually collapse. It's when you're doing the "real" collapse that you find the position of the 'clear' block's top border edge. > <div id="outer" style="border:0.000001px solid red; float:left"> > <div id="interior" style="margin-top:100px;"> > <div id="float" style="float:left; height:100px"></div> > <div id="clear" style="clear:left; margin-top:150px;"></div> > </div> > </div> (Note that the outer element being a float doesn't affect clear or inner floats at all, it just means that it'll shrink wrap to contain everything, including descendent floats. Just like a root element would, in fact.) Here the float's position is between two margins that might collapse, so we invoke section 9.5.1 rule 4 sentence 2: "When the float occurs between two collapsing margins, the float is positioned as if it had an otherwise empty anonymous block parent taking part in the flow. The position of such a parent is defined by the rules in the section on margin collapsing." Thus the markup is now equivalent to: <div id="outer" style="border:0.000001px solid red; float:left"> <div id="interior" style="margin-top:100px;"> <div id="anonymous"> <div id="float" style="float:left; height:100px"></div> </div> <div id="clear" style="clear:left; margin-top:150px;"></div> </div> </div> The 'anonymous' block is self-collapsing, so we invoke 8.3.1, 6th bullet point, first bullet point: "If the element's margins are collapsed with its parent's top margin, the top border edge of the box is defined to be the same as the parent's." Now we have to find the top border edge of 'interior', but that depends on the collapsing of various margins including one that might be affected by whether clearance exists. We therefore find the hypothetical position of 'clear' by hypothetically collapsing the margins at that point -- 100px, (0, 0), 150px, 0, 0, respectively -- and that gives us a top margin of 150px on the 'interior' div, which puts the float at the same level as the 'clear' block, which means the 'clear' block hasn't cleared the floats it needs to clear. We therefore now assume 'clear' has clearance. Knowing that 'clear' has clearance, we can do the real collapsing. We collapse the adjacent margins -- just 100px and the two anonymous 0, 0 margins -- and so find that there is a 100px margin at the top of 'interior'. This puts the float at the top of 'interior', according to the two rules quoted above, 100px below the outer red border. Next we position 'clear', which means discovering its clearance. We need to place that element so that its top border edge is touching the bottom border edge of the float. That's easy enough: 100px distance from the bottom of the last in-flow box to the bottom of the floats we have to clear, - 150px of our margin (that's 150px collapsed with 0 and 0 bottom margins) in that area already covering the float, = -50px clearance. Thus the distance from inner top to inner bottom of the 'outer' block is 200px, the last 100px of which is the float and 'interior' blocks. > We start by assuming that the top margins of "clear" and "interior" collapse. > The top border-edges of "interior", "float" and "clear" would then be 150px > below the top of "outer". So "clear" has clearance. Now we assume that the top > margins of "clear" and "interior" do not collapse. The top edges of "interior" > and "float" are now 100px below the top of "outer", and the top edge of "clear" > is 200px below the top of "outer". Is that correct? Yes. > <div id="outer" style="border:0.000001px solid red; float:left; width:1000px"> > <div id="right" style="float:right; width:900px; height:150px;"></div> > <div id="interior" style="margin-top:100px;"> > <div id="float1" style="float:left; height:100px; width:100px;"></div> > <div id="float2" style="float:left; height:100px; width:100px;"></div> > <div id="clear" style="clear:left; margin-top:150px;"></div> > </div> > </div> (Note that here the 'right' float is _not_ between two collapsing margins, so it doesn't get an anonymous div -- it's just placed at the top of the 'outer' div.) This is quite a nice example. The two renderings aren't actually that different; the second float, for instance, has the same position in the real rendering as in the hypothetical layout used to find if 'clear' has clearance. But yes, this can cascade to quite big differences. The key here is that the moment you start collapsing margins, you find all the involved margins, then size the floats that are introduced between the first and last margins involved (float sizes cannot depend on position), then see if any of the in-flow blocks might have clearance, and if they do, for each one in turn use the hypothetical mechanism to work out if it should have clearance or not. When one of them does, you lay out everything up to that point and start again from there (since clearance is like padding -- it blocks collapsing). If none of them do, then you just do the layout at that point. There are probably other, more performant ways of doing this, but that's the one that makes the most sense when calculating things by hand IMHO.
> the second float, for instance, has the same position in the real rendering as > in the hypothetical layout used to find if 'clear' has clearance. Is that true? My understanding was that in the hypothetical layout, "float1" and "float2" would be placed next to each other with their top edges 150px below the top of "outer". But in the actual layout, the margins don't collapse, "float1"'s top edge is placed 100px below the top of "outer", there's no room to put "float2" beside it because of the right float, and therefore "float2" goes below it, with its top edge 200px below the top of "outer". > (float sizes cannot depend on position) Are you sure? In this testcase, if I'd omitted the width property on the left floats, and given them some wrappable text content, I think float1's size could have changed with position because (in Mozilla parlance) the available width would have been different (1000px in the hypothetical layout, 100px in the actual layout), and text would have wrapped differently. I'm pretty sure that I understand how to do this now. Thanks very much for all the help.
(In reply to comment #31) > > Is that true? My understanding was that in the hypothetical layout, "float1" > and "float2" would be placed next to each other with their top edges 150px > below the top of "outer". Right. > But in the actual layout, the margins don't collapse, "float1"'s > top edge is placed 100px below the top of "outer", there's no room to put > "float2" beside it because of the right float, and therefore "float2" goes > below it, with its top edge 200px below the top of "outer". No, there is room, since the right float ends half way down the first left float. The layout would be: +-----------------------------------------+ | +--------------------------------+ | | | | | | | | | | | | | | XXXX | | | | XXXX +--------------------------------+ | | XXXX YYYY | | XXXX YYYY | | YYYY | ' '''' ' ...etc. If I'm not mistaken. > Are you sure? In this testcase, if I'd omitted the width property on the left > floats, and given them some wrappable text content, I think float1's size > could have changed with position because (in Mozilla parlance) the available > width would have been different (1000px in the hypothetical layout, 100px in > the actual layout), and text would have wrapped differently. Available width is only dependent on the containing block, which is defined by the float's original position in the DOM, not by layout. See section 10.3.5.
I'm not sure about the meaning of hypothetical constructions like the one used here: > Computing the clearance of an element on which 'clear' is set is done by first > determining the hypothetical position of the element's top border edge within > its parent block. This position is determined after the top margin of the > element has been collapsed with previous adjacent margins (including the top > margin of the parent block). The word "after" implies that layout is computed in some order determined by the spec, but that's not really so. You assume it's an outside-in model, and therefore we detect clearance on parents before children, which is natural enough, but as far as I know it's not part of the spec. So it seems unclear exactly what constraints are to apply in the hypothetical world we construct to detect clearance. For example, consider this example: <div id="outer" style="border:0.000001px solid red;"> <div id="float" style="float:left; height:100px;"></div> <div id="clear" style="clear:left; margin-top:120px;"> <div id="child" style="clear:left; margin-top:-50px;">Hello</div> </div> </div> Is there really anything in the spec which forces the conclusion that "clear" has clearance and "child" does not, precluding the possibility that "child" has clearance and "clear" does not?
CSS is always done in document order. I can't find anything in the spec that says so, but it is. I'll raise it with the working group.
Hmm. Most of the CSS spec seems to describe constraints on a valid layout, not a layout algorithm. (In some places it describes algorithms, but these are really just functions used to describe complex, but local, constraints.) I'm not sure that just saying "CSS follows document order" is meaningful. What we have here is a situation where multiple layouts may satisfy the spec as written, and we need to say which layout is preferred. What we really need is a formalization of CSS using something like constraint logic programming (Prolog with arithmetic constraints). It seems that Prolog-style choice points and a suitable ordering on the rules would suffice to select the preferred layout among many possibilities. I wish I had the time.
Implementing CSS in Prolog would just mean we had yet another buggy implementation to worry about... But if you want it in terms of constraints, how about: "The hypothetical position of an element is calculated assuming that all later elements have no clearance."
> Implementing CSS in Prolog would just mean we had yet another buggy > implementation to worry about... The point of formalizing specs is that at some point the formalism is designated as normative. But don't worry, actually formalizing CSS would be a mighty project that no-one will do in the forseeable future :-). > But if you want it in terms of constraints, how about: "The hypothetical > position of an element is calculated assuming that all later elements have no > clearance." That helps a lot. But is it obvious that we can talk about "an element" having clearance? Might an element have multiple boxes some of which have clearance and others not? I suppose clearance only applies to the first-in-flow. But then there are some situations where elements can have multiple distinct presentations; 'position:fixed' and table header/footer repetition, at least. I guess in those cases all copies are guaranteed to have the same clearance. (I do wonder how well the CSS spec language, and CSS itself, are going to hold up in these situations...)
> But is it obvious that we can talk about "an element" having clearance? 'clear' applies to elements and pseudo-elements that are block-level. I guess at the technical level one box of an element in paged media has clearance and the others don't... but then we talk about the margins of an element too, so I don't really see the problem there. > Might an element have multiple boxes some of which have clearance and > others not? Only insofar as the an element has multiple boxes some of which have a top margin and others don't. > But then there are some situations where elements can have multiple distinct > presentations; 'position:fixed' and table header/footer repetition, at least. Good lord, 'clear' shouldn't apply to absolutely positioned elements. Let me file another issue... (Table headers and footers aren't block-level and so 'clear' doesn't apply to them.)
I have coded up a patch for this. There's still a lot of work to do to get it working properly.
The patch is starting to look good. I have to look at the incremental reflow issues though
In fact my patch is good enough that I've burned a lot of time trying to figure out why certain testcases don't work, and it turned out that the code was right and I was wrong.
(In reply to comment #26) > To answer your question with reference to your example: > > > <div id="float" style="float:left; height:100px"></div> > > <div id="outer" style="margin-bottom:100px;"> > > <div id="clear" style="clear:both; margin-top:100px"></div> > > <div id="sib" style="margin-bottom:100px;"></div> > > </div> > > <div id="end" style="border:1px solid red"></div> > > (Again assuming a box X around the whole thing that is not the root > element and that has a solid border and zero padding.) ... > Note that if we had the _exact_ same markup but changed the margin-top > of the 'clear' box to one less pixels -- 99px -- that the result would > be quite different: The hypothetical position of the box would now not > be enough to push the top border edge of the 'clear' box past the > float (collapse 0 and 99px and you get 99px, less than the float's > 100px) and so the box would need clearance, in this case of exactly > 1px. If the 'clear' box has clearance, that means it doesn't collapse > with the 'outer' box, whose top border edge would therefore be at the > top of X. Thus the top border edge of the self-collapsing 'clear' box > would be defined as being 1px+99px (clearance and top margin > respectively) below the top of the parent 'outer' box and thus 100px > below the top of X. This position would be relevant if 'clear' > contained any floats of its own, or was a containing block for any > positioned elements. I believe this was actually incorrect. Here's the adjusted markup: <div id="float" style="float:left; height:100px"></div> <div id="outer" style="margin-bottom:100px;"> <div id="clear" style="clear:both; margin-top:99px"></div> <div id="sib" style="margin-bottom:100px;"></div> </div> <div id="end" style="border:1px solid red"></div> In this case, when we try to determine clearance, the hypothetical box position will still be 100px below the top, because the margin-bottom on 'sib' collapses with the margins of 'clear'. So to get the example to show what we wanted it to show, you also have to reduce the margin-bottom on 'sib' to 99px.
Regarding float sizes depending on position: it can happen in quirks mode, I think (and it's an ugly hack that prevents us from fixing some rather serious real bugs).
> <div id="float" style="float:left; height:100px"></div> > <div id="outer" style="margin-bottom:100px;"> > <div id="clear" style="clear:both; margin-top:99px"></div> > <div id="sib" style="margin-bottom:100px;"></div> > </div> > <div id="end" style="border:1px solid red"></div> > > In this case, when we try to determine clearance, the hypothetical box > position will still be 100px below the top, because the margin-bottom on 'sib' > collapses with the margins of 'clear'. Er, yes, indeed. In fact you'd have to change both of the margin-bottom:100px cases to 99px because until there is clearance, they would all collapse together and the total collapsed margin has to be less than 100px for clearance to be needed. My bad. If you're enjoying your time with floats, please feel free to take a stab at the other float bugs we have filed, or just to go through these tests: http://www.hixie.ch/tests/adhoc/css/box/block/margin-collapse/
Urrrgh. So given <div id="float" style="float:left; height:100px"></div> <div id="outer" style="margin-bottom:90px;"> <div id="clear" style="clear:both; margin-top:90px"></div> <div id="sib" style="margin-bottom:90px;"></div> </div> <div style="margin-top:90px"></div> ... <div style="margin-top:90px"></div> <div style="margin-top:100px"></div> <div id="end" style="border:1px solid red"></div> ... we have to wait indefinitely long before we can decide whether "clear" has clearance. That makes this considerably harder than I thought.
I guess we can compute the collapsed margin just by looking ahead at the following elements, without actually reflowing them, so that won't be so bad. I'm having trouble figuring out how to make this all work efficiently incrementally though. It's going to be really tough to fix all this without hurting Tp.
I believe the float should be pushed down below the margin, adjacent to the text. Currently we don't do that. Note that fixing this will be hard because of the following sort of situation: <div id="float" style="float:left; height:100px"></div> <div style="margin-top:90px"></div> ... <div style="margin-top:90px;"></div> <div style="margin-top:90px;"> <div style="margin-top:100px; clear:both;"></div> </div> Once we discover that clearance is required, we have to roll back all the way to where we put the float, and place it again, then fix up all the following lines. Ugh.
I wonder if this means it would be a good idea to make floats be children of, and reflowed by, the block with the space manager.
It's not indefinitely long, it's just until the end of the set of margins that are collapsing with each other at that point.
(which is rarely many steps, since you get very few empty elements generally)
> Note that fixing this will be hard because of the following sort of situation: > > <div id="float" style="float:left; height:100px"></div> > <div style="margin-top:90px"></div> > ... > <div style="margin-top:90px;"></div> > <div style="margin-top:90px;"> > <div style="margin-top:100px; clear:both;"></div> > </div> > > Once we discover that clearance is required, we have to roll back all the way > to where we put the float, and place it again, then fix up all the following > lines. Ugh. No, that float is not affected by later elements. A float between two adjacent margins is placed as if it had an empty element around it, with the top of that element defined as, _for the purposes of positioning floats_: # * If the element's margins are collapsed with its parent's top margin, the # top border edge of the box is defined to be the same as the parent's. # * Otherwise, either the element's parent is not taking part in the margin # collapsing, or only the parent's bottom margin is involved. The position # of the element's top border edge is the same as it would have been if the # element had a non-zero top border. Note how later margins never affect that.
(In reply to comment #49) > I wonder if this means it would be a good idea to make floats be children of, > and reflowed by, the block with the space manager. I've thought for a while that that might be better, although there are a few tricky issues.
> The position of the element's top border edge is the same as it would have been > if the element had a non-zero top border. OK, so our current rendering is actually correct. Thanks for reminding me about this. I'm finding it hard to keep it all straight in my head.
Attached file testcase (obsolete) —
Here are the testcase summarizing most of the issues raised in this bug. My current patch passes all of them.
1.8a4 passes none of them! except for the first one. Which is strange since that's the one I originally wanted to fix here. Oh well.
Attached patch work in progress (obsolete) — Splinter Review
Another checkpoint. This is pretty good, I think, for static layout. It still falls down badly in incremental reflow. In particular ReflowDirtyLines is nowhere near in sync with what ReflowBlockFrame is doing.
Attachment #159375 - Attachment is obsolete: true
To get clearance working properly with incremental reflow, I had to subsume the fix for bug 260938 into this patch.
Blocks: 260938
Attached patch diff -wSplinter Review
I think this patch is ready for review. I'm afraid it's another monster. I'll provide a detailed commentary soon. There are a few places that have scary performance implications.
Attachment #160705 - Attachment is obsolete: true
Attached file better testcase
I improved my previous testcase(s) and added testing of various dynamic changes when you hover over the testcases. My patch passes all these tests, as far as I can tell. Sometimes I've had a hard time figuring out if it's my code or my own evaluation of the test that's (in)correct :-).
Attachment #160701 - Attachment is obsolete: true
At some point I'll take that test case and make real test cases with actual pass conditions and so forth and add them to the CSS 2.1 test suite...
Whiteboard: [csswg] → (py8ieh: take testcase and make a real one)
Blocks: 262771
Blocks: 263190
Okay, here's an overview of this patch. The basic strategy for handling clearance and parent-child collapsing top margins is in nsBlockFrame::ReflowBlockFrame and is similar to what Hixie described. When we compute the collapsed top margin for a child block in a line L, we assume that none of the clearing elements have clearance. When those elements are actually reflowed we check to see whether they need clearance. If one does, then we complete its reflow as normal but we pass back (via nsHTMLReflowState::mDiscoveredClearance) a notification that this frame needed clearance. When the reflow for line L notices that this notification has occurred, it marks the frame as having clearance, recomputes the correct top margin, and repeats the reflow of the line. This approach doesn't require much rearchitecture and is quite flexible. In particular it does not rely on floats having the same size wherever they are positioned. If the optimistic reflow fails, then we have to revert the space manager state before we retry. But most of the time it won't fail. So we push the space manager state and if we don't fail, we call the new method nsSpaceManager::DiscardState which pops the state without actually restoring it. We have to add a bit to the line state to record whether the line has clearance. For lines whose top margin collapsed with an ancestor's, the clearance bit is managed by the topmost ancestor whose top margin collapses with the line. For all other lines, we compute the clearance bit when reflowing the block in nsBlockFrame::ReflowBlockFrame. nsBlockFrame::CheckForCollapsedBottomMarginFromClearanceLine applies the rule from CSS 2.1 section 8.3.1. Basically whenever we compute the final block size, we scan the line list backwards to see if a child block with clearance had its bottom margin adjacent to our bottom margin. If so, then we convert the carried-out bottom margin of the children into real vertical space which is added to the block height, and restrict the carried out bottom margin to just the block's own bottom margin. The comments in https://bugzilla.mozilla.org/show_bug.cgi?id=260938#c6 apply to this patch too. The combination of incremental reflow and clearance is really broken right now as you can see if you look at the incremental reflow bugs that have been exposed by the reflow-on-focus changes. Of course adding more complex clearing processing to fix this bug made the problem worse. The basic approach now is that whenever ReflowDirtyLines sees a frame with 'clear' set, we call ClearFloats to see what vertical position is required to for clearance, and we compare the results with the line's actual vertical position. If they're different, or the line didn't have clearance before but seems to need it now, or the line had clearance before and doesn't seem to need it now, then we reflow the line. The good news is that according to Martjin this fixes a number of existing clearance/incremental reflow bugs, and in my tests it takes care of even really complex clearance/margins/incremental reflow cases. The bad news is that when we reflow a block with a space manager (i.e. a containing block for floats), we must reflow any block with a child line which has 'clear' set --- and of course the block parents of such blocks, etc. That could potentially be bad for performance in some cases. I just don't see a way around it. We could add code to optimize this away in some restricted cases. To make it reasonably efficient I've added a new block state bit NS_BLOCK_HAS_CLEAR_CHILDREN which is set when the block has descendants with 'clear' not 'none'. It should probably be called NS_BLOCK_HAS_CLEAR_DESCENDANTS. The bit is set by ReflowDirtyLines. The interaction between BR-clearance and CSS clearance is a little unclear but I think what this code does is reasonable. To simplify things I've stopped accumulating BR-clearance across lines like we used to ... I'm not sure it was ever valuable. To simplify code and make things clearer, I consolidated the float-clearing stuff in nsBlockReflowState into a single ClearFloats method which does not change the state at all; it just returns what position is required to clear the floats. This avoids some confusing aState.mY juggling. + clearance = aState.mY - (currentY + topMargin); Note that this line computes exactly the actual CSS 2.1 clearance value for the block. Or so I claim :-). Note that ComputeCollapsedTopMargin tells us whether optimism is really needed in mayNeedRetry --- i.e. whether it encountered a 'clear' element that had to be assumed to have no clearance. If not (the common case I guess) then we can avoid a little bit of work. I've moved the call to nsBlockReflowContext::ComputeCollapsedTopMargin in nsBlockReflowContext::ReflowBlock out to its callers in nsBlockFrame::ReflowBlockFrame and nsBlockFrame::ReflowFloat, because that's where the retry loops are. This also required moving initialization of a block's nsHTMLReflowState out of nsBlockReflowContext::ReflowBlock into the callers. nsBlockReflowContext::ComputeCollapsedTopMargin has changed quite a bit. On the optimistic pass it unsets clearance on lines it encounters. On the second pass it sets clearance on the line that has been determined to need it. Whenever it changes the clearance on a line, it dirties the line and the ancestor lines it traversed, to make sure such lines get reflowed. I've changed nsBlockReflowContext::PlaceBlock to do margin collapsing when lines really are empty rather than just have zero height. Phew! The thing that most worries me about this is performance. We're definitely setting ourselves up for extra work, both in incremental reflow and in normal reflow. I really don't know what the impact will be but I'll be surprised if Tp doesn't feel it. OTOH I can't predict right now what would be the right way to optimize this better.
Comment on attachment 160890 [details] [diff] [review] diff -w Sorry David, but might as well get this on the radar.
Attachment #160890 - Flags: superreview?(dbaron)
Attachment #160890 - Flags: review?(dbaron)
Blocks: 265860
(In reply to comment #34) > I'm not sure about the meaning of hypothetical constructions like the one used > here: > > # Computing the clearance of an element on which 'clear' is set is done by > # first determining the hypothetical position of the element's top border edge > # within its parent block. This position is determined after the top margin of > # the element has been collapsed with previous adjacent margins (including the > # top margin of the parent block). > > The word "after" implies that layout is computed in some order determined by > the spec, but that's not really so. You assume it's an outside-in model, and > therefore we detect clearance on parents before children, which is natural > enough, but as far as I know it's not part of the spec. So it seems unclear > exactly what constraints are to apply in the hypothetical world we construct > to detect clearance. So I raised this with the CSS working group, and pretty much everyone here told me that the layout was done in document order and even quoted parts of the spec to that effect (although I can't find them now), so the working group didn't want to add the sentence I proposed ("The hypothetical position of an element is calculated assuming that all later elements have no clearance."). I can try pushing harder if you want. Should I?
No, I can deal, although I don't believe that "done in document order" is really a well-defined concept when you have intrinsic sizing.
The sentence that I thought already covered the issue was in 9.5.2: This position is determined after the top margin of the element has been collapsed with previous adjacent margins (including the top margin of the parent block). (*previous* adjacent margins)
Blocks: 266454
Blocks: 267274
Blocks: 267116
Blocks: 268644
Comment on attachment 160890 [details] [diff] [review] diff -w I really wanted to review this in more detail, since I did the last rewrite of margin collapsing, but I don't think that's going to happen. So rubber-stamp r+sr=dbaron. Sorry for the delay. >+PRBool >+nsBlockFrame::CheckForCollapsedBottomMarginFromClearanceLine() >+{ >+ for (line_iterator line = --end_lines(), begin = begin_lines(); >+ line != begin; --line) { This doesn't check the first line. Should it?
Attachment #160890 - Flags: superreview?(dbaron)
Attachment #160890 - Flags: superreview+
Attachment #160890 - Flags: review?(dbaron)
Attachment #160890 - Flags: review+
Yes it should. I'll fix that. BTW I mentioned in email that the performance impact of this bug might be mitigated somewhat if we cache the IsEmpty() status of frames. I'd like to do that before checking this in; Boris and I have a plan, but it depends on (or at least would conflict grossly with) the work in bug 261064...
Blocks: 263966
Blocks: 265342
Blocks: 268894
Blocks: 271433
Checked in. Mimimal Tp damage on btek, *perhaps* a 5-10ms loss. I'll keep an eye on the other tboxes.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
There's about a 0.5% Tp loss across the board, plus maybe 2% hit on Tdhtml. It's unfortunate, but some loss was inevitable, we simply have to do more work to check that blocks with clearance are correctly positioned after incremental reflow.
These are my observations on the dependencies, using a 2004-11-25 trunk build, which has the fix: Bug 204831, partly fixed, second testcases still shows a bug in Mozilla, probably Bug 224057, fixed Bug 260938, fixed Bug 262538, fixed Bug 262771, fixed Bug 263190, fixed Bug 263966, fixed Bug 265342, fixed Bug 265860, not fixed Bug 266454, fixed Bug 267116, fixed Bug 267274, fixed Bug 268644, fixed Bug 268894, I don't know Bug 271433, fixed There are some more bugs here and there, that are probably fixed by the fix for this bug. I'll look for these tomorrow.
Thanks again Martjin. Please mark those fixed bugs as fixed.
Blocks: 261153
Blocks: 262104
Blocks: 264628
Blocks: 262027
Blocks: 262347
Blocks: 262420
Blocks: 262576
No longer blocks: 268894
This bug is in 1.8A5. Requesting "blocing 1.8a6" flag status
Flags: blocking1.8a6?
The fix is checked in. What is the point of making this block 1.8a6?
Flags: blocking1.8a6?
Blocks: 273193
Hmm. In hindsight, comment 73 bear all tokens of a Flatus Cerebralis - a fart of the brain, if you will. Sorry about that.
Depends on: 273293
Depends on: 273946
Depends on: 274249
Blocks: 220114
Depends on: 282714
Depends on: 309550
Depends on: 320378
Depends on: 326085
Depends on: 375462
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: