w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.43k stars 656 forks source link

Collapsed table first row width quirk #6230

Open atotic opened 3 years ago

atotic commented 3 years ago

Chrome gets bug reports about incorrect table widths for tables with collapsed borders. Examples: https://crbug.com/1200116 https://crbug.com/711360

The issue happens because per spec, width of the table in collapsed borders mode is determined by borders in the 1st row. I could not find definition of this behavior in the latest spec, but here it is in the old spec: https://www.w3.org/TR/2011/REC-CSS2-20110607/tables.html#collapsing-borders "As must compute an initial left and right border width for the table by examining the first and last cells in the first row of the table.'

If the first row has no borders, this results in a behavior developers consider a bug.

What do you think about using all the rows to compute table size? I am not sure if this is web compatible, but it would increase developer happiness.

FremyCompany commented 3 years ago

My understanding is that fixed tables work that way so that you can add rows without having to recompute the column widths. Checking the second row would defeat that purpose.

atotic commented 3 years ago

add rows without having to recompute the column widths

I believe that original intent was that you were able to determine column widths just by traversing the first row.

This optimization is no longer possible, and might have never been possilble.

Also, computing table's ink overflow requires traversing the entire table.

Today, this behavior is just a historical quirk that makes life more complex for web developers. Also, we might see many more "0-width borders on the first row", because that is currently the only way to get glitch-free collapsed borders + sticky headers.

FremyCompany commented 3 years ago

Ok, so if I understood correctly, what you are saying that with the current complexity level of the web platform, this optimization is no longer useful, and that it's a constraint we should relax to make developers' life easier. Is that correct?

If so, I am not against that.

Regarding web compatibility, could you put a use counter so we can take a look at a few sites and check if that improves or cause issues for them?

atotic commented 3 years ago

Filed a chrome issue asking for a use counter: https://bugs.chromium.org/p/chromium/issues/detail?id=1238243

FremyCompany commented 3 years ago

Wonderful, thanks @atotic!

bfgeek commented 3 years ago

I've added in the use-counter. My expectation is that there won't be a many (if any) sites relying on this quirk. We should have some initial data in a couple of months.

As Aleks says implementations already have to calculate the other value to correctly determine visual-overflow so there really isn't any "performance benefit". (There /potentially/ was in old implementations which has much simpler paint sub-systems, e.g. they didn't care about visual overflow as much, but this isn't true for modern implementations).

bfgeek commented 2 years ago

I finally got around to performing some analysis on our UseCounter data. TL;DR I think this change is going to be fine.

The UseCounter is "high" at ~0.55% of pageloads. However I analyzed a lot of examples URLs - all the changes are all minimal. The changes broadly fall into ~4-5 categories.

Craigslist - e.g. https://sfbay.craigslist.org/ Here the calendar widget on the right changes size by 1px. This doesn't have any affect on page layout. It does have a small (barely noticeable) visual effect making the border between the table body and header uniform. Interestingly this was previously present. E.g. it has the CSS rule:

.cal .dys th, .cal td {
    border: 1px #ccc solid;
}

However this doesn't match the <th> element as it now uses the class .days (instead of .dys).

Discourse - e.g. https://discourse.wicg.io/categories Here the "categories" panel uses visual table overflow to add category color on the left of the table. This is the largest visual effect I've seen in my analysis (3px shift in visual - border is 6px). No layout change. The new behaviour is arguably slightly better at small screen sizes with this change, and a little bit of a wash at large. We could submit a PR to modify discourse directly if they'd like to keep their current behaviour.

Open table reservation widget - e.g. https://www.charcoalgrillbar.com/ Similar to the calendar widget in craigslist - no significant change (0.5px delta on each side) - no visual difference.

Various calendar widgets on blogs etc. Similar to the other calendar widgets no significant changes.

In my opinion if we'd like to do this change - it'll has a high chance of being fine, and given the amount of confusion it has generated over the years (e.g. we still get bug reports about this behaviour) likely a positive for the web platform. Personally my only real concern is the discourse example.

Ian

FremyCompany commented 2 years ago

Sounds like good news to me! Would Chrome volunteer to try the change if I update the spec to allow the behavior?

bfgeek commented 2 years ago

Yeah I think so - its fairly straightforward for us. I'll let you adgenda+ it so that you can be on the call.

FremyCompany commented 2 years ago

This week is APAC call. So yeah not joining, will Agenda+ for the next one.

FremyCompany commented 2 years ago

FWIW the current spec:

Harmonize the table-root border-{top,bottom,left,right} with the corresponding border of all cells forming the border of the table (indenpendently), without actually modifying the border properties of the table-root.

If the table and the cell border styles have the same specificity, keep the cell border style.

Once this is done, set the table-root border-{…}-width to half the maximum width found during the harmonization processes for that border, then set border-{…}-style to solid, and border-{…}-color to transparent.

And the older spec:

UAs must compute an initial left and right border width for the table by examining the first and last cells in the first row of the table. The left border width of the table is half of the first cell's collapsed left border, and the right border width of the table is half of the last cell's collapsed right border. If subsequent rows have larger collapsed left and right borders, then any excess spills into the margin area of the table.

Any borders that spill into the margin are taken into account when determining if the table overflows some ancestor (see 'overflow').

All engines seem to do this latter thing, but rounding issues cause this to be un-noticable in Firefox in the initial test case. Here is a more clear test case: https://jsfiddle.net/gfhLs745/

css-meeting-bot commented 2 years ago

The CSS Working Group just discussed collapsed table row quirk, and agreed to the following:

