From 4cdb9fc35804b7a6e204bc1476af82cdaa280cbe Mon Sep 17 00:00:00 2001 From: Jake Jackson Date: Sat, 22 Aug 2026 04:09:26 +1000 Subject: [PATCH 1/2] Filter the whole match set at once in filter() and children() filter() kept any element that *contained* a match, because it ran a descendant search per node. jQuery's filter() narrows the set to the members that match, so qp($file, 'inner')->filter('li') returned both elements where jQuery returns none. has() already provides the old behaviour. The original author left a comment on the line saying the correct traverser mode "fails unit tests". It fails exactly one, DOMQueryTest::testFilter, which asserts the containment result; that test is rebaselined here. Both methods now filter their candidates as a single set rather than one node at a time. A per-node pass cannot evaluate a selector that describes a position within the set, because each node is the only member of its own one-element set. This also drops the per-node scope node, so children(':scope') no longer matches every child. Co-Authored-By: Claude Opus 5 (1M context) --- src/Helpers/QueryFilters.php | 54 ++++++++++++++------------------ tests/QueryPath/DOMQueryTest.php | 51 ++++++++++++++++++++++++++++-- 2 files changed, 72 insertions(+), 33 deletions(-) diff --git a/src/Helpers/QueryFilters.php b/src/Helpers/QueryFilters.php index 0fa04a8..b2c1f77 100644 --- a/src/Helpers/QueryFilters.php +++ b/src/Helpers/QueryFilters.php @@ -4,7 +4,6 @@ use DOMElement; use DOMNode; -use QueryPath\CSS\DOMTraverser; use QueryPath\CSS\ParseException; use QueryPath\DOMQuery; use QueryPath\Exception; @@ -41,20 +40,18 @@ trait QueryFilters */ public function filter($selector): Query { - $found = new SplObjectStorage(); - $tmp = new SplObjectStorage(); + // The whole match set is filtered in one pass rather than one node at a time. + // A per-node pass cannot evaluate a selector that describes a position within the + // set (:first, :eq(n), :odd, ...), because each node would be the only member of + // its own one-element set. + $matched = NodeMatcher::filter($this->matches, $selector); + // Rebuild the set by walking the original, so the caller's ordering is preserved. + $found = new SplObjectStorage(); foreach ($this->matches as $m) { - $tmp->offsetSet($m); - // Seems like this should be right... but it fails unit - // tests. Need to compare to jQuery. - // $query = new \QueryPath\CSS\DOMTraverser($tmp, TRUE, $m); - $query = new DOMTraverser($tmp); - $query->find($selector); - if (count($query->matches())) { + if ($matched->offsetExists($m)) { $found->offsetSet($m); } - $tmp->offsetUnset($m); } return $this->inst($found, null); @@ -1051,32 +1048,29 @@ public function prevAll($selector = null): Query */ public function children($selector = null): Query { - $found = new SplObjectStorage(); - $filter = is_string($selector) && strlen($selector) > 0; - - if ($filter) { - $tmp = new SplObjectStorage(); - } + $children = new SplObjectStorage(); foreach ($this->matches as $m) { foreach ($m->childNodes as $c) { if ($c->nodeType === XML_ELEMENT_NODE) { - // This is basically an optimized filter() just for children(). - if ($filter) { - $tmp->offsetSet($c); - $query = new DOMTraverser($tmp, true, $c); - $query->find($selector); - if (count($query->matches()) > 0) { - $found->offsetSet($c); - } - $tmp->offsetUnset($c); - } // No filter. Just attach it. - else { - $found->offsetSet($c); - } + $children->offsetSet($c); } } } + if (! is_string($selector) || strlen($selector) === 0) { + return $this->inst($children, null); + } + + // Filter the children as one set, for the same reason filter() does. + $matched = NodeMatcher::filter($children, $selector); + + $found = new SplObjectStorage(); + foreach ($children as $c) { + if ($matched->offsetExists($c)) { + $found->offsetSet($c); + } + } + return $this->inst($found, null); } diff --git a/tests/QueryPath/DOMQueryTest.php b/tests/QueryPath/DOMQueryTest.php index 4e4027f..921247c 100644 --- a/tests/QueryPath/DOMQueryTest.php +++ b/tests/QueryPath/DOMQueryTest.php @@ -560,9 +560,54 @@ public function testIndex() public function testFilter() { $file = DATA_FILE; - $this->assertEquals(1, qp($file)->filter('li')->count()); - $this->assertEquals(2, qp($file, 'inner')->filter('li')->count()); - $this->assertEquals('inner-two', qp($file, 'inner')->filter('li')->eq(1)->attr('id')); + + // filter() narrows the current match set to the members that match the selector. + // It does not search their descendants: and are not
  • elements, + // so an 'li' filter removes them both. + $this->assertEquals(0, qp($file)->filter('li')->count()); + $this->assertEquals(0, qp($file, 'inner')->filter('li')->count()); + + $this->assertEquals(2, qp($file, 'inner')->filter('inner')->count()); + $this->assertEquals(5, qp($file, 'li')->filter('li')->count()); + + $this->assertEquals(1, qp($file, 'inner')->filter('#inner-two')->count()); + $this->assertEquals('inner-two', qp($file, 'inner')->filter('#inner-two')->attr('id')); + + // The original set's ordering is preserved. + $this->assertEquals('inner-one', qp($file, 'inner')->filter('inner')->eq(0)->attr('id')); + $this->assertEquals('inner-two', qp($file, 'inner')->filter('inner')->eq(1)->attr('id')); + } + + /** + * filter() used to keep any element that *contained* a match, which is what has() + * does. This pins the difference, and documents the migration path for anyone who + * was relying on the old behaviour. + */ + public function testFilterMatchesTheSetRatherThanItsDescendants() + { + $file = DATA_FILE; + + $this->assertEquals(0, qp($file, 'inner')->filter('li')->count()); + $this->assertEquals(2, qp($file, 'inner')->has('li')->count()); + + $this->assertEquals(0, qp($file)->filter('li')->count()); + $this->assertEquals(1, qp($file)->has('li')->count()); + } + + /** + * A selector that describes a position within the match set has to be evaluated + * against the whole set, not against each member in turn. + */ + public function testFilterEvaluatesTheSelectorAgainstTheWholeSet() + { + $file = DATA_FILE; + + $this->assertEquals(5, qp($file, 'li')->filter('li')->count()); + $this->assertEquals('one', qp($file, 'li')->filter('#one')->attr('id')); + + // :scope refers to the document element, exactly as it does in find(). + $this->assertEquals(0, qp($file, 'inner')->filter(':scope')->count()); + $this->assertEquals(1, qp($file, 'root')->filter(':scope')->count()); } public function testFilterPreg() From aa8546965c02f13dee731deb59502a84e73b0d93 Mon Sep 17 00:00:00 2001 From: Jake Jackson Date: Sat, 22 Aug 2026 04:09:59 +1000 Subject: [PATCH 2/2] Add CHANGELOG entries for the filter() and children() fixes Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a42a1ad..2e3f0fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ QueryPath Changelog - Fix `parents($selector)` so the selector filters the ancestors themselves, instead of matching any ancestor that merely *contains* an element matching the selector. `qp($xml, 'Demographics > Age > Name')->parents('Demographics')` now returns only ``, matching jQuery (#62) - Apply the same fix to the other selector-filtered traversal methods in `QueryPath\Helpers\QueryFilters`: `parent()`, `parents()`, `parentsUntil()`, `closest()`, `next()`, `nextAll()`, `nextUntil()`, `prev()`, `prevAll()`, `prevUntil()`, `siblings()`, and `not()` (#62) - **Behaviour change:** `parents()` and `parentsUntil()` now return their results in reverse document order with duplicates removed, as jQuery does. Previously a set built from more than one starting element was grouped by starting element (#62) +- **Breaking behaviour change:** `DOMQuery::filter()` now narrows the match set to the members that match the selector, as jQuery does, instead of keeping any element that merely *contains* a match. `qp($file, 'inner')->filter('li')` returned both `` elements and now returns none. Use `has()` for the old behaviour +- Fix `children($selector)`, which resolved `:scope` against each child rather than the document element, so `children(':scope')` matched every child +- `filter()` and `children($selector)` now evaluate the selector against the whole candidate set in a single pass, rather than one node at a time. A per-node pass cannot evaluate a selector describing a position within the set, since each node is the only member of its own one-element set - 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()`