Re: [CSS21] Margin collapsing spec bugs (was: Response to wiki comments on my major comments post)

On 15/03/2011 23:13, L. David Baron wrote:
> On Tuesday 2011-03-15 22:02 +0100, Anton Prowse wrote:
>> I'm responding to the comments presented on
>> http://wiki.csswg.org/spec/css2.1/anton-lc-2010 with regard to my
>> major comments post
>> [http://lists.w3.org/Archives/Public/www-style/2010Dec/0312.html].
>>
>> (MC1)-----------------------------------------------------------
>> wiki:"If the top and bottom margins of an element with clearance are
>> adjoining, they always collapse, so the change is not necessary."
>>
>> Yes, but the margins can also collapse when they are *not*
>> adjoining, since adjoiningness is intransitive but collapsing isn't,
>> so the change is necessary else a case is missed.
>>
>> wiki:"The suggestion to add “all of its in-flow children's margins
>> (if any) collapse.” seem redundant with the existing statement that
>> “A collapsed margin is considered adjoining to another margin if any
>> of its component margins is adjoining to that margin.”"
>>
>> I don't understand this statement, since that wasn't at all what I
>> suggested.  My suggestion concerned a rather more complex issue.
>> I'd appreciate a re-review of this issue!
>
> Is this the same as Issue 211, i.e.,
> http://lists.w3.org/Archives/Public/www-style/2010Sep/0451.html ?
> While I'd like to fix this issue, my argument for fixing it was
> substantially weakened by the fact that fixing it doesn't agree with
> any implementations.

Issue 211 concerns the "transitivity bug" together with your warning 
that the proposed new wording for 8.3.1 (Issue 159) shouldn't 
unintentionally "fix" the bug without appropriate WG discussion and 
resolution (which it doesn't in its latest incarnation).

The transitivity bug is that the current spec contains an internal 
contradiction as to whether the top margin of an auto-height, non-zero 
computed min-height element that collapses with the element's last 
in-flow child's bottom margin also collapses with the element's own 
bottom margin.  This bug arose from a poor resolution to a different 
issue (Issue 79, of which, more below) which resulted in mandating that 
the last in-flow child collapse its bottom margin with the element's 
bottom margin irrespective of what min-height is set to, with the 
consequence that, by transitivity of margin collapsing, the element's 
top and bottom margins collapse in a situation other than those 
explicitly permitted in the spec.

[Note, however, that sloppy wording in 8.3.1 means that I'm not 
convinced that the transitivity bug even exists in the current spec, 
although I think that's actually accidental!  Issue 79 was raised 
because people didn't like the discontinuity that arises when min-height 
transitions from having no effect to having an effect; the parent's 
height is discontinuously increased due to suddenly not collapsing with 
its last child's bottom margin and having to enclose that margin 
instead.  The intent of the resolution was to ensure that margins still 
collapse even when min-height has an effect. 
[http://lists.w3.org/Archives/Public/www-style/2008Nov/0473.html]. 
However, the actual effect of the resolution ensures that margins don't 
collapse when min-height has an effect and instead the child's margin 
overflows.  In other words, the resolution moved from one type of 
discontinuity to another.  This is specifically due to the fact that the 
8.3.1 paragraph concerning bottom margin collapsing between parent and 
child only applies when there's a "'height' of 'auto'" and so it doesn't 
actually come into play when min-height has an effect, since as part of 
determining the parent height in such a situation you have to assume 
that the computed value of height is the same as min-height, ie 
non-auto, and since height is not auto its content including its child's 
margin overflows.  I *think* that's an accident though, and that 79 
resolution really did want margin collapsing to occur but the 
relationship between 10.7 and 8.3.1 wasn't made sufficiently clear. 
Indeed, a note was placed in 10.7 which tries to say something about 
whether you should be treating height as either auto or equal to 
min-height as regards margin collapsing, but that note is actually 
impenetrable.  Even if _nothing_ happens to address any of the margin 
collapsing issues now, that note and/or the paragraph in 8.3.1 need to 
be clarified.  For the rest of this post, I'm going to assume that the 
resolution to Issue 79 did what it set out to do.]

It appears that you wanted to fix up the proposed new text by making the 
paradoxical case explicitly undefined.  You say that that doesn't agree 
with implementations; were you referring to the fact that there seems to 
be interop amongst all but Gecko to support the resolution of Issue 79? 
(The observation that this not-quite-interop exists is Issue 234.)  I 
don't think it can be said that your fix doesn't "agree", since the 
transitivity bug is a logical bug that exists in the spec regardless of 
what implementations actually do; we *must* fix it somehow, even if by 
making certain cases undefined as you suggest.

[When you said in your post for Issue 211 that "I think this also needs 
to say that 'min-height' is nonzero, but that's a fix to a transitivity 
bug existing in the current spec", I'm assuming you meant to say "but 
that's to match the current spec which says that collapsing occurs 
(thanks to the resolution of Issue 79) due to the auto height 
irrespective of the positive min-height", but in fact that particular 
proposed change (which doesn't actually fix the transitivity bug, unlike 
your "make undefined" proposal) to the mooted new wording of 8.3.1 would 
mean that the bottom margin of a last in-flow child and bottom margin of 
its parent would be non-adjoining (and hence non-collapsing) if the 
parent has 'auto' height and zero min-height which is both obviously 
undesirable and trivially false in practice.]

Whilst your issue concerns non-zero min-height, mine concerns zero 
min-height.  The proposed new wording implies that for a box to be 
self-collapsing then it must have zero min-height and either have zero 
height and no in-flow children or auto height and self-collapsing 
children.  However, the existing spec says (modulo the transitivity bug) 
that it's enough that the box to have zero min-height, zero or auto 
height, and self-collapsing children.  I gave an example demonstrating 
that these two situations are not equivalent; integer height (even '0') 
is enough to inhibit margin collapsing between the last child and the 
parent's bottom margin in the proposed new wording, whereas it is not in 
the existing spec.  Hence my issue is that the proposed new wording for 
8.3.1 is not faithful to the existing spec, whereas your issue is that 
the both the existing spec and the proposed new wording contain the 
transitivity bug.

Hence I don't believe our issue to be the same.

> (It's also related to issue 234.)

Indeed.  Issue 234 is the observation that Gecko fails to collapse the 
bottom margin of a child when the auto-height parent's min-height 
exceeds the used height of the child as required by the flawed 
resolution to Issue 79, whereas other UAs do.  Hence 234 is more or less 
the same issue as Issue 79.  Even ignoring the fact that the resolution 
of 79 introduced the transitivity bug, it was a bizarre change anyway: 
it says that margin collapsing can occur even when the min-height so 
large that the last child doesn't "reach" the parent's bottom content 
edge which seems typographically dubious in many cases even through I 
can see why it might have seems desirable in certain cases (generally 
those in which "partial margin collapsing" could occur).

So Issue 79/234 is different again from either your issue or my issue, 
but it did directly cause your issue.

> The testcase was:
> http://software.hixie.ch/utilities/js/live-dom-viewer/?%3C!DOCTYPE%20html%3E%0A%3Cstyle%3E%0A.one%2C%20.three%20%7B%20min-height%3A%202px%20%7D%0A.one%2C%20.two%20%7B%20margin-bottom%3A%201em%20%7D%0A.three%20%7B%20margin%3A%201em%200%7D%0A%3C%2Fstyle%3E%0AShould%20have%20a%201em%20gap%3A%3Cbr%3E%0AHello%0A%3Cdiv%20class%3D%22one%22%3E%3Cdiv%20class%3D%22two%22%3E%3C%2Fdiv%3E%3C%2Fdiv%3E%0AWorld%0A%3Chr%3E%0AShould%20have%20a%201em%20gap%3A%0A%3Cdiv%20class%3D%22one%22%3E%3Cdiv%20class%3D%22two%22%3EHello%3C%2Fdiv%3E%3C%2Fdiv%3E%0AWorld%0A%3Chr%3E%0AShould%20have%20a%202em%20gap%3A%0AHello%0A%3Cdiv%20class%3D%22three%22%3E%3C%2Fdiv%3E%0AWorld
> and the proposal would have changed the specification's requirement
> for the first of the three gaps mentioned in the testcase from 1em
> to 2em.

Sorry, which proposal for which issue would result in that change?  (The 
change would arise as the result of margin collapsing being prevented 
when min-height has an effect, which I believe is what the spec used to 
require before the resolution of Issue 79.)

> (Gecko's behavior, unfortunately, needs to be ignored, since I
> *think* it breaks collapsing based on whether the min-height had an
> effect.  I believe we looked at a testcase earlier in the day that
> showed that.

Assuming we're talking about Issue 79/234, then Gecko's behaviour is 
actually the only sensible one in many cases, right?  Surely we want to 
break collapsing in at least some cases where min-height had an effect 
(ie was "large enough")?  That is, surely we want to revisit Issue 79 
and reintroduce the idea that margin collapsing is inhibited when 
min-height has an effect, since it's typographically nonsensical to 
collapse when the min-height is significantly larger than the content.

> If that's incorrect, then we'd still only have one implementation
> for the desired behavior.)

Sorry, I'm struggling to follow!  If what's incorrect?  What is the 
desired behaviour, and which implementation demonstrates it?

> The raw IRC log of the discussion is in
> http://krijnhoetmer.nl/irc-logs/css/20110307#l-450
> (Unfortunately the W3C copy of the IRC log has permissions set
> incorrectly, so even I can't get to it.  In theory it should be in
> http://www.w3.org/2011/03/07-css-irc.html around 22:04:53 (search
> for "http://wiki.csswg.org/spec/css2.1#issue-211").)

Bert summarized the consequence of transitivity bug well: "Say you have 
an element with a 'min-height'. Its margins don't collapse. Now add an 
empty child. Now its margins do collapse."  And - and this is the bug - 
if its margins now collapse, how is that pathological collapsed margin 
supposed to manifest itself?  At the top of the content area box?  At 
the bottom?  Both?

This situation is farcical, and I have no idea how that resolution for 
Issue 79 got through.  Ironically, that issue is recorded as having been 
sparked by Boris Zbarsky's post 
http://lists.w3.org/Archives/Public/www-style/2008Sep/0101.html which 
successfully _defended_ the original spec ("no collapsing when 
min-height is too big"), explaining why that behaviour is an acceptable 
compromise using the sensible argument that if you have fixed heights 
(sometimes via the use of min-height) then you can't then also expect to 
have meaningful margin collapsing between elements and their last 
in-flow children (and nor would you want it in many cases).  Rather than 
accept that argument, Issue 79 was actually raised on the basis of that 
post and was resolved to break the behaviour!  [I can see an argument 
for breaking that behaviour in favour of "partial margin collapsing" 
where appropriate (and in fact I wish for margin collapsing myself in 
certain other situations unrelated to min-height; details will follow at 
some future point!), but that's a big conceptual change from what we 
have in CSS21.]

To fix this mess, Issue 79 needs to be re-opened.  (Issue 234 is merely 
the observation that most but not all UAs honour the resolution to Issue 
79.)  What does it even mean to honour Issue 79 in the case of the 
transitivity bug?  What do they actually *do* with the pathological 
collapsed margin in the case that Bert describes?  That needs to be 
specified or made undefined.  Issue 211 is simply the request that Issue 
159 (the proposed new wording for 8.3.1) doesn't pretend the problem 
doesn't exist, and it suggests making the case undefined.  My issue is 
not a discovery of any new bug in the spec but rather is the observation 
that new wording for Issue 159 is unfaithful to the current spec in yet 
another way.  (The transitivity bug still exists in the proposed new 
wording even if my issue is fixed.)

Cheers,
Anton Prowse
http://dev.moonhenge.net

Received on Wednesday, 16 March 2011 20:44:00 UTC