Keep the closing "?" of processing instructions in html5qp() documents

CHANGELOG.md

@@ -36,6 +36,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 `<?php echo $title; ?>` came out as `<?php echo $title; >` (issue #86). `html5qp()` documents are now `QueryPath\Document` instances, like those from `qp()` and `htmlqp()`
 - Fix `css()` merging the inline styles of every matched element and writing the combined result back to all of them. The new declarations are now merged into each element's own `style` attribute (issue #77)
 - Fix `not()` keeping only the given nodes, instead of removing them, when passed an `SplObjectStorage` such as `get(null, true)` returns (issue #75)
 - Fix `QueryPath::VERSION` and `QueryPath::VERSION_MAJOR` still reporting `3.2.2` and `3`. They now report `4.2.0` and `4`, and both are deprecated in favour of `\Composer\InstalledVersions::getPrettyVersion('gravitypdf/querypath')`, which cannot drift from the installed version (issue #83)

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

src/QueryPath.php

@@ -87,6 +87,7 @@
 
 namespace QueryPath;
 
+use DOMDocument;
 use Masterminds\HTML5;
 use QueryPath\ExtensionRegistry;
 
@@ -273,6 +274,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);

tests/Issues/Issue86Test.php

@@ -0,0 +1,198 @@
+<?php
+/**
+ * @file
+ *
+ * Regression tests for https://github.com/GravityPDF/querypath/issues/86
+ *
+ * libxml's HTML serializer writes a processing instruction as `<?target data>` and relies on the
+ * closing "?" already being in the data. Masterminds never puts it there, so html() and writeHTML()
+ * turned `<?php echo $title; ?>` from an html5qp() document into `<?php echo $title; >`.
+ */
+
+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 = '<html><body><h1><?php echo $title; ?></h1></body></html>';
+
+	private const EXPECTED = '<h1><?php echo $title; ?></h1>';
+
+	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('<!DOCTYPE html>', $qp->html());
+		$this->assertStringStartsWith('<!DOCTYPE html>', $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('<html><body><h1><?php $a = 1; ??></h1></body></html>', null, self::OPTIONS);
+
+		$this->assertStringContainsString('<h1><?php $a = 1; ??></h1>', $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('<h1><?php echo $title; ></h1>', $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('<?xml version="1.0"?><root><item><?php echo $title; ?></item></root>', null, self::OPTIONS);
+
+		$output = $this->capture(function () use ($qp) {
+			$qp->writeHTML();
+		});
+
+		$this->assertStringContainsString('<item><?php echo $title; ?></item>', $qp->html());
+		$this->assertStringContainsString('<item><?php echo $title; ?></item>', $output);
+	}
+}