Commit 296a759c authored by Aleks Totic's avatar Aleks Totic Committed by Chromium LUCI CQ

[TablesNG] Change the way columns affect grid size

What happens when table has more columns than cells?
Should the extra columns be discarded, or used to table grid?

If the columns are not discarded, and table has border spacing,
each empty column would make table wider.

Spec, and previous implemntations are not clear on what to do.
Initial table spec had a concept of effective columns,
where multiple columns could be treated as one for purpose of layout.
New spec introduces the concept of track merging.

https://www.w3.org/TR/css-tables-3/#dimensioning-the-row-column-grid--step2

It is a messy subject. I think cells could merge, but not columns. But
then in some implementations (ours), columns could merge too.

I investigated this because two wpt tests were failing in TablesNG:

external/wpt/css/css-tables/html5-table-formatting-1.html
external/wpt/css/css-tables/html5-table-formatting-2.html

They also failed in FF/Legacy, but in different ways.

The initial code trimmed the excess columns.
New code does not trim the columns.

This makes html5-table-formatting-2 pass all tests,
and makes one extra test in html5-table-formatting-1 pass.

The fix exposed a bug in ColumnGeometries sorting, ordering was not
monotonic. Fixed that too.

Bug: 958381
Change-Id: I9bd267f7e7782df7224f5db4611751ca10ff1aaa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2583157Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835721}
parent b5e40451
...@@ -303,6 +303,8 @@ class ColumnGeometriesBuilder { ...@@ -303,6 +303,8 @@ class ColumnGeometriesBuilder {
return a.start_column < b.start_column; return a.start_column < b.start_column;
} }
if (a.node.IsTableColgroup()) { if (a.node.IsTableColgroup()) {
if (b.node.IsTableColgroup())
return a.start_column < b.start_column;
if (a.start_column <= b.start_column && if (a.start_column <= b.start_column &&
(a.start_column + a.span) > b.start_column) { (a.start_column + a.span) > b.start_column) {
return true; return true;
......
...@@ -32,7 +32,8 @@ void ApplyCellConstraintsToColumnConstraints( ...@@ -32,7 +32,8 @@ void ApplyCellConstraintsToColumnConstraints(
bool is_fixed_layout, bool is_fixed_layout,
NGTableTypes::ColspanCells* colspan_cell_constraints, NGTableTypes::ColspanCells* colspan_cell_constraints,
NGTableTypes::Columns* column_constraints) { NGTableTypes::Columns* column_constraints) {
column_constraints->data.resize(cell_constraints.size()); if (column_constraints->data.size() < cell_constraints.size())
column_constraints->data.resize(cell_constraints.size());
// Distribute cell constraints to column constraints. // Distribute cell constraints to column constraints.
for (wtf_size_t i = 0; i < cell_constraints.size(); ++i) { for (wtf_size_t i = 0; i < cell_constraints.size(); ++i) {
column_constraints->data[i].Encompass(cell_constraints[i]); column_constraints->data[i].Encompass(cell_constraints[i]);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment