From 7f8e1d5e047e4fa171c9c152d9a20bf6b5a582fc Mon Sep 17 00:00:00 2001 From: Jake Jackson Date: Sat, 22 Aug 2026 04:32:59 +1000 Subject: [PATCH 1/3] Make find() search descendants only, as jQuery does find() treated the nodes already in the match set as candidates for their own selector, so qp($xml, '#a1')->find('a') returned #a1 itself alongside any nested . jQuery's .find() searches descendants; filter() is what asks whether the nodes in hand match. The document element keeps its self-match. qp() seeds the match set with the document element rather than with the document node, so it stands in for $(document), and without the exception qp($xml)->find('root') could never match. This depends on #72. Before it, the selector-filtered traversal methods reach is(), which reached find(), so they relied on the self-match to test a node against a selector; changing find() on its own breaks nine of them. Once those go through NodeMatcher they no longer care. testBefore searched for the node it was standing on. Rewritten to search from top(), which is the shape the next two assertions in that test already use. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 1 + src/CSS/DOMTraverser.php | 9 ++-- tests/QueryPath/DOMQueryTest.php | 4 +- tests/QueryPath/FindDescendantOnlyTest.php | 63 ++++++++++++++++++++++ 4 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 tests/QueryPath/FindDescendantOnlyTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e3f0fa..b5e6f5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ QueryPath Changelog - 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 +- **Breaking behaviour change:** `find()` now searches descendants only, as jQuery's `.find()` does. A node already in the match set is no longer a candidate for its own selector, so `qp($xml, '#a1')->find('a')` no longer returns `#a1` itself. The document element remains matchable, because `qp()` seeds the match set with it rather than with the document. Use `filter()` to match the nodes in hand - 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` diff --git a/src/CSS/DOMTraverser.php b/src/CSS/DOMTraverser.php index 8b8483f..9277574 100644 --- a/src/CSS/DOMTraverser.php +++ b/src/CSS/DOMTraverser.php @@ -587,9 +587,12 @@ protected function initialMatchOnElement(SimpleSelector $selector, SplObjectStor $found = $this->newMatches(); /** @var DOMDocument $node */ foreach ($matches as $node) { - // Capture the case where the initial element is the root element. - if ($node->tagName === $element - || ($element === '*' && $node->parentNode instanceof DOMDocument)) { + // jQuery's find() searches descendants only, so a node in the match set is not + // a candidate for its own selector. The document element is the exception: + // QueryPath seeds the match set with it rather than with the document, so + // without this qp($xml)->find('root') could never match. + if ($node->parentNode instanceof DOMDocument + && ($element === '*' || $node->tagName === $element)) { $found->offsetSet($node); } $nl = $node->getElementsByTagName($element); diff --git a/tests/QueryPath/DOMQueryTest.php b/tests/QueryPath/DOMQueryTest.php index 921247c..ad98eef 100644 --- a/tests/QueryPath/DOMQueryTest.php +++ b/tests/QueryPath/DOMQueryTest.php @@ -918,7 +918,9 @@ public function testPrependTo() public function testBefore() { $file = DATA_FILE; - $this->assertEquals(1, qp($file, 'unary')->before('')->find(':root > test ~ unary')->count()); + // find() searches descendants, so the search starts from the top of the document — + // the same shape as the top() calls below. + $this->assertEquals(1, qp($file, 'unary')->before('')->top()->find(':root > test ~ unary')->count()); $this->assertEquals(1, qp($file, 'unary')->before('')->top('head ~ test')->count()); $this->assertEquals( 'unary', diff --git a/tests/QueryPath/FindDescendantOnlyTest.php b/tests/QueryPath/FindDescendantOnlyTest.php new file mode 100644 index 0000000..78f842f --- /dev/null +++ b/tests/QueryPath/FindDescendantOnlyTest.php @@ -0,0 +1,63 @@ +'; + + public function testFindDoesNotMatchTheNodesItStartsFrom(): void + { + $a1 = qp(self::XML, '#a1'); + $this->assertSame('a1', $a1->attr('id')); + + // is in the match set, so it is not a candidate: only the nested + // is found. + $found = $a1->find('a'); + $this->assertCount(1, $found); + $this->assertSame('a2', $found->attr('id')); + } + + public function testWildcardDoesNotMatchTheNodesItStartsFrom(): void + { + $ids = []; + foreach (qp(self::XML, '#a1')->find('*') as $node) { + $ids[] = $node->attr('id'); + } + + $this->assertSame(['a2', 'b1'], $ids); + } + + public function testTheDocumentElementIsStillReachable(): void + { + // qp() seeds the match set with the document element, which stands in for the + // document, so a selector naming it has to keep working. + $this->assertCount(1, qp(self::XML)->find('root')); + $this->assertSame('root', qp(self::XML)->find('root')->tag()); + + $this->assertCount(1, qp(self::XML)->find(':root')); + } + + public function testDescendantsOfTheDocumentElementAreStillFound(): void + { + $this->assertCount(2, qp(self::XML)->find('a')); + $this->assertCount(1, qp(self::XML)->find('b')); + } + + /** + * filter() is the jQuery-equivalent way to ask whether the nodes in hand match. + */ + public function testFilterIsTheWayToMatchTheNodesInHand(): void + { + $this->assertCount(1, qp(self::XML, '#a1')->filter('a')); + $this->assertSame('a1', qp(self::XML, '#a1')->filter('a')->attr('id')); + } +} From 333f6be2316dfde6a46e373e52b9cee4a335edec Mon Sep 17 00:00:00 2001 From: Jake Jackson Date: Sat, 22 Aug 2026 04:38:03 +1000 Subject: [PATCH 2/3] Make the ID and class initial matchers descendant-only too Only the element matcher was changed, so find('a') stopped self-matching while find('#a1') and find('.c') still did. Three initial matchers, three separate self-tests, and the selector you happened to write decided the semantics. All three now apply the same rule, with the same document-element exception. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 2 +- src/CSS/DOMTraverser.php | 8 ++++++-- tests/QueryPath/FindDescendantOnlyTest.php | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5e6f5c..128ecfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ QueryPath Changelog - 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 -- **Breaking behaviour change:** `find()` now searches descendants only, as jQuery's `.find()` does. A node already in the match set is no longer a candidate for its own selector, so `qp($xml, '#a1')->find('a')` no longer returns `#a1` itself. The document element remains matchable, because `qp()` seeds the match set with it rather than with the document. Use `filter()` to match the nodes in hand +- **Breaking behaviour change:** `find()` now searches descendants only, as jQuery's `.find()` does. A node already in the match set is no longer a candidate for its own selector, so `qp($xml, '#a1')->find('a')`, `find('#a1')` and `find('.c')` no longer return `#a1` itself. The document element remains matchable, because `qp()` seeds the match set with it rather than with the document. Use `filter()` to match the nodes in hand - 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` diff --git a/src/CSS/DOMTraverser.php b/src/CSS/DOMTraverser.php index 9277574..99d457c 100644 --- a/src/CSS/DOMTraverser.php +++ b/src/CSS/DOMTraverser.php @@ -476,7 +476,9 @@ protected function initialMatchOnID(SimpleSelector $selector, SplObjectStorage $ // Now we try to find any matching IDs. /** @var DOMElement $node */ foreach ($matches as $node) { - if ($node->getAttribute('id') === $id) { + // Descendants only, as in find(). The document element is the exception, + // because the match set is seeded with it rather than with the document. + if ($node->parentNode instanceof DOMDocument && $node->getAttribute('id') === $id) { $found->offsetSet($node); } @@ -521,7 +523,9 @@ protected function initialMatchOnClasses(SimpleSelector $selector, SplObjectStor /** @var DOMElement $node */ foreach ($matches as $node) { // Refactor me! - if ($node->hasAttribute('class')) { + // Descendants only, as in find(). The document element is the exception, + // because the match set is seeded with it rather than with the document. + if ($node->parentNode instanceof DOMDocument && $node->hasAttribute('class')) { $intersect = array_intersect($selector->classes, explode(' ', $node->getAttribute('class'))); if (count($intersect) === count($selector->classes)) { $found->offsetSet($node); diff --git a/tests/QueryPath/FindDescendantOnlyTest.php b/tests/QueryPath/FindDescendantOnlyTest.php index 78f842f..b485c29 100644 --- a/tests/QueryPath/FindDescendantOnlyTest.php +++ b/tests/QueryPath/FindDescendantOnlyTest.php @@ -60,4 +60,22 @@ public function testFilterIsTheWayToMatchTheNodesInHand(): void $this->assertCount(1, qp(self::XML, '#a1')->filter('a')); $this->assertSame('a1', qp(self::XML, '#a1')->filter('a')->attr('id')); } + + /** + * The element, ID and class initial matchers each have their own self-test, so all three + * have to agree — otherwise find('#a1') would match a node that find('a') does not. + */ + public function testIdAndClassSelectorsAreDescendantOnlyToo(): void + { + $xml = ''; + + $this->assertCount(0, qp($xml, '#a1')->find('#a1')); + + $found = qp($xml, '#a1')->find('.c'); + $this->assertCount(1, $found); + $this->assertSame('a2', $found->attr('id')); + + // The document element keeps its exception here as well. + $this->assertCount(1, qp($xml)->find('#a1')); + } } From 2254927d5c77c8aa701d25ec06ede99d03b67448 Mon Sep 17 00:00:00 2001 From: Jake Jackson Date: Sat, 22 Aug 2026 04:50:37 +1000 Subject: [PATCH 3/3] Give the document-element exception one home MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule and its four-line explanation were pasted into all three initial matchers. They have to agree — find('#a1') matching a node that find('a') does not is the divergence this PR exists to remove — so they should not be three independent copies. Co-Authored-By: Claude Opus 5 (1M context) --- src/CSS/DOMTraverser.php | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/src/CSS/DOMTraverser.php b/src/CSS/DOMTraverser.php index 99d457c..33cc591 100644 --- a/src/CSS/DOMTraverser.php +++ b/src/CSS/DOMTraverser.php @@ -476,9 +476,7 @@ protected function initialMatchOnID(SimpleSelector $selector, SplObjectStorage $ // Now we try to find any matching IDs. /** @var DOMElement $node */ foreach ($matches as $node) { - // Descendants only, as in find(). The document element is the exception, - // because the match set is seeded with it rather than with the document. - if ($node->parentNode instanceof DOMDocument && $node->getAttribute('id') === $id) { + if ($this->isDocumentElement($node) && $node->getAttribute('id') === $id) { $found->offsetSet($node); } @@ -523,9 +521,7 @@ protected function initialMatchOnClasses(SimpleSelector $selector, SplObjectStor /** @var DOMElement $node */ foreach ($matches as $node) { // Refactor me! - // Descendants only, as in find(). The document element is the exception, - // because the match set is seeded with it rather than with the document. - if ($node->parentNode instanceof DOMDocument && $node->hasAttribute('class')) { + if ($this->isDocumentElement($node) && $node->hasAttribute('class')) { $intersect = array_intersect($selector->classes, explode(' ', $node->getAttribute('class'))); if (count($intersect) === count($selector->classes)) { $found->offsetSet($node); @@ -582,6 +578,26 @@ private function initialXpathQuery(DOMXPath $xpath, DOMElement $node, string $qu * * @return SplObjectStorage */ + /** + * Is this node the element the match set was seeded with? + * + * jQuery's find() searches descendants only, so a node already in the match set is not + * a candidate for its own selector. The document element is the one exception: QueryPath + * seeds the match set with it rather than with the document, so it stands in for the + * document and qp($xml)->find('root') has to keep matching. + * + * All three initial matchers ask this, and they have to agree — otherwise find('#a1') + * would match a node that find('a') does not. + * + * @param DOMNode $node + * + * @return bool + */ + private function isDocumentElement($node): bool + { + return $node->parentNode instanceof DOMDocument; + } + protected function initialMatchOnElement(SimpleSelector $selector, SplObjectStorage $matches): SplObjectStorage { $element = $selector->element; @@ -591,11 +607,7 @@ protected function initialMatchOnElement(SimpleSelector $selector, SplObjectStor $found = $this->newMatches(); /** @var DOMDocument $node */ foreach ($matches as $node) { - // jQuery's find() searches descendants only, so a node in the match set is not - // a candidate for its own selector. The document element is the exception: - // QueryPath seeds the match set with it rather than with the document, so - // without this qp($xml)->find('root') could never match. - if ($node->parentNode instanceof DOMDocument + if ($this->isDocumentElement($node) && ($element === '*' || $node->tagName === $element)) { $found->offsetSet($node); }