Fix CI on ubuntu-26.04, whose libxml2 parses HTML as HTML5

.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

.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

.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 }}

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 `<?php echo $title; ?>` came back out of `html()`, `innerHTML()`, `innerXML()`, `innerXHTML()`, `xml()`, `html5()`, `innerHTML5()`, and `writeXML()` as `<?php echo $title; ??>`. 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 `<?php ... >` without the terminator, since libxml's HTML serializer never adds one
+- With libxml2 2.14+, `htmlqp()` parses `<?php ... ?>` as a comment (`<!--?php ... ?-->`) 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

tests/Issues/Issue65ParserIndependentTest.php

@@ -0,0 +1,107 @@
+<?php
+/**
+ * @file
+ *
+ * Regression tests for https://github.com/GravityPDF/querypath/issues/65 that run on every libxml
+ * version. See Issue65Test for the bug itself.
+ */
+
+namespace QueryPathTests;
+
+use QueryPath\Document;
+
+class Issue65ParserIndependentTest extends TestCase
+{
+	private const HTML = '<html><body><h1><?php echo $title; ?></h1></body></html>';
+
+	private const EXPECTED = '<h1><?php echo $title; ?></h1>';
+
+	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 = '<h1><!--?php echo $title; ?--></h1>';
+		$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 = '<?xml version="1.0"?><root><item><?php echo $title; ?></item></root>';
+		$qp  = qp($xml, null, self::OPTIONS);
+
+		$this->assertSame('<?php echo $title; ?>', $qp->top()->find('item')->innerXML());
+		$this->assertSame('<item><?php echo $title; ?></item>', $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 = '<?xml version="1.0"?><root><item><?php echo $title; ?></item></root>';
+		$qp  = qp($xml, null, self::OPTIONS);
+
+		$output = $this->capture(function () use ($qp) {
+			$qp->top()->writeHTML();
+		});
+
+		$this->assertStringContainsString('<item><?php echo $title; ?></item>', $output);
+	}
+
+	/**
+	 * Documents with no processing instructions must serialize exactly as they did before.
+	 */
+	public function testDocumentsWithoutProcessingInstructionsAreUntouched()
+	{
+		$qp = htmlqp('<html><body><div id="d"><br><p>hi</p></div></body></html>', null, self::OPTIONS);
+
+		$this->assertSame('<br/><p>hi</p>', $qp->top()->find('#d')->innerHTML());
+
+		$output = $this->capture(function () use ($qp) {
+			$qp->top()->writeHTML();
+		});
+
+		$this->assertStringContainsString('<div id="d"><br><p>hi</p></div>', $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);
+	}
+}

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 `<?php echo $title; ?>` came back out as
  * `<?php echo $title; ??>` 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 = '<?xml version="1.0"?><root><item><?php echo $title; ?></item></root>';
-		$qp  = qp($xml, null, self::OPTIONS);
-
-		$this->assertSame('<?php echo $title; ?>', $qp->top()->find('item')->innerXML());
-		$this->assertSame('<item><?php echo $title; ?></item>', $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 = '<?xml version="1.0"?><root><item><?php echo $title; ?></item></root>';
-		$qp  = qp($xml, null, self::OPTIONS);
-
-		$output = $this->capture(function () use ($qp) {
-			$qp->top()->writeHTML();
-		});
-
-		$this->assertStringContainsString('<item><?php echo $title; ?></item>', $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('<html><body><div id="d"><br><p>hi</p></div></body></html>', null, self::OPTIONS);
-
-		$this->assertSame('<br/><p>hi</p>', $qp->top()->find('#d')->innerHTML());
-
-		$output = $this->capture(function () use ($qp) {
-			$qp->top()->writeHTML();
-		});
-
-		$this->assertStringContainsString('<div id="d"><br><p>hi</p></div>', $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.

tests/QueryPath/DOMQueryTest.php

@@ -287,6 +287,8 @@ public function testFailedHTTPLoadWithContext()
 
 	public function testFailedParseHTMLElement()
 	{
+		$this->requireLegacyHtmlParser();
+
 		$this->expectException(ParseException::class);
 
 		try {

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('<p><?x y?></p>');
+			$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.');
+		}
+	}
 }