Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[css-multicol] Definition of adjacent spanners, when to create column boxes #6265

Closed
mstensho opened this issue May 3, 2021 · 5 comments
Closed

[css-multicol] Definition of adjacent spanners, when to create column boxes #6265

mstensho opened this issue May 3, 2021 · 5 comments
Assignees
@rachelandrew
Labels
css-multicol-1 Current Work

Comments

@mstensho
Copy link

mstensho commented May 3, 2021

Should "adjacent spanners" be defined in more detail? Here's what we have today:

https://drafts.csswg.org/css-multicol/#spanning-columns

Spanners are block-level boxes therefore the margins of two adjacent spanners will collapse with each other. As column boxes establish a new block formatting context, margins on elements inside a column box will not collapse with the margin of a spanner.

So it looks like a column box between two spanners will prevent them from being adjacent, but when exactly do we create column boxes?

<div style="columns:3; width:100px; background:blue;">
  <div id="s1" style="column-span:all; margin-bottom:50px; height:25px;"></div>
  <div id="abs" style="position:absolute;"></div>
  <div id="s2" style="column-span:all; margin-top:50px; height:25px;"></div>
</div>

Are #s1 and #s2 adjacent spanners? Not if #abs causes a column box to be created, for sure, but should it? If it shouldn't, should we still treat #s1 and #s2 as adjacent?

Browsers don't agree. Blink and EdgeHTML draw a 100x100 square (i.e. adjacent). Gecko draws a 100x150 rectangle (i.e. not adjacent). Currently the new fragmentation engine that we're working on in Blink behaves like Gecko, but this is merely an outcome of implementation details, and not a conscious choice.
@rachelandrew
Copy link
Contributor

It seems we should definitely make a decision and add something about the abspos case.

I'll agenda+ it. I think my preference would be that if there is an abspos item, it's out of flow and therefore the spanners are adjacent to each other and margins should collapse. That seems like the behavior I'd expect as an author.

Safari is doing what current Chromium does, collapsing the margins.

CodePen: https://codepen.io/rachelandrew/pen/9be69994f15df720cd5b920cfc5d2c10

@rachelandrew rachelandrew self-assigned this May 9, 2021
@dholbert
Copy link
Member

dholbert commented May 11, 2021 •
edited
Loading

Note that margins do collapse in regular block layout, when there's an abspos element separating two adjacent blocks, as shown here:
https://jsfiddle.net/dholbert/46nLusf2/
CSS:

.square-with-margin {
  margin: 50px;
  height: 50px;
  width: 50px;
}
.blue { background: blue; }
.teal { background: teal; }
.abs { position: absolute; }

HTML:

<div class="square-with-margin blue"></div>
<div class="abs">hello</div>
<div class="square-with-margin teal"></div>

So at first glance, I think I'd expect the same collapsing behavior from spanners as well.

@mstensho
Copy link
Author

Then again, abspos descendants do affect a containing multicol container in special ways; see issue #6279 - abspos descendants affect column balancing in browsers (but should they?).

Should (contained) OOFs trigger creation of column boxes? I suppose that answer would determine whether a spanner before and after should have their margins collapsed.

@aethanyc
Copy link

Should (contained) OOFs trigger creation of column boxes? I suppose that answer would determine whether a spanner before and after should have their margins collapsed.

I agree the problem reduces to the above statement. Firefox currently always triggers the creation of column boxes to contain non-column-span elements, whether they are absolutely positioned or not. So in the original example, the margin of #s1 and #s2 not collapsing is just a consequence that there's a column box in between them.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-multicol] Definition of adjacent spanners, when to create column boxes, and agreed to the following:

  • RESOLVED: Confirm that abspos elemetns do not create column boxes and this adjacent spanners sep by abspos will collapse margins
