Skip to content

GitLab

T
tangled
Commit f4f73107 authored by Aleks Totic's avatar Aleks Totic Committed by Commit Bot
Browse files
Options

[TableNG] Fix minor colspan percentage redistribution bug 

Legacy code was not enforcing 2 invariants of colspan
redistribution.
- min_width <= max_width
- redistribution should never shrink width.

The fix introduces a part of new colspan redistribution
test suite.

Bug: 958381
Change-Id: I3cc6e55af86dd376ffbc0a4c4710900df8bccce5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2030805
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738062}
parent c61ac28f
Hide whitespace changes
Inline Side-by-side
Showing with 237 additions and 1 deletion
third_party/blink/renderer/core/layout/table_layout_algorithm_auto.cc
View file @ f4f73107
......@@ -538,8 +538,11 @@ int TableLayoutAlgorithmAuto::CalcEffectiveLogicalWidth() {
layout_struct_[pos].effective_min_logical_width =
std::max(layout_struct_[pos].effective_min_logical_width,
column_min_logical_width);
column_max_logical_width =
std::max(column_max_logical_width, column_min_logical_width);
layout_struct_[pos].effective_max_logical_width =
column_max_logical_width;
std::max(layout_struct_[pos].effective_max_logical_width,
column_max_logical_width);
allocated_min_logical_width += column_min_logical_width;
allocated_max_logical_width += column_max_logical_width;
}
......
third_party/blink/web_tests/external/wpt/css/css-tables/width-distribution/colspan_redistribution.tentative.html 0 → 100644
View file @ f4f73107
<!doctype html>
<script src='/resources/testharness.js'></script>
<script src='/resources/testharnessreport.js'></script>
<script src="/resources/check-layout-th.js"></script>
<link rel="author" title="Aleks Totic" href="atotic@chromium.org" />
<link rel="help" href="https://www.w3.org/TR/css-tables-3/#distributing-width-to-columns" />
<style>
table {
background: gray;
margin-bottom: 24px;
border-spacing: 8px 8px;
}
td {
padding: 0;
background: #BFB;
font-size: 10px;
}
td > div {
display: inline-block;
background: rgba(56,162,56,0.3);
}
.error {
color: red;
}
.brick {
width: 30px;
height: 20px;
}
p {
margin-top:4px;
margin-bottom:4px;
}
.rule1 {
background: #87dc8a;
}
.rule2 {
background: #3ae4cc;
}
.rule1and2 {
background: #fda4a4;
}
</style>
<h1>Colspan > 1 width redistribution</h1>
<p>This page is intended to be used as a wpt testharness test, and for examining what algorithms do. Hovering over cells will display information about the table.</p>
<p>Principles</p>
<ul>
<li>Distribution should be stable: reordered span cells retain their width.</li>
</ul>
<p>Wide cell redistributes its different widths to the spanned cells: percentage, min and max.</p>
<p>Creating understandable tests with percentage cells is complex because the relationship between percentage cell width, and table width is complex. Rules that govern relationship between table grid width and percentage cell width are:</p>
<ol>
<li><span class="rule1">Rule#1</span>, Single column sets minimum table width. Minimum table width is column.min_width / column.percent * 100.</li>
<li><span class="rule2">Rule#2</span>, All percentage columns set minimum table width. <br>Let P% be sum of all percentages, and Mpx sum of minimum widths of all non-percentage columns. The equations below determine minimum table width:
<ul>
<li>table width = Mpx + Ppx </li>
<li>P% + M% = 100%</li>
<li>Ppx * P% = Mpx * M% </li>
<li>Ppx * P% = Mpx * (100-P%) </li>
<li>Ppx = Mpx * (100-P%) / P% </li>
<li>table width = Mpx + Mpx * (100-P%) / P%.</li>
<li>table width = Mpx * (1 + (100-P%)/P%)</li>
</ul>
</li>
<p class="error">FF and Edge interpret rule#2 as maximum table width, while Chrome interprets it as minimum table width. </p>
<p>Table backgrounds will change if they match the 2 rules above: <span class="rule1">Rule#1</span>, <span class="rule2">Rule#2</span>, and both <span class="rule1and2">Rule#1 and Rule#2</span>.</p>
</ol>
<p>All examples have border-spacing:8, td.padding:0.</p>
<h2>Percentage redistribution</h2>
<h3>Percentage CSS constrained span cells</h3>
<p>Rules<br>
<ul>
<li>min width never gets smaller than it started.
<li>min width becomes cell.percent/cells.percent * wide_cell.min_width</li>
<li>max width becomes cell.percent/cells.percent * wide_cell.max_width</li>
</ul>
</p>
<p>10) auto/300px wide cell, 25%/50px span min width<br>
wide min width gets distributed evenly (-border_spacing) to both cells at 146px<br>
Table width: 146/0.25 + 4*8 = 584 + 32 = 616
<table data-expected-width="616">
<tr>
<td style="width:25%" data-expected-width="146"><div style="width:50px">25%/30px</div></td>
<td style="width:25%"><div style="width:50px">25%/30px</div></td>
<td style="width:20px">x</td>
</tr>
<tr>
<td colspan=2 style=""><div style="width:300px">300px min</div></td>
</tr>
</table>
<p>11) auto/400px wide cell, 20%/50px, 60%/50px spans.<br>
400 - 8px min width gets redistributed to span cells as 98px/294px.<br>
98px/0.2 => min grid width 490 + 4*8 = 522.</p>
<p class="error">Edge disagrees, table is 870</p>
<div style="width:600px">
<table data-expected-width="522">
<tr>
<td style="width:20%" data-expected-width="98"><div style="width:50px">20%/50px</div></td>
<td style="width:60%" data-expected-width="294"><div style="width:50px">60%/50px</div></td>
<td style="width:20px">x</td>
</tr>
<tr>
<td colspan=2 style=""><div style="width:400px">400px min</div></td>
</tr>
</table>
</div>
<p>12) auto/400px wide cell, 50%/150px, 30%/150px spans.<br>
This tests conflict resolution where min-width > redistributed width, and all browsers disagree.<br>
min-width of the 2nd cell is larger than redistributed width, causing differences.<br>
400-8px distributed max width tries to redistribute as 245|147, but gets constrained to 245|150 in Chrome.<br>
table width from cell 1 245/0.5 + 4*8 = 522<br>
table width from cell 2 150/0.3 + 4*8 = 532<br>
cell 1 = 50% of 500 = 250, cell 2 = 30% of 500 = 150 , cell 3 gets the remaining 100</p>
<p class="error">Chrome/FF/Edge end up with tables of different widths: 532/590/685. Chrome's 2nd span cell seems 'most correct' at its original max width of 150. In FF, extra min-width seems to cause more width to be redistributed. If you hover over 30%/150 cell, its min width will change to 100px, and all browsers will agree.</p>
<style>
.test12:hover {
width:100px !important;
}
</style>
<table data-expected-width="532">
<tr>
<td style="width:50%" data-expected-width="250"><div style="width:150px">50%/150px</div></td>
<td style="width:30%" data-expected-width="150"><div class="test12" style="width:150px">30%/150px</div></td>
<td style="width:20px" data-expected-width="100">x</td>
</tr>
<tr>
<td colspan=2 style=""><div style="width:400px">400px min</div></td>
</tr>
</table>
<p>13) auto/400px wide cell, 50%/75px/125px, 30%/75px/125px spans.<br>
400-8px min width gets redistributed as 245/147 (no min width limits)<br>
</p>
<p class="error">Edge is different</p>
<table data-expected-width="522">
<tr>
<td style="width:50%" data-expected-width="245"><div style="width:75px">50%/75</div><div style="width:50px">/125</div></td>
<td style="width:30%" data-expected-width="147"><div style="width:75px">30%/75</div><div style="width:50px">/125</div></td>
<td style="width:20px">x</td>
</tr>
<tr>
<td colspan=2 style=""><div style="width:400px">300px min</div></td>
</tr>
</table>
<script>
function measureCellMinMax(cell) {
let d = document.createElement("div");
let clone = cell.cloneNode(true);
for (child of Array.from(clone.childNodes))
d.appendChild(child);
d.style.width = "min-content";
document.body.appendChild(d);
let min = d.offsetWidth;
d.style.width = "max-content";
let max = d.offsetWidth;
d.remove();
return {min: min, max: max};
}
function annotateTable(t) {
let tableWidth = t.offsetWidth;
let firstCell;
let totalCellPercent = 0;
let nonPercentCellWidth = 0;
let tableMinWidthByCellPercent = 0;
let cellCount = 0;
let cells = Array.from(t.querySelectorAll("td"));
let spacing = (2 + cells.length - 1)*8;
for (let cell of cells) {
if (!firstCell)
firstCell = cell;
cellCount++;
let percent = cell.offsetWidth / (tableWidth - spacing) * 100;
let minmax = measureCellMinMax(cell);
let title = `${cell.offsetWidth.toFixed(1)}px\nmin:${minmax.min}px max:${minmax.max}px ${percent.toFixed(1)}%`;
let cssWidth = cell.style.width;
if (cssWidth) {
let w = parseFloat(cssWidth);
if (cssWidth.match(/\%/)) {
totalCellPercent += w;
let tableMinWidth = minmax.min / (w / 100);
tableMinWidthByCellPercent = Math.max(tableMinWidthByCellPercent, tableMinWidth);
title += `\nmin table: ${tableMinWidth.toFixed(0)}px`;
} else {
nonPercentCellWidth += w;
}
}
title += `\nTable: ${tableWidth.toFixed(0)} spacing: ${spacing}px`;
cell.setAttribute("title", title);
}
// Display table statistics in first cell.
if (firstCell) {
let title = firstCell.getAttribute("title");
let ruleMatch = 0;
if (tableMinWidthByCellPercent != 0) {
if ((tableMinWidthByCellPercent + spacing) == tableWidth)
ruleMatch += 1;
title += `\nTable min by single cell percent: ${tableMinWidthByCellPercent.toFixed(1)}`;
} else {
title += "\nTable min by single cell percent NA";
}
if (nonPercentCellWidth && totalCellPercent > 0) {
totalCellPercent = Math.min(totalCellPercent, 100);
title += `\nPercent sum: ${totalCellPercent.toFixed(1)}%, non percent width: ${nonPercentCellWidth}px`;
let tableMinBySum = (totalCellPercent / (100 - totalCellPercent) +1) * nonPercentCellWidth;
if (Math.floor((tableMinBySum + spacing)) == Math.floor(tableWidth))
ruleMatch += 2;
title += `\nTable min by sum ${totalCellPercent}%: ${tableMinBySum.toFixed(1)}px`;
} else {
"Table min by % sum not available";
}
firstCell.setAttribute("title", title);
switch(ruleMatch) {
case 1: t.classList.toggle('rule1'); break;
case 2: t.classList.toggle('rule2'); break;
case 3: t.classList.toggle('rule1and2'); break;
default: break;
}
}
}
for (let t of Array.from(document.querySelectorAll("table")))
annotateTable(t);
checkLayout("table");
</script>
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