- From: Anton Prowse <prowse@moonhenge.net>
- Date: Mon, 27 Feb 2012 04:21:35 -0500
- To: "www-style@w3.org" <www-style@w3.org>
- Cc: "Arron Eicholz" <Arron.Eicholz@microsoft.com>
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