Skip to content

Commit 68fbb66

Browse files
committed
Preserve table rows unfragmented by default, per css-tables-3 6.1
Verified directly against the current W3C Editor's Draft (drafts.csswg.org/css-tables-3/#breaking-rules): "user agents must attempt to preserve the table rows unfragmented if the cells spanning the row do not span any subsequent row, and their height is at least twice smaller than both the fragmentainer height and width" - a required UA default, not something an author opts into. The table's row-shift previously only fired when the table itself had explicit break-inside:avoid, meaning an ordinary multi-page table with no special markup rendered rows split across page boundaries by default - not spec-compliant. CssLayoutEngineTable.LayoutCells now attempts to preserve every row by default, with the spec's two carve-outs implemented as "freely fragmentable" exceptions: a row a cell only starts spanning into a later row (new RowHasCellSpanningIntoSubsequentRow helper), or a row taller than half the page's height or width. The table's own break-inside:avoid still forces the attempt even for an otherwise-freely-fragmentable row, preserving existing behavior for that explicit case.
1 parent 5f56ad2 commit 68fbb66

2 files changed

Lines changed: 176 additions & 10 deletions

File tree

Source/HtmlRenderer/Core/Dom/CssLayoutEngineTable.cs

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -736,19 +736,25 @@ private void LayoutCells(RGraphics g)
736736
}
737737
}
738738

