-moz-column-count on parent element with border causes its margins to collapse with the kids' margins

RESOLVED FIXED in mozilla17

Status

()

Core
Layout
RESOLVED FIXED
7 years ago
3 years ago

People

(Reporter: GPHemsley, Assigned: mats)

Tracking

(Depends on: 2 bugs, Blocks: 2 bugs, {css3, testcase})

Trunk
mozilla17
css3, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 1 obsolete attachment)

(Reporter)

Description

7 years ago
Created attachment 495268 [details]
Reduced testcase

Not sure if this is related to bug 388666.

If you have a parent element (e.g. <div>) with '-moz-column-count' set to something higher than 0, the margins of the (last?) child element (e.g. <p>) will bleed through, even if the parent element's margins are 0.

This does not appear to happen with Safari (which, of course, uses '-webkit-column-count').
Yeah, this seems like a bug.

Presumably the problem here is that the border ends up on the columnset, not on the columns themselves or something?
Summary: -moz-column-count on parent element cause margins on child to bleed through → -moz-column-count on parent element with border causes its margins to collapse with the kids' margins
(Reporter)

Comment 2

7 years ago
Created attachment 495303 [details]
Reduced testcase, with border tests

(In reply to comment #1)
> Yeah, this seems like a bug.
> 
> Presumably the problem here is that the border ends up on the columnset, not on
> the columns themselves or something?

I'm not sure that it is directly related to the borders, though. Removing them (and using, e.g., a background color to demarcate the two divs) actually makes things worse, as I understand it. Without the borders, the margins collapse on both divs.

Here's a new testcase that shows that.
Attachment #495268 - Attachment is obsolete: true
> Without the borders, the margins collapse on both divs.

See <http://www.w3.org/TR/CSS2/box.html#collapsing-margins>.
(Reporter)

Comment 4

7 years ago
Comment on attachment 495268 [details]
Reduced testcase

(In reply to comment #3)
> > Without the borders, the margins collapse on both divs.
> 
> See <http://www.w3.org/TR/CSS2/box.html#collapsing-margins>.

Yeah, I thought I might've been mistaken. I take it you're referring to this part?

"The bottom margin of an in-flow block-level element with a 'height' of 'auto' is adjoining to its last in-flow block-level child's bottom margin if the element has no bottom padding or border."
Attachment #495268 - Attachment is obsolete: false
Yep.
(Assignee)

Comment 6

7 years ago
Created attachment 495318 [details] [diff] [review]
Patch rev. 1 (diff -w)

Change nsBlockFrame::BlockIsMarginRoot() to return separate values
for top/bottom.  Currently there is a special handling for blocks where
the parent frame is a ColumnSetFrame that currently returns false
unconditionally -- make that return true if there is a non-zero
border+padding instead.
Assignee: nobody → matspal
(Assignee)

Updated

7 years ago
status2.0: --- → ?
Keywords: css3, testcase
(Assignee)

Comment 7

7 years ago
Created attachment 495323 [details]
Testcase #3

Here's an interesting edge case.  When the column is empty, (how) should the
margin self-collapse...  when there's one column? more than one column?

My patch makes the height of the one column case 20px, ie. like a normal
block would do.  The three column case has 0px height.  I would expect it
to be 20px.

Comment 8

6 years ago
Bug 388666 is a different issue. It's about the margin-top of the first child of a multi column element that is not being rendered if the multi-column element has a border-top.

This issue is about the the margin-bottom of the last-child of a multi-column element (that has border-bottom) which apparently collapses with the margin-bottom of the multi-column element.

The spec says that "a multi-column element establishes a new block formatting context". See the bottom of section 2, The multi-column model:
http://www.w3.org/TR/2011/CR-css3-multicol-20110412/#the-multi-column-model

That means the margins of a multi-column element do not collapse with themselves nor with the margins of their descendants.

Gecko and WebKit apparently do not implement the spec. Microsoft claims to implement the spec in IE10 Preview 1.

Updated

6 years ago
Depends on: 672043

Updated

6 years ago
Blocks: 684062

Updated

5 years ago
Blocks: 698783
(Assignee)

Comment 9

5 years ago
Created attachment 644305 [details]
Testcase #4
(Assignee)

Comment 10

5 years ago
Created attachment 644312 [details] [diff] [review]
Patch rev. 2

Make ColumnSetFrame a block margin root.  Make the first column a margin root at the top edge, and the last column at the bottom.  (The 368020-1 test change is
to account for its margin which previously was ignored)

https://tbpl.mozilla.org/?usebuildbot=1&tree=Try&rev=f0cde7c76d05
Attachment #495318 - Attachment is obsolete: true
Attachment #644312 - Flags: review?(roc)
(Assignee)

Comment 11

5 years ago
Created attachment 644313 [details] [diff] [review]
Patch rev. 2 (diff -w)
Comment on attachment 644313 [details] [diff] [review]
Patch rev. 2 (diff -w)

Review of attachment 644313 [details] [diff] [review]:
-----------------------------------------------------------------

::: inbound-opt.9b8e56ada40e/layout/generic/nsBlockFrame.cpp
@@ +7020,5 @@
>  }
>  
> +void
> +nsBlockFrame::IsMarginRoot(bool* aTopMarginRoot,
> +                                bool* aBottomMarginRoot)

Fix indent
Attachment #644313 - Flags: review+
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ad2c23374bc
Flags: in-testsuite+
Target Milestone: --- → mozilla17
https://hg.mozilla.org/mozilla-central/rev/8ad2c23374bc
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 787310
(Assignee)

Updated

5 years ago
No longer depends on: 787310
Blocks: 814811
(Reporter)

Updated

5 years ago
status2.0: ? → ---
No longer blocks: 814811
Depends on: 814811

Updated

5 years ago
Depends on: 823451
(Assignee)

Updated

4 years ago
No longer depends on: 823451
(Assignee)

Updated

4 years ago
Attachment #644312 - Flags: review?(roc)

Updated

4 years ago
Duplicate of this bug: 388666

Updated

3 years ago
Depends on: 974837
You need to log in before you can comment on or make changes to this bug.