Skip to content

Commit 67bab41

Browse files
committed
Add text-indent layout tests, surface a real word-spacing inheritance bug
text-indent was fully implemented (parsed + wired into layout) but untested beyond CSS-OM parsing - added first-line-only indent coverage (the plain CSS2.1 behavior; CSS Text Level 4's hanging/ each-line/RTL variants aren't ported, since this fork's text-indent is the simpler CSS2.1 one). Attempting the equivalent word-spacing port surfaced a real, confirmed bug: word-spacing is fully wired (CSS-OM, CssBoxProperties, CssUtils dispatch, CssRect.FullWidth) but has zero effect on ordinary adjacent words in normal text flow, whether set on the text-owning block directly or an ancestor - the anonymous text box that actually owns the words never inherits it, always reading back "normal". PeachPDF's own equivalent test passes (confirmed by running it), so this is a real regression against a working reference implementation, not a PeachPDF gap to report upstream. Two tests document this precisely, [Ignore]d rather than fixed here (root-causing the anonymous-text-box inheritance path was out of this pass's budget); flagged as a follow-up task.
1 parent 4576e52 commit 67bab41

1 file changed

Lines changed: 140 additions & 0 deletions

File tree

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
using System.Linq;
2+
using HtmlRenderer.IntegrationTest.TestSupport;
3+
4+
namespace HtmlRenderer.IntegrationTest.Text;
5+
6+
/// <summary>
7+
/// CSS 2.1 §16.1 (<c>text-indent</c>) and §16.4 (<c>word-spacing</c>) layout coverage - both properties were
8+
/// already fully implemented (unlike <c>letter-spacing</c>, which this port found has zero layout wiring at
9+
/// all - see remarks below), just untested at the layout level (only CSS-OM parsing existed). Ported/adapted
10+
/// from PeachPDF's TextIndentLayoutIntegrationTests.cs/LetterWordSpacingLayoutIntegrationTests.cs, restricted
11+
/// to the CSS 2.1 subset: a single <c>text-indent</c> length applied to a block's first line only (no
12+
/// <c>hanging</c>/<c>each-line</c> - CSS Text Level 4 - and no RTL-specific cases, since this fork's
13+
/// text-indent implementation is the plain CSS2.1 one).
14+
/// </summary>
15+
/// <remarks>
16+
/// <c>letter-spacing</c> is a confirmed, real gap found while porting this file: it parses fine
17+
/// (<c>CSS/FontKeywordPropertyTests.cs</c>) but has zero wiring anywhere in <c>Core</c> - no
18+
/// <c>CssBoxProperties.LetterSpacing</c> field, and no case in <c>CssUtils.GetPropertyValue</c>/
19+
/// <c>SetPropertyValue</c>/<c>_knownPropertyNames</c> (confirmed by grep - contrast with <c>word-spacing</c>,
20+
/// which has all of these). Implementing it is a real, separate word-measurement change (not a small
21+
/// backport - PeachPDF's own fix touched word-box-width reservation, inter-word-gap collapse prevention, and
22+
/// adjacent-inline-run overlap, per that file's own remarks) and is out of scope for this pass; flagged as a
23+
/// follow-up rather than built here.
24+
/// <para>
25+
/// A second, more fundamental gap this file's own tests found: <c>word-spacing</c>, however it's set
26+
/// (directly on the text-owning block, or on an ancestor further up - both fail identically), never
27+
/// actually reaches the inter-word gap for plain adjacent words in normal text flow. Root cause, confirmed
28+
/// by direct inspection: the CssRectWord that owns each word is an anonymous child text box, and that
29+
/// anonymous child's own <c>WordSpacing</c> reads back "normal" regardless of what any ancestor - including
30+
/// the direct parent - has set, so <c>CssRect.FullWidth</c>'s <c>OwnerBox.ActualWordSpacing</c> term is
31+
/// always the unspaced default. (<c>CssLayoutEngine.cs</c> has two call sites that DO add
32+
/// <c>ActualWordSpacing</c> correctly - for a stray whitespace-only box between two separately-styled
33+
/// inline elements - plus a third, commented-out block that looks like an abandoned attempt at exactly
34+
/// this normal-adjacent-word case.) Not fixed here - the anonymous text-box creation/inheritance path
35+
/// (<c>CorrectTextBoxes</c> et al. in <c>Parse/DomParser.cs</c>) is a shared, high-traffic piece of the
36+
/// engine, and root-causing this precisely was out of this pass's budget.
37+
/// </para>
38+
/// </remarks>
39+
[DoNotParallelize]
40+
[TestClass]
41+
public sealed class TextSpacingAndIndentLayoutIntegrationTests
42+
{
43+
[Ignore("word-spacing set directly on the block that owns the words has no effect - see this class's own " +
44+
"remarks for the root cause (the anonymous text child that actually owns the words doesn't inherit " +
45+
"its own parent's directly-set word-spacing, confirmed by direct inspection: the child's own " +
46+
"WordSpacing reads back \"normal\").")]
47+
[TestMethod]
48+
public void WordSpacing_IncreasesGapBetweenAdjacentWords()
49+
{
50+
var spacedGap = InterWordGap("<p id='p' style='word-spacing:10px'>AA BB</p>");
51+
var plainGap = InterWordGap("<p id='p'>AA BB</p>");
52+
53+
Assert.AreEqual(plainGap + 10, spacedGap, 1);
54+
}
55+
56+
[Ignore("Same root cause as WordSpacing_IncreasesGapBetweenAdjacentWords above, and fails identically " +
57+
"whether word-spacing is set directly on the text-owning block or (as here) an ancestor further " +
58+
"up - see this class's own remarks.")]
59+
[TestMethod]
60+
public void WordSpacing_OnAncestor_AffectsDescendantGap()
61+
{
62+
var spacedGap = InterWordGap("<div style='word-spacing:10px'><p id='p'>AA BB</p></div>");
63+
var plainGap = InterWordGap("<p id='p'>AA BB</p>");
64+
65+
Assert.AreEqual(plainGap + 10, spacedGap, 1);
66+
}
67+
68+
[TestMethod]
69+
public void WordSpacingNormal_LeavesGapUnchanged()
70+
{
71+
var normalGap = InterWordGap("<p id='p' style='word-spacing:normal'>AA BB</p>");
72+
var plainGap = InterWordGap("<p id='p'>AA BB</p>");
73+
74+
Assert.AreEqual(plainGap, normalGap, 1);
75+
}
76+
77+
[Ignore("Same root cause as WordSpacing_IncreasesGapBetweenAdjacentWords above - word-spacing set " +
78+
"directly on the text-owning block has no effect.")]
79+
[TestMethod]
80+
public void WordSpacingNegative_ReducesGap()
81+
{
82+
var negativeGap = InterWordGap("<p id='p' style='word-spacing:-5px'>AA BB</p>");
83+
var plainGap = InterWordGap("<p id='p'>AA BB</p>");
84+
85+
Assert.AreEqual(plainGap - 5, negativeGap, 1);
86+
}
87+
88+
[TestMethod]
89+
public void TextIndent_IndentsOnlyTheFirstLine()
90+
{
91+
var html = LayoutHarness.Wrap(
92+
"<p id='p' style='width:120px; text-indent:40px; margin:0;'>"
93+
+ "This text is long enough that it wraps onto a second line for sure.</p>");
94+
var (root, _) = LayoutHarness.Layout(html);
95+
var p = LayoutHarness.FindById(root, "p")!;
96+
97+
var wordsByLine = AllWords(p).GroupBy(w => w.Top).OrderBy(g => g.Key).ToList();
98+
Assert.IsTrue(wordsByLine.Count >= 2, "fixture text should wrap onto at least two lines");
99+
100+
var firstLineLeft = wordsByLine[0].Min(w => w.Left);
101+
var secondLineLeft = wordsByLine[1].Min(w => w.Left);
102+
103+
Assert.AreEqual(40, firstLineLeft, 1);
104+
Assert.AreEqual(0, secondLineLeft, 1);
105+
}
106+
107+
[TestMethod]
108+
public void TextIndentZero_NoIndentOnAnyLine()
109+
{
110+
var html = LayoutHarness.Wrap("<p id='p' style='margin:0;'>Hello world</p>");
111+
var (root, _) = LayoutHarness.Layout(html);
112+
var p = LayoutHarness.FindById(root, "p")!;
113+
114+
var firstWord = AllWords(p).OrderBy(w => w.Left).First();
115+
Assert.AreEqual(0, firstWord.Left, 1);
116+
}
117+
118+
// ── Helpers ────────────────────────────────────────────────────────────
119+
120+
private static double InterWordGap(string bodyHtml)
121+
{
122+
var html = LayoutHarness.Wrap(bodyHtml);
123+
var (root, _) = LayoutHarness.Layout(html);
124+
var p = LayoutHarness.FindById(root, "p")!;
125+
var words = AllWords(p).OrderBy(w => w.Left).ToList();
126+
127+
Assert.AreEqual(2, words.Count);
128+
return words[1].Left - (words[0].Left + words[0].Width);
129+
}
130+
131+
private static System.Collections.Generic.IEnumerable<TheArtOfDev.HtmlRenderer.Core.Dom.CssRect> AllWords(
132+
TheArtOfDev.HtmlRenderer.Core.Dom.CssBox box)
133+
{
134+
foreach (var word in box.Words) yield return word;
135+
foreach (var child in box.Boxes)
136+
{
137+
foreach (var word in AllWords(child)) yield return word;
138+
}
139+
}
140+
}

0 commit comments

Comments
 (0)