Skip to content

Commit 267c3e0

Browse files
committed
Retire Orphans_RejectsZero: not a real gap, fix the test's own premise instead
Investigated the reported gap (OrphansProperty/WidowsProperty accepting 0 at the CSS-parse layer) before touching production code, per this project's own "verify before implementing" discipline. Tried the described fix - switching both properties from NaturalIntegerConverter/IntegerConverter to PositiveIntegerConverter - and it broke two existing, directly-ported-from- PeachPDF tests: Css/PropertyPaginationTests.cs's CssOrphansZeroLegal and CssWidowsZeroLegal (ported verbatim from PeachPDF.Tests/CSS/Property.cs) assert that PeachPDF's OWN Property.Value for "orphans:0"/"widows:0" IS "0", not a fallback - confirming this fork's raw parsing already matches its reference implementation correctly. CssBox.Orphans/Widows are thin wrappers over that same declared string, so returning "0" verbatim is correct, not a bug - it mirrors how a browser's CSS OM keeps an out-of-range "specified value" visible even though layout never uses it directly. The actual css-break-3 constraint (>=1) is enforced exactly once, where it belongs: ActualOrphans/ActualWidows already treat any non-positive parse as unset and fall back to the initial value of 2 - already correct, already covered by every other test in this file that reads those accessors instead of the raw string. Reverted the converter change; rewrote the test itself (Orphans_ZeroResolvesToDefault_ThoughTheDeclaredStringStaysZero, Widows_ZeroResolvesToDefault_ThoughTheDeclaredStringStaysZero, Widows_NegativeResolvesToDefault) to assert BOTH facts explicitly - the declared string stays "0"/"-1", and ActualOrphans/ActualWidows still resolve to 2 - so this distinction stays pinned rather than re-litigated. Full suite: HtmlRenderer.Test 2427 passed/128 skipped/0 failed (unchanged, confirms the revert restored the two PropertyPaginationTests), HtmlRenderer.IntegrationTest 352 passed/94 skipped/0 failed (was 330/94, +22 in this file, 0 net new skips - the file's own Ignore is fully gone now).
1 parent 6c67f40 commit 267c3e0

1 file changed

Lines changed: 30 additions & 8 deletions

File tree

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

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -105,17 +105,39 @@ public async Task Widows_ParsesExplicitValue()
105105
Assert.AreEqual("1", FindById(root, "p").Widows);
106106
}
107107

108-
// orphans/widows must be >= 1 per spec; an invalid value leaves the property at its default.
108+
// orphans/widows must be >= 1 per spec (a used-value constraint) - but the DECLARED value ("0") is
109+
// still syntactically legal CSS and is stored verbatim, exactly matching PeachPDF's own real behavior:
110+
// Css/PropertyPaginationTests.cs (ported directly from PeachPDF.Tests/CSS/Property.cs) already asserts
111+
// OrphansProperty/WidowsProperty parse "0" into Property.Value=="0", not a fallback. CssBox.Orphans/
112+
// Widows are thin wrappers over that same declared string, so they correctly return "0" too - this was
113+
// confirmed the hard way: an earlier attempt to make the raw string reject 0 at the CSS-engine level
114+
// broke CssOrphansZeroLegal/CssWidowsZeroLegal outright. The actual spec constraint (>=1) is enforced
115+
// exactly once, at the point real layout consumes the value: ActualOrphans/ActualWidows (below) already
116+
// treat any non-positive parse as unset and fall back to the CSS initial value of 2 - which is what
117+
// this test should really be pinning, not the raw declared string.
109118
[TestMethod]
110-
[Ignore("Confirmed gap: OrphansProperty/WidowsProperty (Core/CssEngine/StyleProperties/OrphansProperty.cs, "
111-
+ "WidowsProperty.cs) both use Converters.NaturalIntegerConverter.OrDefault(2), which accepts 0 - "
112-
+ "css-break-3 requires a positive integer (>= 1), which is what the separate "
113-
+ "Converters.PositiveIntegerConverter enforces elsewhere in this codebase. 'orphans:0' is parsed and "
114-
+ "stored as \"0\" rather than falling back to the default.")]
115-
public async Task Orphans_RejectsZero()
119+
public async Task Orphans_ZeroResolvesToDefault_ThoughTheDeclaredStringStaysZero()
116120
{
117121
var (root, _) = await BuildAsync("<p id='p' style='orphans:0'>text</p>");
118-
Assert.AreEqual("2", FindById(root, "p").Orphans);
122+
var p = FindById(root, "p");
123+
Assert.AreEqual("0", p.Orphans);
124+
Assert.AreEqual(2, p.ActualOrphans);
125+
}
126+
127+
[TestMethod]
128+
public async Task Widows_ZeroResolvesToDefault_ThoughTheDeclaredStringStaysZero()
129+
{
130+
var (root, _) = await BuildAsync("<p id='p' style='widows:0'>text</p>");
131+
var p = FindById(root, "p");
132+
Assert.AreEqual("0", p.Widows);
133+
Assert.AreEqual(2, p.ActualWidows);
134+
}
135+
136+
[TestMethod]
137+
public async Task Widows_NegativeResolvesToDefault()
138+
{
139+
var (root, _) = await BuildAsync("<p id='p' style='widows:-1'>text</p>");
140+
Assert.AreEqual(2, FindById(root, "p").ActualWidows);
119141
}
120142

121143
[TestMethod]

0 commit comments

Comments
 (0)