CSS 2.1 test parity: floats, positioning, z-index gap - #1
Closed
jhaygood86 wants to merge 3 commits into
Closed
Conversation
FloatPropertyTests.cs claimed this fork had no clear CSS property at all, dropping every clear case from the PeachPDF source it was ported from - that predates ClearProperty being added and is now false, so restore the parsing coverage. Ports PeachPDF's FloatLayoutRegression- Tests.cs and the float/clear Acid2 regression cases as a new IntegrationTest suite; all pass unmodified, confirming float/clear layout parity with PeachPDF.
…/§10.3.7
position:relative previously had no layout effect anywhere in this
fork (parsed but never read); position:absolute only had ad-hoc
partial support (no real containing-block resolution, plain in-flow
placement otherwise); position:fixed resolved only against the page
and dropped the box's own margin; and `right`/`bottom` were parsed at
the CSS-OM level but never dispatched onto a box at all, so they had
zero effect under any positioning scheme.
Backports PeachPDF's CommitBlockChildOffset placement logic, adapted
to this fork's box places-itself (rather than parent-commits-child)
layout shape:
- Right/Bottom become real box properties, wired through CssUtils
the same way Left/Top already were.
- position:relative applies a near/far offset (left wins over right
when both are set, sign-flipped when only the far edge is) that is
purely visual per §9.4.3: RelativeOffsetX/Y record it separately so
the new StaticBottom can back it out again, and every sibling-
placement/margin-collapse/shrink-to-fit call site that used to read
a box's ActualBottom directly now reads StaticBottom instead, so a
relatively-positioned box's offset no longer drags its parent's
auto height or following siblings down with it.
- position:absolute resolves against DomUtils.GetNearestPositioned-
Ancestor's padding edge, on both axes anchoring from whichever of
the near/far offset is set (right-anchoring reads the box's own
already-resolved Size.Width; bottom-anchoring has to wait until
this box's own ApplyHeight has run, since auto height depends on
this box's own content, so it's corrected via a post-hoc OffsetTop
shift instead of resolved inline like every other case here).
Absolute boxes with width:auto also now shrink-to-fit their
content instead of filling the containing block, matching CSS 2.1
§10.3.7's common case (the full seven-case width-auto-resolution
algorithm is not implemented).
- position:fixed's offset is now computed once, in PerformLayoutImp
once margin/container are guaranteed ready, instead of eagerly from
the Left/Top property setters - which used to race ahead of
ActualMarginLeft/Top being resolved and cache a margin-less
Location that never got recomputed.
Porting PeachPDF's shrink-to-fit width tests also surfaced two real,
pre-existing bugs in GetMinMaxSumWords (a border/padding sum that
never reset between sibling "lines", and no explicit-width floor for
a childless block), both already fixed in PeachPDF - backported here
too, and re-approves one baseline PDF whose auto-width table column
render 1-2px differently now that the fix applies generally, not just
to the new absolute-positioning case that surfaced it.
Ports the CSS 2.1 §9.4.3/§10.3.7 Acid2 regression tests from PeachPDF
covering all of the above; all 9 pass.
…by-design test Ports PeachPDF's Acid2 z-index/stacking regression test (CSS 2.1 §9.9/Appendix E: a position:relative;z-index:2 box must paint over a later position:fixed sibling regardless of document order). Left [Ignore]d rather than implemented: FragmentPainter.cs already documents stacking-context paint order as deferred follow-on work, and there is no ZIndex box property or paint-order sorting anywhere in Core to hang a real implementation off of - this is a separate, larger feature port, not a one-line fix like the positioning gaps fixed in the previous commit.
Owner
Author
|
Opened against the wrong repo (this fork's origin instead of upstream, where ArthurHub#263 actually lives) - recreating against ArthurHub/HTML-Renderer. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part 1 of a CSS 2.1 test-parity sweep against PeachPDF's Integration test suite, stacked on ArthurHub#263. Scope: chapter 9 (visual formatting model) — floats/clear, positioning, z-index.
FloatPropertyTests.cs's doc comment claimed this fork had noclearproperty at all, dropping everyclearcase from the PeachPDF source it was ported from - that predatesClearPropertybeing added. Restored the parsing coverage, and ported PeachPDF'sFloatLayoutRegressionTests.cs+ the float/clear Acid2 cases as a newIntegrationTest/Layout/FloatLayoutIntegrationTests.cs(11 tests). All passed unmodified against this fork's existing float engine.position:relativehad zero layout effect anywhere in this fork (parsed, never read);position:absoluteonly had ad-hoc partial support;position:fixedignored the box's own margin;right/bottomwere parsed at the CSS-OM level but never dispatched onto a box under any positioning scheme. Backported PeachPDF'sCommitBlockChildOffsetplacement logic (adapted to this fork's box-places-itself layout shape) — see the second commit's message for the full breakdown, including two genuine pre-existing bugs inGetMinMaxSumWordsthat porting PeachPDF's shrink-to-fit-width tests surfaced and that PeachPDF had already fixed. Ported the CSS 2.1 §9.4.3/§10.3.7 Acid2 regression tests; all 9 pass.ZIndexbox property, no paint-order sorting anywhere inCore—FragmentPainter.csalready documents this as deferred follow-on work). Ported PeachPDF's Acid2 z-index regression test[Ignore]d with an accurate explanation, rather than building a separate, larger stacking-context feature into this PR.Test plan
HtmlRenderer.Testfull suite: 2432 passed, 0 failedHtmlRenderer.IntegrationTestfull suite: 357 passed, 0 failed (90 skipped, all pre-existing or the one new documented z-index gap)HtmlRenderer.PdfSharp.Testfull suite: 39 passed, 0 failedHtmlRenderer.sln) builds clean across net8.0, net8.0-windows, netstandard2.0, net462Tables.png) whose auto-width table column renders 1-2px differently now that theGetMinMaxSumWordsfix applies generally — reviewed the diff image, confirmed it's the intended correction, not a regression