616339 - min-height and max-height should not influence whether margins collapse

Status: NEW

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

Description

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

According to CSS 2.1, the max-height property should not affect whether margins collapse (in particular, the bottom margin of an element with max-height with the bottom margin of its last child).  We incorrectly consider max-height, which makes us fail:
http://test.csswg.org/suites/css2.1/20101001/html4/margin-collapse-038.htm

Comment 1

[Daniel.S]

The current definition is a result of CSS 2.1 Issue 79
http://wiki.csswg.org/spec/css2.1#issue-79

The resolution to that issue lifted the restrictions for min-height as well,
so I think one bug is enough.
(I might add that the current definition leads to some intriguing and unexpected results).

Comment 2

[Gérard Talbot]

Updating URL field to:
http://test.csswg.org/suites/css2.1/latest/html4/margin-collapse-038.htm

Comment 5

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

I have a patch that I believe at least partly fixes this:
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/4da92b976cd1/max-min-height-margin-collapse

It does in fact fix all of the margin-collapsing reftests marked as failing due to this bug.

However, the patch causes the failure of these two tests:
http://mxr.mozilla.org/mozilla-central/find?string=block-no-content-8

I believe both the old and new behavior of that test are incorrect, because all of the margins of #zero-height and all of the margins of #min-height should collapse together:

 1. the top margin of #min-height should collapse with the top margin of #zero-height due to the "top margin of a box and top margin of its first in-flow child" rule in http://dev.w3.org/csswg/css2/box.html#collapsing-margins

 2. the top-margin of #zero-height and the bottom margin of #zero-height should collapse due to the "top and bottom margins of a box that does not establish a new block formatting context and that has zero computed 'min-height', zero or 'auto' computed 'height', and no in-flow children" rule

 3. the bottom margin of #zero-height and the bottom margin of #min-height should collapse due to the "bottom margin of a last in-flow child and bottom margin of its parent if the parent has 'auto' computed height" rule

This in turn means that the top and bottom margins of #min-height should collapse together despite the min-height, due to the vaguely-worded rule "A collapsed margin is considered adjoining to another margin if any of its component margins is adjoining to that margin." which I believe would be improved by the changes proposed in https://lists.w3.org/Archives/Public/www-style/2010Jul/0507.html or https://lists.w3.org/Archives/Public/www-style/2012Mar/0057.html .

This, in turn, means that the lower gap in the test should be neither the 40px that it is before the patch or the 60px that it is after the patch, but instead -20px.  (I'm not sure if that behavior is Web-compatible, though; Gecko and Chrome currently match, but I haven't tested IE.)

Does that analysis seem correct?

Comment 6

[Daniel.S]

(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #5)
> It does in fact fix all of the margin-collapsing reftests marked as failing
> due to this bug.

Great :)

> This, in turn, means that the lower gap in the test should be neither the
> 40px that it is before the patch or the 60px that it is after the patch, but
> instead -20px.  (I'm not sure if that behavior is Web-compatible, though;
> Gecko and Chrome currently match, but I haven't tested IE.)

Both IE and the old Presto engine match with Gecko/Chrome as well.

> Does that analysis seem correct?

I think you missed the line (with annotations):
> "Adjoining vertical margins collapse, except: (...)
> * If the top margin of a box with non-zero computed 'min-height' and 'auto' 
> computed 'height' (#min-height) collapses with the bottom margin of its 
> last in-flow child (#zero-height), then the child's bottom margin does 
> not collapse with the parent's bottom margin."

At least, that's what I had in mind when I created this test.

The top margins of #min-height as well as the bottom margin of #zero-height collapse to a top margin of 60px and the sole margin-bottom of #min-height remains at 40px.

Comment 7

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

(In reply to Daniel.S from comment #6)
> I think you missed the line (with annotations):
> > "Adjoining vertical margins collapse, except: (...)
> > * If the top margin of a box with non-zero computed 'min-height' and 'auto' 
> > computed 'height' (#min-height) collapses with the bottom margin of its 
> > last in-flow child (#zero-height), then the child's bottom margin does 
> > not collapse with the parent's bottom margin."
> 
> At least, that's what I had in mind when I created this test.

But it's not possible for those two margins (A and D) not to collapse, if A collapses with B, B collapses with C, and C collapses with D.

That's what's implied by the sentence quoted after my 1/2/3/ bullet points in comment 5 says.

If it meant something different, it would need to say that other sets of margins also don't collapse.

Comment 8

[Daniel.S]

(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #7)
> (In reply to Daniel.S from comment #6)
> > I think you missed the line (with annotations):
> > > "Adjoining vertical margins collapse, except: (...)
> > > * If the top margin of a box with non-zero computed 'min-height' and 'auto' 
> > > computed 'height' (#min-height) collapses with the bottom margin of its 
> > > last in-flow child (#zero-height), then the child's bottom margin does 
> > > not collapse with the parent's bottom margin."
> > 
> > At least, that's what I had in mind when I created this test.
> 
> But it's not possible for those two margins (A and D) not to collapse, if A
> collapses with B, B collapses with C, and C collapses with D.

Yes, yes, no.

> That's what's implied by the sentence quoted after my 1/2/3/ bullet points
> in comment 5 says.

I think your sentence is a definition that says in which case margins are collapsing. So far you are right, A with B, B with C and C with D.

However, the sentence I quoted is aware that these margins are adjoining. The introductory line says: "Adjoining vertical margins collapse, except:". In my words: Yes, these margins are adjoining, but they still don't collapse because one of these bullet points is true: And the last bullet point describes the situation in the test case.