The full IRC log of that discussion <dael> Topic: [css-multicol] Definition of adjacent spanners, when to create column boxes
<dael> github: https://github.com//issues/6265
<dael> rachelandrew: Couple of linked issue with out of flow items
<dael> rachelandrew: I think example is straightforward. Got adjacent spanners that come directly after. margins collapse is in spec
<dael> rachelandrew: If you have abspos item inbetween which is taken out of flow we don't have interop. Gecko is not margin collapsing. Keeping abspos item separating. Blink and Safari do collapse
<dael> rachelandrew: Morton brought up b/c new fragementation engine doesn't
<dael> rachelandrew: What happens if we have out of flow item between 2 column spanners. Happy with margins not collapsing or want them to collapse
<iank_> yes
<dael> astearns: if you have non-spanning elements sep by out of flow do margins collapse?
<dael> rachelandrew: um...I'd have to test. Not sure
<dael> fantasai: They do collapse
<dael> rachelandrew: Cool
<dael> dholbert: I posted test case on GH showing they do in the simplified scenario
<dael> fantasai: I think this is a little...worth pointing out if you have abspos content contained by a block-level box and you remove the boxes it gets trapped and creates columns.
<dael> fantasai: If abspos is directly contained by multicol I don't see that is should cause creation of margin
<dael> rachelandrew: That's what I thought. It would mean Gecko changing what they're doing
<fantasai> s/and you remove the boxes/
<fantasai> s/and you remove the boxes//
<dael> astearns: fantasai you suggest in case where abspos is contianed by siblining it's different?
<TYLin> q+
<astearns> ack TYLin
<dael> fantasai: If there's an abspos position:releative element it creats column boxes and abspos can cause a height. But it's the block box causing the columns. When it's directly contained no reason to create columns and therefore not effect margins
<fantasai> s/abspos/abspos inside it/
<dael> TYLin: Reason why FF is not collapsing margin is b/c when it's out of flow it still leaves placeholder which triggers column box to be created so there is a gap between the adjacent
<dael> fantasai: But margin collapsing rules generally ignore abspos leements. So no reason why that should be happening
<dael> TYLin: Agree
<dael> dholbert: Create the column box to create placeholder. If say abspos placeholder don't get a box that would fix it
<dael> fantasai: placeholders only exist in tables.
<dael> dholbert: Yeah. Abspos elements don't cause creation of column box. I think that's what you're proposing.
<fantasai> s/tables/table box generation, as far as the specs are concerned. They're otherwise an implementation artifact/
<dael> astearns: Sounds like FF is willing to change
<dael> astearns: It was said Morton brought this up b/c new generation matched FF
<dael> rachelandrew: I don't think it was a concious choice but was impl details. I'm assuming they're happy to go back. We can ask
<dael> iank: I didn't get to talk with Morton before meeting. I think we'd be fine with that
<florian> s/Morton/Morten/
<dael> astearns: prop: Confirm that abspos eleemtns do not create column boxes and this adjacent spanners sep by abspos will collapse margins
<dael> fantasai: It hink it's no change
<dael> astearns: Clarifications and some tests, I suspect
<dael> fantasai: [missed] amrgin collapsing eslewehre might. Need tests
<dael> iank_: Don't know if that's correct about don't create column boxes. Might conflict with next issue
<dael> iank_: And that issue is basically does column balancing apply when you've only got abspos in multicol
<dael> fantasai: Not in conflict b/c columns not being created by abspos in the next issue. Box with position:relative creates it. It might be 0 height and can't see, but it's causing creation of column row
<dael> iank_: I guess this sort of gets at...need to think about it
<fantasai> s/[missed]/if we call this out in the spec, that implies that/
<dael> astearns: Why don't we resolve adjacent spanners sep by abspos elements will collapse margins
<fantasai> s/Need tests/Need tests, but no change to spec/
<dael> astearns: Abspos elements not creating column boxes is probably implied. But we can go with just margin collapsing for now
<dael> iank_: Re-read next, I was wrong
<dael> astearns: Back to orginal proposal. "Confirm that abspos elemetns do not create column boxes and this adjacent spanners sep by abspos will collapse margins"
<dael> RESOLVED: Confirm that abspos elemetns do not create column boxes and this adjacent spanners sep by abspos will collapse margins

@mozilla-apprentice mozilla-apprentice mentioned this issue May 12, 2021
Closed
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Jul 2, 2021
@mstensho
Column spanner margins may collapse through OOF siblings.
48f75c5 
If there's column spanner A, then some out-of-flow content not part of
the fragmentation context, then another spanner B, the block-end margin
of spanner A should collapse with the block-start margin of spanner B,
as no columns should be created for the out-of-flow content (as far as
the spec is concerned).

See w3c/csswg-drafts#6265

In our implementation we DO create column fragments for such out-of-flow
content, though, as we need this to let the OOFs bubble all the way up
to their containing block.

Therefore, let spanner margins collapse through such "empty" column
rows. We'll now update the intrinsic block-size of the multicol and
reset the margin strut only of the column row is considered to be
non-empty. As a consequence of this, we need to avoid using
intrinsic_block_size_ in some cases, but rather require that a row
offset be supplied (it's actually quite tempting to just remove
intrinsic_block_size_ completely as a member, since it's hard to
determine when it's safe to use and not).

This fixes two failing tests. Note that there's still one test failing
(fast/multicol/relpos-becomes-static-has-abspos.html) because of this
problem.

Note that there was actually remnants of code that attempted to deal
with this, but it got messed up by CL:2410242.

Bug: 1151880
Change-Id: I990d42f7b4661bac50ad52f65d40d902cb79249e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2998709
Commit-Queue: Morten Stenshorne <[email protected]>
Reviewed-by: Ian Kilpatrick <[email protected]>
Cr-Commit-Position: refs/heads/master@{#897948}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
@mstensho @copybara-github
Column spanner margins may collapse through OOF siblings.
51a5f30 
If there's column spanner A, then some out-of-flow content not part of
the fragmentation context, then another spanner B, the block-end margin
of spanner A should collapse with the block-start margin of spanner B,
as no columns should be created for the out-of-flow content (as far as
the spec is concerned).

See w3c/csswg-drafts#6265

In our implementation we DO create column fragments for such out-of-flow
content, though, as we need this to let the OOFs bubble all the way up
to their containing block.

Therefore, let spanner margins collapse through such "empty" column
rows. We'll now update the intrinsic block-size of the multicol and
reset the margin strut only of the column row is considered to be
non-empty. As a consequence of this, we need to avoid using
intrinsic_block_size_ in some cases, but rather require that a row
offset be supplied (it's actually quite tempting to just remove
intrinsic_block_size_ completely as a member, since it's hard to
determine when it's safe to use and not).

This fixes two failing tests. Note that there's still one test failing
(fast/multicol/relpos-becomes-static-has-abspos.html) because of this
problem.

Note that there was actually remnants of code that attempted to deal
with this, but it got messed up by CL:2410242.

Bug: 1151880
Change-Id: I990d42f7b4661bac50ad52f65d40d902cb79249e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2998709
Commit-Queue: Morten Stenshorne <[email protected]>
Reviewed-by: Ian Kilpatrick <[email protected]>
Cr-Commit-Position: refs/heads/master@{#897948}
NOKEYCHECK=True
GitOrigin-RevId: 48f75c5d7e2142b979f48f0498df0b5849b87d97
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rachelandrew rachelandrew

Labels
css-multicol-1 Current Work
Projects
css-multicol-1
Status: Done
Milestone
No milestone
Development

No branches or pull requests
5 participants
@aethanyc @dholbert @rachelandrew @mstensho @css-meeting-bot