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.

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

[css-multicol] column rules should only be drawn to the height of the column contents #2309

Closed
dbaron opened this Issue Feb 14, 2018 · 12 comments

Comments

@dbaron
Copy link
Member

dbaron commented Feb 14, 2018

The multicol spec says:

A column rule is drawn in the middle of the column gap with the endpoints at opposing content edges of the multicol container.

However, this multicol breaking test shows that this isn't what implementations do -- at least Firefox and Chrome. (I think Chrome's behavior is correct; Firefox's is less buggy in nightly than in release right now.) They only draw the column rules as far as there is content in the columns.

The spec does say later in the same paragraph:

Column rules are only drawn between two columns that both have content.

but this doesn't justify the ending of the rule partway down the height of the multicol, when the content ends at that height.

dbaron added a commit to dbaron/web-platform-tests that referenced this issue Feb 14, 2018

Add tests for the breaking of a multicolumn across columns, including…
… backgrounds and column rules.

The *-nobackground-* tests pass in Firefox and Chrome, and the others
pass in Chrome and fail in Firefox due to what I (as a Gecko
implementor) consider to be a Firefox bug.  I haven't yet tested other
engines.

Given the level of ambiguity in the multicol spec, I suspect these tests
probably test some things that aren't formally specified.  However, I
think that's probably OK for now.  One issue that I know of that they
depend on the spec for is w3c/csswg-drafts#2309.

dbaron added a commit to dbaron/web-platform-tests that referenced this issue Feb 14, 2018

Add tests for the breaking of a multicolumn across columns, including…
… backgrounds and column rules.

The *-nobackground-* tests pass in Firefox and Chrome, and the others
pass in Chrome and fail in Firefox due to what I (as a Gecko
implementor) consider to be a Firefox bug.  I haven't yet tested other
engines.

Given the level of ambiguity in the multicol spec, I suspect these tests
probably test some things that aren't formally specified.  However, I
think that's probably OK for now.  One issue that I know of that they
depend on the spec for is w3c/csswg-drafts#2309.

@rachelandrew rachelandrew self-assigned this Feb 15, 2018

@mstensho

This comment has been minimized.

Copy link

mstensho commented Feb 15, 2018

See also crbug.com/792435 - Chrome fails at css/css-multicol/multicol-rule-004.xht , and it's related to this ambiguity. As dbaron says, Chome's behavior is for rules to stop where the columns stop.

With column spanners and nested columns in mind, I think this is the only sensible behavior. You don't want rules drawn through spanners, and you don't want rules in the parts of an inner multicol (with non-auto height) that have no content. There may not be enough content to fill all the outer columns that the inner multicol occupies, and it would just look silly to draw rules there.

@rachelandrew

This comment has been minimized.

Copy link
Contributor

rachelandrew commented Feb 15, 2018

I'm inclined to agree with @mstensho here that it would make sense to clarify this to only draw the rule where there is content, I can't think of a decent use case for drawing the rules otherwise. I'll add this to the agenda for discussion.

The suggestion as I understand it is that column rules extend to the edge of the multicol container where content extends to the edge of the container, or to the end of the content where it does not.

@rachelandrew rachelandrew added this to Needs discussion in css-multicol-1 Feb 15, 2018

dbaron added a commit to web-platform-tests/wpt that referenced this issue Feb 15, 2018

