Skip to content

Commit 416ea3d

Browse files
committed
Preserve a box's own top margin at a forced page break
Confirmed gap: BlockFragmentation.TryGetForcedBreakTarget's targetTop was always the raw page-content-top boundary - the box's own MarginTopCollapse was used only to decide WHICH slot the natural (unshifted) position falls in, never added back into the actual placement. css-break-3 5.2 preserves (does not truncate) a box's own margin specifically at a FORCED break - truncation only applies to an UNFORCED break where a margin alone happens to cross a page boundary (BlockFragmentation.ResolveBlockTop, unaffected by this change). Fixed at the two places breakTop actually becomes a box's placement (Dom/CssBox.cs): the immediate-placement path (top = breakTop) and the deferred-pass path (RequestedBreakBeforeTop, which flows through BlockBreakToken.ResumeTopOverride into _resumeTopOverride on the resuming pass) - not inside TryGetForcedBreakTarget itself, which stays a pure boundary value on purpose (its own slot/target output is asserted directly in ForcedBreakTargetIsTheFramesTests.cs and needs to keep meaning "the boundary", not "the boundary plus whatever margin happened to apply"). Un-ignores TargetIsTheBoundary_AndThePreservedMarginIsAddedToIt (HtmlRenderer.Test, ported in Batch 1) and ForcedBreak_MarginAfterBreak_IsPreservedNotTruncated (HtmlRenderer.IntegrationTest, ported in Batch 2). Full suite: HtmlRenderer.Test 2428 passed/127 skipped/0 failed (was 2427/128), HtmlRenderer.IntegrationTest 335 passed/91 skipped/0 failed (was 334/92), HtmlRenderer.PdfSharp.Test 39 passed/0 failed (unchanged).
1 parent dbe33fe commit 416ea3d

3 files changed

Lines changed: 13 additions & 21 deletions

