Skip to content

Serialize entity reference nodes instead of skipping them - #279

Open
Amoifr wants to merge 1 commit into
Masterminds:masterfrom
Amoifr:fix-244-entity-reference-nodes
Open

Amoifr wants to merge 1 commit into
Masterminds:masterfrom
Amoifr:fix-244-entity-reference-nodes

Conversation

@Amoifr

@Amoifr Amoifr commented Sep 14, 2026

Copy link
Copy Markdown

Fixes #244

Traverser::node() had no case for XML_ENTITY_REF_NODE, so the default arm skipped it silently. A manually appended entity reference then disappeared from the output: the reproduction in the issue gives <span>Identit</span> where DOMDocument::saveHTML() gives <span>Identit&eacute;</span>.

OutputRules::entityReference() writes the node with saveXML(), the same one-liner cdata() and comment() already use. It cannot go through text(): an entity reference node carries the name only, its nodeValue is null and its textContent is empty, so text() would write nothing at all.

Why the method is not on RulesInterface

RulesInterface is implemented outside this package, for instance windwalker-io/framework ships its own OutputRules implements RulesInterface rather than extending this one. Adding a method to the interface would be a fatal error for such an implementation, and the interface has not gained a method since it was introduced.

So the traverser guards the call with instanceof OutputRules, which is the dispatch idiom already used in that file. A custom rules implementation keeps exactly the behaviour it has today, the node stays skipped. Happy to move the method onto the interface instead if you would rather take it in a major release.

Scope

Only named entity references are affected. A numeric reference such as &#233; never becomes an entity reference node, it is parsed into a text node, and a literal &amp; already round-trips. So this does not make the output byte-identical to DOMDocument::saveHTML(), which additionally re-encodes non-ASCII characters unless encode_entities is enabled.

Tests

testEntityReference() asserts the traverser path first, so on master it fails with '<span>Identit</span>' rather than erroring on the missing method, then checks the rule on its own. The full suite is green locally (168 tests, 2146 assertions).

One note on verification: php-cs-fixer could not be run the way CI does. The workflow uses the 2.19.3 phar on PHP 7.4, and this machine has no PHP below 8.2, which that phar refuses without PHP_CS_FIXER_IGNORE_ENV.

This branch has not been deployed

No deployments
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.

Traverser::node() does not handle entity references

1 participant