Add tests for the breaking of a multicolumn across columns, including…
… backgrounds and column rules. (#9527)

The *-nobackground-* tests pass in Firefox and Chrome, and the others
pass in Chrome and fail in Firefox due to what I (as a Gecko
implementor) consider to be a Firefox bug.  I haven't yet tested other
engines.  (Though -nobackground-003 fails in Firefox due to a different
bug, and multicol-breaking-002 doesn't trigger the background-related
Firefox bug.)

Given the level of ambiguity in the multicol spec, I suspect these tests
probably test some things that aren't formally specified.  However, I
think that's probably OK for now.  One issue that I know of that they
depend on the spec for is w3c/csswg-drafts#2309.
@dbaron

This comment has been minimized.

Copy link
Member Author

dbaron commented Feb 15, 2018

So it turns out Edge does do what the current spec says: it draws the rules between any chunks where there is content, but not between two empty pieces. See, for example, the behavior on multicol-breaking-001.html (and compare to the reference which presumes the resolution I was suggesting for this issue).

@css-meeting-bot

This comment has been minimized.

Copy link
Member

css-meeting-bot commented Feb 28, 2018

The Working Group just discussed . [css-multicol] column rules should only be drawn to the height of the column contents.

The full IRC log of that discussion <dael> Topic: . [css-multicol] column rules should only be drawn to the height of the column contents
<dael> github: https://github.com//issues/2309
<dael> Rossen_: This came from dbaron a couple weeks ago and added by rachelandrew .
<dael> dbaron: There was a thing in the spec, I had been doing testing and it didn't match FF and CHrome and didn't make sense to me. Edge does impl what hte spec says. Question is what happens to column rules when content of columns doesn't fill the space it could fill
<Rossen_> test case: https://w3c-test.org/css/css-multicol/multicol-breaking-001.html
<dael> dbaron: Either because you have a multi-col with a spec width and height unspec # col. Depending on column-fill balance|auto you might have content in the first and second or the top of the first. Question is where do you draw the rules.
<dael> dbaron: Spec says a full height rule between any pair with content in them. Completely empty is no rule. THat's not what FF and Chrome do. Last comment in GH issue I linked to a test.
<dael> dbaron: I think it...don't look in FF, look in Chrome and Edge.
<dael> Rossen_: I'm assuming you're going to be fixing in FF, that's why you're looking?
<dael> dbaron: What a moment...I think I've gotten things confused.
<dael> s/What/Wait
<dael> dbaron: FF has a bunch of other bugs here that are messing this up in weird ways.
<dael> dbaron: What htis test shows is a multicol inside another.
<dael> dbaron: Spec calls for the purple column to go all the way tot the bottom in the second column. FF & CHrome only take it to the bottom of the content.
<dael> Rossen_: What is wrong with this? I believe what the spec suggests in terms of behavior as well as what IE/Edge/Presto impl for column rules makes a lot of sense.
<dael> Rossen_: Are you suggesting only make app column rules only draw to content type?
<dael> dbaron: That's what I'm suggesting. I made that suggestion before testing edge so I made it before realizing current browsers did the thing the spec said.
<dael> dauwhe: I perfer Chrome to the spec
<dael> dauwhe: Pruely as a visual thing, having the rule extend past empty space strikes me as unexpected.
<dael> plinss: I think it should be an author rule. Having rules only to content seems common, but you might want rules to go all the way when you're doing something sophisticated. If you're going to content you should make sure all rules are to the same length
<dael> dbaron: Makes sense for balancing, but not column fill auto.
<dael> dauwhe: I agree with plinss about author control though.
<dbaron> s/Makes sense/Might make sense/
<AmeliaBR> Does anyone have the current spec text? Is it really supposed to apply to columns inside columns?
<dael> Rossen_: To answer AmeliaBR, columns inside of columns brings nothing interesting. dbaron did it for illistrative purposes to show when the multicol itself is the content of another column what the behavior is.
<dbaron> AmeliaBR, the spec text is quoted in the first comment of the issue
<dael> Rossen_: In this case I would have expected that...if we have content height as the limit to column gaps, what Chrome does is the behavior which would be compliant. And if column rules are used to show where columns are and their height, because as dbaron illustrates when you have a background you'll see the heights, that looks just as silly as the other way where you don't have the BG but you have the rule.
<dael> Rossen_: I sympathize to plinss suggestion of an author control. Having the background & the column rules not be the same height is pretty silly. THat's my personal opinion.
<dael> Rossen_: It seems strange that the two are not going hand in hand.
<dael> AmeliaBR: I brought it up because it seems browsers are consistant with outer level being full height. It's only when you add the inner level and how it break accross multiple it's weird
<dael> Rossen_: THe multicol inside the top level extends all the way down. That's why the rules are they way they are. It is visually deciving.
<dael> Rossen_: In this case if you had max-height on the inner and height on the outer so it stretches beyond I would expect column rules to be the same height
<dbaron> I'd note that if you remove the .outer { column-fill: auto } in the testcase than the thick blue rules get shorter
<dael> liam: There's a danger here that...in trying to decide based on weird test cases. If I had spec a height explicitly I'd expect the rule to go all the way down. Either you want a control as plinss said or it should honor the height if you gave one. Maybe you need to look at actual use cases.
<dael> Rossen_: Now you're suggesting tie in column height and rule properties which goes against our groups
<dael> liam: No no, I'm askign what is user's expectation. if you have an area with the height and column rules chances are you want the rules all the way donwn. I'm supporting a property.
<dael> Rossen_: I can't speak for the billions of users of the web. From a pure layout PoV what I said is if you ahev column boxes with backgrounds that exend the rules should extend the same way. THat's consistant with other properties.
<dael> Rossen_: Let's see if we can narrow down this issue and get to something actionable.
<dbaron> I think this was also a case where this was implemented in browsers *before* the current spec text existed.
<dael> Rossen_: Current proposal a 1) make a user control so people can explicitly control height of column rules to be that of the content height or the multicol height
<dael> Rossen_: 2) Stay with current spec which suggests height fo column rules is the same as the multicol element height
<dael> Rossen_: 3) Height of the column rule is based on height of the content fo the multi-height
<dael> Rossen_: We can try to make a resolution or we can stop discussing here and continue on GH. Get user feedback from designers, come back and see what stands out.
<dael> Rossen_: Preference dbaron ?
<dael> dbaron: I'm fine discussing more.
<dael> Rossen_: Okay.

