diff --git a/Source/HtmlRenderer/Core/Dom/CssBox.cs b/Source/HtmlRenderer/Core/Dom/CssBox.cs index ed14dc5ac..1ef899648 100644 --- a/Source/HtmlRenderer/Core/Dom/CssBox.cs +++ b/Source/HtmlRenderer/Core/Dom/CssBox.cs @@ -888,10 +888,19 @@ protected virtual void PerformLayoutImp(RGraphics g) else { left = ContainingBlock.Location.X + ContainingBlock.ActualPaddingLeft + ActualMarginLeft + ContainingBlock.ActualBorderLeftWidth; + // The baseline to add this box's own collapsed top margin (below) on top of. A + // self-collapsing prevSibling (CSS 2.1 8.3.1) contributes no space of its own and + // is transparent to margin collapsing - its own position already reflects part of + // the SAME adjoining-margin set MarginTopCollapse resolves below, so anchoring here + // against it directly would double-count that shared portion. marginAnchor walks + // back through any run of self-collapsing siblings to the first real (non-empty) + // one, matching the same walk CollectAdjoiningMarginsBeforeSibling does when + // gathering the margin SET itself. + var marginAnchor = FindMarginAnchorSibling(prevSibling); // StaticBottom (not ActualBottom): a relatively-positioned previous sibling's visual // offset must not drag this box down with it (CSS 2.1 9.4.3 - relative positioning // "has no effect on the position of any other box"). Ported from PeachPDF. - var baseTopWithoutMargin = (prevSibling == null && ParentBox != null ? ParentBox.ClientTop : ParentBox == null ? Location.Y : 0) + (prevSibling != null ? prevSibling.StaticBottom + prevSibling.ActualBorderBottomWidth : 0); + var baseTopWithoutMargin = (marginAnchor == null && ParentBox != null ? ParentBox.ClientTop : ParentBox == null ? Location.Y : 0) + (marginAnchor != null ? marginAnchor.StaticBottom + marginAnchor.ActualBorderBottomWidth : 0); if (_incomingToken != null && ReferenceEquals(_incomingToken.Box, this)) { @@ -1597,25 +1606,71 @@ internal bool HasJustInlineSiblings() } /// - /// Gets the result of collapsing the vertical margins of the two boxes - /// - /// the previous box under the same parent + /// Gets the result of collapsing the vertical margins of the two boxes, per CSS 2.1 §8.3.1's real + /// "adjoining margin set" rule (collapse to max(positives) + min(negatives) over the WHOLE + /// set of margins that adjoin - not a pairwise Math.Max between exactly two, which only + /// happens to match when both margins are positive). The set can span: + /// + /// this box's own top margin, extended forward through an unblocked chain of first-in-flow + /// children () - collapsing "through" the box so a nested + /// child sits flush with it rather than being pushed down twice; + /// 's own bottom-margin chain, and - if + /// is itself self-collapsing (empty, zero border/padding/height) - reaching further back through + /// however many more self-collapsing siblings (and, if the chain runs out of siblings, the + /// parent's own top-margin chain) precede it, since a self-collapsing box is transparent to margin + /// collapsing entirely (its own top and bottom margins merge into the same set too). + /// + /// A floated box's own margin never participates in collapsing at all (CSS 2.1 §8.3.1: floats are + /// out of flow) - the predecessor's trailing margin is still real space, but this box's own margin + /// adds on top of it rather than merging. + /// + /// the previous in-flow box under the same parent, or null if this is the first /// Resulting top margin - internal double MarginTopCollapse(CssBoxProperties prevSibling) + internal double MarginTopCollapse(CssBox prevSibling) { double value; - if (prevSibling != null) + if (IsFloated) { - value = Math.Max(prevSibling.ActualMarginBottom, ActualMarginTop); + value = (prevSibling?.ActualMarginBottom ?? 0) + ActualMarginTop; CollapsedMarginTop = value; } - else if (_parentBox != null && ActualPaddingTop < 0.1 && ActualPaddingBottom < 0.1 && _parentBox.ActualPaddingTop < 0.1 && _parentBox.ActualPaddingBottom < 0.1) + else if (prevSibling != null) { - value = Math.Max(0, ActualMarginTop - Math.Max(_parentBox.ActualMarginTop, _parentBox.CollapsedMarginTop)); + var margins = new List(); + CollapsedMarginTopChain(this, margins); + CollectAdjoiningMarginsBeforeSibling(prevSibling, margins); + + value = CollapseMarginSet(margins); + // A real sibling gap is always genuinely, geometrically realized right here (this box's + // own position is baseTopWithoutMargin + value) - so it's a fresh anchor for whatever + // comes next to build on, independent of anything before prevSibling. + CollapsedMarginTop = value; + } + else if (_parentBox != null && !BlocksParentChildTopJoin(_parentBox, this)) + { + // This box's own semantic top margin, extended forward into its own subtree (its first + // in-flow child's chain, if unblocked, and so on). + var forwardMargins = new List(); + CollapsedMarginTopChain(this, forwardMargins); + var forwardValue = CollapseMarginSet(forwardMargins); + + // Only the excess beyond what the parent's OWN placement already geometrically realized is + // left for this box to apply - parent.CollapsedMarginTop is the CUMULATIVE amount actually + // applied so far along this unblocked first-in-flow-child chain (not merely this box's own + // semantic total): critically, that's 0 at the true document root, which cannot itself + // move no matter how large a margin collapses into it - see + // FirstChildMarginCollapsingToTheRoot_IsStillTruncated_UnlikePeachPDF, where the excess + // must still surface as real displacement (later truncated by page-break logic) rather + // than being silently absorbed by a root that only ever contributes its own 0 margin. + value = Math.Max(0, forwardValue - _parentBox.CollapsedMarginTop); + CollapsedMarginTop = _parentBox.CollapsedMarginTop + value; } else { value = ActualMarginTop; + // Blocked (or no parent at all): a fresh realized-origin, unrelated to whatever the parent + // chain was carrying - matches every other "stop the chain here" case in this method. + CollapsedMarginTop = value; } // fix for hr tag @@ -1627,6 +1682,215 @@ internal double MarginTopCollapse(CssBoxProperties prevSibling) return value; } + /// + /// Adds 's own top margin, and - if unblocked - its first in-flow child's + /// own chain (recursively), to . This is the "forward" half of §8.3.1's + /// adjoining set: it's what lets a parent's own top-margin placement already account for a nested, + /// unblocked child's top margin too, so the child can sit flush with the parent instead of being + /// pushed down a second time once the child is itself placed (see 's + /// no-prevSibling branch). + /// + private static void CollapsedMarginTopChain(CssBox box, List margins) + { + margins.Add(ResolveMarginTopForChain(box)); + + var firstChild = FirstInFlowChild(box); + if (firstChild != null && !BlocksParentChildTopJoin(box, firstChild)) + { + CollapsedMarginTopChain(firstChild, margins); + } + } + + /// + /// Adds 's own bottom margin, and - if unblocked - its last in-flow child's + /// own chain (recursively), to . The "forward" half for the bottom edge, + /// mirroring . + /// + private static void CollapsedMarginBottomChain(CssBox box, List margins) + { + margins.Add(ResolveMarginBottomForChain(box)); + + var lastChild = LastInFlowChild(box); + if (lastChild != null && !BlocksParentChildBottomJoin(box)) + { + CollapsedMarginBottomChain(lastChild, margins); + } + } + + /// + /// resolves a percentage margin against this box's + /// OWN .Width, not its containing block's (a pre-existing quirk elsewhere in + /// this engine, left as-is here to avoid a wider blast radius) - fine once a box's own width has + /// actually been computed, but deliberately reads a + /// not-yet-laid-out descendant's margin (that's the whole point of the forward lookahock - see its + /// own remarks), so Size.Width is still the zero default there. Resolves against + /// 's width instead when this box's own margin is a percentage + /// and its containing block is one level up (already sized, since this is always called from that + /// ancestor's own placement, after its own width computation ran) - correct AND safe. A percentage + /// margin more than one unblocked level deep still can't be resolved this way (the ancestor two + /// levels up hasn't been sized either at this point) and falls back to the same 0 the pre-existing + /// bug already produced. + /// + private static double ResolveMarginTopForChain(CssBox box) + { + if (!string.IsNullOrEmpty(box.MarginTop) && box.MarginTop.EndsWith("%") && box.ParentBox != null) + { + return CssValueParser.ParseLength(box.MarginTop, box.ContainingBlock.Size.Width, box); + } + return box.ActualMarginTop; + } + + /// + private static double ResolveMarginBottomForChain(CssBox box) + { + if (!string.IsNullOrEmpty(box.MarginBottom) && box.MarginBottom.EndsWith("%") && box.ParentBox != null) + { + return CssValueParser.ParseLength(box.MarginBottom, box.ContainingBlock.Size.Width, box); + } + return box.ActualMarginBottom; + } + + /// + /// The "backward" half of §8.3.1's adjoining set: adds 's own bottom- + /// margin chain to , and - if is itself + /// self-collapsing (so its own top and bottom margins merge into the very same set, per spec) - + /// reaches further back through however many more self-collapsing predecessors adjoin before it, + /// including (once siblings run out) the parent's own forward chain. + /// + private static void CollectAdjoiningMarginsBeforeSibling(CssBox sibling, List margins) + { + CollapsedMarginBottomChain(sibling, margins); + + if (!IsSelfCollapsingEmpty(sibling)) return; + + CollapsedMarginTopChain(sibling, margins); + + var earlierSibling = DomUtils.GetPreviousSibling(sibling); + if (earlierSibling != null) + { + CollectAdjoiningMarginsBeforeSibling(earlierSibling, margins); + } + else if (sibling.ParentBox != null && !BlocksParentChildTopJoin(sibling.ParentBox, sibling)) + { + // sibling was itself its parent's first in-flow child - the parent's own placement already + // folded its own chain in via CollapsedMarginTopChain, so that value is already correct and + // complete (no need to walk further back past the parent - see MarginTopCollapse's + // no-prevSibling branch for why the parent's own value is already the full answer). + CollapsedMarginTopChain(sibling.ParentBox, margins); + } + } + + /// + /// Walks back from through any run of self-collapsing (empty) + /// siblings to the first real one - the box whose own border-box bottom edge is a genuine position + /// baseline, since a self-collapsing box's own position already reflects part of the shared + /// adjoining-margin set rather than occupying real space of its own. Returns null if + /// itself is null, or the ENTIRE preceding run turns out to be + /// self-collapsing all the way back (meaning there's no real sibling to anchor against - falls + /// through to the parent's own content-top instead, same as having no previous sibling at all). + /// + internal static CssBox FindMarginAnchorSibling(CssBox prevSibling) + { + var current = prevSibling; + while (current != null && IsSelfCollapsingEmpty(current)) + { + current = DomUtils.GetPreviousSibling(current); + } + return current; + } + + /// + /// CSS 2.1 §8.3.1: an empty box (no border/padding on either the top or bottom edge, and zero + /// height) is "self-collapsing" - transparent to margin collapsing, its own top and bottom margins + /// merge into whatever set adjoins either edge, rather than keeping them apart. Relies on + /// already being fully laid out (true for a previous sibling, which is + /// always placed - including its own subtree - before this box is). + /// + private static bool IsSelfCollapsingEmpty(CssBox box) + { + return box.ActualBorderTopWidth + box.ActualBorderBottomWidth + box.ActualPaddingTop + box.ActualPaddingBottom < 0.1 + && box.ActualBottom - box.Location.Y < 0.1; + } + + /// A parent's own top border/padding, or the child's own clearance, blocks the top-margin join. + private static bool BlocksParentChildTopJoin(CssBox parent, CssBox child) + { + return parent.ActualBorderTopWidth > 0.1 || parent.ActualPaddingTop > 0.1 + || (!string.IsNullOrEmpty(child.Clear) && child.Clear != CssConstants.None) + // CSS 2.1 §8.3.1 margin collapsing is a block-formatting-context concept - an inline + // parent (e.g. a block-level child anonymously boxed inside a ) is not a block + // container to collapse "through" in the first place. + || parent.Display == CssConstants.Inline || parent.Display == CssConstants.InlineBlock + || EstablishesBlockFormattingContext(parent); + } + + /// + /// A box's own bottom border/padding, a height other than auto, or a new block formatting context + /// blocks the bottom-margin join with its last child. + /// + private static bool BlocksParentChildBottomJoin(CssBox box) + { + return box.ActualBorderBottomWidth > 0.1 || box.ActualPaddingBottom > 0.1 + || (!string.IsNullOrEmpty(box.Height) && box.Height != CssConstants.Auto) + || EstablishesBlockFormattingContext(box); + } + + /// + /// Floats, absolutely positioned boxes, table cells and captions, and boxes with overflow other than + /// visible start a new block formatting context, so their margins never collapse with their + /// children's (CSS 2.1 §8.3.1, §9.4.1). + /// + private static bool EstablishesBlockFormattingContext(CssBox box) + { + return box.IsFloated + || box.Position == CssConstants.Absolute || box.Position == CssConstants.Fixed + || box.Display == CssConstants.TableCell || box.Display == CssConstants.TableCaption + || (!string.IsNullOrEmpty(box.Overflow) && box.Overflow != CssConstants.Visible); + } + + /// + /// The first in-flow child that's itself a block container - i.e. one margin collapsing could + /// possibly apply to (CSS 2.1 §8.3.1 is a block-formatting-context concept). Skips not just + /// out-of-flow/display:none children but inline/inline-block ones too: an anonymous inline/text + /// run preceding a real block child (e.g. "text<p>...</p>") is not itself a candidate + /// to collapse through, and must not stop the search before reaching the real block child. + /// + private static CssBox FirstInFlowChild(CssBox box) + { + return box._boxes.Find(IsBlockLevelInFlowChild); + } + + /// + private static CssBox LastInFlowChild(CssBox box) + { + return box._boxes.FindLast(IsBlockLevelInFlowChild); + } + + private static bool IsBlockLevelInFlowChild(CssBox b) + { + return !b.IsOutOfFlow && b.Display != CssConstants.None + && b.Display != CssConstants.Inline && b.Display != CssConstants.InlineBlock; + } + + /// + /// CSS 2.1 §8.3.1: an adjoining margin set collapses to the largest positive margin plus the most + /// negative (smallest) margin in the set - equal to a plain max() only when every member shares the + /// same sign. + /// + private static double CollapseMarginSet(List margins) + { + double maxPositive = 0; + double minNegative = 0; + + foreach (var m in margins) + { + if (m > maxPositive) maxPositive = m; + if (m < minNegative) minNegative = m; + } + + return maxPositive + minNegative; + } + /// /// Calculate the actual right of the box by the actual right of the child boxes if this box actual right is not set. /// diff --git a/Source/HtmlRenderer/Core/Dom/CssBoxHr.cs b/Source/HtmlRenderer/Core/Dom/CssBoxHr.cs index a2b31c828..2495c4e5a 100644 --- a/Source/HtmlRenderer/Core/Dom/CssBoxHr.cs +++ b/Source/HtmlRenderer/Core/Dom/CssBoxHr.cs @@ -48,9 +48,13 @@ protected override void PerformLayoutImp(RGraphics g) var prevSibling = DomUtils.GetPreviousSibling(this); double left = ContainingBlock.Location.X + ContainingBlock.ActualPaddingLeft + ActualMarginLeft + ContainingBlock.ActualBorderLeftWidth; + // The baseline for this rule's own collapsed top margin, matching CssBox.PerformLayoutImp's + // own placement: a self-collapsing prevSibling (CSS 2.1 8.3.1) contributes no space of its own + // and must not be anchored against directly (see FindMarginAnchorSibling). + var marginAnchor = FindMarginAnchorSibling(prevSibling); // StaticBottom (not ActualBottom): a relatively-positioned previous sibling's visual offset must // not drag this rule down with it (CSS 2.1 9.4.3), matching the same fix in CssBox.PerformLayoutImp. - double top = (prevSibling == null && ParentBox != null ? ParentBox.ClientTop : ParentBox == null ? Location.Y : 0) + MarginTopCollapse(prevSibling) + (prevSibling != null ? prevSibling.StaticBottom + prevSibling.ActualBorderBottomWidth : 0); + double top = (marginAnchor == null && ParentBox != null ? ParentBox.ClientTop : ParentBox == null ? Location.Y : 0) + MarginTopCollapse(prevSibling) + (marginAnchor != null ? marginAnchor.StaticBottom + marginAnchor.ActualBorderBottomWidth : 0); Location = new RPoint(left, top); ActualBottom = top; diff --git a/Source/Test/HtmlRenderer.IntegrationTest/Baselines/ACID 1.png b/Source/Test/HtmlRenderer.IntegrationTest/Baselines/ACID 1.png index 0ba3367e5..f9c82c9c2 100644 Binary files a/Source/Test/HtmlRenderer.IntegrationTest/Baselines/ACID 1.png and b/Source/Test/HtmlRenderer.IntegrationTest/Baselines/ACID 1.png differ diff --git a/Source/Test/HtmlRenderer.IntegrationTest/Baselines/BlockInInline.png b/Source/Test/HtmlRenderer.IntegrationTest/Baselines/BlockInInline.png index 0541b326f..0f94e7bcd 100644 Binary files a/Source/Test/HtmlRenderer.IntegrationTest/Baselines/BlockInInline.png and b/Source/Test/HtmlRenderer.IntegrationTest/Baselines/BlockInInline.png differ diff --git a/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Iframes.png b/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Iframes.png index 89ab0ac35..f6162d1f9 100644 Binary files a/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Iframes.png and b/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Iframes.png differ diff --git a/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Languages.png b/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Languages.png index 7b87e4ca1..9a85759a9 100644 Binary files a/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Languages.png and b/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Languages.png differ diff --git a/Source/Test/HtmlRenderer.IntegrationTest/Baselines/MaxWidth.png b/Source/Test/HtmlRenderer.IntegrationTest/Baselines/MaxWidth.png index 48522a053..16a5fa4e6 100644 Binary files a/Source/Test/HtmlRenderer.IntegrationTest/Baselines/MaxWidth.png and b/Source/Test/HtmlRenderer.IntegrationTest/Baselines/MaxWidth.png differ diff --git a/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Misc.png b/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Misc.png index ef95f22c5..6fdc3244c 100644 Binary files a/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Misc.png and b/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Misc.png differ diff --git a/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Paragraphs.png b/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Paragraphs.png index 64a988737..d29c91200 100644 Binary files a/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Paragraphs.png and b/Source/Test/HtmlRenderer.IntegrationTest/Baselines/Paragraphs.png differ diff --git a/Source/Test/HtmlRenderer.IntegrationTest/BoxModel/CollapsedMarginBeforeTests.cs b/Source/Test/HtmlRenderer.IntegrationTest/BoxModel/CollapsedMarginBeforeTests.cs index 1bb22a75f..459f48f99 100644 --- a/Source/Test/HtmlRenderer.IntegrationTest/BoxModel/CollapsedMarginBeforeTests.cs +++ b/Source/Test/HtmlRenderer.IntegrationTest/BoxModel/CollapsedMarginBeforeTests.cs @@ -7,22 +7,15 @@ namespace HtmlRenderer.IntegrationTest.BoxModel; /// set, resolved at one break point by the frame the boxes are children of. /// /// -/// -/// The set spans two frames, and these pin both halves: what precedes a box is the frame's to see (a -/// predecessor's bottom margin, and everything a run of self-collapsing predecessors folds in), while the box's -/// own top margin and the first-in-flow-child chain adjoining it are a walk into its own subtree. §8.3.1 -/// collapses the whole set at once, so the two are folded together rather than resolved separately. -/// -/// -/// This fork's margin-collapse machinery (CssBox.MarginTopCollapse/MarginBottomCollapse, -/// Dom/CssBox.cs ~1103-1183) only ever resolves a pairwise Math.Max between exactly two margins at -/// a time - never the full CSS2.1 "adjoining margin set" (which can span an arbitrary run of self-collapsing -/// boxes and nested first-in-flow children, and which spec requires collapsing to -/// max(positives) + min(negatives), not Math.Max alone). Math.Max on two same-signed margins -/// happens to equal the spec result by coincidence, but it diverges the moment the set has to look past a single -/// pair - whether because a negative margin is mixed in, or because more than two margins are actually -/// adjoining. See each test's own for the specific way it trips over this. -/// +/// CssBox.MarginTopCollapse (Dom/CssBox.cs) implements the real adjoining-margin-set algorithm: +/// CollapsedMarginTopChain/CollapsedMarginBottomChain walk forward through an unblocked run of +/// first/last-in-flow children (so a nested child sits flush with its parent instead of being pushed down a +/// second time), CollectAdjoiningMarginsBeforeSibling walks backward through however many self-collapsing +/// (empty) siblings precede a box - reaching all the way to the parent's own top margin if the whole run turns +/// out self-collapsing - and CollapseMarginSet collapses the resulting set to +/// max(positives) + min(negatives), not a plain Math.Max between just two values (which only +/// happens to match when both margins share a sign). A floated box's own margin never participates in +/// collapsing at all (CSS 2.1 §8.3.1: floats are out of flow). /// [DoNotParallelize] [TestClass] @@ -45,12 +38,6 @@ public void AdjoiningSiblingMargins_CollapseToTheLargerPositive() Assert.AreEqual(30, b.Location.Y - a.ActualBottom, Delta); } - [Ignore("Confirmed real bug, not a porting mistake: CssBox.MarginTopCollapse's sibling branch (Dom/CssBox.cs " + - "~line 1108) computes `Math.Max(prevSibling.ActualMarginBottom, ActualMarginTop)` for the gap between " + - "two in-flow siblings - for a positive/negative mixed pair (30, -10) that picks 30 (the larger " + - "value, treating the negative margin as if it weren't there), instead of CSS2.1's mixed-sign rule of " + - "summing the largest positive and the most-negative value (30 + (-10) = 20). Confirmed: " + - "b.Location.Y - a.ActualBottom is 30 on this fork, not the spec-correct 20.")] [TestMethod] public void MixedSignSiblingMargins_Sum() { @@ -64,14 +51,6 @@ public void MixedSignSiblingMargins_Sum() Assert.AreEqual(20, b.Location.Y - a.ActualBottom, Delta); } - [Ignore("Confirmed real bug, not a porting mistake: because MarginTopCollapse only ever looks at the single " + - "immediate previous sibling (Dom/CssBox.cs ~1103-1127), it cannot treat a self-collapsing empty box " + - "as transparent for a single 3-way adjoining set the way CSS2.1 requires. Instead it resolves two " + - "SEPARATE pairwise gaps in series - a-to-middle: Math.Max(a.marginBottom=10, middle.marginTop=40)=40, " + - "then middle-to-b: Math.Max(middle.marginBottom=40, b.marginTop=10)=40 - and since the empty middle " + - "box has zero height (its own margin-bottom is never folded into its own ActualBottom), those two " + - "40px gaps stack in series instead of collapsing to one shared 40px value. Confirmed: " + - "b.Location.Y - a.ActualBottom is 80 on this fork (40+40), not the spec-correct 40.")] [TestMethod] public void ASelfCollapsingSiblingBetweenTwoBoxes_JoinsTheSet() { @@ -87,16 +66,6 @@ public void ASelfCollapsingSiblingBetweenTwoBoxes_JoinsTheSet() Assert.AreEqual(40, b.Location.Y - a.ActualBottom, Delta); } - [Ignore("Confirmed real bug, not a porting mistake: MarginTopCollapse's parent-child branch (Dom/CssBox.cs " + - "~1111-1114) is `Math.Max(0, ActualMarginTop - Math.Max(parent.ActualMarginTop, parent.CollapsedMarginTop))` " + - "- an approximation that does not implement 'the whole chain collapses to one value, taken once' the " + - "way CSS2.1 requires. For outer(margin-top:10)/inner(margin-top:30) here: outer's own placement " + - "against 'a' only applies outer's OWN 10px margin (Math.Max(a.marginBottom=0, outer.marginTop=10)=10, " + - "not the spec-correct 30 which should reflect the whole outer+inner set), and inner then gets pushed " + - "an ADDITIONAL Math.Max(0, 30-Math.Max(10,10))=20px below outer's own top - so inner ends up 20px " + - "below outer (not flush with it), and outer is only 10px below 'a' (not 30px). Confirmed on this " + - "fork: outer.Location.Y - a.ActualBottom is 10 (not 30), and inner.Location.Y - outer.Location.Y is " + - "20 (not 0).")] [TestMethod] public void AFirstInFlowChildsTopMargin_JoinsItsParentsOwnSet() { @@ -115,16 +84,6 @@ public void AFirstInFlowChildsTopMargin_JoinsItsParentsOwnSet() Assert.AreEqual(outer.Location.Y, inner.Location.Y, Delta); } - [Ignore("Confirmed real bug, not a porting mistake: the parent-child join's guard condition (Dom/CssBox.cs " + - "~1111) only checks `ActualPaddingTop < 0.1 && ActualPaddingBottom < 0.1` on the box and its parent - " + - "it never checks BORDER width at all, even though CSS2.1 requires ANY border or padding between " + - "parent and child to block the join, not just padding. So outer's border-top:1px here fails to block " + - "the outer/inner join: inner still gets pulled into the same Math.Max(0, inner.marginTop=30 - " + - "Math.Max(outer.marginTop=10, outer.CollapsedMarginTop=10))=20px-below-outer's-ClientTop computation " + - "as the no-border case, landing it 1px (border) + 20px = 21px below outer, not the spec-correct " + - "1px + 30px = 31px where the border fully blocks the join and both margins stand independently. (The " + - "outer-vs-'a' gap of 10px does coincidentally match here, since that part never depended on the " + - "border in the first place.)")] [TestMethod] public void ABorderOnTheParent_BlocksTheChainAndLeavesBothMarginsStanding() { @@ -141,15 +100,6 @@ public void ABorderOnTheParent_BlocksTheChainAndLeavesBothMarginsStanding() Assert.AreEqual(31, inner.Location.Y - outer.Location.Y, Delta); } - [Ignore("Confirmed real bug, not a porting mistake, compounding two gaps: (1) this fork's block-placement " + - "seam does not special-case Position=float the way it does display:none/absolute/fixed siblings (the " + - "same gap BoxModel.BlockPlacementSeamTests.AFloatedSibling_IsNotThePredecessorTheFrameResolvesAgainst " + - "documents) - so the floated 'f' box is positioned via the exact same in-flow " + - "top = prevSibling.ActualBottom + MarginTopCollapse(prevSibling) formula as any ordinary block, " + - "instead of being excluded from margin collapsing entirely per CSS2.1 (a float's margin should never " + - "adjoin anything). (2) That collapse is then itself the mixed-sign Math.Max bug this class's remarks " + - "describe: Math.Max(a.marginBottom=20, f.marginTop=-5)=20, not the spec-correct sum " + - "20+(-5)=15. Confirmed: f.Location.Y - a.ActualBottom is 20 on this fork, not 15.")] [TestMethod] public void AFloatsOwnMarginDoesNotMerge_ButThePredecessorsStillOccupiesSpace() { @@ -164,4 +114,32 @@ public void AFloatsOwnMarginDoesNotMerge_ButThePredecessorsStillOccupiesSpace() // predecessor's trailing margin is real space it must sit after. Assert.AreEqual(15, f.Location.Y - a.ActualBottom, Delta); } + + [TestMethod] + [DataRow("overflow:hidden")] + [DataRow("float:left;width:100px")] + [DataRow("position:absolute;width:100px")] + public void NewBlockFormattingContext_KeepsItsFirstChildsTopMarginInside(string parentStyle) + { + var (root, _) = LayoutHarness.Layout(LayoutHarness.Wrap( + $"
")); + + var p = LayoutHarness.FindById(root, "p")!; + var c = LayoutHarness.FindById(root, "c")!; + + Assert.AreEqual(20, c.Location.Y - p.Location.Y, Delta); + } + + [TestMethod] + public void FixedHeightParent_DoesNotPassItsLastChildsBottomMarginOn() + { + var (root, _) = LayoutHarness.Layout(LayoutHarness.Wrap( + "
" + + "
")); + + var p = LayoutHarness.FindById(root, "p")!; + var n = LayoutHarness.FindById(root, "n")!; + + Assert.AreEqual(p.ActualBottom, n.Location.Y, Delta); + } } diff --git a/Source/Test/HtmlRenderer.IntegrationTest/BoxModel/MinMaxHeightIntegrationTests.cs b/Source/Test/HtmlRenderer.IntegrationTest/BoxModel/MinMaxHeightIntegrationTests.cs new file mode 100644 index 000000000..563f258a3 --- /dev/null +++ b/Source/Test/HtmlRenderer.IntegrationTest/BoxModel/MinMaxHeightIntegrationTests.cs @@ -0,0 +1,53 @@ +using HtmlRenderer.IntegrationTest.TestSupport; + +namespace HtmlRenderer.IntegrationTest.BoxModel; + +/// +/// Ported from PeachPDF.Tests' Acid2 min-height/max-height regression cases (CSS 2.1 §10.7). +/// +[DoNotParallelize] +[TestClass] +public sealed class MinMaxHeightIntegrationTests +{ + private const double Delta = 0.5; + + [TestMethod] + public void MinHeightWinsOverConflictingMaxHeight() + { + // min-height (20px) and max-height (5px) conflict; min-height must win per §10.7, so the box's + // actual height must be 20px, not clamped down to 5px. + var html = LayoutHarness.Wrap("
"); + var (root, _) = LayoutHarness.Layout(html); + var box = LayoutHarness.FindById(root, "b")!; + + Assert.AreEqual(20, box.ActualBottom - box.Location.Y, Delta); + } + + [TestMethod] + public void MaxHeightAppliesWhenNotConflictingWithMinHeight() + { + var html = LayoutHarness.Wrap("
"); + var (root, _) = LayoutHarness.Layout(html); + var box = LayoutHarness.FindById(root, "b")!; + + Assert.AreEqual(20, box.ActualBottom - box.Location.Y, Delta); + } + + [TestMethod] + public void PercentageHeight_AgainstAutoHeightContainingBlock_ResolvesToAuto_LettingMaxHeightWin() + { + // "height: 60%" must resolve to auto (zero intrinsic content) against an auto-height containing + // block (CSS 2.1 §10.5/§10.7), not against the container's own content-driven height - so + // max-height:20px is the only thing constraining #b's height. + var html = LayoutHarness.Wrap( + "
" + + "
" + + "
" + + "
"); + var (root, _) = LayoutHarness.Layout(html); + var box = LayoutHarness.FindById(root, "b")!; + + var height = box.ActualBottom - box.Location.Y; + Assert.IsTrue(height is >= 0 and <= 20.5, $"expected height clamped to max-height:20px, was {height}"); + } +} diff --git a/Source/Test/HtmlRenderer.IntegrationTest/Layout/GlobalKeywordCascadeTests.cs b/Source/Test/HtmlRenderer.IntegrationTest/Layout/GlobalKeywordCascadeTests.cs new file mode 100644 index 000000000..923939a37 --- /dev/null +++ b/Source/Test/HtmlRenderer.IntegrationTest/Layout/GlobalKeywordCascadeTests.cs @@ -0,0 +1,185 @@ +using HtmlRenderer.IntegrationTest.TestSupport; + +namespace HtmlRenderer.IntegrationTest.Layout; + +/// +/// Ported from PeachPDF.Tests' GlobalKeywordCascadeTests.cs - the CSS 2.1-relevant subset (CSS 2.1 §6.1/ +/// §6.2's inheritance and cascade-order rules, plus inherit/initial/unset, the latter +/// from CSS Cascading and Inheritance Level 3 but already ported at the CSS-OM/parse level in +/// Css/GlobalKeywordPropertyTests.cs - this file is the cascade-RESOLUTION level that was still +/// untested). revert/revert-layer, cascade layers, and PeachPDF's own eager em-to-pt UA +/// font-size conversion are not ported - out of CSS 2.1 scope or specific to PeachPDF's own architecture. +/// +[DoNotParallelize] +[TestClass] +public sealed class GlobalKeywordCascadeTests +{ + // ── inherit ──────────────────────────────────────────────────────────── + + [TestMethod] + public void Inherit_ColorFromParent_ChildGetsParentColor() + { + var html = LayoutHarness.Wrap( + "
text
"); + var (root, _) = LayoutHarness.Layout(html); + var child = LayoutHarness.FindById(root, "child")!; + + Assert.AreEqual("rgb(255, 0, 0)", child.Color); + } + + [TestMethod] + public void Inherit_NonInheritedProperty_ChildGetsParentValue() + { + // margin-top is NOT inherited by default; explicit "inherit" should force it. + var html = LayoutHarness.Wrap( + "
text
"); + var (root, _) = LayoutHarness.Layout(html); + var child = LayoutHarness.FindById(root, "child")!; + + Assert.AreEqual("30px", child.MarginTop); + } + + [TestMethod] + public void Inherit_ColorWithoutExplicitParent_UsesInitialBlack() + { + var html = LayoutHarness.Wrap("
text
"); + var (root, _) = LayoutHarness.Layout(html); + var el = LayoutHarness.FindById(root, "el")!; + + Assert.AreEqual("black", el.Color); + } + + // ── initial ─────────────────────────────────────────────────────────── + + [TestMethod] + public void Initial_Color_ResetsToBlackIgnoringParent() + { + var html = LayoutHarness.Wrap( + "
text
"); + var (root, _) = LayoutHarness.Layout(html); + var child = LayoutHarness.FindById(root, "child")!; + + Assert.AreEqual("black", child.Color); + } + + [TestMethod] + public void Initial_FontSize_ResetsToMedium() + { + var html = LayoutHarness.Wrap("

heading

"); + var (root, _) = LayoutHarness.Layout(html); + var el = LayoutHarness.FindById(root, "el")!; + + Assert.AreEqual("medium", el.FontSize); + } + + [TestMethod] + public void Initial_MarginTop_ResetsToZero() + { + var html = LayoutHarness.Wrap( + "
text
"); + var (root, _) = LayoutHarness.Layout(html); + var el = LayoutHarness.FindById(root, "el")!; + + Assert.AreEqual("0", el.MarginTop); + } + + // ── unset ───────────────────────────────────────────────────────────── + + [TestMethod] + public void Unset_InheritedProperty_BehavesLikeInherit() + { + var html = LayoutHarness.Wrap( + "
text
"); + var (root, _) = LayoutHarness.Layout(html); + var child = LayoutHarness.FindById(root, "child")!; + + Assert.AreEqual("rgb(0, 128, 0)", child.Color); + } + + [TestMethod] + public void Unset_NonInheritedProperty_BehavesLikeInitial() + { + var html = LayoutHarness.Wrap( + "
text
"); + var (root, _) = LayoutHarness.Layout(html); + var el = LayoutHarness.FindById(root, "el")!; + + Assert.AreEqual("0", el.MarginTop); + } + + [TestMethod] + public void Unset_InheritedPropertyWithNoParentValue_UsesInitial() + { + var html = LayoutHarness.Wrap("
text
"); + var (root, _) = LayoutHarness.Layout(html); + var el = LayoutHarness.FindById(root, "el")!; + + Assert.AreEqual("black", el.Color); + } + + // ── cascade order regressions ──────────────────────────────────────── + + [TestMethod] + public void Regression_ColorInheritsFromParentWithoutKeyword() + { + var html = LayoutHarness.Wrap( + "
text
"); + var (root, _) = LayoutHarness.Layout(html); + var child = LayoutHarness.FindById(root, "child")!; + + Assert.AreEqual("rgb(128, 0, 128)", child.Color); + } + + [TestMethod] + public void Regression_InlineStyleOverridesAuthorRule() + { + var html = LayoutHarness.Wrap( + "
text
"); + var (root, _) = LayoutHarness.Layout(html); + var el = LayoutHarness.FindById(root, "el")!; + + Assert.AreEqual("rgb(0, 128, 0)", el.Color); + } + + [TestMethod] + public void Regression_AuthorRuleOverridesUaDefault() + { + var html = LayoutHarness.Wrap("

heading

"); + var (root, _) = LayoutHarness.Layout(html); + var el = LayoutHarness.FindById(root, "el")!; + + Assert.AreEqual("10px", el.FontSize); + } + + [TestMethod] + public void Regression_ImportantAuthorRuleBeatsInlineStyle() + { + var html = LayoutHarness.Wrap( + "
text
"); + var (root, _) = LayoutHarness.Layout(html); + var el = LayoutHarness.FindById(root, "el")!; + + Assert.AreEqual("rgb(0, 0, 255)", el.Color); + } + + [TestMethod] + public void InlineImportant_BeatsAuthorImportantRule() + { + var html = LayoutHarness.Wrap( + "
text
"); + var (root, _) = LayoutHarness.Layout(html); + var el = LayoutHarness.FindById(root, "el")!; + + Assert.AreEqual("rgb(255, 0, 0)", el.Color); + } + + [TestMethod] + public void Regression_LaterAuthorRuleOverridesEarlierSameSpecificity() + { + var html = LayoutHarness.Wrap("
text
"); + var (root, _) = LayoutHarness.Layout(html); + var el = LayoutHarness.FindById(root, "el")!; + + Assert.AreEqual("rgb(0, 0, 255)", el.Color); + } +} diff --git a/Source/Test/HtmlRenderer.IntegrationTest/Tables/Acid2TableIntegrationTests.cs b/Source/Test/HtmlRenderer.IntegrationTest/Tables/Acid2TableIntegrationTests.cs new file mode 100644 index 000000000..b3d72a8cc --- /dev/null +++ b/Source/Test/HtmlRenderer.IntegrationTest/Tables/Acid2TableIntegrationTests.cs @@ -0,0 +1,46 @@ +using HtmlRenderer.IntegrationTest.TestSupport; + +namespace HtmlRenderer.IntegrationTest.Tables; + +/// +/// Ported from PeachPDF.Tests' Acid2 table regression cases (CSS 2.1 §17.5.3 row-stretch, §17.6.1 +/// border-spacing). +/// +[DoNotParallelize] +[TestClass] +public sealed class Acid2TableIntegrationTests +{ + private const double Delta = 0.5; + + [TestMethod] + public void TableCell_ExplicitHeightShorterThanRow_StretchesToRowHeight() + { + var html = LayoutHarness.Wrap( + "" + + "" + + "" + + "
"); + var (root, _) = LayoutHarness.Layout(html); + + var tall = LayoutHarness.FindById(root, "tall")!; + var shortCell = LayoutHarness.FindById(root, "short")!; + + Assert.AreEqual(tall.ActualBottom, shortCell.ActualBottom, Delta); + } + + [TestMethod] + public void BorderSpacingZero_AdjacentCells_HaveNoGapBetweenThem() + { + var html = LayoutHarness.Wrap( + "" + + "" + + "" + + "
ab
"); + var (root, _) = LayoutHarness.Layout(html); + + var c1 = LayoutHarness.FindById(root, "c1")!; + var c2 = LayoutHarness.FindById(root, "c2")!; + + Assert.AreEqual(c1.ActualRight, c2.Location.X, Delta); + } +} diff --git a/Source/Test/HtmlRenderer.IntegrationTest/Text/TextSpacingAndIndentLayoutIntegrationTests.cs b/Source/Test/HtmlRenderer.IntegrationTest/Text/TextSpacingAndIndentLayoutIntegrationTests.cs new file mode 100644 index 000000000..e579e9882 --- /dev/null +++ b/Source/Test/HtmlRenderer.IntegrationTest/Text/TextSpacingAndIndentLayoutIntegrationTests.cs @@ -0,0 +1,140 @@ +using System.Linq; +using HtmlRenderer.IntegrationTest.TestSupport; + +namespace HtmlRenderer.IntegrationTest.Text; + +/// +/// CSS 2.1 §16.1 (text-indent) and §16.4 (word-spacing) layout coverage - both properties were +/// already fully implemented (unlike letter-spacing, which this port found has zero layout wiring at +/// all - see remarks below), just untested at the layout level (only CSS-OM parsing existed). Ported/adapted +/// from PeachPDF's TextIndentLayoutIntegrationTests.cs/LetterWordSpacingLayoutIntegrationTests.cs, restricted +/// to the CSS 2.1 subset: a single text-indent length applied to a block's first line only (no +/// hanging/each-line - CSS Text Level 4 - and no RTL-specific cases, since this fork's +/// text-indent implementation is the plain CSS2.1 one). +/// +/// +/// letter-spacing is a confirmed, real gap found while porting this file: it parses fine +/// (CSS/FontKeywordPropertyTests.cs) but has zero wiring anywhere in Core - no +/// CssBoxProperties.LetterSpacing field, and no case in CssUtils.GetPropertyValue/ +/// SetPropertyValue/_knownPropertyNames (confirmed by grep - contrast with word-spacing, +/// which has all of these). Implementing it is a real, separate word-measurement change (not a small +/// backport - PeachPDF's own fix touched word-box-width reservation, inter-word-gap collapse prevention, and +/// adjacent-inline-run overlap, per that file's own remarks) and is out of scope for this pass; flagged as a +/// follow-up rather than built here. +/// +/// A second, more fundamental gap this file's own tests found: word-spacing, however it's set +/// (directly on the text-owning block, or on an ancestor further up - both fail identically), never +/// actually reaches the inter-word gap for plain adjacent words in normal text flow. Root cause, confirmed +/// by direct inspection: the CssRectWord that owns each word is an anonymous child text box, and that +/// anonymous child's own WordSpacing reads back "normal" regardless of what any ancestor - including +/// the direct parent - has set, so CssRect.FullWidth's OwnerBox.ActualWordSpacing term is +/// always the unspaced default. (CssLayoutEngine.cs has two call sites that DO add +/// ActualWordSpacing correctly - for a stray whitespace-only box between two separately-styled +/// inline elements - plus a third, commented-out block that looks like an abandoned attempt at exactly +/// this normal-adjacent-word case.) Not fixed here - the anonymous text-box creation/inheritance path +/// (CorrectTextBoxes et al. in Parse/DomParser.cs) is a shared, high-traffic piece of the +/// engine, and root-causing this precisely was out of this pass's budget. +/// +/// +[DoNotParallelize] +[TestClass] +public sealed class TextSpacingAndIndentLayoutIntegrationTests +{ + [Ignore("word-spacing set directly on the block that owns the words has no effect - see this class's own " + + "remarks for the root cause (the anonymous text child that actually owns the words doesn't inherit " + + "its own parent's directly-set word-spacing, confirmed by direct inspection: the child's own " + + "WordSpacing reads back \"normal\").")] + [TestMethod] + public void WordSpacing_IncreasesGapBetweenAdjacentWords() + { + var spacedGap = InterWordGap("

AA BB

"); + var plainGap = InterWordGap("

AA BB

"); + + Assert.AreEqual(plainGap + 10, spacedGap, 1); + } + + [Ignore("Same root cause as WordSpacing_IncreasesGapBetweenAdjacentWords above, and fails identically " + + "whether word-spacing is set directly on the text-owning block or (as here) an ancestor further " + + "up - see this class's own remarks.")] + [TestMethod] + public void WordSpacing_OnAncestor_AffectsDescendantGap() + { + var spacedGap = InterWordGap("

AA BB

"); + var plainGap = InterWordGap("

AA BB

"); + + Assert.AreEqual(plainGap + 10, spacedGap, 1); + } + + [TestMethod] + public void WordSpacingNormal_LeavesGapUnchanged() + { + var normalGap = InterWordGap("

AA BB

"); + var plainGap = InterWordGap("

AA BB

"); + + Assert.AreEqual(plainGap, normalGap, 1); + } + + [Ignore("Same root cause as WordSpacing_IncreasesGapBetweenAdjacentWords above - word-spacing set " + + "directly on the text-owning block has no effect.")] + [TestMethod] + public void WordSpacingNegative_ReducesGap() + { + var negativeGap = InterWordGap("

AA BB

"); + var plainGap = InterWordGap("

AA BB

"); + + Assert.AreEqual(plainGap - 5, negativeGap, 1); + } + + [TestMethod] + public void TextIndent_IndentsOnlyTheFirstLine() + { + var html = LayoutHarness.Wrap( + "

" + + "This text is long enough that it wraps onto a second line for sure.

"); + var (root, _) = LayoutHarness.Layout(html); + var p = LayoutHarness.FindById(root, "p")!; + + var wordsByLine = AllWords(p).GroupBy(w => w.Top).OrderBy(g => g.Key).ToList(); + Assert.IsTrue(wordsByLine.Count >= 2, "fixture text should wrap onto at least two lines"); + + var firstLineLeft = wordsByLine[0].Min(w => w.Left); + var secondLineLeft = wordsByLine[1].Min(w => w.Left); + + Assert.AreEqual(40, firstLineLeft, 1); + Assert.AreEqual(0, secondLineLeft, 1); + } + + [TestMethod] + public void TextIndentZero_NoIndentOnAnyLine() + { + var html = LayoutHarness.Wrap("

Hello world

"); + var (root, _) = LayoutHarness.Layout(html); + var p = LayoutHarness.FindById(root, "p")!; + + var firstWord = AllWords(p).OrderBy(w => w.Left).First(); + Assert.AreEqual(0, firstWord.Left, 1); + } + + // ── Helpers ──────────────────────────────────────────────────────────── + + private static double InterWordGap(string bodyHtml) + { + var html = LayoutHarness.Wrap(bodyHtml); + var (root, _) = LayoutHarness.Layout(html); + var p = LayoutHarness.FindById(root, "p")!; + var words = AllWords(p).OrderBy(w => w.Left).ToList(); + + Assert.AreEqual(2, words.Count); + return words[1].Left - (words[0].Left + words[0].Width); + } + + private static System.Collections.Generic.IEnumerable AllWords( + TheArtOfDev.HtmlRenderer.Core.Dom.CssBox box) + { + foreach (var word in box.Words) yield return word; + foreach (var child in box.Boxes) + { + foreach (var word in AllWords(child)) yield return word; + } + } +}