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'));
+ }
+}