diff --git a/CHANGELOG.md b/CHANGELOG.md
index cbb6125..dff1ca9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -56,6 +56,7 @@ QueryPath Changelog
- **Behaviour change.** `remove($selector)` and `replaceAll($selector)` now select with `QueryPath\CSS\DOMTraverser`, the engine behind `find()`, instead of the legacy `QueryPath\CSS\QueryPathEventHandler` (issues #126, #123, #124). `remove($selector)` searches the descendants of the match set, as `find()` and `detach($selector)` do, so `qp($xml, 'p')->remove('p')` no longer removes the `
` elements themselves. `replaceAll($selector, $document)` still searches the whole of `$document`, document element included. Both now accept `:matches()`, which threw "Unknown Pseudo-Class: matches", and apply a positional `:not()` argument to the whole set, so `remove('li:not(:first)')` keeps only the first `
` instead of removing nothing. `QueryPathEventHandler` is no longer used and is deprecated
- Add `QueryPath\CSS\DOMTraverser::filterNodes($nodes, $selector)`, which returns the given nodes that themselves match a selector, in the order given (issue #116). `filter()`, `is()`, the selector-filtered traversal methods, and `:not()`/`:matches()` all use it. `DOMTraverser` no longer modifies a parsed selector, so queries sharing a `PseudoClass` handler parse each selector once: `nextUntil()` over 2000 siblings drops from about 16 ms to 6 ms, and `tr:not(:nth-child(2n))` on a 2000-row table from 15 ms to 6 ms
- Consolidate the sibling and ancestor walks behind `next()`, `prev()`, `nextAll()`, `prevAll()`, `nextUntil()`, `prevUntil()`, `parent()`, `parents()`, `parentsUntil()` and `closest()` into one helper, so a fix to one no longer has to be repeated in its mirror (issue #115). As a side effect, `parentsUntil()` no longer loops forever on an element that is not attached to the document, and `nextUntil('')` and `prevUntil('')` treat an empty selector as no selector, as the other methods already did, instead of throwing `QueryPath\CSS\ParseException`. `closest('')` returns an empty set, as in jQuery. `nextUntil()`, `prevUntil()` and `parentsUntil()` also stop at an element already collected from an earlier starting element, as `nextAll()` and `prevAll()` do, so `nextUntil()` on each of 2000 siblings falls from about 100 ms to 1 ms
+- **Behaviour change.** `prevAll()`, `prevUntil()`, `nextAll()`, `nextUntil()`, `siblings()`, `parent()` and `closest()` now return the results for a multi-element set in jQuery's order, instead of grouped by starting element (issue #117). `prevAll()` and `prevUntil()` return reverse document order, as `parents()` and `parentsUntil()` already did, and the others return document order. `qp($xml, 'c')->add('d')->prevAll()` now returns `c, b, a` rather than `b, a, c`. `next()` and `prev()` keep the order of the starting elements, as in jQuery, and the results for a single starting element are unchanged
# 4.2.0
diff --git a/src/Helpers/QueryFilters.php b/src/Helpers/QueryFilters.php
index df379e1..3141d0e 100644
--- a/src/Helpers/QueryFilters.php
+++ b/src/Helpers/QueryFilters.php
@@ -557,7 +557,7 @@ public function lastChild(): Query
*/
public function nextUntil($selector = null): Query
{
- return $this->inst($this->walk('nextSibling', $selector, false), null);
+ return $this->inDocumentOrder($this->walk('nextSibling', $selector, false));
}
/**
@@ -583,7 +583,7 @@ public function nextUntil($selector = null): Query
*/
public function prevUntil($selector = null): Query
{
- return $this->inst($this->walk('previousSibling', $selector, false), null);
+ return $this->inReverseDocumentOrder($this->walk('previousSibling', $selector, false));
}
/**
@@ -605,7 +605,7 @@ public function prevUntil($selector = null): Query
*/
public function parentsUntil($selector = null): Query
{
- return $this->inst($this->sortReverseDocumentOrder($this->walk('parentNode', $selector, false)), null);
+ return $this->inReverseDocumentOrder($this->walk('parentNode', $selector, false));
}
/**
@@ -694,7 +694,7 @@ public function closest($selector): Query
return $this->inst(new SplObjectStorage(), null);
}
- return $this->inst($this->walk('parentNode', $selector, true, true), null);
+ return $this->inDocumentOrder($this->walk('parentNode', $selector, true, true));
}
/**
@@ -715,7 +715,7 @@ public function closest($selector): Query
*/
public function parent($selector = null): Query
{
- return $this->inst($this->walk('parentNode', $selector, true), null);
+ return $this->inDocumentOrder($this->walk('parentNode', $selector, true));
}
/**
@@ -736,11 +736,7 @@ public function parent($selector = null): Query
*/
public function parents($selector = null): Query
{
- // jQuery returns the ancestors of a multi-element set in reverse document order, with
- // duplicates removed. The selector is applied after that ordering.
- $ancestors = $this->sortReverseDocumentOrder($this->walk('parentNode', null, false));
-
- return $this->inst($this->filterCandidates($ancestors, $selector), null);
+ return $this->inReverseDocumentOrder($this->walk('parentNode', null, false), $selector);
}
/**
@@ -788,7 +784,7 @@ public function next($selector = null): Query
*/
public function nextAll($selector = null): Query
{
- return $this->inst($this->filterCandidates($this->walk('nextSibling', null, false), $selector), null);
+ return $this->inDocumentOrder($this->walk('nextSibling', null, false), $selector);
}
/**
@@ -837,7 +833,7 @@ public function prev($selector = null): Query
*/
public function prevAll($selector = null): Query
{
- return $this->inst($this->filterCandidates($this->walk('previousSibling', null, false), $selector), null);
+ return $this->inReverseDocumentOrder($this->walk('previousSibling', null, false), $selector);
}
/**
@@ -943,7 +939,7 @@ public function siblings($selector = null): Query
}
}
- return $this->inst($this->filterCandidates($found, $selector), null);
+ return $this->inDocumentOrder($found, $selector);
}
/**
@@ -994,26 +990,54 @@ private function walk(string $axis, $selector, bool $first, bool $inclusive = fa
}
/**
- * Sort a set of nodes into reverse document order.
+ * Wrap a traversal's results, optionally filtered by a selector, in document order.
+ *
+ * The results of a traversal are accumulated per starting element, so a set built from
+ * more than one would otherwise be grouped by starting element, as jQuery's never are.
*
- * jQuery's ancestor traversal methods 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.
+ * @param SplObjectStorage $nodes
+ * @param string|null $selector
+ *
+ * @return Query
+ * @throws ParseException
+ */
+ private function inDocumentOrder(SplObjectStorage $nodes, $selector = null): Query
+ {
+ return $this->inst($this->sortDocumentOrder($this->filterCandidates($nodes, $selector), false), null);
+ }
+
+ /**
+ * As inDocumentOrder(), in reverse document order, as jQuery returns the walks that head
+ * backwards: parents(), parentsUntil(), prevAll() and prevUntil().
*
* @param SplObjectStorage $nodes
- * The nodes to sort. Duplicates are already removed by SplObjectStorage.
+ * @param string|null $selector
+ *
+ * @return Query
+ * @throws ParseException
+ */
+ private function inReverseDocumentOrder(SplObjectStorage $nodes, $selector = null): Query
+ {
+ return $this->inst($this->sortDocumentOrder($this->filterCandidates($nodes, $selector), true), null);
+ }
+
+ /**
+ * @param SplObjectStorage $nodes
+ * @param bool $reverse
*
* @return SplObjectStorage
- * The same nodes, in reverse document order.
*/
- private function sortReverseDocumentOrder(SplObjectStorage $nodes): SplObjectStorage
+ private function sortDocumentOrder(SplObjectStorage $nodes, bool $reverse): SplObjectStorage
{
- // A single starting element's walk up the tree already yields reverse document order.
+ // A single starting element's walk is already in order.
if (count($nodes) < 2 || count($this->matches) < 2) {
return $nodes;
}
- $ordered = array_reverse(Util::sortDocumentOrder(iterator_to_array($nodes, false)));
+ $ordered = Util::sortDocumentOrder(iterator_to_array($nodes, false));
+ if ($reverse) {
+ $ordered = array_reverse($ordered);
+ }
$sorted = new SplObjectStorage();
foreach ($ordered as $node) {
diff --git a/tests/Issues/Issue117Test.php b/tests/Issues/Issue117Test.php
new file mode 100644
index 0000000..089a6ff
--- /dev/null
+++ b/tests/Issues/Issue117Test.php
@@ -0,0 +1,54 @@
+';
+
+ public const ANCESTORS = '';
+
+ public function testBackwardWalksReturnReverseDocumentOrder()
+ {
+ $this->assertSame('c,b,a', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'd')->prevAll()));
+ $this->assertSame('d,c,b', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'e')->prevUntil('#a')));
+ $this->assertSame('t2,s2,t1,s1,r', $this->idList($this->selectIds(self::ANCESTORS, 'u1', 'u2')->parents()));
+ $this->assertSame('t2,s2,t1,s1', $this->idList($this->selectIds(self::ANCESTORS, 'u1', 'u2')->parentsUntil('#r')));
+ }
+
+ public function testForwardWalksReturnDocumentOrder()
+ {
+ $this->assertSame('b,c,d,e', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'a')->nextAll()));
+ $this->assertSame('b,c,d', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'a')->nextUntil('#e')));
+ $this->assertSame('a,b,c,d,e', $this->idList($this->selectIds(self::SIBLINGS, 'd', 'b')->siblings()));
+ }
+
+ public function testParentAndClosestReturnDocumentOrder()
+ {
+ $this->assertSame('t1,t2', $this->idList($this->selectIds(self::ANCESTORS, 'u2', 'u1')->parent()));
+ $this->assertSame('s1,s2', $this->idList($this->selectIds(self::ANCESTORS, 'u2', 'u1')->closest('s')));
+ }
+
+ public function testNextAndPrevKeepTheOrderOfTheStartingElements()
+ {
+ $this->assertSame('d,b', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'a')->next()));
+ $this->assertSame('d,b', $this->idList($this->selectIds(self::SIBLINGS, 'e', 'c')->prev()));
+ }
+
+ public function testSingleStartingElementKeepsItsWalkOrder()
+ {
+ $this->assertSame('d,c,b,a', $this->idList($this->selectIds(self::SIBLINGS, 'e')->prevAll()));
+ $this->assertSame('t1,s1,r', $this->idList($this->selectIds(self::ANCESTORS, 'u1')->parents()));
+ }
+}
diff --git a/tests/QueryPath/DOMQueryTest.php b/tests/QueryPath/DOMQueryTest.php
index 01deac5..674f925 100644
--- a/tests/QueryPath/DOMQueryTest.php
+++ b/tests/QueryPath/DOMQueryTest.php
@@ -2139,8 +2139,8 @@ public function testNextAllAndPrevAllFromOverlappingStarts()
};
$this->assertSame('c,d,e', $names(qp($xml, 'b')->add('d')->nextAll()));
- $this->assertSame('e,c,d', $names(qp($xml, 'd')->add('b')->nextAll()));
- $this->assertSame('a,c,b', $names(qp($xml, 'b')->add('d')->prevAll()));
+ $this->assertSame('c,d,e', $names(qp($xml, 'd')->add('b')->nextAll()));
+ $this->assertSame('c,b,a', $names(qp($xml, 'b')->add('d')->prevAll()));
$this->assertSame('c,b,a', $names(qp($xml, 'd')->add('b')->prevAll()));
}
diff --git a/tests/QueryPath/TestCase.php b/tests/QueryPath/TestCase.php
index 63386cd..089dc35 100644
--- a/tests/QueryPath/TestCase.php
+++ b/tests/QueryPath/TestCase.php
@@ -34,6 +34,38 @@ protected function capture(callable $callback): string
}
}
+ /**
+ * Build a match set from element IDs, in the order given rather than document order.
+ *
+ * @param string $xml
+ * @param string ...$ids
+ *
+ * @return \QueryPath\DOMQuery
+ */
+ protected function selectIds(string $xml, string ...$ids)
+ {
+ $qp = qp($xml, '#' . array_shift($ids));
+ foreach ($ids as $id) {
+ $qp = $qp->add('#' . $id);
+ }
+
+ return $qp;
+ }
+
+ /**
+ * The IDs of a match set's elements, comma-separated, in match-set order.
+ *
+ * @param \QueryPath\DOMQuery $query
+ *
+ * @return string
+ */
+ protected function idList($query): string
+ {
+ return implode(',', array_map(function ($node) {
+ return $node->getAttribute('id');
+ }, $query->get()));
+ }
+
/**
* Whether libxml's HTML parser follows HTML5, as it does from 2.14.
*