From 43fb6a3d3a3cccf206d77731e0e0cf7e40797d82 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 10:09:32 +1100 Subject: [PATCH 1/7] Keep the PHP warning text in exceptions raised from an error initializeFromError() passed the error number as the message and the warning text, cast to int, as the code. Pass them in the constructor's order, and throw static so IOException no longer needs its own copy. Fixes #91 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/IOException.php | 6 ------ src/ParseException.php | 3 +-- tests/QueryPath/DOMQueryTest.php | 22 ++++++++++++++++++++++ 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2dff43fe..2ad4119d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ QueryPath Changelog - Fix the `:text` pseudo-class so it matches jQuery: it selects `input` elements whose `type` attribute is absent or is `text` (case-insensitively). It never indicated, and still does not indicate, whether a node is a text node - `find('*')` now matches the nodes in the match set as well as their descendants, so that a selector can be tested against an element already in hand - `writeHTML5()` now returns the `DOMQuery` like the other `write*()` methods, and throws `QueryPath\IOException` instead of a `TypeError` when the file cannot be written (issue #80) +- Fix `QueryPath\ParseException` and `QueryPath\IOException` losing the PHP warning they were raised from (issue #91). The message was the error number, as in `2 (/path/to/src/DOMQuery.php: 1343)`, and the code was always `0`. The message now carries the warning text and the code is the PHP error level (for example `E_WARNING`), so code that matches on the old message text will need updating # 4.1.0 diff --git a/src/IOException.php b/src/IOException.php index 71195c3e..30ac7961 100644 --- a/src/IOException.php +++ b/src/IOException.php @@ -14,10 +14,4 @@ */ class IOException extends ParseException { - - public static function initializeFromError($errno, $errstr, $errfile, $errline, $context = null) - { - $class = __CLASS__; - throw new $class($errno, (int) $errstr, $errfile, $errline); - } } diff --git a/src/ParseException.php b/src/ParseException.php index dd21d93e..5a511793 100644 --- a/src/ParseException.php +++ b/src/ParseException.php @@ -44,7 +44,6 @@ public function __construct($msg = '', $code = 0, $file = null, $line = null) public static function initializeFromError($errno, $errstr, $errfile, $errline, $context = null) { - $class = __CLASS__; - throw new $class($errno, (int) $errstr, $errfile, $errline); + throw new static($errstr, $errno, $errfile, $errline); } } diff --git a/tests/QueryPath/DOMQueryTest.php b/tests/QueryPath/DOMQueryTest.php index e448e394..1f673628 100644 --- a/tests/QueryPath/DOMQueryTest.php +++ b/tests/QueryPath/DOMQueryTest.php @@ -311,6 +311,17 @@ public function testFailedParseXMLElement() } } + public function testFailedParseReportsWarning() + { + try { + qp('foonator;', null); + $this->fail('Expected a ParseException.'); + } catch (ParseException $e) { + $this->assertStringContainsString('DOMDocument::loadXML()', $e->getMessage()); + $this->assertSame(E_WARNING, $e->getCode()); + } + } + public function testIgnoreParserWarnings() { $qp = @qp('BAD!', null, ['ignore_parser_warnings' => true]); @@ -1473,6 +1484,17 @@ public function testFailWriteXML() } } + public function testFailWriteXMLReportsWarning() + { + try { + qp()->writeXML('./test/no-writing.xml'); + $this->fail('Expected an IOException.'); + } catch (IOException $e) { + $this->assertStringContainsString('DOMDocument::save(./test/no-writing.xml)', $e->getMessage()); + $this->assertSame(E_WARNING, $e->getCode()); + } + } + public function testFailWriteXHTML() { $this->expectException(IOException::class); From 7357097d01f8aa59aa1d970aec4491d98a5c96dc Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 10:09:53 +1100 Subject: [PATCH 2/7] Throw IOException from writeHTML() when the file cannot be written writeHTML() installed ParseException's error handler where the other writers install IOException's. testFailWriteHTML() called writeXML(), so the failure path went untested; it now calls writeHTML(). Fixes #92 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/DOMQuery.php | 2 +- tests/QueryPath/DOMQueryTest.php | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ad4119d..49beb616 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ QueryPath Changelog - `find('*')` now matches the nodes in the match set as well as their descendants, so that a selector can be tested against an element already in hand - `writeHTML5()` now returns the `DOMQuery` like the other `write*()` methods, and throws `QueryPath\IOException` instead of a `TypeError` when the file cannot be written (issue #80) - Fix `QueryPath\ParseException` and `QueryPath\IOException` losing the PHP warning they were raised from (issue #91). The message was the error number, as in `2 (/path/to/src/DOMQuery.php: 1343)`, and the code was always `0`. The message now carries the warning text and the code is the PHP error level (for example `E_WARNING`), so code that matches on the old message text will need updating +- 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 # 4.1.0 diff --git a/src/DOMQuery.php b/src/DOMQuery.php index 2c3be61f..71c26a97 100644 --- a/src/DOMQuery.php +++ b/src/DOMQuery.php @@ -1377,7 +1377,7 @@ public function writeHTML($path = null) print $this->saveDocumentHTML(); } else { try { - set_error_handler(['\QueryPath\ParseException', 'initializeFromError']); + set_error_handler([IOException::class, 'initializeFromError']); $this->saveDocumentHTML($path); } catch (Exception $e) { restore_error_handler(); diff --git a/tests/QueryPath/DOMQueryTest.php b/tests/QueryPath/DOMQueryTest.php index 1f673628..4e248b43 100644 --- a/tests/QueryPath/DOMQueryTest.php +++ b/tests/QueryPath/DOMQueryTest.php @@ -1512,7 +1512,7 @@ public function testFailWriteHTML() $this->expectException(IOException::class); try { - qp('')->writeXML('./test/no-writing.xml'); + htmlqp('

