From 657ab6f119cc77448729096fdfaa4c79a064dbd4 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 17:00:57 +1100 Subject: [PATCH 1/5] Count a positional compound left of a combinator across the document in filter() In filterNodes(), a non-subject compound carrying a positional pseudo-class was resolved by searching the candidates' descendants, so filter('ul:first > li') and is('ul:first > li') matched nothing. It is now resolved from the document element, as jQuery's .filter() does. Fixes #132. Co-Authored-By: Claude Opus 5.5 --- src/CSS/DOMTraverser.php | 27 ++++++++++++++-- tests/Issues/Issue132Test.php | 61 +++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 tests/Issues/Issue132Test.php diff --git a/src/CSS/DOMTraverser.php b/src/CSS/DOMTraverser.php index 7f443c9..bed6495 100644 --- a/src/CSS/DOMTraverser.php +++ b/src/CSS/DOMTraverser.php @@ -173,8 +173,9 @@ public function find($selector): DOMTraverser * * Unlike find(), which searches the descendants of the nodes it starts from, only the given * nodes can match: for ``, `b` matches the `b` element and not `a`. Combinators are - * still resolved against the document, so `div > p` works, and positional pseudo-classes - * (`:first`, `:eq(n)`, ...) count within the given set. Nodes that are not elements never match. + * still resolved against the document, so `div > p` works. Positional pseudo-classes (`:first`, + * `:eq(n)`, ...) on the subject count within the given set, and on a compound left of a + * combinator across the document, as in jQuery's .filter(). Nodes that are not elements never match. * * @param SplObjectStorage $nodes * The candidates, all from one document. @@ -336,8 +337,13 @@ protected function resolveSubject(array $selectorGroup, int $index): SplObjectSt } else { // initialMatch() consumes the parts of the simple selector it matched on. Give it a copy, // so the caller's parsed selector can be reused. + // When the given nodes are tested as themselves, a positional compound left of a combinator + // counts across the document, as jQuery's .filter() does. $selectorGroup[$index] = clone $selectorGroup[$index]; - $candidates = $this->initialMatch($selectorGroup[$index], $this->matches); + $candidates = $this->initialMatch( + $selectorGroup[$index], + $this->initialized ? $this->documentScope() : $this->matches + ); } $setFilters = $selectorGroup[$index]->setFilterPseudoClasses(); @@ -377,6 +383,21 @@ protected function resolveSubject(array $selectorGroup, int $index): SplObjectSt return $this->toMatches($matched); } + /** + * The document element, standing in for the document as it does in qp($document). + * + * @return SplObjectStorage + */ + protected function documentScope(): SplObjectStorage + { + $scope = $this->newMatches(); + if ($this->dom instanceof DOMDocument && $this->dom->documentElement) { + $scope->offsetSet($this->dom->documentElement); + } + + return $scope; + } + /** * Apply a single set-level pseudo-class to an ordered list of nodes. * diff --git a/tests/Issues/Issue132Test.php b/tests/Issues/Issue132Test.php new file mode 100644 index 0000000..c915774 --- /dev/null +++ b/tests/Issues/Issue132Test.php @@ -0,0 +1,61 @@ +
'; + + /** + * @testWith ["ul:first > li", "a,b"] + * ["ul:eq(1) > li", "c"] + * ["ul:last li", "c"] + * ["ul:eq(0) + ul > li", "c"] + * ["ul:gt(0) > li:first", "c"] + * ["ul:first > li:last", "b"] + * ["ul:first > li, ul:last > li", "a,b,c"] + */ + public function testFilterCountsALeftHandPositionalCompoundAcrossTheDocument($selector, $expected) + { + $this->assertSame($expected, $this->idList(qp(self::XML, 'li')->filter($selector))); + } + + public function testTheIssueExamples() + { + $this->assertSame('a,b', $this->idList(qp(self::XML)->find('ul:first > li'))); + $this->assertSame('a,b', $this->idList(qp(self::XML, 'li')->filter('ul:first > li'))); + $this->assertTrue(qp(self::XML, 'li')->is('ul:first > li')); + $this->assertSame('c', $this->idList(qp(self::XML, 'li')->filter('ul:eq(1) > li'))); + } + + public function testTheCompoundIsNotLimitedToTheNodesInHand() + { + // u1 is not in the set, but is still the document's first
    . + $this->assertSame('b', $this->idList(qp(self::XML, '#b')->filter('ul:first > li'))); + $this->assertSame('', $this->idList(qp(self::XML, '#c')->filter('ul:first > li'))); + $this->assertSame('c', $this->idList(qp(self::XML, 'li')->not('ul:first > li'))); + } + + public function testFilterNodes() + { + $nodes = new SplObjectStorage(); + foreach (qp(self::XML, 'li')->get() as $node) { + $nodes->offsetSet($node); + } + + $this->assertCount(2, DOMTraverser::filterNodes($nodes, 'ul:first > li')); + $this->assertCount(1, DOMTraverser::filterNodes($nodes, 'ul:eq(1) > li')); + } +} From 21efb93f758d754615cc51380e07f768a81d8421 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 17:18:22 +1100 Subject: [PATCH 2/5] Add a CHANGELOG entry Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 527a87d..2960a68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ QueryPath Changelog - Consolidate the sibling and ancestor walks behind `next()`, `prev()`, `nextAll()`, `prevAll()`, `nextUntil()`, `prevUntil()`, `parent()`, `parents()`, `parentsUntil()` and `closest()` into one helper, so a fix to one no longer has to be repeated in its mirror (issue #115). As a side effect, `parentsUntil()` no longer loops forever on an element that is not attached to the document, and `nextUntil('')` and `prevUntil('')` treat an empty selector as no selector, as the other methods already did, instead of throwing `QueryPath\CSS\ParseException`. `closest('')` returns an empty set, as in jQuery. `nextUntil()`, `prevUntil()` and `parentsUntil()` also stop at an element already collected from an earlier starting element, as `nextAll()` and `prevAll()` do, so `nextUntil()` on each of 2000 siblings falls from about 100 ms to 1 ms - **Behaviour change.** `prevAll()`, `prevUntil()`, `nextAll()`, `nextUntil()`, `siblings()`, `parent()` and `closest()` now return the results for a multi-element set in jQuery's order, instead of grouped by starting element (issue #117). `prevAll()` and `prevUntil()` return reverse document order, as `parents()` and `parentsUntil()` already did, and the others return document order. `qp($xml, 'c')->add('d')->prevAll()` now returns `c, b, a` rather than `b, a, c`. `next()` and `prev()` keep the order of the starting elements, as in jQuery, and the results for a single starting element are unchanged - **Behaviour change.** A positional pseudo-class (`:first`, `:eq()`, `:last`, ...) in the stop selector of `nextUntil()`, `prevUntil()` and `parentsUntil()` now counts across the whole document, as jQuery's does, instead of seeing each sibling or ancestor as a one-element set (issue #118). `nextUntil(':first')` used to stop at once and now walks to the end, and `nextUntil(':eq(3)')` stops at the fourth element in the document. `closest()` no longer matches a positional selector at all, as in jQuery, where `closest(':first')` used to return the element itself. `next()`, `prev()` and `parent()` still test each candidate on its own, which agrees with jQuery for a single starting element +- **Behaviour change.** Fix `filter()`, `not()` and `is()` matching nothing when a positional pseudo-class sits on a compound left of a combinator, as in `ul:first > li` (issue #132). That compound now counts across the whole document, as in jQuery's `.filter()` and in `find()`, so `qp($xml, 'li')->filter('ul:first > li')` returns the items of the document's first `
      `. A positional pseudo-class on the subject still counts within the set. `QueryPath\CSS\DOMTraverser::filterNodes()` follows suit # 4.2.0 From 022b5ed76fcca353938cc265689e50dbe6a2ca7c Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 17:19:23 +1100 Subject: [PATCH 3/5] Share the document scope with NodeMatcher Co-Authored-By: Claude Opus 5.5 --- src/CSS/DOMTraverser.php | 23 ++++++++++++----------- src/Helpers/NodeMatcher.php | 10 +++------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/CSS/DOMTraverser.php b/src/CSS/DOMTraverser.php index bed6495..caa9e62 100644 --- a/src/CSS/DOMTraverser.php +++ b/src/CSS/DOMTraverser.php @@ -173,9 +173,9 @@ public function find($selector): DOMTraverser * * Unlike find(), which searches the descendants of the nodes it starts from, only the given * nodes can match: for ``, `b` matches the `b` element and not `a`. Combinators are - * still resolved against the document, so `div > p` works. Positional pseudo-classes (`:first`, - * `:eq(n)`, ...) on the subject count within the given set, and on a compound left of a - * combinator across the document, as in jQuery's .filter(). Nodes that are not elements never match. + * still resolved against the document, so `div > p` works. As in jQuery's .filter(), a positional + * pseudo-class (`:first`, `:eq(n)`, ...) counts within the given set when it is on the subject, and + * across the document when it is left of a combinator. Nodes that are not elements never match. * * @param SplObjectStorage $nodes * The candidates, all from one document. @@ -336,13 +336,12 @@ protected function resolveSubject(array $selectorGroup, int $index): SplObjectSt }); } else { // initialMatch() consumes the parts of the simple selector it matched on. Give it a copy, - // so the caller's parsed selector can be reused. - // When the given nodes are tested as themselves, a positional compound left of a combinator - // counts across the document, as jQuery's .filter() does. + // so the caller's parsed selector can be reused. In filterNodes(), a compound left of a + // combinator is looked up across the document, so its positional filters count there. $selectorGroup[$index] = clone $selectorGroup[$index]; $candidates = $this->initialMatch( $selectorGroup[$index], - $this->initialized ? $this->documentScope() : $this->matches + $this->initialized ? self::documentScope($this->dom) : $this->matches ); } @@ -386,13 +385,15 @@ protected function resolveSubject(array $selectorGroup, int $index): SplObjectSt /** * The document element, standing in for the document as it does in qp($document). * + * @param DOMDocument|null $document + * * @return SplObjectStorage */ - protected function documentScope(): SplObjectStorage + public static function documentScope($document): SplObjectStorage { - $scope = $this->newMatches(); - if ($this->dom instanceof DOMDocument && $this->dom->documentElement) { - $scope->offsetSet($this->dom->documentElement); + $scope = new SplObjectStorage(); + if ($document instanceof DOMDocument && $document->documentElement) { + $scope->offsetSet($document->documentElement); } return $scope; diff --git a/src/Helpers/NodeMatcher.php b/src/Helpers/NodeMatcher.php index 8df96fd..d117795 100644 --- a/src/Helpers/NodeMatcher.php +++ b/src/Helpers/NodeMatcher.php @@ -115,13 +115,9 @@ public function matchesInDocument($node): bool */ private function documentMatches($document): SplObjectStorage { - // As in qp($document), the document element stands in for the document, so it can match. - $scope = new SplObjectStorage(); - if ($document->documentElement) { - $scope->offsetSet($document->documentElement); - } - - return (new DOMTraverser($scope, false, null, $this->psHandler))->find($this->selector)->matches(); + return (new DOMTraverser(DOMTraverser::documentScope($document), false, null, $this->psHandler)) + ->find($this->selector) + ->matches(); } /** From 96ebf9509c6fbfa42745e3a3e0876a1938308a3b Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 17:25:57 +1100 Subject: [PATCH 4/5] Drop assertions the data provider already covers Co-Authored-By: Claude Opus 5.5 --- tests/Issues/Issue132Test.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/Issues/Issue132Test.php b/tests/Issues/Issue132Test.php index c915774..02b8299 100644 --- a/tests/Issues/Issue132Test.php +++ b/tests/Issues/Issue132Test.php @@ -32,12 +32,10 @@ public function testFilterCountsALeftHandPositionalCompoundAcrossTheDocument($se $this->assertSame($expected, $this->idList(qp(self::XML, 'li')->filter($selector))); } - public function testTheIssueExamples() + public function testFilterAgreesWithFindAndIs() { $this->assertSame('a,b', $this->idList(qp(self::XML)->find('ul:first > li'))); - $this->assertSame('a,b', $this->idList(qp(self::XML, 'li')->filter('ul:first > li'))); $this->assertTrue(qp(self::XML, 'li')->is('ul:first > li')); - $this->assertSame('c', $this->idList(qp(self::XML, 'li')->filter('ul:eq(1) > li'))); } public function testTheCompoundIsNotLimitedToTheNodesInHand() From ef8723b6dc34223f3420253d2832c9aac5a9f96e Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 17:26:05 +1100 Subject: [PATCH 5/5] Rename a test after trimming it Co-Authored-By: Claude Opus 5.5 --- tests/Issues/Issue132Test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Issues/Issue132Test.php b/tests/Issues/Issue132Test.php index 02b8299..799fd21 100644 --- a/tests/Issues/Issue132Test.php +++ b/tests/Issues/Issue132Test.php @@ -32,7 +32,7 @@ public function testFilterCountsALeftHandPositionalCompoundAcrossTheDocument($se $this->assertSame($expected, $this->idList(qp(self::XML, 'li')->filter($selector))); } - public function testFilterAgreesWithFindAndIs() + public function testIsMatchesWhatFindSelects() { $this->assertSame('a,b', $this->idList(qp(self::XML)->find('ul:first > li'))); $this->assertTrue(qp(self::XML, 'li')->is('ul:first > li'));