RE: [CSS21] Errata - proposals for review - Margin collapsing, I

Thanks for the review, Arron!  I've added my comments and a revised
proposal below.

On 23/02/2012 20:44, Arron Eicholz wrote:

> Proposal 1 (Anton's current proposals)
> =Bug 16049=
> Replace:
>     # These steps do not affect the real computed values of the above #
properties. The change of used 'height' has no effect on margin #
collapsing except as specifically required by rules for
>     # 'min-height' or 'max-height' in "Collapsing margins" (8.3.1).
> with:
>     | These steps do not affect the real computed values of the above |
properties. The change of used 'height' has no effect on margin |
collapsing; the real computed value of 'height' is used to |
determine which margins are adjoining in "Collapsing margins" |
(8.3.1) during these steps.
> =Bug 16036=
> Replace:
>     #   - bottom margin of a last in-flow child and bottom margin of its
#     parent if the parent has 'auto' computed height
> with:
>     |   - bottom margin of a last in-flow child and bottom margin of its
|     parent if the parent has 'auto' computed height and either the
|     parent has a zero computed min-height or the bottom margin of
|     the last in-flow child does not collapse with the top margin
of
>     |     the parent
> =Bug 16037=
> Replace:
>     # The bottom margin of an in-flow block box with a 'height' of
'auto'
>     # and a 'min-height' of zero collapses with its last in-flow #
block-level child's bottom margin if the box has no bottom padding #
and no bottom border and the child's bottom margin does not
collapse
>     # with a top margin that has clearance.
> with:
>     | The bottom margin of an in-flow block box with a 'height' of
'auto'
>     | collapses with its last in-flow block-level child's bottom margin
if
>     | the box has no bottom padding and no bottom border and the child's
| bottom margin neither collapses with a top margin that has
clearance
>     | nor (if the box's min-height is non-zero) with the box's top
margin.
> Pros:
> - Matches the functionality of Webkit, Opera and Internet Explorer - No
current CSS 2.1 test cases affected
> Cons:
> - Does not match Firefox
> - Is not consistent with how authors think min-height and height work
together
> - Mixes concepts of margin collapsing and adjacency in the updated text
- Complicated to read and understand


> -----------------------------------
> Proposal 2 (Arron's proposals):
> This proposal keeps the logic of "Proposal 1" the same but separates the
concepts of collapsing and adjacency which we are trying to do in the
spec. This also simplifies some of the working to make the topics easier
to read without sacrificing necessary logic.
> =Bug 16049=
> Replace:
>     # These steps do not affect the real computed values of the above #
properties. The change of used 'height' has no effect on margin #
collapsing except as specifically required by rules for
>     # 'min-height' or 'max-height' in "Collapsing margins" (8.3.1).
> with:
>     | These steps do not affect the real computed values of the above |
properties. The change of used 'height' has no effect on margin |
collapsing.

I agree that removing the clause is better than leaving the current one in
place, but for me the problem is that I don't find it clear what the
second sentence actually means; that's why I added an explicit explanation
in Proposal 1 above.  Perhaps I just have a mental block about it, since
others seem not to have a problem with it!

I believe the sentence is trying to reinforce the following subtle point:
in steps 2 and 3, we are not performing entire layout again with a
different /computed/ value of 'height' (which would affect the margin
collapsing relationships, and could potentially affect other aspects of
layout as well since most layout rules throughout the spec revolve around
computed value - although I don't think there actually are any other
aspect that would be affected in CSS21).  Instead, we are just performing
the rules of, specifically, 10.6 again with a substituted computed value
of 'height', which might result in different used values of 'margin-top',
'margin-bottom', 'top' and 'bottom' but nothing else AFAICT.

Technically the sentence isn't needed; it's just a Note, after all.  But I
do think that some sort of comment is useful.  Perhaps the problem I have
with the current formulation is that it says "change of used 'height'",
but it's not the used height that's actually changing; the used height is
the final outcome after three calculations.

How about:

  | These steps do not affect the real computed values of the above |
properties. _Collapsed margins_<link to 8.3.1> remain unchanged | for
each step, since the real computed value of 'height' is used | when
determining which margins collapse.


> =Bug 16036=
> Replace:
>      #   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
> with:
>      |   top and bottom margins of a box that does not establish a new |
  block formatting context and zero or 'auto' computed 'height', | 
 and no in-flow children
> Add a new bullet to:
> "Adjoining vertical margins collapse, except:"
>     | - The top and bottom margins, of a box with a 'min-height' value
that
>     |    does not compute to zero, do not collapse. If the bottom
margin, of such
>     |    a box's last inflow-child, collapses with the box's top margin
then the
>     |    last inflow-child's bottom margin does not collapse with its
parent
>     |    box's bottom margin.

I agree with your observation that my proposal uses the concept of margin
collapsing in the specification of adjacency, and that in fact it would be
better to keep them separated.

However, it seems rather perverse to define two margins as adjoining which
intuitively aren't, only to then immediately prevent them from collapsing.

How do you feel about the following alternative:

Do not modify:
  #   - bottom margin of a last in-flow child and bottom margin of its #  
  parent if the parent has 'auto' computed height

but add a new bullet to:
  # "Adjoining vertical margins collapse, except:"

  | - If the top margin of a box with non-zero computed 'min-height' |  
and 'auto' computed 'height' collapses with the bottom margin of |   its
last in-flow child, then the child's bottom margin does not |   collapse
with the parent's bottom margin.

> =Bug 16037=
> Replace:
>     # The bottom margin of an in-flow block box with a 'height' of
'auto'
>     # and a 'min-height' of zero collapses with its last in-flow #
block-level child's bottom margin if the box has no bottom padding #
and no bottom border and the child's bottom margin does not
collapse
>     # with a top margin that has clearance.
> This proposal I think work better written as a list of sub-bullets. with:
>     | - The bottom margin of an in-flow block box with a 'height' of
'auto'
>     |    collapses with its last in-flow block-level child's bottom
margin, if:
>     |        - the box has no bottom padding, and
>     |        - the box has no bottom border, and
>     |        - the child's bottom margin neither collapses with a top
margin
>     |           that has clearance, nor (if the box's min-height is
non-zero)
>     |           with the box's top margin.

I completely agree here.  The sentence was already hard to read, and with
the extra clause it becomes downright convoluted.  The list approach works
better.

> Pros:
> - Matches the functionality of Webkit, Opera and Internet Explorer - No
current CSS 2.1 test cases affected
> - Adjacency and margin collapsing are still separate sections within
>   margin collapsing's normative text
> - Much easier to read and understand
> Cons:
> - Does not match Firefox
> - Is not consistent with how authors think min-height and height work
together


> -----------------------------------
> Proposal 3
> This proposal is more in line with what I can gather Firefox does. I
hope I have captured the correct proposal here. I think it's close but
in my opinion if we are going for the proposal with the most interop
then this proposal might not be worth discussing.
> =Bug 16049=
> Replace:
>     # These steps do not affect the real computed values of the above #
properties. The change of used 'height' has no effect on margin #
collapsing except as specifically required by rules for
>     # 'min-height' or 'max-height' in "Collapsing margins" (8.3.1).
> with:
>     | These steps do not affect the real computed values of the above |
properties.
> =Bug 16036=
> Replace:
>     #   - bottom margin of a last in-flow child and bottom margin of its
#     parent if the parent has 'auto' computed height
> with:
>     |   - bottom margin of a last in-flow child and bottom margin of its
|     parent if the parent has 'auto' computed height and the |    
parent has a computed min-height that is less than or equal |     to
the parent's used height

The trouble here is that you don't know the used height until you've
completed the three steps in 10.7, yet at least the first step requires
you to know how the margin collapsing works.

Is this proposal trying to make these two margins adjoining if and only if
min-height doesn't "have an effect"?  Several people in the WG said that
they didn't want to introduce a condition like that, although I recall
dbaron mentioning somewhere that that is indeed what Gecko does.  (Bear in
mind that we'd have to introduce a cycle breaker into the spec if we
wanted that behaviour.)

> =Bug 16037=
> Replace:
>     # The bottom margin of an in-flow block box with a 'height' of
'auto'
>     # and a 'min-height' of zero collapses with its last in-flow #
block-level child's bottom margin if the box has no bottom padding #
and no bottom border and the child's bottom margin does not
collapse
>     # with a top margin that has clearance.
> with:
>     | - The bottom margin of an in-flow block box with a 'height' of
'auto'
>     |   and a 'min-height' of zero collapses with its last in-flow
block-level
>     |  child's bottom margin, if:
>     |        - the box has no bottom padding, and
>     |        - the box has no bottom border, and
>     |        - the child's bottom margin does not collapse with a top
margin
>     |           that has clearance
> Pros:
> - Matches the functionality of Firefox
> - More consistent with how authors think min-height and height work
together
> - Adjacency and margin collapsing are still separate sections within
>   margin collapsing's normative text
> - Much easier to read and understand
> - No current CSS 2.1 test cases affected
> Cons:
> - Does not match Webkit, Opera and Internet Explorer

So Proposal 3 tries to achieve a different behaviour.  Leaving that to one
side for the moment, here's a Proposal 4 describing the same behaviour as
Proposals 1 and 2, based on my comments above.

Proposal 4:

=Bug 16049=

Replace:
  # These steps do not affect the real computed values of the above #
properties. The change of used 'height' has no effect on margin #
collapsing except as specifically required by rules for
  # 'min-height' or 'max-height' in "Collapsing margins" (8.3.1).

with:

  | These steps do not affect the real computed values of the above |
properties. _Collapsed margins_<link to 8.3.1> remain unchanged | for
each step, since the real computed value of 'height' is used | when
determining which margins collapse.

=Bug 16036=

Add a new bullet to:
  # "Adjoining vertical margins collapse, except:"

  | - If the top margin of a box with non-zero computed 'min-height' |  
and 'auto' computed 'height' collapses with the bottom margin of |   its
last in-flow child, then the child's bottom margin does not |   collapse
with the parent's bottom margin.

=Bug 16037=

Replace:
  #  - The bottom margin of an in-flow block box with a 'height' of #   
'auto' and a 'min-height' of zero collapses with its last in-flow #   
block-level child's bottom margin if the box has no bottom
  #    padding and no bottom border and the child's bottom margin does #  
 not collapse with a top margin that has clearance.

with:
  | - The bottom margin of an in-flow block box with a 'height' of |  
'auto' collapses with its last in-flow block-level child's bottom |  
margin, if:
  |      - the box has no bottom padding, and
  |      - the box has no bottom border, and
  |      - the child's bottom margin neither collapses with a top margin |
       that has clearance, nor (if the box's min-height is non-zero) |  
     with the box's top margin.

(The 'height' and 'min-height' being talked about here are /computed/
values; that should be the subject of another bug, I guess.)


>> =====================
>> Bug 16072 - Margin collapsing: clarification needed as to where a
collapsed margin manifests itself
> I would like to handle this issue as a separately from the other issues
above and I will send a separate email addressing this issue.

OK.

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

Received on Monday, 27 February 2012 09:22:09 UTC