From 7ffe1aabe0c5c1e67b477ce27e7f2752c56fed22 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 09:44:32 +1100 Subject: [PATCH] Fix CI on ubuntu-26.04, whose libxml2 parses HTML as HTML5 GitHub's ubuntu-latest is now ubuntu-26.04, which ships libxml2 2.15. libxml2 2.14 rewrote the HTML tokenizer to follow HTML5: `` is read as a bogus comment, not a processing instruction, and parse errors are no longer reported. - Skip the tests that need the old parser, detected by probing libxml rather than reading LIBXML_VERSION (the build-time version) - Move the issue #65 tests that run on any libxml into their own class, so Issue65Test skips once in set_up() - Pin the comment round trip libxml 2.14+ produces - Run the coverage job on ubuntu-24.04 so the issue #65 fix stays exercised, and only on PHP 8.3 (the old include applied report: true to every job) - Bump actions/checkout to v7 and codecov/codecov-action to v7 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/coding-standard.yml | 2 +- .github/workflows/examples.yml | 2 +- .github/workflows/unit-testing.yml | 11 +- CHANGELOG.md | 1 + tests/Issues/Issue65ParserIndependentTest.php | 107 ++++++++++++++++++ tests/Issues/Issue65Test.php | 79 ++----------- tests/QueryPath/DOMQueryTest.php | 2 + tests/QueryPath/TestCase.php | 29 +++++ 8 files changed, 157 insertions(+), 76 deletions(-) create mode 100644 tests/Issues/Issue65ParserIndependentTest.php diff --git a/.github/workflows/coding-standard.yml b/.github/workflows/coding-standard.yml index 29e2b44..0bc15c3 100644 --- a/.github/workflows/coding-standard.yml +++ b/.github/workflows/coding-standard.yml @@ -8,7 +8,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Setup PHP uses: shivammathur/setup-php@v2 diff --git a/.github/workflows/examples.yml b/.github/workflows/examples.yml index 6a450c2..4347abf 100644 --- a/.github/workflows/examples.yml +++ b/.github/workflows/examples.yml @@ -23,7 +23,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Setup PHP uses: shivammathur/setup-php@v2 diff --git a/.github/workflows/unit-testing.yml b/.github/workflows/unit-testing.yml index be8b87d..5e99e9a 100644 --- a/.github/workflows/unit-testing.yml +++ b/.github/workflows/unit-testing.yml @@ -4,18 +4,21 @@ on: [ pull_request ] jobs: phpunit: - runs-on: ubuntu-latest + runs-on: ${{ matrix.os || 'ubuntu-latest' }} strategy: matrix: php-versions: [ '7.1', '7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3', '8.4', '8.5' ] include: - - php: '8.3' + # libxml2 2.14+ (ubuntu-26.04 onwards) parses HTML as HTML5, which skips the tests for + # libxml's older HTML parser, so the coverage job stays on a runner that still has it. + - php-versions: '8.3' + os: ubuntu-24.04 report: true name: PHP ${{ matrix.php-versions }} PHPUnit Test steps: - name: Checkout repository - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Setup PHP uses: shivammathur/setup-php@v2 @@ -43,7 +46,7 @@ jobs: - name: Check Code Coverage if: ${{ matrix.report }} - uses: codecov/codecov-action@v5 + uses: codecov/codecov-action@v7 with: directory: ./coverage/ token: ${{ secrets.CODECOV_TOKEN }} diff --git a/CHANGELOG.md b/CHANGELOG.md index c9c4dc6..b953986 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ QueryPath Changelog - Rewrite the cURL example against the PubMed E-utilities API. MusicBrainz throttles by IP address, which made the example unusable from any shared address - Add `composer run test:examples` (and `test:examples:network`) to run the examples locally - Fix processing instructions gaining an extra `?` each time an HTML-parsed document was serialized, so `` came back out of `html()`, `innerHTML()`, `innerXML()`, `innerXHTML()`, `xml()`, `html5()`, `innerHTML5()`, and `writeXML()` as ``. libxml's HTML parser keeps the closing `?` as part of the node's data, unlike its XML parser and the Masterminds HTML5 parser, so QueryPath now strips it on load. `DOMProcessingInstruction::$data` consequently no longer has a stray `?` on the end for documents read with `htmlqp()` or `qp()` on an `.html` file. Note that this normalisation applies only to documents QueryPath parses itself, and that taking the underlying `DOMDocument` out of QueryPath and calling libxml's own `saveHTML()` on it will emit `` without the terminator, since libxml's HTML serializer never adds one +- With libxml2 2.14+, `htmlqp()` parses `` as a comment (``) and no longer throws `QueryPath\ParseException` on malformed HTML. Use `html5qp()` to keep processing instructions - Add `QueryPath\Document`, a `DOMDocument` subclass QueryPath parses into. The type is how QueryPath records that a document's processing instruction data does not carry the closing `?`, which cannot be determined by inspecting the document afterwards. It travels with the document, so every route to a second `DOMQuery` over one document -- iteration, `add()`, `remove()`, `replaceAll()`, `branch()`, `QueryPath::with()`, and the bundled extensions -- serializes it correctly. A `DOMDocument` supplied by the caller is a plain `DOMDocument`, makes no such promise, and is still serialized exactly as it was handed over - Fix fatal error when running a CSS selector against a match set that contains non-element nodes (text, comment, CDATA or processing instruction), as `contents()` returns. Those nodes now simply do not match, instead of calling element-only DOM methods on them - Fix `find('a > b')` throwing a `TypeError` when the left-hand side matched the root element, whose parent is the document rather than an element diff --git a/tests/Issues/Issue65ParserIndependentTest.php b/tests/Issues/Issue65ParserIndependentTest.php new file mode 100644 index 0000000..487796e --- /dev/null +++ b/tests/Issues/Issue65ParserIndependentTest.php @@ -0,0 +1,107 @@ +

