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-tables] visibility: collapse #478

Closed
gregwhitworth opened this issue Sep 13, 2016 · 29 comments
Closed

[css-tables] visibility: collapse #478

gregwhitworth opened this issue Sep 13, 2016 · 29 comments
Assignees
Labels
css-tables-3 Current Work

Comments

@gregwhitworth
Copy link
Contributor

gregwhitworth commented Sep 13, 2016

Compat Issues: yes

Description:
Visiblity:collapse is implemented very differently in various browsers.
One outstanding issue is what happens to cells that used to span the erased tracks and are now oversized.

Based on 2.1 this is a visual change only, so our questions are:

When collapsing and a portion of the cell is visible, what should occur?
A: The cell's contents are clipped (What Safari and Edge currently do)
B: The cell's contents overflow (What Gecko does)
C: You don't collapse the track since the spanning cell is in the track
D: You make this a layout change and you re-layout the cell in the new constraint space

When you collapse a track do you change the size of the table wrapper box?
Currently Gecko/Edge/Webkit all change the size of the wrapper box

CSS 2.1 Option:
Undefined

Testcases:
http://jsbin.com/zodaderama/edit?html,css,output For collapsing of tracks
https://jsfiddle.net/fa5q3rf6/ For showing re-layout of the table wrapper box

@FremyCompany
Copy link
Contributor

This was discussed at TPAC, but with no conclusion reached. It was mentioned the proposed resolution written above was not about the issue at hand, so I just removed it.

An important thing is that something people wanted to keep visibility:collapse NOT affect track sizing to make it possible for an user to collapse rows/columns without affecting scroll position of things BEFORE the removed track. That being said, since we change the size of the table wrapper, we might move the table itself if it is floated... or affect parent grid, fragmentation, etc... It is not clear what the way forward is in this case.

In case we cannot find an agreement on this, I would note that in additions to previously proposed options, we can also decide to drop the feature or adopt Edge/Webkit behavior, which is the closest you can get from CSSS 2.1.

@davidsgrogan
Copy link
Member

Chrome's top requested table feature is making visibility:collapse work with rows and columns. We are planning on tackling it (for some definition of "it") next summer with an intern's help. I might corner one of you at the January F2F to get more details.

https://crbug.com/174167 (visibility:collapse does not work correctly for table columns or rows)

@gregwhitworth
Copy link
Contributor Author

Sure, we can definitely have a chat at the face to face - but it sure would be great to get something more solid from you before the face to face as we may be able to resolve on that behavior.

@davidsgrogan
Copy link
Member

One more option surfaced at TPAC
E: if overflow:visible then re-layout, else clip
and I want to introduce
F: Hide any cells that span the collapsed track

The consensus at TPAC seemed to be that A was bad, and B/C weren't discussed. E gives authors the option of A or D. But D, a re-layout, is specifically what visibility:collapse was introduced to prevent, right? And if A is bad, why do we want to offer it as an option? So my initial take is to vote against E.

I don't like C because it could be a point of confusion/frustration for authors: visibility:collapse works for some tracks and doesn't work for others and it would take some investigation that spanned cells are the culprit.

I'm not necessarily advocating for F, but want to discuss it. Authors would immediately understand the effect that collapsing has on spanned cells, while with the other potential behaviors may not be so obvious (i.e. Edge and FF do the same thing with https://jsfiddle.net/dgrogan/9emqs2zh/5/)

I don't want to drop the feature. It's chrome's most requested table feature; developers want it.

I'm probably overlooking some important constraints, hopefully we can have an off-the record convo about all this (off-the-record just so I don't look stupid).

Asides:

I don't see that Safari has implemented this at all (tested r210282 from a few days ago). They seem to treat visibility:collapse the same as Chrome. And their bug for it is still open https://bugs.webkit.org/show_bug.cgi?id=8735 Am I missing something?