bar

')->writeHTML('./test/no-writing.html'); } catch (IOException $e) { // print $e->getMessage(); throw $e; From 336e05447d027f0c9e4bbe3def1206c026a15e79 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 10:11:59 +1100 Subject: [PATCH 3/7] Restore the error handler in finally, through one shared helper Seven blocks installed initializeFromError() as the error handler and restored it only from catch (QueryPath\Exception), so any other throwable left the handler installed. Route them all through DOM::withErrorHandler(), which restores it in finally. Fixes #95 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/DOM.php | 69 ++++++++++------------- src/DOMQuery.php | 25 ++------ tests/QueryPath/DOMQueryTest.php | 50 ++++++++++++++++ tests/QueryPath/ThrowingStreamWrapper.php | 23 ++++++++ 5 files changed, 111 insertions(+), 57 deletions(-) create mode 100644 tests/QueryPath/ThrowingStreamWrapper.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 49beb616..1053962f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ QueryPath Changelog - `writeHTML5()` now returns the `DOMQuery` like the other `write*()` methods, and throws `QueryPath\IOException` instead of a `TypeError` when the file cannot be written (issue #80) - Fix `QueryPath\ParseException` and `QueryPath\IOException` losing the PHP warning they were raised from (issue #91). The message was the error number, as in `2 (/path/to/src/DOMQuery.php: 1343)`, and the code was always `0`. The message now carries the warning text and the code is the PHP error level (for example `E_WARNING`), so code that matches on the old message text will need updating - 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 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). Every place that converts PHP warnings into exceptions now shares one helper that restores the previous handler in a `finally` block # 4.1.0 diff --git a/src/DOM.php b/src/DOM.php index b9e95eb6..45879137 100644 --- a/src/DOM.php +++ b/src/DOM.php @@ -177,9 +177,7 @@ private function parseXMLString($string, $flags = 0) { $document = self::createDocument('1.0'); $lead = strtolower(substr($string, 0, 5)); // errTypes); - + self::withErrorHandler(ParseException::class, function () use ($document, $string, $flags, $lead) { if (isset($this->options['convert_to_encoding'])) { // Is there another way to do this? @@ -217,12 +215,7 @@ private function parseXMLString($string, $flags = 0) else { self::loadHTMLString($document, $string); } - } // Emulate 'finally' behavior. - catch (Exception $e) { - restore_error_handler(); - throw $e; - } - restore_error_handler(); + }, $this->errTypes); if (empty($document)) { throw new ParseException('Unknown parser exception.'); @@ -453,15 +446,9 @@ protected function prepareInsert($item) } $frag = $this->document->createDocumentFragment(); - try { - set_error_handler([ParseException::class, 'initializeFromError'], $this->errTypes); + self::withErrorHandler(ParseException::class, function () use ($frag, $item) { $frag->appendXML($item); - } // Simulate a finally block. - catch (Exception $e) { - restore_error_handler(); - throw $e; - } - restore_error_handler(); + }, $this->errTypes); return $frag; } @@ -498,6 +485,28 @@ protected function prepareInsert($item) throw new Exception('Cannot prepare item of unsupported type: ' . gettype($item)); } + /** + * Run a callback with PHP errors converted into exceptions. + * + * @param string $exceptionClass + * ParseException or a subclass, whose initializeFromError() handles the errors. + * @param callable $callback + * @param int $types + * The error levels to convert. + * + * @return mixed + * Whatever the callback returns. + */ + protected static function withErrorHandler($exceptionClass, callable $callback, $types = E_ALL) + { + set_error_handler([$exceptionClass, 'initializeFromError'], $types); + try { + return $callback(); + } finally { + restore_error_handler(); + } + } + /** * Convenience function for getNthMatch(0). */ @@ -537,17 +546,9 @@ private function parseXMLFile($filename, $flags = 0, $context = null) // If a context is specified, we basically have to do the reading in // two steps: if (! empty($context)) { - try { - set_error_handler(['\QueryPath\ParseException', 'initializeFromError'], $this->errTypes); - $contents = file_get_contents($filename, false, $context); - } - // Apparently there is no 'finally' in PHP, so we have to restore the error - // handler this way: - catch (Exception $e) { - restore_error_handler(); - throw $e; - } - restore_error_handler(); + $contents = self::withErrorHandler(ParseException::class, function () use ($filename, $context) { + return file_get_contents($filename, false, $context); + }, $this->errTypes); if ($contents == false) { throw new ParseException(sprintf( @@ -576,9 +577,7 @@ private function parseXMLFile($filename, $flags = 0, $context = null) $ext = $lastDot !== false ? strtolower(substr($filename, $lastDot)) : ''; - try { - set_error_handler([ParseException::class, 'initializeFromError'], $this->errTypes); - + self::withErrorHandler(ParseException::class, function () use ($document, $filename, $flags, $useParser, $htmlExtensions, $ext) { // If the parser is explicitly set to XML, use that parser. if ($useParser === 'xml') { $document->load($filename, $flags); @@ -590,13 +589,7 @@ private function parseXMLFile($filename, $flags = 0, $context = null) else { $document->load($filename, $flags); } - - } // Emulate 'finally' behavior. - catch (Exception $e) { - restore_error_handler(); - throw $e; - } - restore_error_handler(); + }, $this->errTypes); return $document; } diff --git a/src/DOMQuery.php b/src/DOMQuery.php index 71c26a97..7cb4178e 100644 --- a/src/DOMQuery.php +++ b/src/DOMQuery.php @@ -1338,14 +1338,9 @@ public function writeXML($path = null, $options = 0) if ($path === null) { print $this->document->saveXML(null, $options); } else { - try { - set_error_handler([IOException::class, 'initializeFromError']); + self::withErrorHandler(IOException::class, function () use ($path, $options) { $this->document->save($path, $options); - } catch (Exception $e) { - restore_error_handler(); - throw $e; - } - restore_error_handler(); + }); } return $this; @@ -1376,14 +1371,9 @@ public function writeHTML($path = null) if ($path === null) { print $this->saveDocumentHTML(); } else { - try { - set_error_handler([IOException::class, 'initializeFromError']); + self::withErrorHandler(IOException::class, function () use ($path) { $this->saveDocumentHTML($path); - } catch (Exception $e) { - restore_error_handler(); - throw $e; - } - restore_error_handler(); + }); } return $this; @@ -1413,12 +1403,9 @@ public function writeHTML5($path = null) // Print the document to stdout. print $html5->saveHTML($this->document); } else { - set_error_handler([IOException::class, 'initializeFromError']); - try { + self::withErrorHandler(IOException::class, function () use ($html5, $path) { $html5->save($this->document, $path); - } finally { - restore_error_handler(); - } + }); } return $this; diff --git a/tests/QueryPath/DOMQueryTest.php b/tests/QueryPath/DOMQueryTest.php index 4e248b43..3cb85754 100644 --- a/tests/QueryPath/DOMQueryTest.php +++ b/tests/QueryPath/DOMQueryTest.php @@ -16,6 +16,7 @@ use QueryPath\ParseException; use QueryPath\QueryPath; use QueryPath\DOMQuery; +use RuntimeException; use SplDoublyLinkedList; use stdClass; @@ -1526,6 +1527,55 @@ public function testFailWriteHTML5() html5qp('bar')->writeHTML5('./test/no-writing.html'); } + public function throwingCallProvider(): array + { + $url = ThrowingStreamWrapper::PROTOCOL . '://out'; + + return [ + 'writeXML' => [function () use ($url) { + qp('')->writeXML($url); + }], + 'writeHTML' => [function () use ($url) { + htmlqp('

