From 36027d3c869ca5c3d0fcb9bde5c73cf10e2b5069 Mon Sep 17 00:00:00 2001 From: Jake Jackson Date: Fri, 21 Aug 2026 14:35:53 +1000 Subject: [PATCH 1/2] Make is() test the current match set, like jQuery is() ran a descendant search (`branch($selector)->count() > 0`), so it returned TRUE whenever anything *below* an element in the match set matched the selector. That made `html5qp('

foo

', 'p')->is('span')` return TRUE and `is()` largely useless as a predicate. It now filters the elements held in the match set instead, by handing them to CSS\DOMTraverser with `$initialized = true`, and returns TRUE when at least one of them matches. Ancestors and descendants no longer cause a match, while combinators and positional pseudo-classes are still evaluated against the full document. Non-element nodes are skipped rather than passed to the traverser, which also stops `is()` fataling on a DOMText. The DOMNode and Traversable overloads are unchanged. Fixes #51 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 3 + src/Helpers/QueryChecks.php | 45 ++++++++-- tests/Issues/Issue51Test.php | 154 +++++++++++++++++++++++++++++++++++ 3 files changed, 196 insertions(+), 6 deletions(-) create mode 100644 tests/Issues/Issue51Test.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e390a2..75e72d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ QueryPath Changelog # Unreleased changes +- **Breaking behaviour change:** `DOMQuery::is()` now behaves like jQuery's `.is()`. It tests the elements held in the current match set and returns `true` when at least one of them matches the selector. Previously it ran a descendant search, so `html5qp('

foo

', 'p')->is('span')` returned `true`. Use `has()` if you need the old "does the set contain something matching this selector" behaviour (#51) +- `DOMQuery::is()` no longer raises a fatal error when the match set contains non-element nodes (text nodes, comments, processing instructions). Those nodes cannot match a CSS selector, so they are skipped +- Fix `parents()`, `parentsUntil()`, `next()`, `nextAll()`, `nextUntil()`, `prev()`, `prevAll()`, `prevUntil()`, `closest()`, and `not()`, which filter with `is()` and therefore also matched elements that merely *contained* the selector. For example, `parents('div')` returned every ancestor that contained a `div`, not just the ancestors that are a `div` - Reorganise, modernise, and repair the `examples/` directory. Each example now lives in its own subdirectory with an `index.php`, and the full set is indexed in `examples/quickstart-guide.md` - Convert the remaining legacy examples: `simple_example.php`, `techniques.php`, `svg.php`, `rss.php`, `odt.php`, `parse_php.php`, and `sparql.php` - Fix examples that no longer ran: send a `User-Agent` where remote hosts now require one, resolve paths relative to the example rather than the working directory, and stop relying on the removed `qp.php` autoloader and the PHP 8 incompatible `eachLambda()` diff --git a/src/Helpers/QueryChecks.php b/src/Helpers/QueryChecks.php index cdb4b6c..cd298da 100644 --- a/src/Helpers/QueryChecks.php +++ b/src/Helpers/QueryChecks.php @@ -2,7 +2,9 @@ namespace QueryPath\Helpers; +use DOMElement; use DOMNode; +use QueryPath\CSS\DOMTraverser; use QueryPath\CSS\ParseException; use QueryPath\DOMQuery; use QueryPath\Exception; @@ -21,20 +23,33 @@ trait QueryChecks { /** - * Given a selector, this checks to see if the current set has one or more matches. + * Check the current set of elements against a selector, and return TRUE if at least + * one of them matches. + * + * This behaves like jQuery's is(): only the elements in the current match set are + * tested. Neither the descendants nor the ancestors of those elements are considered. + * Use has() if you need to know whether the current set contains something that + * matches a selector. * * Unlike jQuery's version, this supports full selectors (not just simple ones). * - * @param string|DOMNode $selector - * The selector to search for. As of QueryPath 2.1.1, this also supports passing a - * DOMNode object. + * Non-element nodes in the match set (text nodes, comments, processing instructions, and + * so on) can never match a CSS selector, and are simply skipped. + * + * @param string|DOMNode|Traversable $selector + * The selector to test the current match set against. As of QueryPath 2.1.1, this also + * supports passing a DOMNode object, in which case the current match set must consist of + * exactly that one node, or a Traversable (e.g. another DOMQuery's match set), in which + * case the two sets must contain exactly the same nodes. * * @return boolean * TRUE if one or more elements match. FALSE if no match is found. * @throws Exception - * @throws Exception + * @throws ParseException * @see get() * @see eq() + * @see has() + * @see filter() */ public function is($selector): bool { @@ -63,7 +78,25 @@ public function is($selector): bool throw new Exception('Cannot compare an object to a DOMQuery.'); } - return $this->branch($selector)->count() > 0; + // Only elements can be matched against a CSS selector, so anything else in the + // match set is discarded before the selector is evaluated. + $candidates = new SplObjectStorage(); + foreach ($this->matches as $match) { + if ($match instanceof DOMElement) { + $candidates->offsetSet($match); + } + } + + if (count($candidates) === 0) { + return false; + } + + // The second argument tells the traverser that the match set is already the set of + // candidates, so the selector filters those elements rather than searching below them. + $traverser = new DOMTraverser($candidates, true); + $traverser->find($selector); + + return count($traverser->matches()) > 0; } /** diff --git a/tests/Issues/Issue51Test.php b/tests/Issues/Issue51Test.php new file mode 100644 index 0000000..930f0c2 --- /dev/null +++ b/tests/Issues/Issue51Test.php @@ -0,0 +1,154 @@ +foo

