From 987cace7dad4162ff51c271e2078d5a18f2877a9 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 11:25:30 +1100 Subject: [PATCH] Keep the closing "?" of processing instructions in html5qp() documents html()/writeHTML() go through libxml's HTML serializer, which relies on the terminator being in the PI data. Masterminds never leaves it there, and saveDocumentHTML() only restores it for a QueryPath\Document. Retype the Masterminds document as a Document so it gets the same treatment. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/Document.php | 1 + src/QueryPath.php | 9 ++ tests/Issues/Issue86Test.php | 198 +++++++++++++++++++++++++++++++++++ 4 files changed, 209 insertions(+) create mode 100644 tests/Issues/Issue86Test.php diff --git a/CHANGELOG.md b/CHANGELOG.md index cfe5b3b..d3861a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ QueryPath Changelog - Fix `tag()` emitting an "Undefined property" warning and returning `NULL` when the first match is not an element, such as a text, comment or processing instruction node returned by `contents()`. It now returns an empty string, as it does for an empty match set (issue #93) - Fix QueryPath's error handler staying installed for the rest of the request when an exception other than a `QueryPath\Exception` (a `TypeError`, or one thrown by a stream wrapper, for example) escaped from parsing a document, inserting markup, or `writeXML()`/`writeHTML()` (issue #95) - Fix `writeHTML()` throwing `QueryPath\ParseException` instead of `QueryPath\IOException` when the file cannot be written, as `writeXML()` and `writeHTML5()` do (issue #92). `IOException` extends `ParseException`, so existing `catch (ParseException $e)` blocks still catch it +- Fix `html()` and `writeHTML()` dropping the closing `?` of a processing instruction in an `html5qp()` document, so `` came out as `` (issue #86). `html5qp()` documents are now `QueryPath\Document` instances, like those from `qp()` and `htmlqp()` # 4.1.0 diff --git a/src/Document.php b/src/Document.php index dd303fa..d49e0d1 100644 --- a/src/Document.php +++ b/src/Document.php @@ -25,6 +25,7 @@ * QueryPath serializes it exactly as it was handed over. * * @see DOM::normalizeProcessingInstructions() + * @see QueryPath::withHTML5(), which retypes the document Masterminds builds * @see DOMQuery::saveDocumentHTML() * * @ingroup querypath_core diff --git a/src/QueryPath.php b/src/QueryPath.php index 2d08a76..36a6f48 100644 --- a/src/QueryPath.php +++ b/src/QueryPath.php @@ -87,6 +87,7 @@ namespace QueryPath; +use DOMDocument; use Masterminds\HTML5; use QueryPath\ExtensionRegistry; @@ -284,6 +285,14 @@ public static function withHTML5($source = null, $selector = null, array $option } else { $source = $html5->load($source); } + + // Masterminds already satisfies the invariant Document records, but builds a plain + // DOMDocument. registerNodeClass() cannot retype a live wrapper, so release it and + // let PHP rebuild it as a Document when it is next reached through a node. + $source->registerNodeClass(DOMDocument::class, Document::class); + $doctype = $source->doctype; + $source = null; + $source = $doctype->ownerDocument; } return new $qpClass($source, $selector, $options); diff --git a/tests/Issues/Issue86Test.php b/tests/Issues/Issue86Test.php new file mode 100644 index 0000000..1bdb13e --- /dev/null +++ b/tests/Issues/Issue86Test.php @@ -0,0 +1,198 @@ +` and relies on the + * closing "?" already being in the data. Masterminds never puts it there, so html() and writeHTML() + * turned `` from an html5qp() document into ``. + */ + +namespace QueryPathTests; + +use DOMDocument; +use Masterminds\HTML5; +use QueryPath\Document; + +class Issue86Test extends TestCase +{ + private const PI_FILE_HTML = 'tests/processing-instruction.html'; + + private const HTML = '

'; + + private const EXPECTED = '

'; + + private const OPTIONS = ['format_output' => false]; + + /** + * Every serializer in the issue's table, with the selector each one is called on. + * + * @return array + */ + public function serializerProvider(): array + { + return [ + 'html' => ['html', 'html'], + 'html5' => ['html', 'html5'], + 'xml' => ['html', 'xml'], + 'innerHTML' => ['body', 'innerHTML'], + 'innerHTML5' => ['body', 'innerHTML5'], + 'innerXML' => ['body', 'innerXML'], + ]; + } + + /** + * @dataProvider serializerProvider + * + * @param string $selector + * @param string $method + */ + public function testSerializersKeepTheTerminator($selector, $method) + { + $qp = html5qp(self::HTML, null, self::OPTIONS); + + $this->assertStringContainsString(self::EXPECTED, $qp->find($selector)->$method()); + } + + /** + * @return array + */ + public function writerProvider(): array + { + return [ + 'writeHTML' => ['writeHTML'], + 'writeHTML5' => ['writeHTML5'], + 'writeXML' => ['writeXML'], + ]; + } + + /** + * @dataProvider writerProvider + * + * @param string $method + */ + public function testWritersKeepTheTerminator($method) + { + $qp = html5qp(self::HTML, null, self::OPTIONS); + + $output = $this->capture(function () use ($qp, $method) { + $qp->$method(); + }); + + $this->assertStringContainsString(self::EXPECTED, $output); + } + + public function testWriteHtmlToAFileKeepsTheTerminator() + { + $qp = html5qp(self::HTML, null, self::OPTIONS); + $path = tempnam(sys_get_temp_dir(), 'qp86'); + + try { + $qp->writeHTML($path); + $output = file_get_contents($path); + } finally { + unlink($path); + } + + $this->assertStringContainsString(self::EXPECTED, $output); + } + + /** + * html5qp() reads a path through a different Masterminds method than it reads markup. + */ + public function testDocumentLoadedFromAFileKeepsTheTerminator() + { + $qp = html5qp(self::PI_FILE_HTML, null, self::OPTIONS); + + $this->assertStringContainsString(self::EXPECTED, $qp->html()); + } + + public function testHtmlAndWriteHtmlKeepTheDoctype() + { + $qp = html5qp(self::HTML, null, self::OPTIONS); + + $output = $this->capture(function () use ($qp) { + $qp->writeHTML(); + }); + + $this->assertStringStartsWith('', $qp->html()); + $this->assertStringStartsWith('', $output); + } + + public function testDocumentIsADocumentThroughOwnerDocument() + { + $element = html5qp(self::HTML, null, self::OPTIONS)->get(0); + + $this->assertInstanceOf(Document::class, $element->ownerDocument); + } + + /** + * Only the restored "?" is added, so content that itself ends in "?" keeps it. + */ + public function testProcessingInstructionEndingInAQuestionMarkRoundTrips() + { + $qp = html5qp('

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

', $qp->html()); + } + + public function testWriteHtmlLeavesTheDocumentUnchanged() + { + $qp = html5qp(self::HTML, null, self::OPTIONS); + + $this->capture(function () use ($qp) { + $qp->writeHTML(); + }); + + $this->assertSame('echo $title; ', $qp->find('h1')->get(0)->firstChild->data); + } + + /** + * A Masterminds document the caller built is a plain DOMDocument, and is written as-is. + */ + public function testCallerSuppliedHtml5DocumentIsSerializedAsIs() + { + $document = (new HTML5())->loadHTML(self::HTML); + $qp = qp($document, null, self::OPTIONS); + + $this->assertNotInstanceOf(Document::class, $qp->document()); + $this->assertStringContainsString('

', $qp->html()); + } + + public function testCallerSuppliedHtmlDocumentIsSerializedAsIs() + { + $this->requireLegacyHtmlParser(); + + $document = new DOMDocument(); + @$document->loadHTML(self::HTML); + + $this->assertStringContainsString(self::EXPECTED, qp($document, null, self::OPTIONS)->html()); + } + + public function testHtmlParserIsUnchanged() + { + $this->requireLegacyHtmlParser(); + + $qp = htmlqp(self::HTML, null, self::OPTIONS); + + $output = $this->capture(function () use ($qp) { + $qp->writeHTML(); + }); + + $this->assertStringContainsString(self::EXPECTED, $qp->html()); + $this->assertStringContainsString(self::EXPECTED, $output); + } + + public function testXmlParserIsUnchanged() + { + $qp = qp('', null, self::OPTIONS); + + $output = $this->capture(function () use ($qp) { + $qp->writeHTML(); + }); + + $this->assertStringContainsString('', $qp->html()); + $this->assertStringContainsString('', $output); + } +}