@astearns astearns removed the Agenda+ label Mar 6, 2018

@TalbotG

This comment has been minimized.

Copy link

TalbotG commented Mar 13, 2018

Shouldn't the column-rule shine through transparent vertical (50px) margins of column spanning <h2> element in http://www.gtalbot.org/BrowserBugsSection/CSS3Multi-Columns/column-span-vert-margins.xhtml?

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 1, 2018

Bug 1438361 [wpt PR 9527] - Add tests for the breaking of a multicolu…
…mn across columns, including backgrounds and column rules, a=testonly

Automatic update from web-platform-testsAdd tests for the breaking of a multicolumn across columns, including backgrounds and column rules. (#9527)

The *-nobackground-* tests pass in Firefox and Chrome, and the others
pass in Chrome and fail in Firefox due to what I (as a Gecko
implementor) consider to be a Firefox bug.  I haven't yet tested other
engines.  (Though -nobackground-003 fails in Firefox due to a different
bug, and multicol-breaking-002 doesn't trigger the background-related
Firefox bug.)

Given the level of ambiguity in the multicol spec, I suspect these tests
probably test some things that aren't formally specified.  However, I
think that's probably OK for now.  One issue that I know of that they
depend on the spec for is w3c/csswg-drafts#2309.

wpt-commits: d04c0d50dbbf7752c27957005a5a659701b781ad
wpt-pr: 9527
wpt-commits: d04c0d50dbbf7752c27957005a5a659701b781ad
wpt-pr: 9527

mykmelez pushed a commit to mozilla/gecko that referenced this issue Apr 2, 2018

Bug 1438361 [wpt PR 9527] - Add tests for the breaking of a multicolu…
…mn across columns, including backgrounds and column rules, a=testonly

Automatic update from web-platform-testsAdd tests for the breaking of a multicolumn across columns, including backgrounds and column rules. (#9527)

The *-nobackground-* tests pass in Firefox and Chrome, and the others
pass in Chrome and fail in Firefox due to what I (as a Gecko
implementor) consider to be a Firefox bug.  I haven't yet tested other
engines.  (Though -nobackground-003 fails in Firefox due to a different
bug, and multicol-breaking-002 doesn't trigger the background-related
Firefox bug.)

Given the level of ambiguity in the multicol spec, I suspect these tests
probably test some things that aren't formally specified.  However, I
think that's probably OK for now.  One issue that I know of that they
depend on the spec for is w3c/csswg-drafts#2309.

wpt-commits: d04c0d50dbbf7752c27957005a5a659701b781ad
wpt-pr: 9527
wpt-commits: d04c0d50dbbf7752c27957005a5a659701b781ad
wpt-pr: 9527
@rachelandrew

This comment has been minimized.

Copy link
Contributor

rachelandrew commented Apr 7, 2018

@TalbotG when there is a spanner the columns below the spanner are a new set of column boxes, and margins do not collapse with the spanner so I think the rendering is correct. If you think the spec needs clarifying in that regard then it's probably a different issue to the one at hand.

"If the multi-column container is paginated, the height of each row is constrained by the page and the content continues in a new row of column boxes on the next page; a column box never splits across pages.

The same effect occurs when a spanning element divides the multi-column container: the columns before the spanning element are balanced and shortened to fit their content. Content after the spanning element then flows into a new row of column boxes." - The multi-column model

@rachelandrew

This comment has been minimized.

Copy link
Contributor

rachelandrew commented Apr 7, 2018

In the interest in getting feedback from designers on this issue, I'm writing it up with some screenshots and clarifications.

The specification says:

"A column rule is drawn in the middle of the column gap with the endpoints at opposing content edges of the multicol container."

This means that if you were to have a multicol container (an element with column-count or column-width set on it) which has a height, and there is not enough content to fill that height, you end up with column rules stretching past the content to the height of the container.

Edge currently does this, Chrome instead has the rules extending to the height of the content only. I made a very simple example to show the difference, which you can see here. The silver border shows a multicol element with a height.

Screenshot in Edge (spec behaviour):

multicol2309-edge

Screenshot in Chrome:

multicol2309-chrome

We have three options as detailed in the IRC discussion above:

  1. Create a user control so people can explicitly control height of column rules to choose content height or multicol container height.
  2. Stay with current spec which suggests height of column rules is the same as the multicol container height
  3. Change the spec so the height of the column rule is the content height.

If you have a preference and especially if you have a use case, please comment!

@astearns

This comment has been minimized.

Copy link
Member

astearns commented Apr 7, 2018

@rachelandrew it may help to have screenshots of the case where columns 1 and 2 are filled with content but column 3 is only half filled. In that case does the second column rule run the whole height or only to the bottom of the third column’s content height?

@dwds

This comment has been minimized.

Copy link

dwds commented Apr 7, 2018

@rachelandrew I think it would be nice to have the choice (option 1). I'm trying to think of good use cases for having a multicol container with height greater than its content ... maybe if you were using vh to make it vertically fluid, and it had a background image. In that case, I might want the columns rules to be the content height, to avoid disrupting the bg image. In a different scenario, with nested contexts (like columns inside grid) where you might have items in a row stretching to be the same height, you might want the consistency of column rules all being the same container height.

CodePen

Screenshot in FF:
column-rule_ff

Screenshot in Chrome:
column-rule_chrome

@shaunrashid

This comment has been minimized.

Copy link

shaunrashid commented Apr 9, 2018

Played with this and came up with a similar example to @dwds. It made more sense to have the rules extend to the edges of the content. I think I prefer that to having the rules extend to the edges of the container. If we do have options, then I would prefer that rules to the edge of content be the default behaviour.

@css-meeting-bot

This comment has been minimized.

Copy link
Member

css-meeting-bot commented Apr 12, 2018

The Working Group just discussed column rules should only be drawn to the height of the column contents, and agreed to the following resolutions:

  • RESOLVED: won't change
The full IRC log of that discussion <dael> Topic: column rules should only be drawn to the height of the column contents
<dael> github: https://github.com//issues/2309
<dael> rachelandrew: We wanted feedback from designers. A couple of people commented on the thread.
<dael> rachelandrew: 3 options. 1) create user controls so people can explicitly control column rules 2) keep current spec 3) change spec so height of column rule is content height
<dael> florian: When it's column heing if two adacjed you pick longer?
<dael> rachelandrew: Yes.
<dael> rachelandrew: Generally people I spoke to if there was a height they would expect the rules to go to the height. THat's current spec. So it's if we want to add a switch to allow content height which is option 1.
<dael> florian: I'm not opposed but I'd rather this in L2. I'd be opposed to this in L1.
<dael> rachelandrew: Have L1 remain as is which is height on container.
<dael> fantasai: Makes sense because you can always adjust the height by wrapping hte multi col
<dael> rachelandrew: I haven't seen many use cases for content height.
<fantasai> s/adjust the height/get the content height option/
<dael> myles: Why would anyone want it?
<dael> rachelandrew: I haven't see many use cases
<dael> myles: If one is never valuable don't make a switch. Close it.
<dael> fantasai: If people want it we can open an issue.
<dael> Rossen: I think we resolve this as don't change and move on.
<dael> rachelandrew: Happy with that.
<dael> Rossen: Objections?
<dael> RESOLVED: won't change
<dael> astearns: I do think adding a comment to the issue telling people that have connented.
@rachelandrew

This comment has been minimized.

Copy link
Contributor

rachelandrew commented Apr 12, 2018

Thanks for the use cases and comments. We discussed this today at the face to face meeting and have decided that since the current spec matches most use cases, we will leave level 1 as per the existing spec, with the column rules drawn to the height of the container.

If people have use cases for the alternative feel free to raise an issue including those and we can consider a switch for level 2 of multicol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.