The full IRC log of that discussion <TabAtkins> Topic: collapsed table row quirk
<TabAtkins> github: https://github.com/w3c/csswg-drafts/issues/6230
<TabAtkins> fremy: while reimplementing tables in chrome, the dev found a lot of bugs that werne't bugs per spec, but were unexpected by authors
<TabAtkins> fremy: We realized the new version of the spec was causing the issues
<TabAtkins> fremy: But behavior that is currently implemented was mentioned in an old version of the spec
<TabAtkins> fremy: So chrome dev decided to implement the old behavior, since we didn't ahve resolution for the spec change, but they'd like to change to the new beahvior because the old didn't make much sense
<TabAtkins> fremy: They feared there would be some breakage with new behavior, so they added a UseCounter and studied websites
<TabAtkins> fremy: Found that on all websites, the changes to layout would be very small, if any, and often slightly better. Found one case where it was slightly worse, but in no case was it worse than about 3px.
<TabAtkins> fremy: So they want to update their impl to the new spec, if the WG approves it
<TabAtkins> fremy: So that's context. Here's the behavior.
<iank_> https://www.w3.org/TR/CSS2/tables.html#:~:text=17.6.2-,The%20collapsing%20border%20model,-In%20the%20collapsing
<TabAtkins> fremy: When you have a table in collapsed border mode
<bradk> 3px narrower or 3px wider?
<TabAtkins> fremy: Two cells next to each other, the border needs to be "harmonized" - pick one of the two borders and draw.
<TabAtkins> fremy: Must also harmonize border between cells and the table itself.
<TabAtkins> fremy: In the new spec you harmonize all the cells on the table edge, take the biggest one. Do that for each edge.
<TabAtkins> fremy: The older spec for some reason only harmonized the first cell of the first row.
<astearns> https://www.w3.org/TR/CSS2/tables.html#collapsing-borders is a more interoperable version of iank_'s URL
<TabAtkins> fremy: If you have a table you don't draw borders around the header row, but you use borders for the rest,
<TabAtkins> fremy: According to the old spec, the borders will then be drawn outside the table
<TabAtkins> fremy: so it often just causes an overflow of like 1px, not noticeable
<TabAtkins> fremy: But scrollbars can pop
<TabAtkins> fremy: And devs can't figure out why it's happening
<bradk> That answers my question
<TabAtkins> fremy: Bug is marked as Doesn'tReproduce in Firefox, technically true, because they do something differently but still wrong, it's based on rounding.
<TabAtkins> fremy: So proposal is to accept the new spec text: when you decide the border of a table, you look at *all* the cells against each border edge, not just the first
<TabAtkins> fremy: Should be more in line with dev expectations, but it's currently a breaking change so we wanted a resolution.
<TabAtkins> iank_: Ironically this is one of the better specified areas in 2.1 regarding tables, which is why everyone has the same (bad) behavior.
<astearns> ack fantasai
<TabAtkins> fantasai: I worked on this issue in 2.1. I was really unhappy with the old resolution.
<TabAtkins> fantasai: Happy to move away from that.
<TabAtkins> fantasai: There was a proposal to have the borders not belonging to the table overlfow; another was to count thema s part of the border with. I think there was a third to allow it to overlap with the margin, but take up space if there wasn't enough amrgin.
<TabAtkins> fantasai: Say you have a table with anrrow borders, and you used a thick border to highlight an interesting cell.
<TabAtkins> fantasai: If you want the table to stay centered when the highlighted cell happens to be on the edge, we can't include the difference in the border width, it'll shift the table slighitly
<TabAtkins> fantasai: But that is a more complicated behavior, but I want to put it out for consideration.
<TabAtkins> fantasai: That effect is why, I suspect, why we had not taken the max border width, so those borders could overlap into the margin.
<TabAtkins> iank_: I don't believe we've found anyone using it that way. Might exist, but broadly the usage fell into a few buckets.
<TabAtkins> iank_: Primary is calendar widgets - craigslist was the big one. Had some dead code to add a transparent border to fix the behavior.
<TabAtkins> astearns: So the current spec describes what you prefer?
<TabAtkins> iank_: I think the max-border beahvior is defined in Tables 3.
<TabAtkins> iank_: caveated on us not breaking too much
<TabAtkins> iank_: Use counter is high-ish but layout changes are very minor.
<TabAtkins> fremy: Current Tables spec does have symmetrical behavior
<TabAtkins> astearns: francois, do you ahve opinions on the behavior fantasai described?
<TabAtkins> fremy: I see it's useful behavior
<TabAtkins> fremy: But think you should probably be using outline to add this kind of call-out border
<TabAtkins> fremy: So I think the simple behavior is fine. Wouldn't be mad either way.
<TabAtkins> fantasai: Think it's probably fine.
<TabAtkins> astearns: So proposed resolution is no change to the Tables spec, just confirming that the current spec is intended.
<fantasai> The thing I *wish* we could fix in table borders model is to invert the priority so that table wins over rowgroup over row over cell.
<fantasai> The current version is just so impossible to deal with if you want different colored but same-thickness borders for rows vs rowgroups, etc.
<TabAtkins> iank_: Think the spec fixes some unintuitive behavior for webdevs. Will report if there's problems.
<TabAtkins> RESOLVED: Accept current Tables 3 spec text regarding calculation of shared borders in collapsed-border mode between table and cells.
atotic commented 2 years ago

\o/ When I filed this issue, I did not think we'd actually fix it. Thanks everyone!