bar

')->writeHTML($url); + }], + 'writeHTML5' => [function () use ($url) { + html5qp('

bar

')->writeHTML5($url); + }], + 'load with context' => [function () use ($url) { + qp($url . '.xml', null, ['context' => stream_context_create()]); + }], + ]; + } + + /** + * A throwable QueryPath does not convert must not leave its error handler installed. + * + * @dataProvider throwingCallProvider + * + * @param callable $callback + */ + public function testErrorHandlerRestoredAfterForeignThrowable(callable $callback) + { + $sentinel = function () { + return false; + }; + + stream_wrapper_register(ThrowingStreamWrapper::PROTOCOL, ThrowingStreamWrapper::class); + set_error_handler($sentinel); + try { + $callback(); + $this->fail('Expected a RuntimeException.'); + } catch (RuntimeException $e) { + $current = set_error_handler(null); + restore_error_handler(); + } finally { + restore_error_handler(); + stream_wrapper_unregister(ThrowingStreamWrapper::PROTOCOL); + } + + $this->assertSame($sentinel, $current); + } + public function testWriteHTML5() { $qp = html5qp('bar'); diff --git a/tests/QueryPath/ThrowingStreamWrapper.php b/tests/QueryPath/ThrowingStreamWrapper.php new file mode 100644 index 00000000..cc8db9cd --- /dev/null +++ b/tests/QueryPath/ThrowingStreamWrapper.php @@ -0,0 +1,23 @@ + Date: Mon, 26 Oct 2026 10:09:17 +1100 Subject: [PATCH 4/7] Return '' from tag() when the first match is not an element tag() read tagName on whatever node came first, so a text, comment or processing instruction node from contents() raised an "Undefined property" warning and returned NULL. Return an empty string instead, matching the empty match set. Fixes #93. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/DOMQuery.php | 9 +++++++-- tests/QueryPath/DOMQueryTest.php | 19 +++++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1053962f..159aa495 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ QueryPath Changelog - Fix `QueryPath\ParseException` and `QueryPath\IOException` losing the PHP warning they were raised from (issue #91). The message was the error number, as in `2 (/path/to/src/DOMQuery.php: 1343)`, and the code was always `0`. The message now carries the warning text and the code is the PHP error level (for example `E_WARNING`), so code that matches on the old message text will need updating - 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 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). Every place that converts PHP warnings into exceptions now shares one helper that restores the previous handler in a `finally` block +- 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) # 4.1.0 diff --git a/src/DOMQuery.php b/src/DOMQuery.php index 7cb4178e..c06e3934 100644 --- a/src/DOMQuery.php +++ b/src/DOMQuery.php @@ -14,6 +14,7 @@ namespace QueryPath; use DOMDocument; +use DOMElement; use DOMNode; use DOMXPath; use QueryPath\CSS\DOMTraverser; @@ -547,7 +548,9 @@ public function index($subject) * The tag name of the first element in the list. * * This returns the tag name of the first element in the list of matches. If - * the list is empty, an empty string will be used. + * the list is empty, or the first match is not an element (for example, a + * text, comment or processing instruction node returned by contents()), an + * empty string will be used. * * @return string * The tag name of the first element in the list. @@ -556,7 +559,9 @@ public function index($subject) */ public function tag() { - return ($this->matches->count() > 0) ? $this->getFirstMatch()->tagName : ''; + $first = ($this->matches->count() > 0) ? $this->getFirstMatch() : null; + + return ($first instanceof DOMElement) ? $first->tagName : ''; } /** diff --git a/tests/QueryPath/DOMQueryTest.php b/tests/QueryPath/DOMQueryTest.php index 3cb85754..8632d6b8 100644 --- a/tests/QueryPath/DOMQueryTest.php +++ b/tests/QueryPath/DOMQueryTest.php @@ -779,6 +779,25 @@ public function testTag() { $file = DATA_FILE; $this->assertEquals('li', qp($file, 'li')->tag()); + $this->assertSame('', qp($file, 'nonexistent')->tag()); + } + + /** + * Issue #93 + */ + public function testTagNonElementFirstMatch() + { + $qp = qp('text', 'root')->contents(); + $this->assertSame('', $qp->tag()); + + $qp = qp('', 'root')->contents(); + $this->assertSame('', $qp->tag()); + + $qp = qp('', 'root')->contents(); + $this->assertSame('', $qp->tag()); + + $qp = qp('text', 'root')->contents(); + $this->assertSame('a', $qp->tag()); } public function testAppend() From 267d3bc4f8859ecb6346af48d916a07f602c9b16 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 10:09:29 +1100 Subject: [PATCH 5/7] Resolve createElement() prefixes against the first match, not the root QPXML::createElement() looked an element name's prefix up on the owner document, which only sees declarations on the document element, so a prefix declared on the matched element or an ancestor below the root threw "Undefined namespace". Look it up on the element instead; root declarations are in scope there too. The foreach returned on its first pass, so only the first match was ever used. Use get(0) so the code says so. Fixes #94 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/Extension/QPXML.php | 43 +++++++++++++------------ tests/QueryPath/Extension/QPXMLTest.php | 39 ++++++++++++++++++++++ 3 files changed, 62 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 159aa495..8b3b4d85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ QueryPath Changelog - Fix `QPXML::createElement()` emitting an "Only variables should be passed by reference" notice for a namespace-prefixed element name without an explicit namespace URI (issue #82) - Fix the `:text` pseudo-class so it matches jQuery: it selects `input` elements whose `type` attribute is absent or is `text` (case-insensitively). It never indicated, and still does not indicate, whether a node is a text node - `find('*')` now matches the nodes in the match set as well as their descendants, so that a selector can be tested against an element already in hand +- Fix `QPXML::createElement()` throwing "Undefined namespace" for a prefixed element name whose prefix is declared on the matched element or an ancestor below the document element. The prefix is now resolved against the namespaces in scope on the first matched element (issue #94) - `writeHTML5()` now returns the `DOMQuery` like the other `write*()` methods, and throws `QueryPath\IOException` instead of a `TypeError` when the file cannot be written (issue #80) - Fix `QueryPath\ParseException` and `QueryPath\IOException` losing the PHP warning they were raised from (issue #91). The message was the error number, as in `2 (/path/to/src/DOMQuery.php: 1343)`, and the code was always `0`. The message now carries the warning text and the code is the PHP error level (for example `E_WARNING`), so code that matches on the old message text will need updating - 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 diff --git a/src/Extension/QPXML.php b/src/Extension/QPXML.php index 85d7f34a..ec227da9 100644 --- a/src/Extension/QPXML.php +++ b/src/Extension/QPXML.php @@ -174,40 +174,41 @@ public function createNilElement($text, $value) /** * Create an element with the given namespace. * + * The element is created in the document of the first matched element. If + * $text has a prefix and no $nsUri is given, the prefix is resolved against + * the namespaces in scope on that element. + * * @param string $text * @param string $nsUri * The namespace URI for the given element. * * @return DOMQuery + * The new element, or an empty DOMQuery if there are no matches. + * @throws Exception + * If the prefix of $text is not bound to a namespace on the first match. */ public function createElement($text, $nsUri = null) { - if (isset($text)) { - foreach ($this->qp->get() as $element) { - if ($nsUri === null && strpos($text, ':') !== false) { - $ns = strstr($text, ':', true); - $nsUri = $element->ownerDocument->lookupNamespaceURI($ns); - - if ($nsUri === null) { - throw new Exception("Undefined namespace for: " . $text); - } - } + $element = $this->qp->get(0); + if (! isset($text) || $element === null) { + return new DOMQuery(); + } - $node = null; - if ($nsUri !== null) { - $node = $element->ownerDocument->createElementNS( - $nsUri, - $text - ); - } else { - $node = $element->ownerDocument->createElement($text); - } + if ($nsUri === null && strpos($text, ':') !== false) { + $nsUri = $element->lookupNamespaceURI(strstr($text, ':', true)); - return QueryPath::with($node); + if ($nsUri === null) { + throw new Exception("Undefined namespace for: " . $text); } } - return new DOMQuery(); + if ($nsUri !== null) { + $node = $element->ownerDocument->createElementNS($nsUri, $text); + } else { + $node = $element->ownerDocument->createElement($text); + } + + return QueryPath::with($node); } /** diff --git a/tests/QueryPath/Extension/QPXMLTest.php b/tests/QueryPath/Extension/QPXMLTest.php index 37671c45..dd9c8588 100644 --- a/tests/QueryPath/Extension/QPXMLTest.php +++ b/tests/QueryPath/Extension/QPXMLTest.php @@ -38,6 +38,45 @@ public function testCreateElementWithPrefix() $this->assertEquals('x:item', $element->nodeName); } + public function testCreateElementWithPrefixDeclaredBelowRoot() + { + $xml = ''; + + // A prefix declared on the matched element is in scope for it. + $element = qp($xml, 'child')->createElement('y:item')->get(0); + $this->assertEquals('urn:y', $element->namespaceURI); + $this->assertEquals('y:item', $element->nodeName); + + // So is one declared on the root. + $element = qp($xml, 'child')->createElement('x:item')->get(0); + $this->assertEquals('urn:x', $element->namespaceURI); + } + + public function testCreateElementWithPrefixOutOfScope() + { + $xml = ''; + + $this->expectException(\QueryPath\Exception::class); + $this->expectExceptionMessage('Undefined namespace for: y:item'); + qp($xml, 'b')->createElement('y:item'); + } + + public function testCreateElementUsesFirstMatch() + { + $xml = ''; + + $element = qp($xml, 'a, b')->createElement('y:item'); + $this->assertCount(1, $element); + $this->assertEquals('urn:a', $element->get(0)->namespaceURI); + } + + public function testCreateElementWithNoMatches() + { + $xml = ''; + + $this->assertCount(0, qp($xml, 'missing')->createElement('x:item')); + } + public function testComment() { $this->assertEquals('This is a comment.', trim(qp($this->file, 'root')->comment())); From 94f0c947d36da9e44008a5ba834ab48399ba92c2 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 10:10:11 +1100 Subject: [PATCH 6/7] Share one implementation between the QPXML cdata/comment/pi getters The three getters were copies of the same loop that had drifted: cdata() used === and returned null explicitly, while comment() and pi() used == and fell off the end. Move the loop into a private firstChildText() helper, keeping pi()'s filter on the processing instruction's target, and give pi() a docblock to match the other two. No behaviour change. Tests pin down the not-found, first-across-matches and child-only cases for all three getters. Fixes #96 Co-Authored-By: Claude Opus 5.5 --- src/Extension/QPXML.php | 69 ++++++++++++++++--------- tests/QueryPath/Extension/QPXMLTest.php | 32 ++++++++++++ 2 files changed, 77 insertions(+), 24 deletions(-) diff --git a/src/Extension/QPXML.php b/src/Extension/QPXML.php index ec227da9..88782485 100644 --- a/src/Extension/QPXML.php +++ b/src/Extension/QPXML.php @@ -73,18 +73,7 @@ public function cdata($text = null) return $this->qp; } - // Look for CDATA sections. - foreach ($this->qp->get() as $ele) { - foreach ($ele->childNodes as $node) { - if ($node->nodeType === XML_CDATA_SECTION_NODE) { - // Return first match. - return $node->textContent; - } - } - } - - return null; - // Nothing found + return $this->firstChildText(XML_CDATA_SECTION_NODE); } /** @@ -117,18 +106,32 @@ public function comment($text = null) return $this->qp; } - foreach ($this->qp->get() as $ele) { - foreach ($ele->childNodes as $node) { - if ($node->nodeType == XML_COMMENT_NODE) { - // Return first match. - return $node->textContent; - } - } - } + + return $this->firstChildText(XML_COMMENT_NODE); } /** - * Get or set a processor instruction. + * Get or set a processing instruction. + * + * If $text is passed in (and is not NULL), this will add a processing instruction + * with the target $prefix and the data $text to every match in the set. + * + * If no text is passed in, this will return the data of the first processing + * instruction in the set of matches, limited to those with the target $prefix + * if one is given. If none is found, NULL will be returned. + * + * @param string $prefix + * The target of the processing instruction (the `xml-stylesheet` in + * ``). When getting, NULL matches any target. + * @param string $text + * The data of the processing instruction. If set, a new processing instruction + * will be created in every item wrapped by the current {@link QueryPath}. + * + * @return mixed + * If $text is set, this will return a {@link QueryPath}. If no text is set, this + * will return the string value of the first matching processing instruction, or + * NULL if none is found. + * @see comment() */ public function pi($prefix = null, $text = null) { @@ -140,13 +143,31 @@ public function pi($prefix = null, $text = null) return $this->qp; } + + return $this->firstChildText(XML_PI_NODE, $prefix); + } + + /** + * Get the text of the first child node of the given type in the set of matches. + * + * @param int $nodeType + * One of the XML_*_NODE constants. + * @param string $target + * If set, only processing instructions with this target match. + * + * @return string|null + */ + private function firstChildText($nodeType, $target = null) + { foreach ($this->qp->get() as $ele) { foreach ($ele->childNodes as $node) { - if ($node->nodeType == XML_PI_NODE && ($prefix === null || $node->target === $prefix)) { + if ($node->nodeType === $nodeType && ($target === null || $node->target === $target)) { return $node->textContent; } - } // foreach - } // foreach + } + } + + return null; } public function toXml() diff --git a/tests/QueryPath/Extension/QPXMLTest.php b/tests/QueryPath/Extension/QPXMLTest.php index dd9c8588..389075d1 100644 --- a/tests/QueryPath/Extension/QPXMLTest.php +++ b/tests/QueryPath/Extension/QPXMLTest.php @@ -100,4 +100,36 @@ public function testProcessingInstructionByPrefix() $this->assertEquals('two', $qp->pi('second')); $this->assertNull($qp->pi('missing')); } + + public function testGettersReturnNullWhenNothingFound() + { + $xml = 'text'; + + // Only child nodes of the matches are searched, not deeper descendants. + $qp = qp($xml, 'a'); + $this->assertNull($qp->cdata()); + $this->assertNull($qp->comment()); + $this->assertNull($qp->pi()); + $this->assertNull($qp->pi('nested')); + + $qp = qp($xml, 'missing'); + $this->assertNull($qp->cdata()); + $this->assertNull($qp->comment()); + $this->assertNull($qp->pi()); + } + + public function testGettersReturnFirstFoundAcrossMatches() + { + $xml = '' + . '' + . '' + . '' + . ''; + + $qp = qp($xml, 'a'); + $this->assertEquals('one', $qp->cdata()); + $this->assertEquals('one', $qp->comment()); + $this->assertEquals('one', $qp->pi()); + $this->assertEquals('one', $qp->pi('y')); + } } From 6eeb460bd3f8505ec68c53d8e547ee02ff3198bf Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 10:14:53 +1100 Subject: [PATCH 7/7] Simplify the follow-up fixes and their tests Decide the file parser before entering the error handler, use expectException*() and a data provider in the new tests, drop the rethrow-only try/catch blocks in the write-failure tests, and trim comments that restated the code. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- src/DOM.php | 15 ++--- tests/QueryPath/DOMQueryTest.php | 72 +++++++++-------------- tests/QueryPath/Extension/QPXMLTest.php | 2 - tests/QueryPath/ThrowingStreamWrapper.php | 5 +- 5 files changed, 36 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b3b4d85..9712f50c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,7 @@ QueryPath Changelog - `writeHTML5()` now returns the `DOMQuery` like the other `write*()` methods, and throws `QueryPath\IOException` instead of a `TypeError` when the file cannot be written (issue #80) - Fix `QueryPath\ParseException` and `QueryPath\IOException` losing the PHP warning they were raised from (issue #91). The message was the error number, as in `2 (/path/to/src/DOMQuery.php: 1343)`, and the code was always `0`. The message now carries the warning text and the code is the PHP error level (for example `E_WARNING`), so code that matches on the old message text will need updating - 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 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). Every place that converts PHP warnings into exceptions now shares one helper that restores the previous handler in a `finally` block +- 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 `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) # 4.1.0 diff --git a/src/DOM.php b/src/DOM.php index 45879137..f0596878 100644 --- a/src/DOM.php +++ b/src/DOM.php @@ -577,16 +577,13 @@ private function parseXMLFile($filename, $flags = 0, $context = null) $ext = $lastDot !== false ? strtolower(substr($filename, $lastDot)) : ''; - self::withErrorHandler(ParseException::class, function () use ($document, $filename, $flags, $useParser, $htmlExtensions, $ext) { - // If the parser is explicitly set to XML, use that parser. - if ($useParser === 'xml') { - $document->load($filename, $flags); - } // Otherwise, see if it looks like HTML. - elseif ($useParser === 'html' || isset($htmlExtensions[$ext])) { - // Try parsing it as HTML. + // An explicit parser setting wins; otherwise go by the file extension, defaulting to XML. + $asHtml = $useParser === 'html' || ($useParser !== 'xml' && isset($htmlExtensions[$ext])); + + self::withErrorHandler(ParseException::class, function () use ($document, $filename, $flags, $asHtml) { + if ($asHtml) { self::loadHTMLFile($document, $filename); - } // Default to XML. - else { + } else { $document->load($filename, $flags); } }, $this->errTypes); diff --git a/tests/QueryPath/DOMQueryTest.php b/tests/QueryPath/DOMQueryTest.php index 8632d6b8..77be4fdd 100644 --- a/tests/QueryPath/DOMQueryTest.php +++ b/tests/QueryPath/DOMQueryTest.php @@ -314,13 +314,11 @@ public function testFailedParseXMLElement() public function testFailedParseReportsWarning() { - try { - qp('foonator;', null); - $this->fail('Expected a ParseException.'); - } catch (ParseException $e) { - $this->assertStringContainsString('DOMDocument::loadXML()', $e->getMessage()); - $this->assertSame(E_WARNING, $e->getCode()); - } + $this->expectException(ParseException::class); + $this->expectExceptionMessage('DOMDocument::loadXML()'); + $this->expectExceptionCode(E_WARNING); + + qp('foonator;', null); } public function testIgnoreParserWarnings() @@ -782,22 +780,25 @@ public function testTag() $this->assertSame('', qp($file, 'nonexistent')->tag()); } + public function tagFirstMatchProvider(): array + { + return [ + 'text' => ['text', ''], + 'comment' => ['', ''], + 'processing instruction' => ['', ''], + 'element' => ['text', 'a'], + ]; + } + /** * Issue #93 + * + * @dataProvider tagFirstMatchProvider */ - public function testTagNonElementFirstMatch() + public function testTagFirstMatchOfContents($children, $expected) { - $qp = qp('text', 'root')->contents(); - $this->assertSame('', $qp->tag()); - - $qp = qp('', 'root')->contents(); - $this->assertSame('', $qp->tag()); - - $qp = qp('', 'root')->contents(); - $this->assertSame('', $qp->tag()); - - $qp = qp('text', 'root')->contents(); - $this->assertSame('a', $qp->tag()); + $qp = qp('' . $children . '', 'root')->contents(); + $this->assertSame($expected, $qp->tag()); } public function testAppend() @@ -1496,47 +1497,30 @@ public function testFailWriteXML() { $this->expectException(IOException::class); - try { - qp()->writeXML('./test/no-writing.xml'); - } catch (Exception $e) { - //print $e->getMessage(); - throw $e; - } + qp()->writeXML('./test/no-writing.xml'); } public function testFailWriteXMLReportsWarning() { - try { - qp()->writeXML('./test/no-writing.xml'); - $this->fail('Expected an IOException.'); - } catch (IOException $e) { - $this->assertStringContainsString('DOMDocument::save(./test/no-writing.xml)', $e->getMessage()); - $this->assertSame(E_WARNING, $e->getCode()); - } + $this->expectException(IOException::class); + $this->expectExceptionMessage('DOMDocument::save(./test/no-writing.xml)'); + $this->expectExceptionCode(E_WARNING); + + qp()->writeXML('./test/no-writing.xml'); } public function testFailWriteXHTML() { $this->expectException(IOException::class); - try { - qp()->writeXHTML('./test/no-writing.xml'); - } catch (IOException $e) { - //print $e->getMessage(); - throw $e; - } + qp()->writeXHTML('./test/no-writing.xml'); } public function testFailWriteHTML() { $this->expectException(IOException::class); - try { - htmlqp('

