Conversation
This branch has not been deployed
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
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.
Fixes #244
Traverser::node()had no case forXML_ENTITY_REF_NODE, so thedefaultarm skipped it silently. A manually appended entity reference then disappeared from the output: the reproduction in the issue gives<span>Identit</span>whereDOMDocument::saveHTML()gives<span>Identité</span>.OutputRules::entityReference()writes the node withsaveXML(), the same one-linercdata()andcomment()already use. It cannot go throughtext(): an entity reference node carries the name only, itsnodeValueisnulland itstextContentis empty, sotext()would write nothing at all.Why the method is not on
RulesInterfaceRulesInterfaceis implemented outside this package, for instancewindwalker-io/frameworkships its ownOutputRules implements RulesInterfacerather 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
énever becomes an entity reference node, it is parsed into a text node, and a literal&already round-trips. So this does not make the output byte-identical toDOMDocument::saveHTML(), which additionally re-encodes non-ASCII characters unlessencode_entitiesis enabled.Tests
testEntityReference()asserts the traverser path first, so onmasterit 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.