File tree

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -895,14 +895,22 @@ protected virtual void PerformLayoutImp(RGraphics g)
895895
}
896896
else if (BlockFragmentation.TryGetForcedBreakTarget(this, prevSibling, baseTopWithoutMargin, out var breakSlot, out var breakTop))
897897
{
898+
// css-break-3 5.2 preserves a box's own top margin at a FORCED break (unlike an
899+
// unforced one, where BlockFragmentation.ResolveBlockTop truncates it to avoid
900+
// paginating through blank space) - breakTop itself is the page's own content
901+
// top (TryGetForcedBreakTarget's own contract, kept a pure boundary value so its
902+
// slot/target stay meaningful on their own), so the margin is added here, once,
903+
// at the point it becomes this box's actual placement.
904+
var breakTopWithMargin = breakTop + MarginTopCollapse(prevSibling);
905+
898906
if (CanDeferToLaterPass())
899907
{
900908
// A forced break-before/after applies and this is a genuinely fresh entry
901909
// (no resume state of any kind) - defer this box (and everything after it
902910
// in its parent's child loop) to a later pass entirely, rather than
903911
// positioning it now.
904912
RequestedBreakBeforeSlot = breakSlot;
905-
RequestedBreakBeforeTop = breakTop;
913+
RequestedBreakBeforeTop = breakTopWithMargin;
906914
return;
907915
}
908916

@@ -911,7 +919,7 @@ protected virtual void PerformLayoutImp(RGraphics g)
911919
// before real pass-based deferral existed. Not ideal (this content doesn't
912920
// get a fresh fragmentainer pass the way top-level content does), but correct
913921
// rather than silently measured-but-never-positioned.
914-
top = breakTop;
922+
top = breakTopWithMargin;
915923
}
916924
else
917925
{

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

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,11 @@ namespace TheArtOfDev.HtmlRenderer.IntegrationTest.Fragmentation;
2828
/// only ever tests <c>BreakValues.IsForcedBreak</c> on <c>break-before</c>/<c>break-after</c>. Named-page
2929
/// attribution/transitions are out of scope per the port plan's general exclusion list.
3030
/// <para>
31-
/// Confirmed by actually running these against the real engine (not just reading source): three more real
31+
/// Confirmed by actually running these against the real engine (not just reading source): two more real
3232
/// behavioral differences from PeachPDF surfaced, each documented on its own test below -
3333
/// <c>BreakBeforeAlways_IsAcceptedAsAForcedBreak_UnlikePeachPDF</c> (inverted, not dropped: a deliberate,
34-
/// documented design choice - see <c>BreakValues.IsForcedBreak</c>'s own remark),
35-
/// <c>ForcedBreak_MarginAfterBreak_IsPreservedNotTruncated</c> (Ignored: <c>TryGetForcedBreakTarget</c>'s
36-
/// <c>targetTop</c> is always the raw <c>PageTopOf(slot)</c>, discarding the box's own margin entirely
37-
/// rather than adding it back), and the "container left behind" gap extending to margin-truncation-caused
34+
/// documented design choice - see <c>BreakValues.IsForcedBreak</c>'s own remark), and the "container left
35+
/// behind" gap extending to margin-truncation-caused
3836
/// overflow specifically (2 tests Ignored - <c>EnforceKeepWithNext</c>'s pull only fires on an actual slot
3937
/// gap between a container and ITS OWN previous sibling, which a grandchild's margin truncation alone never
4038
/// creates, unlike <c>RelocateIfNeeded</c>'s straddle-triggered relocation - the case
@@ -283,12 +281,6 @@ public async Task HugeMultiPageMargin_TruncatesToZero_LandsOnVeryNextPage()
283281
// A forced break already relocates the previous sibling's bottom to the next page's top - per
284282
// css-break-3 §5.2, PeachPDF preserves (does not truncate) the margin AFTER a forced break.
285283
[TestMethod]
286-
[Ignore("Confirmed gap: BlockFragmentation.TryGetForcedBreakTarget's targetTop is always the raw "
287-
+ "container.PageTopOf(slot) - the box's own MarginTopCollapse is used only to decide WHICH slot "
288-
+ "the natural position falls in, never added back into the final target. So a forced-break box's "
289-
+ "own margin-top is silently discarded, not preserved, unlike PeachPDF. Confirmed by running this "
290-
+ "test unignored: the box lands at exactly PageTopOf(1) (320 in this fixture's original 300/20 "
291-
+ "page grid) rather than PageTopOf(1)+50.")]
292284
public async Task ForcedBreak_MarginAfterBreak_IsPreservedNotTruncated()
293285
{
294286
var (root, container) = await BuildAsync(

‎Source/Test/HtmlRenderer.Test/Fragmentation/ForcedBreakTargetIsTheFramesTests.cs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -143,14 +143,6 @@ public void ConsecutiveForcedBreaks_StepPastTheMarkerRatherThanCollapsingOntoIt(
143143
// §5.2 preserves the margin on the new page's side of a FORCED break, so the box lands one margin
144144
// below the target rather than on it - which is exactly why the target is worth asserting
145145
// separately from the position.
146-
[Ignore("This port does not preserve a box's own top margin at a forced-break target - confirmed by " +
147-
"direct source read and empirically. BlockFragmentation.TryGetForcedBreakTarget returns the raw " +
148-
"slot boundary (Core/Fragmentation/BlockFragmentation.cs ~87: 'targetTop = " +
149-
"container.PageTopOf(slot)'), and every consumer places the box flush there with no margin " +
150-
"added: CssBox.cs ~914 ('top = breakTop') for the immediate-placement path, and ~974 " +
151-
"(ResumeTopOverride: childBox.RequestedBreakBeforeTop) for the deferred-pass path that resumes " +
152-
"via ~894 ('top = _resumeTopOverride.Value'). A 30px break-before box lands at SlotTop(1) " +
153-
"exactly, not SlotTop(1)+30, unlike PeachPDF's css-break-3 §5.2 margin preservation.")]
154146
[TestMethod]
155147
public void TargetIsTheBoundary_AndThePreservedMarginIsAddedToIt()
156148
{

0 commit comments

Comments
 (0)