diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e3f0fa..128ecfd 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')`, `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 8b8483f..33cc591 100644 --- a/src/CSS/DOMTraverser.php +++ b/src/CSS/DOMTraverser.php @@ -476,7 +476,7 @@ 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) { + if ($this->isDocumentElement($node) && $node->getAttribute('id') === $id) { $found->offsetSet($node); } @@ -521,7 +521,7 @@ protected function initialMatchOnClasses(SimpleSelector $selector, SplObjectStor /** @var DOMElement $node */ foreach ($matches as $node) { // Refactor me! - if ($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); @@ -578,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; @@ -587,9 +607,8 @@ 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)) { + if ($this->isDocumentElement($node) + && ($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..b485c29 --- /dev/null +++ b/tests/QueryPath/FindDescendantOnlyTest.php @@ -0,0 +1,81 @@ +'; + + 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')); + } + + /** + * 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')); + } +}