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-span' and tables #1071

Closed
dbaron opened this issue Mar 3, 2017 · 14 comments
Closed

[css-multicol] 'column-span' and tables #1071

dbaron opened this issue Mar 3, 2017 · 14 comments
Assignees
Labels
css-multicol-1 Current Work css-tables-3 Current Work

Comments

@dbaron
Copy link
Member

dbaron commented Mar 3, 2017

The definition of column-span says:

Applies to: in-flow block-level elements

The definition of block-level element says that it applies to things with display: block, display: list-item and display:table.

Then the definition of column-span goes on to say:

The element establishes a new block formatting context.

This means that (1) column-span: all applies to tables and (2) when it does, they become block formatting contexts.

Having a single edge case in which tables can establish block formatting contexts doesn't make sense. This shouldn't happen.

That said, given the lack of consideration to this issue, I wonder whether column-span is really intended to apply to tables.

What do existing implementations do? (IIRC, Gecko is the last engine to implement column-span, which would have made me expect the spec to be in better shape than it appears to be.)

/cc @tabatkins @gregwhitworth @neerjapancholi

@gregwhitworth
Copy link
Contributor

Looking at this testcase: https://jsfiddle.net/kememj9w/2/

It works on tables in Chrome/Edge (didn't test Safari). Granted following normal table layout rules it shrinks to fit its content so you have to set the width to 100% in order to consume the available width of the columns that span: all provides it.

image

@frivoal
Copy link
Collaborator

frivoal commented Mar 4, 2017

I think this is merely imprecise wording, not an actual expectation that tables should establish a BFC. The goal was, IIRC, the margins of descendants of two adjoining spanners would not collapse with each other, and that floats from one would not intrude in the other. For that, block spanners need to establish a BFC, but tables are fine as is.

@FremyCompany FremyCompany added the css-tables-3 Current Work label Mar 5, 2017
@FremyCompany
Copy link
Contributor

Just wanted to mention that this property would have to be hoisted if we want to apply it on tables (like every single css property except background and border, but hey as long as we want the spec to be a whitelist...)

@mstensho
Copy link

mstensho commented Mar 6, 2017

The way I see it, the goal was to keep column row content from interacting with spanners, and to keep things inside spanners from interacting with things inside other spanners, be it margins of floats. Just saying that they establish a new block formatting context may be too simplistic.

Tables should be allowed to become spanners. I've never worked on an engine that has this distinction between "table wrapper box" (which includes captions) and "table box" explicit in code (neither Presto nor Blink have this), so I don't really know, but for tables to become spanners, you obviously also need to isolate column content from the captions. We shouldn't leave the captions in the column rows, for instance.

Back to Blink, implementing column-span:all for tables was really simple, since Blink's table layout object contains all captions, and the caption margins do never collapse with content on the outside. Nor does it allow intruding floats. Is it a bug in Blink for the "table wrapper box" to always establish a new block formatting context? Because that is what we effectively do.

@FremyCompany
Copy link
Contributor

FremyCompany commented Mar 7, 2017

I've never worked on an engine that has this distinction between "table wrapper box" (which includes captions) and "table box" explicit in code (neither Presto nor Blink have this)

Oh yeah I have a strong opinion this concept should be removed from the spec altogether. If your engine does that for any reason, up to you, just make sure the observable behavior is identical. But some people have diverging opinions about it. I will make a case for that change next f2f I go to. 

[EDIT] Adding these links so I don't forget https://jsfiddle.net/vhLpok8g/ https://jsfiddle.net/vhLpok8g/1/

@astearns astearns removed the Agenda+ label Mar 8, 2017
@astearns
Copy link
Member

astearns commented Mar 8, 2017

Removing agenda+ until #1072 is resolved

@neerjapancholi
Copy link

neerjapancholi commented Mar 20, 2017

Tested this on different browsers and posting results with screenshots:

Test case: https://jsfiddle.net/kememj9w/2/
Description: ‘Column-span: all” is applied to a table to check if it spans all columns. (Same as test case provided by @gregwhitworth).

Chrome 56.0.2924.87 (64-bit) Safari 10.0.3 (11602.4.8.0.1) Edge 38.14393.0.0
Table spans all columns. But ‘width: 100%’ must be set on the spanner Table spans all columns. But ‘width: 100%’ must be set on the spanner (**One bug that I see in Safari only is that on resizing, content at the end of the last column before the spanner seems to be duplicated from the first column after the spanner.) Table spans all columns. But ‘width: 100%’ must be set on the spanner

Result screenshot (Taken from Safari but result for table spanning is identical to Chrome and Edge)

screen shot 2017-03-20 at 3 32 07 pm

@frivoal
Copy link
Collaborator

frivoal commented Mar 27, 2017

So bugs aside, we seem to have the same (sensible, IMHO) behavior everywhere. Does that look like a resolution or does someone disagree?

@mstensho
Copy link

Yeah, looks like they all agree, right? Having to set width:100% to make the table take up all available width of the multicol content box is only sensible, since a table with auto width will "shrink to fit".

See https://www.w3.org/TR/2011/REC-CSS2-20110607/tables.html#auto-table-layout

"If the 'table' or 'inline-table' element has 'width: auto', the used width is the greater of the table's containing block width, CAPMIN, and MIN. However, if either CAPMIN or the maximum width required by the columns plus cell spacing or borders (MAX) is less than that of the containing block, use max(MAX, CAPMIN)."

@fantasai
Copy link
Collaborator

I agree with @frivoal, I think the sentence is supposed to say that the element establishes “a new formatting context” (minus the word “block”).

@fantasai
Copy link
Collaborator

WG just resolved to take the edit of removing “block” from this sentence.

@frivoal
Copy link
Collaborator

frivoal commented Mar 30, 2017

I am still a tad fuzzy on how to make that edit, due to a lack of anchoring terminology. We do have a definition for "formatting context", but not as far as I can tell for "establishes new a formatting context". If it doesn't already establish one in the absence of column span, which one should it use: block, inline, something else? The answer is block, but where does that come form, spec wise?

css-contain used to have a very similar problem, which was solved by adding this section: https://drafts.csswg.org/css-contain/#becoming-formatting-context

Should we move that section / definition from css-contain to css-display, and refer to it from both css-contain and css-multicol?

@frivoal
Copy link
Collaborator

frivoal commented Apr 5, 2017

@fantasai @tabatkins what do you think about the comment above?

@tabatkins
Copy link
Member

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-multicol-1 Current Work css-tables-3 Current Work
Projects
None yet
Development

No branches or pull requests

9 participants