Skip to content

Commit 36bf0d3

Browse files
jakejackson1claude
authored andcommitted
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 <a>. 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) <noreply@anthropic.com>
1 parent 5b0b2c2 commit 36bf0d3

4 files changed

Lines changed: 72 additions & 3 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ QueryPath Changelog
1515
- 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)
1616
- **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)
1717
- **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 `<inner>` elements and now returns none. Use `has()` for the old behaviour
18+
- **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
1819
- Fix `children($selector)`, which resolved `:scope` against each child rather than the document element, so `children(':scope')` matched every child
1920
- `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
2021
- `nextAll()` and `prevAll()` stop walking at the first sibling already collected from an earlier starting element, rather than re-walking to the end of the list. On a 4,000-item list `nextAll()` fell from about 430 ms to under 10 ms

‎src/CSS/DOMTraverser.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -814,8 +814,11 @@ protected function initialMatchOnElement(SimpleSelector $selector, SplObjectStor
814814
continue;
815815
}
816816

817-
// Capture the case where the node itself matches the element.
818-
if ($node instanceof DOMElement
817+
// jQuery's find() searches descendants only, so a node in the match set is not
818+
// a candidate for its own selector. The document element is the exception:
819+
// QueryPath seeds the match set with it rather than with the document, so
820+
// without this qp($xml)->find('root') could never match.
821+
if ($node->parentNode instanceof DOMDocument
819822
&& ($element === '*' || $node->tagName === $element)) {
820823
$found->offsetSet($node);
821824
}

‎tests/QueryPath/DOMQueryTest.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1092,7 +1092,9 @@ public function testPrependTo()
10921092
public function testBefore()
10931093
{
10941094
$file = DATA_FILE;
1095-
$this->assertEquals(1, qp($file, 'unary')->before('<test/>')->find(':root > test ~ unary')->count());
1095+
// find() searches descendants, so the search starts from the top of the document —
1096+
// the same shape as the top() calls below.
1097+
$this->assertEquals(1, qp($file, 'unary')->before('<test/>')->top()->find(':root > test ~ unary')->count());
10961098
$this->assertEquals(1, qp($file, 'unary')->before('<test/>')->top('head ~ test')->count());
10971099
$this->assertEquals(
10981100
'unary',
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
3+
namespace QueryPathTests;
4+
5+
/**
6+
* find() searches descendants, as jQuery's find() does.
7+
*
8+
* A node already in the match set is not a candidate for its own selector. The document
9+
* element is the one exception, because QueryPath seeds the match set with the document
10+
* element rather than with the document node.
11+
*/
12+
class FindDescendantOnlyTest extends TestCase
13+
{
14+
15+
const XML = '<?xml version="1.0"?><root><a id="a1"><a id="a2"/><b id="b1"/></a></root>';
16+
17+
public function testFindDoesNotMatchTheNodesItStartsFrom(): void
18+
{
19+
$a1 = qp(self::XML, '#a1');
20+
$this->assertSame('a1', $a1->attr('id'));
21+
22+
// <a id="a1"> is in the match set, so it is not a candidate: only the nested
23+
// <a id="a2"> is found.
24+
$found = $a1->find('a');
25+
$this->assertCount(1, $found);
26+
$this->assertSame('a2', $found->attr('id'));
27+
}
28+
29+
public function testWildcardDoesNotMatchTheNodesItStartsFrom(): void
30+
{
31+
$ids = [];
32+
foreach (qp(self::XML, '#a1')->find('*') as $node) {
33+
$ids[] = $node->attr('id');
34+
}
35+
36+
$this->assertSame(['a2', 'b1'], $ids);
37+
}
38+
39+
public function testTheDocumentElementIsStillReachable(): void
40+
{
41+
// qp() seeds the match set with the document element, which stands in for the
42+
// document, so a selector naming it has to keep working.
43+
$this->assertCount(1, qp(self::XML)->find('root'));
44+
$this->assertSame('root', qp(self::XML)->find('root')->tag());
45+
46+
$this->assertCount(1, qp(self::XML)->find(':root'));
47+
}
48+
49+
public function testDescendantsOfTheDocumentElementAreStillFound(): void
50+
{
51+
$this->assertCount(2, qp(self::XML)->find('a'));
52+
$this->assertCount(1, qp(self::XML)->find('b'));
53+
}
54+
55+
/**
56+
* filter() is the jQuery-equivalent way to ask whether the nodes in hand match.
57+
*/
58+
public function testFilterIsTheWayToMatchTheNodesInHand(): void
59+
{
60+
$this->assertCount(1, qp(self::XML, '#a1')->filter('a'));
61+
$this->assertSame('a1', qp(self::XML, '#a1')->filter('a')->attr('id'));
62+
}
63+
}

0 commit comments

Comments
 (0)