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

On Monday, February 27, 2012 12:52 PM Anton Prowse wrote:
> 
> On 23/02/2012 20:44, Arron Eicholz wrote:
> 

I snipped out Proposal 1 to focus on the updated proposals. I have added proposal 5 which is the culmination of my comments from this mail. This will eliminate the need to talk about Proposal 2 and 4 in the next mail. Also my other comments will allow you to cut our Proposal 3 too I think.

> 
> > -----------------------------------
> > 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.
> 

It's funny the more we work on this particular bug the more I think we just shouldn't say anything about margin collapsing here. Margin collapsing explicitly uses the term 'computed' value all throughout the margin collapsing section. That means that restating it in the min/max-height section makes it unnecessary and somewhat confusing.

We could simplify this by just saying:
Option 1 (super simple)
| These steps do not affect the computed values of the above properties.

This would simplify things greatly and eliminate the connection to margin collapsing that really isn't there in any way because min/max-height do not affect computed values. If we want to tie margin collapsing with this note then I think we should just say, "See Margin collapsing (8.3.1) on how these properties may interact with the margin collapsing model."

Option 2 (simple and passing off to margin collapsing)
| These steps do not affect the computed values of the above properties. 
| See Margin collapsing (8.3.1) on how these properties may interact with the 
| margin collapsing model.

If we keep it simple or just say go look at margin collapsing then we break up the complication and confusion and put the ownership of the explanation in one place where it can be complete and not disjointed across different sections of the spec.

> 
> > =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.
> 

It is a little strange but we already do it with other rules. Specifically the 'except' rules all may contradict the collapse of margins that are adjoining.

I think what really needs to happen is we have to flip the order of the 2 sections. The "Two margins are adjoining if and only if:" should come first and the "Adjoining margins collapse, except:" section should come next. Just switch the order.

> 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
> 

I believe you meant rule 4 not rule 3?

> 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.
> 

I still think this is incorrect and why my last proposal covered what it did. Let me explain.

Let A and D be 2 adjoining margins. Let us add a box with adjoining top and bottom margins, B and C, between A and D.

It seems inconsistent that adding the adjoining margins of B and C would break the transitive adjacency of A and D.

I think it is better to have adjacency rules consistent and have the 'collapse except rules' cause lack of collapse in this scenario, than have the inconsistent adjacency rules enforce the lack of collapse.

> > =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.

Great, so I think we have this bug solved since we both agree.

> 
> > -----------------------------------
> > 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.)
> 

Yeah this is a complicated case and I was trying to define Gecko's behavior. I think having the correct understanding of Gecko's behavior is good but maybe we can just eliminate it now due to its complications and something we really don't want to specify.

> > =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.
> 

As I stated above, Proposal 3 is intending to spec and define Firefox's behavior which is completely different behavior. It has some complications to it and I don't think I even defined it completely correct. I personally think this proposal is dead on arrival since it would be defining a behavior that fully includes min/max-height into the Margin collapsing behavior to the point that they change if collapsing happens or not.

> 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.)
> 
 
Yes I agree it is another bug that should address that. I think we just need to say that margin collapsing only ever deals with computed values of all properties or something like that.


Proposal 5:
We are getting closer to a solid proposal.

=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:
Option 1:
| These steps do not affect the computed values of the above properties.

Or:
Option 2:
| These steps do not affect the computed values of the above properties. 
| See Margin collapsing (8.3.1) on how these properties may interact with the 
| margin collapsing model.

I also removed the word 'real' since 'real computed values' isn't a defined term.

=Bug 16036=
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.

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

Also switch the order of "Two margins are adjoining if and only if:", and "Adjoining vertical margins collapse, except:" sections. This would make more sense because you would first talk about adjoining margins and then talk about if they are adjoining what rules prevent them from collapsing. The order they are in now uses the term adjoining (in the title of the paragraph "Adjoining vertical margins collapse, except:") before you define how adjoining really works. This is why I think the order is weird.

=Bug 16037=
Same as before since I think we have settled on this issue:
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.

--
Thanks,
Arron Eicholz

Received on Tuesday, 28 February 2012 00:15:57 UTC