', 'p'); + + self::assertTrue($dom->is('p'), 'Should match an element held directly in the collection'); + self::assertFalse($dom->is('span'), 'Should not match a descendant of an element held in the collection'); + } + + /** + * The same document, but with the collection left at the document element. Neither the + *

nor the is in the collection, so neither may match. + */ + public function testIsDoesNotMatchDeeperDescendants(): void + { + $dom = html5qp('

foo

'); + + self::assertTrue($dom->is('html'), 'The collection holds the document element'); + self::assertFalse($dom->is('p'), 'Should not match a descendant of an element held in the collection'); + self::assertFalse($dom->is('span'), 'Should not match a descendant of an element held in the collection'); + } + + public function testIsDoesNotMatchAncestors(): void + { + $dom = html5qp('

foo

', 'span'); + + self::assertTrue($dom->is('span')); + self::assertFalse($dom->is('p'), 'Should not match the parent of an element held in the collection'); + self::assertFalse($dom->is('#outer'), 'Should not match an ancestor of an element held in the collection'); + } + + /** + * jQuery returns TRUE when *at least one* of the elements in the set matches. + */ + public function testIsMatchesWhenAnyElementInTheSetMatches(): void + { + $dom = html5qp('', 'li'); + + self::assertCount(3, $dom); + self::assertTrue($dom->is('li')); + self::assertTrue($dom->is('#one'), 'The first element matches'); + self::assertTrue($dom->is('.two'), 'The second element matches'); + self::assertTrue($dom->is('li, dt'), 'Selector groups are supported'); + self::assertFalse($dom->is('.missing')); + self::assertFalse($dom->is('ul'), 'Should not match the parent of the elements in the collection'); + } + + /** + * The elements are still tested in the context of their own document, so combinators and + * positional pseudo-classes continue to work. + */ + public function testIsSupportsFullSelectors(): void + { + $dom = html5qp('

foo

', 'p'); + + self::assertTrue($dom->is('div p'), 'Descendant combinators are evaluated against the document'); + self::assertTrue($dom->is('div > p'), 'Child combinators are evaluated against the document'); + self::assertTrue($dom->is('p#pp.one.two')); + self::assertTrue($dom->is('p[id="pp"]')); + self::assertTrue($dom->is(':first-child')); + self::assertFalse($dom->is('section p'), 'A non-matching ancestor means no match'); + self::assertFalse($dom->is(':root')); + } + + public function testIsOnAnEmptyCollectionIsFalse(): void + { + $dom = html5qp('

foo

', 'section'); + + self::assertCount(0, $dom); + self::assertFalse($dom->is('*')); + } + + /** + * Nodes that are not elements can never match a CSS selector, and must not raise an error. + */ + public function testIsOnNonElementNodesIsFalse(): void + { + $contents = html5qp('
Sample
', 'div')->contents(); + + self::assertCount(2, $contents); + self::assertFalse($contents->is('div')); + self::assertFalse($contents->is('*')); + } + + /** + * The DOMNode and Traversable overloads are unchanged. + */ + public function testIsStillAcceptsADomNode(): void + { + $dom = html5qp(''); + $one = $dom->top('#one'); + $node = $one->get(0); + + self::assertTrue($one->is($node)); + self::assertFalse($dom->top('#two')->is($node)); + self::assertFalse($dom->top('li')->is($node), 'A single node cannot equal a set of two'); + } + + public function testIsStillAcceptsATraversable(): void + { + $dom = html5qp(''); + + $list = new SplDoublyLinkedList(); + $list->push($dom->top('#one')->get(0)); + $list->push($dom->top('#two')->get(0)); + + self::assertTrue($dom->top('#one,#two')->is($list)); + self::assertFalse($dom->top('#one')->is($list)); + } + + /** + * has() is the migration path for anyone who relied on the old containment behaviour. + */ + public function testHasProvidesTheOldContainmentBehaviour(): void + { + $dom = html5qp('

foo

', 'p'); + + self::assertFalse($dom->is('span')); + self::assertCount(1, $dom->branch()->has('span')); + self::assertCount(0, $dom->branch()->has('em')); + } + + /** + * parents() filters with is(), so it inherited the descendant-matching bug: every ancestor + * that merely *contained* a matching element was returned. + */ + public function testParentsNoLongerMatchesAncestorsThatOnlyContainTheSelector(): void + { + $dom = html5qp('

foo

'); + + self::assertCount(1, $dom->top('#target')->parents('div')); + self::assertSame('outer', $dom->top('#target')->parents('div')->attr('id')); + } +} From f0cb2524dd060f71bac7857bb9b797128e5684b9 Mon Sep 17 00:00:00 2001 From: Jake Jackson Date: Fri, 21 Aug 2026 14:51:59 +1000 Subject: [PATCH 2/2] Extract the single-node selector match into QueryPath\Helpers\NodeMatcher is() built its DOMTraverser inline. The selector-filtered traversal methods in QueryFilters need the same "is this node a match?" test, so the logic is moved into a small helper both can share rather than being written twice and left to drift apart. NodeMatcher deliberately leaves the traverser's scope node at its default, so :scope resolves against the document element exactly as it does in find(). Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 1 - src/Helpers/NodeMatcher.php | 99 +++++++++++++++++++++++++++++++++++++ src/Helpers/QueryChecks.php | 24 ++------- 3 files changed, 102 insertions(+), 22 deletions(-) create mode 100644 src/Helpers/NodeMatcher.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 75e72d1..e274e6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,6 @@ QueryPath Changelog - **Breaking behaviour change:** `DOMQuery::is()` now behaves like jQuery's `.is()`. It tests the elements held in the current match set and returns `true` when at least one of them matches the selector. Previously it ran a descendant search, so `html5qp('

foo

', 'p')->is('span')` returned `true`. Use `has()` if you need the old "does the set contain something matching this selector" behaviour (#51) - `DOMQuery::is()` no longer raises a fatal error when the match set contains non-element nodes (text nodes, comments, processing instructions). Those nodes cannot match a CSS selector, so they are skipped -- Fix `parents()`, `parentsUntil()`, `next()`, `nextAll()`, `nextUntil()`, `prev()`, `prevAll()`, `prevUntil()`, `closest()`, and `not()`, which filter with `is()` and therefore also matched elements that merely *contained* the selector. For example, `parents('div')` returned every ancestor that contained a `div`, not just the ancestors that are a `div` - Reorganise, modernise, and repair the `examples/` directory. Each example now lives in its own subdirectory with an `index.php`, and the full set is indexed in `examples/quickstart-guide.md` - Convert the remaining legacy examples: `simple_example.php`, `techniques.php`, `svg.php`, `rss.php`, `odt.php`, `parse_php.php`, and `sparql.php` - Fix examples that no longer ran: send a `User-Agent` where remote hosts now require one, resolve paths relative to the example rather than the working directory, and stop relying on the removed `qp.php` autoloader and the PHP 8 incompatible `eachLambda()` diff --git a/src/Helpers/NodeMatcher.php b/src/Helpers/NodeMatcher.php new file mode 100644 index 0000000..e66996c --- /dev/null +++ b/src/Helpers/NodeMatcher.php @@ -0,0 +1,99 @@ +descendants of the nodes it is given, so for `` it would report + * that the `a` element "matches" the selector `b`. Here the supplied nodes are + * themselves the only candidates, so `b` matches the `b` element and nothing else. + * + * The DOMTraverser is built in "initialized" mode, which tells it to treat the + * supplied set as the candidate set rather than seeding it with a descendant + * search. Combinators are still resolved against the real document by walking up + * from each candidate, so full selectors (e.g. `div > p`) keep working. + * + * The scope node is deliberately left at its default (the document element) so that + * `:scope` means the same thing here as it does in find(). Passing a candidate as the + * scope node would make every candidate match `:scope`. + */ +final class NodeMatcher +{ + + /** + * Reduce a set of nodes to those that match a selector. + * + * Nodes that are not elements can never match a CSS selector, and are skipped + * rather than raising an error. + * + * @param SplObjectStorage $nodes + * The candidate nodes. + * @param string $selector + * A valid CSS selector. + * + * @return SplObjectStorage + * The subset of $nodes that match the selector. + * @throws ParseException + */ + public static function filter(SplObjectStorage $nodes, $selector): SplObjectStorage + { + $candidates = new SplObjectStorage(); + foreach ($nodes as $node) { + if ($node instanceof DOMElement) { + $candidates->offsetSet($node); + } + } + + if (count($candidates) === 0) { + return $candidates; + } + + $traverser = new DOMTraverser($candidates, true); + $traverser->find($selector); + + return $traverser->matches(); + } + + /** + * Test whether at least one of the given nodes matches a selector. + * + * @param SplObjectStorage $nodes + * @param string $selector + * + * @return bool + * @throws ParseException + */ + public static function matchesAny(SplObjectStorage $nodes, $selector): bool + { + return count(self::filter($nodes, $selector)) > 0; + } + + /** + * Test whether a single node, taken as an element, matches a selector. + * + * @param mixed $node + * The node to test. Anything that is not an element returns FALSE. + * @param string $selector + * + * @return bool + * @throws ParseException + */ + public static function matchesNode($node, $selector): bool + { + if (! $node instanceof DOMElement) { + return false; + } + + $nodes = new SplObjectStorage(); + $nodes->offsetSet($node); + + return self::matchesAny($nodes, $selector); + } +} diff --git a/src/Helpers/QueryChecks.php b/src/Helpers/QueryChecks.php index cd298da..68a5232 100644 --- a/src/Helpers/QueryChecks.php +++ b/src/Helpers/QueryChecks.php @@ -2,9 +2,7 @@ namespace QueryPath\Helpers; -use DOMElement; use DOMNode; -use QueryPath\CSS\DOMTraverser; use QueryPath\CSS\ParseException; use QueryPath\DOMQuery; use QueryPath\Exception; @@ -78,25 +76,9 @@ public function is($selector): bool throw new Exception('Cannot compare an object to a DOMQuery.'); } - // Only elements can be matched against a CSS selector, so anything else in the - // match set is discarded before the selector is evaluated. - $candidates = new SplObjectStorage(); - foreach ($this->matches as $match) { - if ($match instanceof DOMElement) { - $candidates->offsetSet($match); - } - } - - if (count($candidates) === 0) { - return false; - } - - // The second argument tells the traverser that the match set is already the set of - // candidates, so the selector filters those elements rather than searching below them. - $traverser = new DOMTraverser($candidates, true); - $traverser->find($selector); - - return count($traverser->matches()) > 0; + // Only the elements in the match set are candidates: a descendant that matches + // the selector must not make is() true. See NodeMatcher for the details. + return NodeMatcher::matchesAny($this->matches, $selector); } /**