diff --git a/CHANGELOG.md b/CHANGELOG.md index e274e6a..a42a1ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,10 @@ QueryPath Changelog # Unreleased changes - **Breaking behaviour change:** `DOMQuery::is()` now behaves like jQuery's `.is()`. It tests the elements held in the current match set and returns `true` when at least one of them matches the selector. Previously it ran a descendant search, so `html5qp('

foo

', 'p')->is('span')` returned `true`. Use `has()` if you need the old "does the set contain something matching this selector" behaviour (#51) -- `DOMQuery::is()` no longer raises a fatal error when the match set contains non-element nodes (text nodes, comments, processing instructions). Those nodes cannot match a CSS selector, so they are skipped +- `DOMQuery::is()` no longer raises a fatal error when the match set contains non-element nodes (text nodes, comments, processing instructions). Those nodes cannot match a CSS selector, so they are skipped (#51) +- 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) - 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()` diff --git a/src/Helpers/QueryFilters.php b/src/Helpers/QueryFilters.php index 6b39a07..0fa04a8 100644 --- a/src/Helpers/QueryFilters.php +++ b/src/Helpers/QueryFilters.php @@ -3,12 +3,12 @@ namespace QueryPath\Helpers; use DOMElement; +use DOMNode; use QueryPath\CSS\DOMTraverser; use QueryPath\CSS\ParseException; use QueryPath\DOMQuery; use QueryPath\Exception; use QueryPath\Query; -use QueryPath\QueryPath; use SplObjectStorage; use stdClass; @@ -577,7 +577,7 @@ public function nextUntil($selector = null): Query while (isset($m->nextSibling)) { $m = $m->nextSibling; if ($m->nodeType === XML_ELEMENT_NODE) { - if (null !== $selector && QueryPath::with($m, null, $this->options)->is($selector) > 0) { + if (null !== $selector && $this->matchesNodeSelector($m, $selector)) { break; } $found->offsetSet($m); @@ -616,7 +616,7 @@ public function prevUntil($selector = null): Query while (isset($m->previousSibling)) { $m = $m->previousSibling; if ($m->nodeType === XML_ELEMENT_NODE) { - if (null !== $selector && QueryPath::with($m, null, $this->options)->is($selector)) { + if (null !== $selector && $this->matchesNodeSelector($m, $selector)) { break; } @@ -654,7 +654,7 @@ public function parentsUntil($selector = null): Query // Is there any case where parent node is not an element? if ($m->nodeType === XML_ELEMENT_NODE) { if (! empty($selector)) { - if (QueryPath::with($m, null, $this->options)->is($selector) > 0) { + if ($this->matchesNodeSelector($m, $selector)) { break; } $found->offsetSet($m); @@ -665,7 +665,7 @@ public function parentsUntil($selector = null): Query } } - return $this->inst($found, null); + return $this->inst($this->sortReverseDocumentOrder($found), null); } /** @@ -726,7 +726,7 @@ public function not($selector): Query } } else { foreach ($this->matches as $m) { - if (! QueryPath::with($m, null, $this->options)->is($selector)) { + if (! $this->matchesNodeSelector($m, $selector)) { $found->offsetSet($m); } } @@ -756,17 +756,13 @@ public function closest($selector): Query { $found = new SplObjectStorage(); foreach ($this->matches as $m) { - if (QueryPath::with($m, null, $this->options)->is($selector) > 0) { + if ($this->matchesNodeSelector($m, $selector)) { $found->offsetSet($m); } else { while ($m->parentNode->nodeType !== XML_DOCUMENT_NODE) { $m = $m->parentNode; // Is there any case where parent node is not an element? - if ($m->nodeType === XML_ELEMENT_NODE && QueryPath::with( - $m, - null, - $this->options - )->is($selector) > 0) { + if ($this->matchesNodeSelector($m, $selector)) { $found->offsetSet($m); break; } @@ -844,7 +840,7 @@ private function getParentElements(?string $selector, bool $immediate): Query // Is there any case where parent node is not an element? if ($m->nodeType === XML_ELEMENT_NODE) { if (! empty($selector)) { - if (QueryPath::with($m, null, $this->options)->is($selector) > 0) { + if ($this->matchesNodeSelector($m, $selector)) { $found->offsetSet($m); if ($immediate) { break; @@ -860,6 +856,13 @@ private function getParentElements(?string $selector, bool $immediate): Query } } + // jQuery returns the ancestors of a multi-element set in reverse + // document order, with duplicates removed. parent() keeps the + // legacy per-element ordering. + if (! $immediate) { + $found = $this->sortReverseDocumentOrder($found); + } + return $this->inst($found, null); } @@ -890,7 +893,7 @@ public function next($selector = null): Query $m = $m->nextSibling; if ($m->nodeType === XML_ELEMENT_NODE) { if (! empty($selector)) { - if (QueryPath::with($m, null, $this->options)->is($selector) > 0) { + if ($this->matchesNodeSelector($m, $selector)) { $found->offsetSet($m); break; } @@ -932,7 +935,7 @@ public function nextAll($selector = null): Query $m = $m->nextSibling; if ($m->nodeType === XML_ELEMENT_NODE) { if (! empty($selector)) { - if (QueryPath::with($m, null, $this->options)->is($selector) > 0) { + if ($this->matchesNodeSelector($m, $selector)) { $found->offsetSet($m); } } else { @@ -973,7 +976,7 @@ public function prev($selector = null): Query $m = $m->previousSibling; if ($m->nodeType === XML_ELEMENT_NODE) { if (! empty($selector)) { - if (QueryPath::with($m, null, $this->options)->is($selector)) { + if ($this->matchesNodeSelector($m, $selector)) { $found->offsetSet($m); break; } @@ -1015,7 +1018,7 @@ public function prevAll($selector = null): Query $m = $m->previousSibling; if ($m->nodeType === XML_ELEMENT_NODE) { if (! empty($selector)) { - if (QueryPath::with($m, null, $this->options)->is($selector)) { + if ($this->matchesNodeSelector($m, $selector)) { $found->offsetSet($m); } } else { @@ -1143,14 +1146,124 @@ public function siblings($selector = null): Query $parent = $m->parentNode; foreach ($parent->childNodes as $n) { if ($n->nodeType === XML_ELEMENT_NODE && $n !== $m) { + if (! empty($selector) && ! $this->matchesNodeSelector($n, $selector)) { + continue; + } + $found->offsetSet($n); } } } - if (empty($selector)) { - return $this->inst($found, null); + + return $this->inst($found, null); + } + + /** + * Test whether a single node, taken as an element, matches a CSS selector. + * + * The node itself is the only candidate, so this asks "is this node a match?" + * rather than "does this node contain a match?" — which is what running a + * find() against the node would ask. + * + * @param DOMNode $node + * The node to test. + * @param string $selector + * A valid CSS selector. + * + * @return bool + * TRUE if the node is an element and matches the selector. + * @throws ParseException + * @see NodeMatcher + */ + private function matchesNodeSelector($node, $selector): bool + { + return NodeMatcher::matchesNode($node, $selector); + } + + /** + * Sort a set of nodes into reverse document order. + * + * jQuery's ancestor traversal methods (parents(), parentsUntil()) return + * their results in reverse document order with duplicates removed. Because + * the results are accumulated per source element, a set built from more than + * one starting element would otherwise be grouped by source element instead. + * + * @param SplObjectStorage $nodes + * The nodes to sort. Duplicates are already removed by SplObjectStorage. + * + * @return SplObjectStorage + * The same nodes, in reverse document order. + */ + private function sortReverseDocumentOrder(SplObjectStorage $nodes): SplObjectStorage + { + if (count($nodes) < 2) { + return $nodes; + } + + $indexed = []; + foreach ($nodes as $node) { + $indexed[] = [$this->documentOrderPath($node), $node]; + } + + usort($indexed, function ($a, $b) { + // Reverse document order, so the comparison operands are swapped. + return $this->compareDocumentOrderPaths($b[0], $a[0]); + }); + + $sorted = new SplObjectStorage(); + foreach ($indexed as $entry) { + $sorted->offsetSet($entry[1]); + } + + return $sorted; + } + + /** + * Build a comparable representation of a node's position in its document. + * + * The path is the list of child offsets from the document down to the node, + * which can be compared element by element to determine document order. + * + * @param DOMNode $node + * + * @return array + */ + private function documentOrderPath($node): array + { + $path = []; + while ($node instanceof DOMNode && $node->parentNode !== null) { + $offset = 0; + $sibling = $node->previousSibling; + while ($sibling !== null) { + ++$offset; + $sibling = $sibling->previousSibling; + } + array_unshift($path, $offset); + $node = $node->parentNode; + } + + return $path; + } + + /** + * Compare two paths produced by documentOrderPath(). + * + * @param array $a + * @param array $b + * + * @return int + * A negative number if $a precedes $b in the document, positive if it + * follows it, and zero if they are the same node. + */ + private function compareDocumentOrderPaths(array $a, array $b): int + { + $shared = min(count($a), count($b)); + for ($i = 0; $i < $shared; ++$i) { + if ($a[$i] !== $b[$i]) { + return $a[$i] < $b[$i] ? -1 : 1; + } } - return $this->inst($found, null)->filter($selector); + return count($a) - count($b); } } diff --git a/tests/Issues/Issue62Test.php b/tests/Issues/Issue62Test.php new file mode 100644 index 0000000..589362f --- /dev/null +++ b/tests/Issues/Issue62Test.php @@ -0,0 +1,239 @@ +tag(); + } + + return $tags; + } + + /** + * @throws Exception + */ + public function testParentsFiltersAncestorsBySelector() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(1, $qp->count()); + $this->assertEquals(['Demographics'], $this->tags($qp->parents('Demographics'))); + } + + /** + * The ancestor must be matched as an element, not by asking whether it + * contains something matching the selector. + * + * @throws Exception + */ + public function testParentsDoesNotMatchAncestorsThatMerelyContainTheSelector() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(['Age'], $this->tags($qp->parents('Age'))); + $this->assertEquals(['AmplifyReturn'], $this->tags($qp->parents('AmplifyReturn'))); + $this->assertEquals(0, $qp->parents('Name')->count()); + $this->assertEquals(0, $qp->parents('Value')->count()); + } + + /** + * @throws Exception + */ + public function testParentsWithoutSelectorReturnsEveryAncestor() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals( + ['Age', 'Demographics', 'AmplifyReturn', 'ns1:AmplifyResponse'], + $this->tags($qp->parents()) + ); + } + + /** + * @throws Exception + */ + public function testParentsMatchesNamespacedAncestors() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(['ns1:AmplifyResponse'], $this->tags($qp->parents('ns1|AmplifyResponse'))); + $this->assertEquals(['ns1:AmplifyResponse'], $this->tags($qp->parents('*|AmplifyResponse'))); + + // The namespaced root must not be reported for a selector it only contains. + $this->assertEquals(0, $qp->parents('ns1|Demographics')->count()); + } + + /** + * @throws Exception + */ + public function testParentsAcceptsFullSelectorsWithCombinators() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(['Demographics'], $this->tags($qp->parents('AmplifyReturn > Demographics'))); + $this->assertEquals(0, $qp->parents('Styles > Demographics')->count()); + } + + /** + * jQuery returns ancestors closest-first, and for a set built from more than + * one element the result is in reverse document order with duplicates removed. + * + * @throws Exception + */ + public function testParentsReturnsReverseDocumentOrder() + { + $xml = ''; + $qp = qp($xml, 'i'); + + $this->assertEquals(2, $qp->count()); + $this->assertEquals(['d', 'c', 'b', 'a', 'root'], $this->tags($qp->parents())); + } + + /** + * Shared ancestors must appear exactly once. + * + * @throws Exception + */ + public function testParentsRemovesDuplicates() + { + $xml = ''; + $qp = qp($xml, 'i'); + + $this->assertEquals(2, $qp->count()); + $this->assertEquals(['a', 'root'], $this->tags($qp->parents())); + } + + /** + * @throws Exception + */ + public function testParentsUntilStopsAtTheMatchingAncestor() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(['Age'], $this->tags($qp->parentsUntil('Demographics'))); + + // Before the fix AmplifyReturn was collected, because AmplifyReturn does + // not contain a descendant called AmplifyReturn. + $this->assertEquals(['Age', 'Demographics'], $this->tags($qp->parentsUntil('AmplifyReturn'))); + $this->assertEquals( + ['Age', 'Demographics', 'AmplifyReturn'], + $this->tags($qp->parentsUntil('ns1|AmplifyResponse')) + ); + } + + /** + * @throws Exception + */ + public function testClosestMatchesTheAncestorItself() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(['Age'], $this->tags($qp->closest('Age'))); + $this->assertEquals(['Demographics'], $this->tags($qp->closest('Demographics'))); + + // Before the fix this returned ns1:AmplifyResponse, the first ancestor + // that contained an AmplifyReturn element. + $this->assertEquals(['AmplifyReturn'], $this->tags($qp->closest('AmplifyReturn'))); + $this->assertEquals(['Name'], $this->tags($qp->closest('Name'))); + } + + /** + * @throws Exception + */ + public function testParentMatchesTheAncestorItself() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(['Age'], $this->tags($qp->parent('Age'))); + $this->assertEquals(['AmplifyReturn'], $this->tags($qp->parent('AmplifyReturn'))); + $this->assertEquals(0, $qp->parent('Name')->count()); + } + + /** + * @throws Exception + */ + public function testSiblingTraversalMatchesTheSiblingItself() + { + $qp = qp(self::AMPLIFY_FILE, 'Demographics > Age > Name'); + + $this->assertEquals(['Value'], $this->tags($qp->siblings('Value'))); + $this->assertEquals(['Value'], $this->tags($qp->nextAll('Value'))); + $this->assertEquals(['Value'], $this->tags($qp->next('Value'))); + $this->assertEquals(0, $qp->siblings('Name')->count()); + } + + /** + * `nextUntil()`/`prevUntil()` must stop on a sibling that matches, not on a + * sibling that contains a match. + * + * @throws Exception + */ + public function testNextUntilAndPrevUntilStopOnMatchingSibling() + { + $xml = ''; + + $this->assertEquals(['b'], $this->tags(qp($xml, 'root > a')->nextUntil('c'))); + $this->assertEquals([], $this->tags(qp($xml, 'root > a')->nextUntil('b'))); + $this->assertEquals(['c', 'b'], $this->tags(qp($xml, 'root > d')->prevUntil('a'))); + } + + /** + * `not()` must exclude elements that match the selector themselves. + * + * @throws Exception + */ + public function testNotExcludesElementsThatMatchTheSelector() + { + $xml = ''; + $qp = qp($xml, 'a, b'); + + $this->assertEquals(3, $qp->count()); + $this->assertEquals(['b', 'b'], $this->tags($qp->not('a'))); + $this->assertEquals(['a'], $this->tags($qp->not('b'))); + } + + /** + * The candidate node must not be passed to the traverser as its scope node, or every + * candidate matches :scope and the selector stops filtering anything at all. + * + * @see \QueryPath\Helpers\NodeMatcher + */ + public function testScopePseudoClassIsResolvedAgainstTheDocument(): void + { + $xml = 'x'; + + $this->assertSame('root', qp($xml, 'c')->top()->find(':scope')->tag()); + + $parents = qp($xml, 'c')->parents(':scope'); + $this->assertCount(1, $parents, ':scope must match the document element, not every ancestor'); + $this->assertSame('root', $parents->tag()); + + $this->assertTrue(qp($xml, 'root')->is(':scope')); + $this->assertFalse(qp($xml, 'c')->is(':scope')); + } +}