'; + + private const EXPECTED = '

'; + + private const OPTIONS = ['format_output' => false]; + + /** + * The comment libxml 2.14+ produces must round trip unchanged. + */ + public function testHtml5LibxmlKeepsTheBlockAsAComment() + { + if (! self::libxmlParsesHtmlAsHtml5()) { + $this->markTestSkipped('libxml ' . LIBXML_DOTTED_VERSION . ' parses as a processing instruction.'); + } + + $expected = '

'; + $markup = self::HTML; + + for ($i = 0; $i < 3; $i++) { + $markup = htmlqp($markup, null, self::OPTIONS)->top()->html(); + $this->assertStringContainsString($expected, $markup); + } + } + + /** + * html5qp() was never affected, because the Masterminds parser does not keep the "?". + */ + public function testHtml5ParserIsUnaffected() + { + $qp = html5qp(self::HTML, null, self::OPTIONS); + + $this->assertSame(self::EXPECTED, $qp->top()->find('body')->innerHTML()); + $this->assertSame(self::EXPECTED, $qp->top()->find('body')->innerHTML5()); + } + + /** + * A document parsed in XML mode was never affected either. + */ + public function testXmlParserIsUnaffected() + { + $xml = ''; + $qp = qp($xml, null, self::OPTIONS); + + $this->assertSame('', $qp->top()->find('item')->innerXML()); + $this->assertSame('', $qp->top()->find('item')->xml()); + + $instruction = $qp->top()->find('item')->get(0)->firstChild; + $this->assertSame('echo $title; ', $instruction->data); + } + + /** + * writeHTML() on an XML-parsed document used to drop the terminator entirely, because libxml's + * HTML serializer writes a processing instruction verbatim and never adds one of its own. + */ + public function testWriteHtmlOnAnXmlParsedDocumentEmitsATerminator() + { + $xml = ''; + $qp = qp($xml, null, self::OPTIONS); + + $output = $this->capture(function () use ($qp) { + $qp->top()->writeHTML(); + }); + + $this->assertStringContainsString('', $output); + } + + /** + * Documents with no processing instructions must serialize exactly as they did before. + */ + public function testDocumentsWithoutProcessingInstructionsAreUntouched() + { + $qp = htmlqp('

hi

', null, self::OPTIONS); + + $this->assertSame('

hi

', $qp->top()->find('#d')->innerHTML()); + + $output = $this->capture(function () use ($qp) { + $qp->top()->writeHTML(); + }); + + $this->assertStringContainsString('

hi

', $output); + } + + /** + * The marker is the document's type, and PHP rebuilds that wrapper object whenever a document + * is reached through ownerDocument after the original wrapper has been released. + */ + public function testTheDocumentKeepsItsTypeWhenReachedThroughOwnerDocument() + { + $element = htmlqp(self::HTML, null, self::OPTIONS)->top()->get(0); + + $this->assertInstanceOf(Document::class, $element->ownerDocument); + } +} diff --git a/tests/Issues/Issue65Test.php b/tests/Issues/Issue65Test.php index c0ececf..2f51feb 100644 --- a/tests/Issues/Issue65Test.php +++ b/tests/Issues/Issue65Test.php @@ -8,13 +8,15 @@ * data, while its XML parser and the Masterminds HTML5 parser do not. Serializers that append * their own "?>" therefore doubled it up, so `` came back out as * `` and grew another "?" on every round trip. + * + * Skipped on libxml 2.14+, which reads `` in HTML as a comment. Issue65ParserIndependentTest + * holds the tests that do not need libxml's HTML parser to produce a processing instruction. */ namespace QueryPathTests; use DOMDocument; use DOMProcessingInstruction; -use QueryPath\Document; class Issue65Test extends TestCase { @@ -35,6 +37,12 @@ class Issue65Test extends TestCase */ private const OPTIONS = ['format_output' => false]; + protected function set_up() + { + parent::set_up(); + $this->requireLegacyHtmlParser(); + } + /** * Every serializer that appends its own "?>", with the selector each one is called on. * @@ -192,48 +200,6 @@ public function testWriteHtmlLeavesTheDocumentUnchanged() $this->assertSame(self::EXPECTED, $qp->top()->find('body')->innerHTML()); } - /** - * html5qp() was never affected, because the Masterminds parser does not keep the "?". - */ - public function testHtml5ParserIsUnaffected() - { - $qp = html5qp(self::HTML, null, self::OPTIONS); - - $this->assertSame(self::EXPECTED, $qp->top()->find('body')->innerHTML()); - $this->assertSame(self::EXPECTED, $qp->top()->find('body')->innerHTML5()); - } - - /** - * A document parsed in XML mode was never affected either. - */ - public function testXmlParserIsUnaffected() - { - $xml = ''; - $qp = qp($xml, null, self::OPTIONS); - - $this->assertSame('', $qp->top()->find('item')->innerXML()); - $this->assertSame('', $qp->top()->find('item')->xml()); - - $instruction = $qp->top()->find('item')->get(0)->firstChild; - $this->assertSame('echo $title; ', $instruction->data); - } - - /** - * writeHTML() on an XML-parsed document used to drop the terminator entirely, because libxml's - * HTML serializer writes a processing instruction verbatim and never adds one of its own. - */ - public function testWriteHtmlOnAnXmlParsedDocumentEmitsATerminator() - { - $xml = ''; - $qp = qp($xml, null, self::OPTIONS); - - $output = $this->capture(function () use ($qp) { - $qp->top()->writeHTML(); - }); - - $this->assertStringContainsString('', $output); - } - /** * An HTML processing instruction with no "?" before the ">" has nothing to strip. */ @@ -246,22 +212,6 @@ public function testProcessingInstructionWithoutATerminatorIsLeftAlone() $this->assertSame('bar', $instruction->data); } - /** - * Documents with no processing instructions must serialize exactly as they did before. - */ - public function testDocumentsWithoutProcessingInstructionsAreUntouched() - { - $qp = htmlqp('

hi

', null, self::OPTIONS); - - $this->assertSame('

hi

', $qp->top()->find('#d')->innerHTML()); - - $output = $this->capture(function () use ($qp) { - $qp->top()->writeHTML(); - }); - - $this->assertStringContainsString('

hi

', $output); - } - /** * Normalisation only happens on documents QueryPath parses, so the HTML serializer must leave a * caller-supplied document alone -- its processing instructions still carry their own "?". @@ -350,17 +300,6 @@ public function testRemoveKeepsTheDocumentSerializingCorrectly() $this->assertStringContainsString(self::EXPECTED, $qp->top()->html()); } - /** - * The marker is the document's type, and PHP rebuilds that wrapper object whenever a document - * is reached through ownerDocument after the original wrapper has been released. - */ - public function testTheDocumentKeepsItsTypeWhenReachedThroughOwnerDocument() - { - $element = htmlqp(self::HTML, null, self::OPTIONS)->top()->get(0); - - $this->assertInstanceOf(Document::class, $element->ownerDocument); - } - /** * Normalisation runs once, when the document is parsed. Re-using it must not strip a second * "?" from content that legitimately ends in one. diff --git a/tests/QueryPath/DOMQueryTest.php b/tests/QueryPath/DOMQueryTest.php index 228b662..8eb2fd4 100644 --- a/tests/QueryPath/DOMQueryTest.php +++ b/tests/QueryPath/DOMQueryTest.php @@ -287,6 +287,8 @@ public function testFailedHTTPLoadWithContext() public function testFailedParseHTMLElement() { + $this->requireLegacyHtmlParser(); + $this->expectException(ParseException::class); try { diff --git a/tests/QueryPath/TestCase.php b/tests/QueryPath/TestCase.php index f425ccd..63386cd 100644 --- a/tests/QueryPath/TestCase.php +++ b/tests/QueryPath/TestCase.php @@ -33,4 +33,33 @@ protected function capture(callable $callback): string ob_end_clean(); } } + + /** + * Whether libxml's HTML parser follows HTML5, as it does from 2.14. + * + * Such a parser reads `` as a comment rather than a processing instruction, and reports no + * parse errors. Probed rather than read from LIBXML_VERSION, which is the version PHP was built + * against, not necessarily the one it runs with. + * + * @return bool + */ + protected static function libxmlParsesHtmlAsHtml5(): bool + { + static $html5; + + if ($html5 === null) { + $document = new \DOMDocument(); + @$document->loadHTML('

'); + $html5 = ! $document->getElementsByTagName('p')->item(0)->firstChild instanceof \DOMProcessingInstruction; + } + + return $html5; + } + + protected function requireLegacyHtmlParser() + { + if (self::libxmlParsesHtmlAsHtml5()) { + $this->markTestSkipped('libxml ' . LIBXML_DOTTED_VERSION . ' parses HTML as HTML5.'); + } + } }