739-
// break-inside: avoid (or the legacy page-break-inside) on the table: if this row
740-
// straddles a page boundary and fits whole on one page, shift the whole row - not
741-
// just one cell - down to the next page's content top. Rows aren't avoided from
742-
// splitting by default (css-tables-3 6.1 permits a row to fragment, each cell
743-
// independently, which is what happens here with no correction: a cell's own content
744-
// already flows across the boundary via BlockFragmentation/InlineFragmentation) -
745-
// only when the table author actually asked for it.
746-
if (pageGridContainer != null && pageGridContainer.HasRealPageGrid && BreakValues.AvoidsBreak(_tableBox.BreakInside)
747-
&& maxBottom > cury)
739+
// css-tables-3 §6.1: "user agents must attempt to preserve the table rows unfragmented
740+
// if the cells spanning the row do not span any subsequent row, and their height is at
741+
// least twice smaller than both the fragmentainer height and width" - a UA-default
742+
// requirement, not something an author has to opt into. If this row straddles a page
743+
// boundary and isn't "freely fragmentable" by that rule, shift the whole row - not just
744+
// one cell - down to the next page's content top. The table's own break-inside:avoid
745+
// still forces the attempt even for an otherwise-freely-fragmentable row (an author's
746+
// explicit, stronger request), matching this port's existing behavior for that case.
747+
if (pageGridContainer != null && pageGridContainer.HasRealPageGrid && maxBottom > cury)
748748
{
749749
var topSlot = pageGridContainer.PageIndexOf(cury);
750750
var bottomSlot = pageGridContainer.PageIndexOf(Math.Max(cury, maxBottom - 0.01));
751-
if (bottomSlot > topSlot && maxBottom - cury < pageGridContainer.PageSize.Height)
751+
var rowHeight = maxBottom - cury;
752+
var freelyFragmentable = RowHasCellSpanningIntoSubsequentRow(row, currentrow)
753+
|| rowHeight >= pageGridContainer.PageSize.Height / 2
754+
|| rowHeight >= pageGridContainer.PageSize.Width / 2;
755+
var shouldPreserve = !freelyFragmentable || BreakValues.AvoidsBreak(_tableBox.BreakInside);
756+
757+
if (bottomSlot > topSlot && shouldPreserve && rowHeight < pageGridContainer.PageSize.Height)
752758
{
753759
var delta = pageGridContainer.PageTopOf(topSlot + 1) - cury;
754760
foreach (CssBox cell in row.Boxes)
@@ -878,6 +884,31 @@ private static int GetRowSpan(CssBox b)
878884
return rowspan;
879885
}
880886

887+
/// <summary>
888+
/// css-tables-3 §6.1's "the cells spanning the row do not span any subsequent row" test: true
889+
/// if any cell in <paramref name="row"/> - real or the <see cref="CssSpacingBox"/> placeholder
890+
/// standing in for one that started earlier - continues into a row after
891+
/// <paramref name="currentrow"/>, meaning this row cannot be preserved unfragmented on its own
892+
/// without also pulling along content that belongs to a row not yet reached.
893+
/// </summary>
894+
private static bool RowHasCellSpanningIntoSubsequentRow(CssBox row, int currentrow)
895+
{
896+
foreach (CssBox cell in row.Boxes)
897+
{
898+
if (cell is CssSpacingBox spacer)
899+
{
900+
if (spacer.EndRow > currentrow)
901+
return true;
902+
}
903+
else if (GetRowSpan(cell) > 1)
904+
{
905+
return true;
906+
}
907+
}
908+
909+
return false;
910+
}
911+
881912
/// <summary>
882913
/// Recursively measures words inside the box
883914
/// </summary>
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
using System.Collections.Generic;
2+
using System.Drawing;
3+
using System.Linq;
4+
using System.Reflection;
5+
using Microsoft.VisualStudio.TestTools.UnitTesting;
6+
using TheArtOfDev.HtmlRenderer.Core;
7+
using TheArtOfDev.HtmlRenderer.Core.Dom;
8+
using TheArtOfDev.HtmlRenderer.WinForms;
9+
10+
namespace TheArtOfDev.HtmlRenderer.IntegrationTest;
11+
12+
/// <summary>
13+
/// Verifies a real, spec-confirmed default-behavior gap found while auditing this port's fragmentation
14+
/// engine against PeachPDF a third time, then checking the actual W3C text directly
15+
/// (<see href="https://drafts.csswg.org/css-tables-3/#breaking-rules">css-tables-3 §6.1</see>, current
16+
/// Editor's Draft): "When fragmenting a table, user agents <b>must</b> attempt to preserve the table rows
17+
/// unfragmented if the cells spanning the row do not span any subsequent row, and their height is at
18+
/// least twice smaller than both the fragmentainer height and width. Other rows are said <i>freely
19+
/// fragmentable</i>." This is phrased as a required UA default, not something an author opts into -
20+
/// <c>CssLayoutEngineTable.LayoutCells</c> previously only preserved a row when the TABLE had explicit
21+
/// <c>break-inside:avoid</c>, meaning an ordinary multi-page table with no special markup at all rendered
22+
/// rows split across page boundaries by default, which the spec does not permit as the default.
23+
/// </summary>
24+
[TestClass]
25+
[DoNotParallelize]
26+
public sealed class TableRowDefaultAtomicityTest
27+
{
28+
private static HtmlContainerInt GetInternal(HtmlContainer wrapper)
29+
{
30+
var prop = typeof(HtmlContainer).GetProperty("HtmlContainerInt", BindingFlags.NonPublic | BindingFlags.Instance)!;
31+
return (HtmlContainerInt)prop.GetValue(wrapper)!;
32+
}
33+
34+
private static IEnumerable<CssBox> Walk(CssBox box)
35+
{
36+
yield return box;
37+
foreach (var b in box.Boxes)
38+
foreach (var d in Walk(b))
39+
yield return d;
40+
}
41+
42+
[TestMethod]
43+
public async Task OrdinaryRowWithNoBreakInsideAvoid_IsStillPreservedUnfragmented_ByDefault()
44+
{
45+
var checkedAnyStraddleCandidate = false;
46+
47+
for (var fillerCount = 1; fillerCount < 60; fillerCount++)
48+
{
49+
using var wrapper = new HtmlContainer();
50+
var filler = string.Concat(Enumerable.Repeat("<p style='margin:0;'>filler line</p>", fillerCount));
51+
// Deliberately no break-inside:avoid anywhere - this is the plain, no-special-markup case
52+
// css-tables-3 §6.1 says every conformant UA must handle this way by default.
53+
await wrapper.SetHtml(
54+
$"""
55+
<html><body>
56+
{filler}
57+
<table style="border-collapse:collapse;">
58+
<tr><td>RowOneCell</td></tr>
59+
<tr><td>TargetRowCellText with several words giving it real, non-trivial height</td></tr>
60+
</table>
61+
</body></html>
62+
""");
63+
64+
var container = GetInternal(wrapper);
65+
container.PageSize = new TheArtOfDev.HtmlRenderer.Adapters.Entities.RSize(300, 300);
66+
container.MarginTop = 0;
67+
wrapper.MaxSize = new SizeF(300, 0);
68+
69+
using var bitmap = new Bitmap(300, 20000);
70+
using var g = Graphics.FromImage(bitmap);
71+
wrapper.PerformLayout(g);
72+
73+
var tds = Walk(container.Root).Where(b => b.HtmlTag?.Name == "td").ToList();
74+
if (tds.Count < 2)
75+
continue;
76+
var targetCell = tds[1];
77+
78+
var topSlot = container.PageIndexOf(targetCell.Location.Y);
79+
var bottomSlot = container.PageIndexOf(System.Math.Max(targetCell.Location.Y, targetCell.ActualBottom - 0.01));
80+
81+
checkedAnyStraddleCandidate = true;
82+
Assert.AreEqual(topSlot, bottomSlot,
83+
$"at fillerCount={fillerCount}, the second row straddles page slots {topSlot}->{bottomSlot} with no break-inside:avoid anywhere - css-tables-3 6.1 requires it stay whole by default");
84+
}
85+
86+
Assert.IsTrue(checkedAnyStraddleCandidate, "no filler count in range produced a target cell - test is not meaningful as written");
87+
}
88+
89+
[TestMethod]
90+
public async Task RowSpanningIntoASubsequentRow_RemainsFreelyFragmentable()
91+
{
92+
// css-tables-3 6.1's own carve-out: a row a rowspan cell only STARTS in (spanning further rows)
93+
// is explicitly excluded from the "preserve unfragmented" default - confirming the new default
94+
// atomicity doesn't overreach into content the spec says must stay freely fragmentable.
95+
var foundAStraddle = false;
96+
97+
for (var fillerCount = 1; fillerCount < 30; fillerCount++)
98+
{
99+
using var wrapper = new HtmlContainer();
100+
var filler = string.Concat(Enumerable.Repeat("<p style='margin:0;'>filler line</p>", fillerCount));
101+
await wrapper.SetHtml(
102+
$"""
103+
<html><body>
104+
{filler}
105+
<table style="border-collapse:collapse;">
106+
<tr><td rowspan="2">SpanCell</td><td>Row1Cell2 with enough words to make this row meaningfully tall for the straddle test to matter</td></tr>
107+
<tr><td>Row2Cell</td></tr>
108+
</table>
109+
</body></html>
110+
""");
111+
112+
var container = GetInternal(wrapper);
113+
container.PageSize = new TheArtOfDev.HtmlRenderer.Adapters.Entities.RSize(300, 300);
114+
container.MarginTop = 0;
115+
wrapper.MaxSize = new SizeF(300, 0);
116+
117+
using var bitmap = new Bitmap(300, 20000);
118+
using var g = Graphics.FromImage(bitmap);
119+
wrapper.PerformLayout(g);
120+
121+
var row1Cell2 = Walk(container.Root)
122+
.FirstOrDefault(b => b.Words.Any(w => w.Text.Contains("Row1Cell2")))
123+
?.ParentBox;
124+
if (row1Cell2 == null)
125+
continue;
126+
127+
var topSlot = container.PageIndexOf(row1Cell2.Location.Y);
128+
var bottomSlot = container.PageIndexOf(System.Math.Max(row1Cell2.Location.Y, row1Cell2.ActualBottom - 0.01));
129+
if (topSlot != bottomSlot)
130+
foundAStraddle = true;
131+
}
132+
133+
Assert.IsTrue(foundAStraddle, "expected at least one filler count where the rowspan-starting row straddles a page boundary - if none do, this test isn't exercising the carve-out");
134+
}
135+
}

0 commit comments

Comments
 (0)