Looks like Edge (38.14393.0.0) has a bug when a row is collapsed and uncollapsed: the clipped text and border are not redrawn:
https://jsfiddle.net/dgrogan/9emqs2zh/

@davidsgrogan
Copy link
Member

I'm also interested in what got missed in the TPAC minutes from fremy:

dbaron: One of the effects that can happen is that if you have a
row and one of the cells in the row significantly impacts
width, then collapsing the row causes all of the cells to
change width, and scroll shifts accordingly.
frremy: If you have a cell that spans multiple columns with
centered text.
frremy: Collapse center columns [missed] and can't see anything
any more.

@gregwhitworth
Copy link
Contributor Author

Yes, please let's discuss this week - that said, this statement:

what visibility:collapse was introduced to prevent, right

Maybe, but that's why we included the separate test to show that Edge/FF currently do a re-layout so that they aren't taking up space - so avoiding a layout isn't necessarily a top requirement of mine. I'd prefer to look at the use cases and what the author would expect/want out of it. Thanks for the heads up on the bug as well :)

@FremyCompany
Copy link
Contributor

https://jsfiddle.net/bygh1zo6/ shows that Firefox and Edge do not clip the middle part when you remove the middle row (Edge clips the bottom part, Firefox does let the text overflow the cell).

The behavior currently in the spec does seem to match what Firefox is doing but as mentioned in the spec this is just a placeholder before we resolve on the issue.

If you have a chance to review it, please do so and tell us your opinion.

@gregwhitworth
Copy link
Contributor Author

Overflowing doesn't make much sense to me at all - maybe the perf win outweighs the UX issue, but I don't think that's what an author or end user would want or expect.

@davidsgrogan
Copy link
Member

Fremy: I can't tell what you're asking my opinion about. A vs B? And could you clarify what you mean by middle part in

Firefox and Edge do not clip the middle part

Also, do you remember what you said at TPAC about collapsing center columns that the minutes [missed]?

Greg: Sorry, I was unclear when I said relayout. I meant that a relayout could cause inline content to change, causing a jiggle that would be jarring to users. I didn't mean that our implementations would be able to avoid the layout path. Agreed that we should be figuring out what authors expect/want.

@gregwhitworth
Copy link
Contributor Author

@davidsgrogan and I discussed this as the face to face. We agreed that how Edge currently handle this is the best approach. Additionally, I shared that this is the same approach that Excel utilizes when hiding a row of data, that they clip the content that is still visible in the spanning cell. And while I can sympathize with the desire mentioned by @FremyCompany in the initial statement brought up at TPAC. What I would like to stress here, is that we currently don't have any interop in this feature, besides that of the FF/Edge do re-layout the table wrapper, which will cause the same issues raised. I recommend that we do not cause the overflow from row into another like FF does as that makes the visible data unusable in some circumstances. I'd like to get this solution resolved in a way that makes sense for what you're trying to achieve: collapse a row, in a relatively straight forward way to implement and specify.

@davidsgrogan
Copy link
Member

I committed to writing tests but haven't done so yet.

gsnedders pushed a commit to web-platform-tests/wpt that referenced this issue Apr 11, 2017
@FremyCompany
Copy link
Contributor

FremyCompany commented Jun 20, 2017

Visiblity:collapse on table with overflowing absolute content
https://wptest.center/#/kv8k51

@gregwhitworth
Copy link
Contributor Author

@FremyCompany Is there a point to this, or are you just providing a test?

@davidsgrogan
Copy link
Member

Has there been any investigation about how visibility:collapse affects border collapsing?

Pasting from WebKit's attempt at implementation:

Collapsed borders: What is the "cell above" a given cell for the purpose of determining that cell's top border if the row above is collapsed? [...] consider a case where due to collapsing the row above a given cell, the "cell above" now has a thicker border, which pushes down on the content of the given cell, making it taller and thus going against the principle that collapsing does not affect table layout

@FremyCompany
Copy link
Contributor