bar

')->writeHTML('./test/no-writing.html'); - } catch (IOException $e) { - // print $e->getMessage(); - throw $e; - } + htmlqp('

bar

')->writeHTML('./test/no-writing.html'); } public function testFailWriteHTML5() diff --git a/tests/QueryPath/Extension/QPXMLTest.php b/tests/QueryPath/Extension/QPXMLTest.php index 389075d1..8f16badc 100644 --- a/tests/QueryPath/Extension/QPXMLTest.php +++ b/tests/QueryPath/Extension/QPXMLTest.php @@ -42,12 +42,10 @@ public function testCreateElementWithPrefixDeclaredBelowRoot() { $xml = ''; - // A prefix declared on the matched element is in scope for it. $element = qp($xml, 'child')->createElement('y:item')->get(0); $this->assertEquals('urn:y', $element->namespaceURI); $this->assertEquals('y:item', $element->nodeName); - // So is one declared on the root. $element = qp($xml, 'child')->createElement('x:item')->get(0); $this->assertEquals('urn:x', $element->namespaceURI); } diff --git a/tests/QueryPath/ThrowingStreamWrapper.php b/tests/QueryPath/ThrowingStreamWrapper.php index cc8db9cd..a43b7a30 100644 --- a/tests/QueryPath/ThrowingStreamWrapper.php +++ b/tests/QueryPath/ThrowingStreamWrapper.php @@ -5,10 +5,7 @@ use RuntimeException; /** - * A stream wrapper that throws when a stream is opened. - * - * Gives a way to raise an exception that is not a QueryPath\Exception from inside libxml's and - * PHP's own file handling. + * A stream wrapper that throws a non-QueryPath exception when a stream is opened. */ class ThrowingStreamWrapper {