Skip to content

Add HtmlFragment class for improved HTML manipulation in HtmlDiff - #33

Merged
thijskok merged 1 commit into
mainfrom
added-dom-support
Jul 30, 2026
Merged

Add HtmlFragment class for improved HTML manipulation in HtmlDiff#33
thijskok merged 1 commit into
mainfrom
added-dom-support

Conversation

@thijskok

Copy link
Copy Markdown
Contributor

No description provided.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.80851% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/Renderers/Support/HtmlFragment.php 96.10% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a DOM-based HtmlFragment helper to make HtmlDiff’s before/after view extraction more robust (especially around removing fully-added/fully-deleted nested elements), and adds regression tests for nested list-item cases.

Changes:

  • Added HtmlFragment (DOMDocument + XPath wrapper) for targeted, in-place HTML mutations.
  • Updated HtmlDiff to use HtmlFragment instead of regex-based post-processing when building before/after views.
  • Added tests to ensure nested list items that become empty after diff-marker stripping are omitted rather than left as empty structural tags.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tests/HtmlDiffRendererTest.php Adds regression coverage for nested list-item omission behavior in before/after views.
src/Renderers/Support/HtmlFragment.php Introduces a DOM/XPath-based mutation utility for HTML snippets used by HtmlDiff.
src/Renderers/HtmlDiff.php Switches view-extraction logic from regexes to HtmlFragment transformations.
Comments suppressed due to low confidence (2)

src/Renderers/Support/HtmlFragment.php:40

  • $this->xpath->query(...)->item(0) can return null if parsing fails, which will trigger a TypeError when assigning to the typed DOMElement property. Consider validating the lookup and throwing a clear exception so failures are diagnosable.
        $this->xpath = new DOMXPath($this->dom);
        $this->root = $this->xpath->query('//*[@id="__root__"]')->item(0);
    }

src/Renderers/Support/HtmlFragment.php:162

  • toHtml() appends an empty text node to every childless element to force a closing tag. This will also target void elements like
    , , , etc., which must not have children and can serialize into invalid HTML (e.g.,

    ) or unexpected output.
        // Childless elements would otherwise have their closing tag silently dropped by
        // libxml's HTML serializer (e.g. <li></li> would come back out as just <li>); a
        // placeholder text node forces it to always emit both tags.
        foreach (iterator_to_array($this->xpath->query('.//*[not(node())]', $this->root)) as $element) {
            $element->appendChild($this->dom->createTextNode(''));

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +29 to +36
$previousSetting = libxml_use_internal_errors(true);

$this->dom->loadHTML(
'<?xml encoding="utf-8"?><div id="__root__">' . $html . '</div>',
LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD
);

libxml_use_internal_errors($previousSetting);
@thijskok
thijskok requested review from Frankisgek and stefanius July 30, 2026 05:05
@thijskok
thijskok merged commit 0b2318b into main Jul 30, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants