Skip to content

Commit d710fff

Browse files
committed
Update a table's own Location when row-atomicity shifts its first row
Confirmed gap: CssLayoutEngineTable's css-tables-3 6.1 row-atomicity shift (LayoutCells) moves each straddling row's cells (cell.OffsetTop) but never touched the outer <table> box's own Location - so when the shifted row was the table's own first content (a single-row table straddling a boundary, or any table whose first row alone needed the shift), the table's visual top effectively moved but Location/EffectiveTop stayed stale at the original page. EnforceKeepWithNext(table) - called on a table exactly like any other block child - reads EffectiveTop to decide whether a break falls between the table and its previous sibling, so it never saw the crossing and could never pull an avoid-chained heading along with it. Fixed by updating _tableBox.Location when the shifting row's own cury equals starty (nothing rendered above it within the table yet - the row moving IS the table's content moving wholesale, not one of several independently-straddling rows further down a multi-page table). Deliberately does NOT call BlockFragmentation.PropagateContainerRelocation to climb further into the table's own ancestors, unlike every other relocation trigger in this file: this method can run more than once per overall document pass whenever an ancestor is independently relocated by RelocateIfNeeded (which re-lays the whole subtree out fresh at its own target) - climbing here too double-counted that ancestor's already-correct shift on top of RelocateIfNeeded's own. Confirmed by a real regression this caused on first attempt: BoxContainingARepeatingTable_IsStillRelocated (a table inside its own break-inside:avoid card) landed 2px past its correct relocated position - reverted the propagation call once the full suite caught it. A plain, non-avoid wrapper div around a table whose first row alone triggers this path is consequently not covered - narrower than full css-break-3 3.1 propagation, matching what the two tests this fixes actually exercise (the table itself as EnforceKeepWithNext's own child). Un-ignores TableMovedToNextPage_PullsAvoidChainedHeadingAlong and TableMovedToNextPage_ChainSkipsDisplayNoneSibling_PullsHeadingAndIntroAlong in KeepWithNextIntegrationTests.cs. Two other Ignored tests in the same file (HeaderFitsButNoBodyRowDoes_MovesWholeTableAndHeadingTogether, LongRepeatingHeaderTable_StartingNearPageBottom_...) cover a related but different <thead>-interaction gap this fix does not address, left as-is. Full suite: HtmlRenderer.Test 2428 passed/127 skipped/0 failed (unchanged), HtmlRenderer.IntegrationTest 337 passed/89 skipped/0 failed (was 335/91, +2 unignored here, 0 regressions elsewhere after reverting the propagation call), HtmlRenderer.PdfSharp.Test 39 passed/0 failed (unchanged).
1 parent 416ea3d commit d710fff

2 files changed

Lines changed: 30 additions & 8 deletions

File tree

‎Source/HtmlRenderer/Core/Dom/CssLayoutEngineTable.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,31 @@ private void LayoutCells(RGraphics g)
757757
if (bottomSlot > topSlot && shouldPreserve && rowHeight < pageGridContainer.PageSize.Height)
758758
{
759759
var delta = pageGridContainer.PageTopOf(topSlot + 1) - cury;
760+
761+
// cury == starty means nothing has been drawn above this row within the table yet
762+
// (no earlier row consumed space on the table's original page) - so this row moving
763+
// IS the table's own content moving wholesale, not one row among several straddling
764+
// independently. The table's own Location was set once, before this method ever
765+
// ran, by its parent's child loop - row-atomicity shifting cell rectangles alone
766+
// left it stale, so EnforceKeepWithNext(table) (called on the table exactly like any
767+
// other child) never saw the boundary crossing and could never pull an avoid-chained
768+
// heading along. Only the table's own Location follows here - deliberately NOT
769+
// BlockFragmentation.PropagateContainerRelocation's further climb into an ancestor:
770+
// this method can run more than once per overall document pass whenever an ancestor
771+
// is independently relocated by RelocateIfNeeded (which re-lays the whole subtree
772+
// out fresh at its own target) - climbing here too would double-count that ancestor's
773+
// own already-correct shift on top of RelocateIfNeeded's (confirmed: caused a real
774+
// regression in BoxContainingARepeatingTable_IsStillRelocated, a table inside its own
775+
// break-inside:avoid card, off by the same few pixels PageSlotOf's collapsed-border
776+
// tolerance allows). A plain, non-avoid wrapper around a table whose first row alone
777+
// triggers this path is not climbed to - a narrower fix than full css-break-3 3.1
778+
// propagation, matching what the two tests this fixes actually exercise (the table
779+
// itself as EnforceKeepWithNext's own child, not a further-wrapped one).
780+
if (Math.Abs(cury - starty) < 0.01)
781+
{
782+
_tableBox.Location = new RPoint(_tableBox.Location.X, _tableBox.Location.Y + delta);
783+
}
784+
760785
foreach (CssBox cell in row.Boxes)
761786
{
762787
// A rowspan-crossing cell's real content lives on CssSpacingBox.ExtendedBox,

‎Source/Test/HtmlRenderer.IntegrationTest/Fragmentation/KeepWithNextIntegrationTests.cs‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -102,17 +102,16 @@ private static CssBox FindByClass(CssBox root, string className)
102102
}
103103

104104
// css-tables-3 §6.1's row-atomicity shift (CssLayoutEngineTable.LayoutCells) moves each CELL's own
105-
// rectangle (cell.OffsetTop) - it never touches the outer <table> box's own Location, which stays
106-
// wherever the table's own (unmoved) top naturally fell. So "did the table move" has to be read off a
107-
// cell inside it, not the <table> element itself.
105+
// rectangle (cell.OffsetTop); the outer <table> box's own Location follows too, but only when the
106+
// shifted row is the table's very first content (nothing rendered above it within the table yet) - an
107+
// ordinary row straddling further down a multi-page table correctly leaves the table's Location where
108+
// its real first row is. Reading a cell directly is the robust check either way, so tests use this
109+
// rather than assuming which case applies.
108110
private static CssBox FindFirstCell(CssBox table) => Walk(table).FirstOrDefault(b => b.HtmlTag?.Name == "td")!;
109111

110112
// A table moved wholesale to the next page (css-tables-3 §6.1 row-atomicity, its only row too tall to
111113
// fit) must pull its avoid-chained heading along instead of stranding it at the bottom of the old page.
112114
[TestMethod]
113-
[Ignore("Confirmed gap: CssLayoutEngineTable's row-atomicity shift only offsets the cell's own "
114-
+ "rectangle, never the outer <table> box's own Location/EffectiveTop, so EnforceKeepWithNext(table) "
115-
+ "never observes the boundary crossing - see this class's own doc remark.")]
116115
public async Task TableMovedToNextPage_PullsAvoidChainedHeadingAlong()
117116
{
118117
var (root, container) = await BuildAsync(
@@ -157,8 +156,6 @@ public async Task TableMovedToNextPage_LeavesNonAvoidHeadingBehind()
157156
// The chain walk must skip a display:none sibling and pull BOTH the heading and an avoid-chained intro
158157
// paragraph along when the table moves to the next page.
159158
[TestMethod]
160-
[Ignore("Same confirmed gap as TableMovedToNextPage_PullsAvoidChainedHeadingAlong - see this class's "
161-
+ "own doc remark.")]
162159
public async Task TableMovedToNextPage_ChainSkipsDisplayNoneSibling_PullsHeadingAndIntroAlong()
163160
{
164161
var (root, container) = await BuildAsync(

0 commit comments

Comments
 (0)