From 91f0a89b7aeb6e95445167aeb71e5727836eb4f3 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 13:32:29 +1100 Subject: [PATCH 1/2] Return traversal results in jQuery's order prevAll(), prevUntil(), nextAll(), nextUntil(), siblings(), parent() and closest() grouped a multi-element set's results by starting element. They now sort them as jQuery does, in one place in walk(). Fixes #117. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/Helpers/QueryFilters.php | 37 +++++++-------- tests/Issues/Issue117Test.php | 79 ++++++++++++++++++++++++++++++++ tests/QueryPath/DOMQueryTest.php | 4 +- 4 files changed, 101 insertions(+), 20 deletions(-) create mode 100644 tests/Issues/Issue117Test.php 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..01816bb 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->inst($this->sortDocumentOrder($this->walk('nextSibling', $selector, false)), null); } /** @@ -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->inst($this->sortDocumentOrder($this->walk('previousSibling', $selector, false), true), null); } /** @@ -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->inst($this->sortDocumentOrder($this->walk('parentNode', $selector, false), true), null); } /** @@ -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->inst($this->sortDocumentOrder($this->walk('parentNode', $selector, true, true)), null); } /** @@ -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->inst($this->sortDocumentOrder($this->walk('parentNode', $selector, true)), null); } /** @@ -738,7 +738,7 @@ 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)); + $ancestors = $this->sortDocumentOrder($this->walk('parentNode', null, false), true); return $this->inst($this->filterCandidates($ancestors, $selector), null); } @@ -788,7 +788,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->inst($this->sortDocumentOrder($this->filterCandidates($this->walk('nextSibling', null, false), $selector)), null); } /** @@ -837,7 +837,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->inst($this->sortDocumentOrder($this->filterCandidates($this->walk('previousSibling', null, false), $selector), true), null); } /** @@ -943,7 +943,7 @@ public function siblings($selector = null): Query } } - return $this->inst($this->filterCandidates($found, $selector), null); + return $this->inst($this->sortDocumentOrder($this->filterCandidates($found, $selector)), null); } /** @@ -994,26 +994,27 @@ private function walk(string $axis, $selector, bool $first, bool $inclusive = fa } /** - * Sort a set of nodes into reverse document order. + * Sort a set of nodes into document order, or reverse document order. * - * 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. + * 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. * * @param SplObjectStorage $nodes - * The nodes to sort. Duplicates are already removed by SplObjectStorage. + * @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 = false): 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..5d1dd86 --- /dev/null +++ b/tests/Issues/Issue117Test.php @@ -0,0 +1,79 @@ +'; + + public const ANCESTORS = ''; + + /** + * Build a match set from IDs, in the order given rather than document order. + * + * @param string $xml + * @param string ...$ids + * + * @return \QueryPath\DOMQuery + */ + private function start(string $xml, string ...$ids) + { + $qp = qp($xml, '#' . array_shift($ids)); + foreach ($ids as $id) { + $qp = $qp->add('#' . $id); + } + + return $qp; + } + + private function ids($query): string + { + return implode(',', array_map(function ($node) { + return $node->getAttribute('id'); + }, $query->get())); + } + + public function testBackwardWalksReturnReverseDocumentOrder() + { + $this->assertSame('c,b,a', $this->ids($this->start(self::SIBLINGS, 'c', 'd')->prevAll())); + $this->assertSame('d,c,b', $this->ids($this->start(self::SIBLINGS, 'c', 'e')->prevUntil('#a'))); + $this->assertSame('t2,s2,t1,s1,r', $this->ids($this->start(self::ANCESTORS, 'u1', 'u2')->parents())); + $this->assertSame('t2,s2,t1,s1', $this->ids($this->start(self::ANCESTORS, 'u1', 'u2')->parentsUntil('#r'))); + } + + public function testForwardWalksReturnDocumentOrder() + { + $this->assertSame('b,c,d,e', $this->ids($this->start(self::SIBLINGS, 'c', 'a')->nextAll())); + $this->assertSame('b,c,d', $this->ids($this->start(self::SIBLINGS, 'c', 'a')->nextUntil('#e'))); + $this->assertSame('a,b,c,d,e', $this->ids($this->start(self::SIBLINGS, 'd', 'b')->siblings())); + } + + public function testParentAndClosestReturnDocumentOrder() + { + $this->assertSame('t1,t2', $this->ids($this->start(self::ANCESTORS, 'u2', 'u1')->parent())); + $this->assertSame('s1,s2', $this->ids($this->start(self::ANCESTORS, 'u2', 'u1')->closest('s'))); + } + + public function testNextAndPrevKeepTheOrderOfTheStartingElements() + { + $this->assertSame('d,b', $this->ids($this->start(self::SIBLINGS, 'c', 'a')->next())); + $this->assertSame('d,b', $this->ids($this->start(self::SIBLINGS, 'e', 'c')->prev())); + } + + public function testSingleStartingElementKeepsItsWalkOrder() + { + $this->assertSame('d,c,b,a', $this->ids($this->start(self::SIBLINGS, 'e')->prevAll())); + $this->assertSame('t1,s1,r', $this->ids($this->start(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())); } From 56a0b36abff9b82ccc5c9850649dabf505c61ad9 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 15:56:33 +1100 Subject: [PATCH 2/2] Order traversal results through two named helpers inDocumentOrder() and inReverseDocumentOrder() filter, sort and wrap a traversal's results, so each public method is one line and names its order. The test ID helpers move to TestCase. Co-Authored-By: Claude Opus 5.5 --- src/Helpers/QueryFilters.php | 53 +++++++++++++++++++++++++---------- tests/Issues/Issue117Test.php | 51 +++++++++------------------------ tests/QueryPath/TestCase.php | 32 +++++++++++++++++++++ 3 files changed, 83 insertions(+), 53 deletions(-) diff --git a/src/Helpers/QueryFilters.php b/src/Helpers/QueryFilters.php index 01816bb..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->sortDocumentOrder($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->sortDocumentOrder($this->walk('previousSibling', $selector, false), true), 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->sortDocumentOrder($this->walk('parentNode', $selector, false), true), 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->sortDocumentOrder($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->sortDocumentOrder($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->sortDocumentOrder($this->walk('parentNode', null, false), true); - - 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->sortDocumentOrder($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->sortDocumentOrder($this->filterCandidates($this->walk('previousSibling', null, false), $selector), true), null); + return $this->inReverseDocumentOrder($this->walk('previousSibling', null, false), $selector); } /** @@ -943,7 +939,7 @@ public function siblings($selector = null): Query } } - return $this->inst($this->sortDocumentOrder($this->filterCandidates($found, $selector)), null); + return $this->inDocumentOrder($found, $selector); } /** @@ -994,17 +990,44 @@ private function walk(string $axis, $selector, bool $first, bool $inclusive = fa } /** - * Sort a set of nodes into document order, or 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. * + * @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 + * @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 */ - private function sortDocumentOrder(SplObjectStorage $nodes, bool $reverse = false): SplObjectStorage + private function sortDocumentOrder(SplObjectStorage $nodes, bool $reverse): SplObjectStorage { // A single starting element's walk is already in order. if (count($nodes) < 2 || count($this->matches) < 2) { diff --git a/tests/Issues/Issue117Test.php b/tests/Issues/Issue117Test.php index 5d1dd86..089a6ff 100644 --- a/tests/Issues/Issue117Test.php +++ b/tests/Issues/Issue117Test.php @@ -19,61 +19,36 @@ class Issue117Test extends TestCase public const ANCESTORS = ''; - /** - * Build a match set from IDs, in the order given rather than document order. - * - * @param string $xml - * @param string ...$ids - * - * @return \QueryPath\DOMQuery - */ - private function start(string $xml, string ...$ids) - { - $qp = qp($xml, '#' . array_shift($ids)); - foreach ($ids as $id) { - $qp = $qp->add('#' . $id); - } - - return $qp; - } - - private function ids($query): string - { - return implode(',', array_map(function ($node) { - return $node->getAttribute('id'); - }, $query->get())); - } - public function testBackwardWalksReturnReverseDocumentOrder() { - $this->assertSame('c,b,a', $this->ids($this->start(self::SIBLINGS, 'c', 'd')->prevAll())); - $this->assertSame('d,c,b', $this->ids($this->start(self::SIBLINGS, 'c', 'e')->prevUntil('#a'))); - $this->assertSame('t2,s2,t1,s1,r', $this->ids($this->start(self::ANCESTORS, 'u1', 'u2')->parents())); - $this->assertSame('t2,s2,t1,s1', $this->ids($this->start(self::ANCESTORS, 'u1', 'u2')->parentsUntil('#r'))); + $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->ids($this->start(self::SIBLINGS, 'c', 'a')->nextAll())); - $this->assertSame('b,c,d', $this->ids($this->start(self::SIBLINGS, 'c', 'a')->nextUntil('#e'))); - $this->assertSame('a,b,c,d,e', $this->ids($this->start(self::SIBLINGS, 'd', 'b')->siblings())); + $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->ids($this->start(self::ANCESTORS, 'u2', 'u1')->parent())); - $this->assertSame('s1,s2', $this->ids($this->start(self::ANCESTORS, 'u2', 'u1')->closest('s'))); + $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->ids($this->start(self::SIBLINGS, 'c', 'a')->next())); - $this->assertSame('d,b', $this->ids($this->start(self::SIBLINGS, 'e', 'c')->prev())); + $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->ids($this->start(self::SIBLINGS, 'e')->prevAll())); - $this->assertSame('t1,s1,r', $this->ids($this->start(self::ANCESTORS, 'u1')->parents())); + $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/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. *