The completely-rewritten border-collapsing rules currently do not take into account that columns or rows might have been visually hidden. Given how the spec recommends to draw borders, this is technically ok; you would just end up with two half-borders that potentially do not match in style and color in that case. I guess that it could be ugly in some cases, but I don't see this being a common situation either.

Now, the issue here is that the border-rendering strategy that the spec currently proposes is not what any browser is doing today, so what updating one of the existing implementation to render nicely in that case is probably an implementation-dependent question.

A quick testcase shows that no two browser render the same in this case, which is not surprising given each browser has its own rendering strategy to start with:
https://wptest.center/#/w3x844

The spec currently recommends rendering like this:
image

@gregwhitworth
Copy link
Contributor Author

@davidsgrogan here is the commit trying to specify this behavior. Can you please give me your feedback ASAP on this. Thanks.
7e4647b

@davidsgrogan
Copy link
Member

@joysyu and I took a look, commented on the commit, thanks for the alert.

@davidsgrogan
Copy link
Member

Something came up while Blink was implementing row collapsing. (Which just landed on trunk, should be available in 2017-07-28 canary after enabling "Experimental Web Platform features" in chrome://flags/).

https://drafts.csswg.org/css-tables-3/#visibility-collapse-cell-rendering says

If the table-cell is spanning more than one table-track, and the table-track is set to visibility: collapse then clip the content to the table-cell’s border-box.

If the cell's content overflowed only because of visibility:collapse then clipping to the border box makes perfect sense. But if a rowspan cell already has overflow (https://jsfiddle.net/dgrogan/9uduq99L/3/) and then one of the rows it spans is collapsed, the existing overflow also disappears.

That's what happens in Edge (modulo the painting bug) and what @joysyu implemented in Blink.

Was losing the existing overflow considered? I think it'd be surprising to a web author.

@gregwhitworth
Copy link
Contributor Author

Summary
In the basic scenario, FF/Edge agree that setting visibility: collapse will clip any content within the collapsed row/column. The area where FF/Edge diverge is when content that spans into a collapsed row or column, what should happen to this content? Edge clips to the border box, Firefox overflows the visible bounds.

Face to face recommendation
We have specified the Edge behavior, as the author has specifically requested that the content within the row/column should be collapsed, as such the content should be clipped to remain consistent with the basic behavior.

Examples
https://jsfiddle.net/v23h0338/

@w3c w3c deleted a comment from FremyCompany Aug 1, 2017
@joysyu
Copy link

joysyu commented Aug 9, 2017

When a row-spanning cell begins in a collapsed row, the entire cell isn't painted. When a col-spanning cell begins in a collapsed column, however, both Edge and Firefox still at least partially paint the cell (https://jsfiddle.net/0ok0d8oa/). This bug mentions that any cell starting in a collapsed row/col should go away completely.

Is it correct that both Edge and Firefox have bugs when rendering a col-spanning cell that begins in a collapsed column?

@dbaron
Copy link
Member

dbaron commented Aug 23, 2017

This was discussed at our face-to-face meeting at the beginning of this month, but didn't get linked back to this github issue.

RESOLVED: cells spanning collapsed rows/columns are clipped to their resulting bounds (in both axes)

Expand for the full IRC log
<fantasai> topic: visibility collapse: clip or not to club
<fantasai> s/club/clip
<fantasai> gregwhitworth: Talked about this at TPAC
<fantasai> gregwhitworth: Specified Edge behavior currently
<fantasai> gregwhitworth: To not get rid of data
<fantasai> gregwhitworth: But when author asks for visibility collapse on column or row
<fantasai> gregwhitworth: They've asked that row or column to be gone
<fantasai> gregwhitworth: If you have a spanning cell that goes into the collapsed column/row, then clipped to visible columns/rows
<fantasai> fantasai: I think that's what's in the spec. Not really that great
<fantasai> gregwhitworth: FF overflows the content
<fantasai> gregwhitworth: Behind a flag in Chrome
<fantasai> dgrogan: We also clip to border box. Behind a flag
<fantasai> fantasai: I thin there were two options we were considering here, and overflowing was definitely not one of them
<fantasai> fantasai: ...
* fantasai continues to ignore her comments
<fantasai> dbaron: This would ...
* fantasai doesn't have anything to say worth minuting
<gregwhitworth> https://jsfiddle.net/v23h0338/
<fantasai> gregwhitworth explain the demo
<fantasai> dbaron: Suppose before collapsing ou have a cell here and it's part of 3 rows that are conceptually here
<fantasai> dbaron: cell has some content which overflows in theses various directions
<fantasai> dbaron: And then you collapse the middle one
<fantasai> dbaron: When it's not collapsed, these overflow visibly
<fantasai> dbaron: Are you saying when its collapsed, we only draw...
<fantasai> dbaron: I'm collapsing the middle row
<fantasai> gregwhitworth: You only draw what's in the first / third cells
<fantasai> [drawing on whiteboard, behind Bert, can't be captured for minutes]
<fantasai> fremy: If you're clipping the middle row, do you draw the content in the 1st and 3rd rows, or do you show the 1st and 2nd rows and clip out the end
<fantasai> dbaron: Once you collapse the middle row, you're clip everything that overflows the top and you're clip out stuff [over there, over there nobody is telling me what that means and I can't see it]
<fantasai> gregwhitworth: This is an edge case
<fantasai> gregwhitworth: Haven't seen people hiding rows/cols except in data grids, and ...
<fantasai> gregwhitworth: What you're proposing is complicated
<fantasai> dbaron: If it's not a use case why clip anything?
<fantasai> Rossen: That is the use case
* fantasai wants a link to the previous minutes
<fantasai> Rossen: Your primary purpose was to have a layout
<fantasai> Rossen: We go to extreme lengths to avoid overflow
<fantasai> Rossen: To make sure that all content is displayed inall cells
<fantasai> Rossen: Then you have user facing behavior that allow ppl to collapse things away, purposefully make things appear or disappear
<fantasai> Rossen: When they explictily aske for something to disapear, and you continue to show them what disappeared, weird no?
<fantasai> dbaron: I just meant don't clip this cell
<fantasai> gregwhitworth: In dbaron's example ...
<gregwhitworth> https://jsfiddle.net/dgrogan/9uduq99L/3/
<fantasai> gregwhitworth: If you collapse
<fantasai> gregwhitworth: we end up clipping in both axes
<fantasai> gregwhitworth: My statement ot dbaron is, he forcefully asked for no wraps which will overflow
<fantasai> gregwhitworth: I don't see circumstances where it isn't primariy the excel-based scenario
<fantasai> gregwhitworth: this is the most performant way to accomplish, which is clipping to bounding box
<fantasai> gregwhitworth: weird side effect
<fantasai> dbaron: Proposal is that if you have a cell that spans some collapsed rows or columns
<fantasai> dbaron: that cell clips its content to its resulting bounds
<fantasai> gregwhitworth: yes
<fantasai> RESOLVED: cells spanning collapsed rows/columns are clipped to their resulting bounds
<fantasai> (in both axes)
<fantasai> Rossen: Goal of work on this spec is to minimize interop pain
<fantasai> Rossen: not trying to rewrite history
<fantasai> Rossen: issues aren't introduced, we're trying to write out the path of least resistance
* fantasai found the minutes to last time we discussed
<fantasai> https://lists.w3.org/Archives/Public/www-style/2016Nov/0069.html
@dbaron
Copy link
Member

dbaron commented Aug 23, 2017

This bug mentions that any cell starting in a collapsed row/col should go away completely.

I thought I remembered somebody saying that when I filed the bug (which was within minutes of the discussion), but I don't see it in the minutes. I guess it does make sense for rows and columns to be treated similarly. I'm not particularly keen on the idea of making the collapsing the first row or column special, though, so if there are existing implementations that don't do that, I'd certainly be happy to drop that bit, and to treat collapsing any row or column of a column/row-spanning cell the same.

@FremyCompany
Copy link
Contributor

This bug mentions that any cell starting in a collapsed row/col should go away completely.

I thought I remembered somebody saying that when I filed the bug (which was within minutes of the discussion), but I don't see it in the minutes. I guess it does make sense for rows and columns to be treated similarly.

I recall saying that. Looks like I wasn't entirely correct (see below).

I'm not particularly keen on the idea of making the collapsing the first row or column special, though, so if there are existing implementations that don't do that, I'd certainly be happy to drop that bit, and to treat collapsing any row or column of a column/row-spanning cell the same.

I agree and just realized the behavior we are seeing is not that the first cell is hidden if its rows is hidden, but rather that it inherits "visibility:collapse" from its row, which gets interpreted as "visibility:hidden" and hides the cell. If you specify "visibility:visible" on it, it reappears.

https://wptest.center/#/uznwd6

I'll update the spec, this is just a mistake.

@davidsgrogan
Copy link
Member

https://jsfiddle.net/dgrogan/e1du2oaL/ compares behavior for collapsing a row containing a visible rowspan cell.

I think the behavior agreed on in the last two comments (#478 (comment) #478 (comment)) is Chrome's -- rowspan cell renders identically no matter which row is collapsed.

But we haven't explicitly talked about cell backgrounds.

The current spec says:

When a table-track or table-track-group has visibility: collapse, none of backgrounds, borders or outlines that are contributed by the cells within the given table-track or table-track-group are painted.

No exception is made for backgrounds of cells that are still partially visible because of rowspan / visibility:visible like in the jsfiddle. And indeed, neither FF nor Edge paints the background when the visible spanning cell is a child of a collapsed row. But that's in conflict with

treat collapsing any row or column of a column/row-spanning cell the same

So I propose that the planned spec edits include an exception in the above section that allows for painting backgrounds/borders/outlines when the cell is partially visible. Thoughts?

@FremyCompany, in the jsfiddle, when the top row is collapsed Edge doesn't paint the spanning cell's background. But Edge does paint the background for A in https://wptest.center/#/uznwd6, which is the same case as far as I can tell.

@FremyCompany
Copy link
Contributor

Looks like it is because the background is applied on the <col> and not the cell in your example. Sounds like an Edge bug, I agree.

@davidsgrogan
Copy link
Member

Oh, yes. Quite a different case. Sorry :(

This is what I meant to do. Here, Edge and Chrome both paint the cell background in both cases which I think is the desired behavior, but in violation of the spec about not painting backgrounds (the part I quoted in #478 (comment)). FF kind of obeys the spec in the first case because it doesn't paint the background, but it also doesn't paint the text, so that's probably just a bug.

So I suppose my proposal still stands: include an exception that allows for painting backgrounds/borders/outlines when the cell is partially visible despite it being in a collapsed track.

@FremyCompany
Copy link
Contributor

That sounds reasonable to me.

@davidsgrogan
Copy link
Member

davidsgrogan commented Nov 7, 2017

TPAC conversation notes:

  • impls should make outline show like chrome does in https://jsfiddle.net/dgrogan/vtw3u9gz/
  • impls should paint cell background like Edge / chrome in https://jsfiddle.net/dgrogan/0au7rsvn/
  • but when a vis:collapse row with a background has a vis:visible spanning cell without a background, do we paint the collapsed row's background behind the visible cell? Could go either way (Edge and Chrome differ in https://jsfiddle.net/dgrogan/jy57cxng/ ) but chrome behavior -- painting the tr background color behind the cell -- is probably what web authors expect, so we'll go with that.

Deleting the section 5.4.2 should get all these changes.

@FremyCompany FremyCompany self-assigned this Nov 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-tables-3 Current Work
6 participants