diff --git a/CHANGELOG.md b/CHANGELOG.md index 527a87d..737c889 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ QueryPath Changelog - 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 - **Behaviour change.** A positional pseudo-class (`:first`, `:eq()`, `:last`, ...) in the stop selector of `nextUntil()`, `prevUntil()` and `parentsUntil()` now counts across the whole document, as jQuery's does, instead of seeing each sibling or ancestor as a one-element set (issue #118). `nextUntil(':first')` used to stop at once and now walks to the end, and `nextUntil(':eq(3)')` stops at the fourth element in the document. `closest()` no longer matches a positional selector at all, as in jQuery, where `closest(':first')` used to return the element itself. `next()`, `prev()` and `parent()` still test each candidate on its own, which agrees with jQuery for a single starting element +- **Behaviour change.** A positional pseudo-class in the selector of `prevAll()`, `parents()` and `nextAll()` now counts in walk order, before the results are sorted, as jQuery does (issue #134). `qp($xml, '#e')->prevAll(':first')` returns the nearest previous sibling, `d`, instead of the first one, `a`, and `parents(':first')` returns the parent. For a set of more than one element, the walks from each element are joined in match-set order, repeats included, and counted along that list. Add `QueryPath\CSS\DOMTraverser::filterList()`, which filters a list of nodes counting positional pseudo-classes in the order given # 4.2.0 diff --git a/src/CSS/DOMTraverser.php b/src/CSS/DOMTraverser.php index 7f443c9..690865b 100644 --- a/src/CSS/DOMTraverser.php +++ b/src/CSS/DOMTraverser.php @@ -98,6 +98,13 @@ class DOMTraverser implements Traverser */ protected $resolvingIndex; + /** + * The candidates in the order filterList() counts them, or NULL to count in document order. + * + * @var DOMNode[]|null + */ + protected $candidateList; + /** * Build a new DOMTraverser. * @@ -195,18 +202,74 @@ public static function filterNodes( $selector, $scopeNode = null, ?PseudoClass $psHandler = null + ): SplObjectStorage { + return self::filterInOrder($nodes, null, $selector, $scopeNode, $psHandler); + } + + /** + * As filterNodes(), but positional pseudo-classes on the subject count along the list in the + * order given, repeats included, as jQuery.filter() counts a traversal's walk. + * + * @param DOMNode[] $nodes + * The candidates, all from one document, in the order to count them. + * @param string $selector + * A CSS selector. + * @param DOMNode|null $scopeNode + * @param PseudoClass|null $psHandler + * + * @return SplObjectStorage + * The matching nodes, once each, in the order they first appear in the list. + * @throws ParseException + */ + public static function filterList( + array $nodes, + $selector, + $scopeNode = null, + ?PseudoClass $psHandler = null + ): SplObjectStorage { + $unique = new SplObjectStorage(); + foreach ($nodes as $node) { + $unique->offsetSet($node); + } + + return self::filterInOrder($unique, array_values($nodes), $selector, $scopeNode, $psHandler); + } + + /** + * Match the given nodes themselves against a selector, returning them in the order given. + * + * @param SplObjectStorage $nodes + * @param DOMNode[]|null $list + * The order to count positional pseudo-classes in, or NULL for document order. + * @param string $selector + * @param DOMNode|null $scopeNode + * @param PseudoClass|null $psHandler + * + * @return SplObjectStorage + * @throws ParseException + */ + private static function filterInOrder( + SplObjectStorage $nodes, + ?array $list, + $selector, + $scopeNode, + ?PseudoClass $psHandler ): SplObjectStorage { if (count($nodes) === 0) { return new SplObjectStorage(); } - $traverser = new self($nodes, true, $scopeNode, $psHandler); - $groups = $traverser->psHandler->parse($selector); - $matched = $traverser->findGroups($groups)->matches(); + $traverser = new self($nodes, true, $scopeNode, $psHandler); + $traverser->candidateList = $list; + $groups = $traverser->psHandler->parse($selector); + $matched = $traverser->findGroups($groups)->matches(); // A single group without a set filter is matched in the order given. Otherwise the result // is in document order, or a union of groups, and has to be put back in input order. - if (count($matched) < 2 || (count($groups) === 1 && ! $traverser->groupIsSetFiltered($groups->toArray()[0]))) { + $inOrder = $list === null + && count($groups) === 1 + && ! $traverser->groupIsSetFiltered($groups->toArray()[0]); + if (count($matched) < 2 || $inOrder) { return $matched; } @@ -330,7 +393,7 @@ protected function resolveSubject(array $selectorGroup, int $index): SplObjectSt // Initialize matches if necessary. A caller-supplied set may hold non-element nodes, // which can never match a selector. if ($index === 0 && $this->initialized) { - $candidates = array_filter(iterator_to_array($this->matches, false), function ($node) { + $candidates = array_filter($this->candidateList ?? iterator_to_array($this->matches, false), function ($node) { return $node instanceof DOMElement; }); } else { @@ -369,7 +432,9 @@ protected function resolveSubject(array $selectorGroup, int $index): SplObjectSt $this->resolvingIndex = $previous; - $matched = Util::sortDocumentOrder($matched); + if ($index !== 0 || $this->candidateList === null) { + $matched = Util::sortDocumentOrder($matched); + } foreach ($setFilters as $pseudoClass) { $matched = $this->applySetFilter($matched, $pseudoClass); } @@ -398,7 +463,7 @@ protected function applySetFilter(array $nodes, array $pseudoClass): array // :not() and :matches() whose argument uses a positional filter. // The argument has to see the whole result set, exactly as jQuery's // :not(:first) does, so run it over this set and keep or drop the result. - $inner = self::filterNodes($this->toMatches($nodes), $value, $this->scopeNode, $this->psHandler); + $inner = self::filterList($nodes, $value, $this->scopeNode, $this->psHandler); if ($name !== 'not') { return iterator_to_array($inner, false); } diff --git a/src/Helpers/QueryFilters.php b/src/Helpers/QueryFilters.php index 8b73c7b..4628556 100644 --- a/src/Helpers/QueryFilters.php +++ b/src/Helpers/QueryFilters.php @@ -3,6 +3,7 @@ namespace QueryPath\Helpers; use DOMElement; +use QueryPath\CSS\DOMTraverser; use QueryPath\CSS\DOMTraverser\Util; use QueryPath\CSS\ParseException; use QueryPath\DOMQuery; @@ -726,7 +727,8 @@ public function parent($selector = null): Query /** * Get all ancestors of each element in the DOMQuery. * - * If a selector is present, only matching ancestors will be retrieved. + * If a selector is present, only matching ancestors will be retrieved. As in jQuery, a + * positional selector counts from the nearest ancestor, so `parents(':first')` is the parent. * * @param string $selector * A valid CSS 3 Selector. @@ -741,7 +743,7 @@ public function parent($selector = null): Query */ public function parents($selector = null): Query { - return $this->inReverseDocumentOrder($this->walk('parentNode', null, false), $selector); + return $this->inReverseDocumentOrder($this->walkAll('parentNode', $selector)); } /** @@ -793,7 +795,7 @@ public function next($selector = null): Query */ public function nextAll($selector = null): Query { - return $this->inDocumentOrder($this->walk('nextSibling', null, false), $selector); + return $this->inDocumentOrder($this->walkAll('nextSibling', $selector)); } /** @@ -825,7 +827,8 @@ public function prev($selector = null): Query * Get the previous siblings for each element in the DOMQuery. * * For each element in the DOMQuery, get all previous siblings. If a - * selector is provided, only matching siblings will be retrieved. + * selector is provided, only matching siblings will be retrieved. As in jQuery, a + * positional selector counts from the nearest sibling, so `prevAll(':first')` is prev(). * * @param string $selector * A valid CSS 3 selector. @@ -842,7 +845,7 @@ public function prev($selector = null): Query */ public function prevAll($selector = null): Query { - return $this->inReverseDocumentOrder($this->walk('previousSibling', null, false), $selector); + return $this->inReverseDocumentOrder($this->walkAll('previousSibling', $selector)); } /** @@ -1000,6 +1003,38 @@ private function walk(string $axis, $selector, bool $first, bool $inclusive = fa return $found; } + /** + * Walk from each matched element to the end of a DOM axis, keeping the elements that match + * an optional selector. + * + * As in jQuery, a positional selector counts along the walks joined in walk order, so + * prevAll(':first') is the nearest previous sibling. An element reached from more than one + * starting element is counted once per walk. + * + * @param string $axis + * @param string|null $selector + * + * @return SplObjectStorage + * @throws ParseException + */ + private function walkAll(string $axis, $selector): SplObjectStorage + { + if (empty($selector) || ! Util::selectorHasPositionalPseudoClass($selector)) { + return $this->filterCandidates($this->walk($axis, null, false), $selector); + } + + $walked = []; + foreach ($this->matches as $m) { + for ($n = $m->{$axis}; $n !== null && $n->nodeType !== XML_DOCUMENT_NODE; $n = $n->{$axis}) { + if ($n->nodeType === XML_ELEMENT_NODE) { + $walked[] = $n; + } + } + } + + return DOMTraverser::filterList($walked, $selector); + } + /** * Wrap a traversal's results, optionally filtered by a selector, in document order. * diff --git a/tests/Issues/Issue118Test.php b/tests/Issues/Issue118Test.php index d9c9113..1107f3b 100644 --- a/tests/Issues/Issue118Test.php +++ b/tests/Issues/Issue118Test.php @@ -71,18 +71,18 @@ public function testNextPrevAndParentTestAPositionalSelectorPerNode() } /** - * The set methods count a positional selector within the set, in document order - * (issue #66), whatever order the results come back in (issue #117). jQuery counts - * in walk order, so for prevAll() and parents() its :first is the nearest node. + * The set methods count a positional selector along the walk, before the results are + * sorted (issue #134), so for prevAll() and parents() :first is the nearest node. + * siblings() still counts in document order. */ - public function testSetMethodsCountPositionsInDocumentOrder() + public function testSetMethodsCountPositionsInWalkOrder() { - $this->assertSame('a', $this->idList($this->selectIds(self::SIBLINGS, 'e')->prevAll(':first'))); - $this->assertSame('d', $this->idList($this->selectIds(self::SIBLINGS, 'e')->prevAll(':last'))); - $this->assertSame('b', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'e')->prevAll(':eq(1)'))); - $this->assertSame('b', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'a')->nextAll(':first'))); + $this->assertSame('d', $this->idList($this->selectIds(self::SIBLINGS, 'e')->prevAll(':first'))); + $this->assertSame('a', $this->idList($this->selectIds(self::SIBLINGS, 'e')->prevAll(':last'))); + $this->assertSame('a', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'e')->prevAll(':eq(1)'))); + $this->assertSame('b', $this->idList($this->selectIds(self::SIBLINGS, 'a', 'c')->nextAll(':first'))); $this->assertSame('a', $this->idList($this->selectIds(self::SIBLINGS, 'd', 'b')->siblings(':first'))); - $this->assertSame('r', $this->idList($this->selectIds(self::ANCESTORS, 'u1', 'u2')->parents(':first'))); - $this->assertSame('t2', $this->idList($this->selectIds(self::ANCESTORS, 'u1', 'u2')->parents(':last'))); + $this->assertSame('t1', $this->idList($this->selectIds(self::ANCESTORS, 'u1', 'u2')->parents(':first'))); + $this->assertSame('r', $this->idList($this->selectIds(self::ANCESTORS, 'u1', 'u2')->parents(':last'))); } } diff --git a/tests/Issues/Issue134Test.php b/tests/Issues/Issue134Test.php new file mode 100644 index 0000000..a45c5f3 --- /dev/null +++ b/tests/Issues/Issue134Test.php @@ -0,0 +1,145 @@ +'; + + // Document order: r, s1, t1, u1, s2, t2, u2. + private const ANCESTORS = ''; + + /** + * @testWith [":first", "d"] + * [":last", "a"] + * [":eq(0)", "d"] + * [":eq(1)", "c"] + * [":eq(-1)", "a"] + * [":lt(2)", "d,c"] + * [":gt(1)", "b,a"] + * [":odd", "c,a"] + * [":even", "d,b"] + * ["*:not(:first)", "c,b,a"] + * [":matches(:last)", "a"] + * ["b, :first", "d,b"] + */ + public function testPrevAllCountsFromTheNearestSibling($selector, $expected) + { + $this->assertSame($expected, $this->idList(qp(self::SIBLINGS, '#e')->prevAll($selector))); + } + + /** + * The walks from c and e join as b, a, d, c, b, a. + * + * @testWith [":first", "b"] + * [":last", "a"] + * [":eq(2)", "d"] + * [":eq(4)", "b"] + * [":lt(3)", "d,b,a"] + * [":gt(3)", "b,a"] + * [":odd", "c,a"] + */ + public function testPrevAllOnASetCountsAlongTheJoinedWalks($selector, $expected) + { + $this->assertSame($expected, $this->idList($this->selectIds(self::SIBLINGS, 'c', 'e')->prevAll($selector))); + } + + /** + * @testWith ["u2", ":first", "t2"] + * ["u2", ":last", "r"] + * ["u2", ":eq(1)", "s2"] + * ["u2", "s:first", "s2"] + * ["u1,u2", ":first", "t1"] + * ["u1,u2", ":eq(3)", "t2"] + * ["u1,u2", ":lt(2)", "t1,s1"] + * ["u1,u2", ":gt(3)", "s2,r"] + * ["u1,u2", "s:last", "s2"] + */ + public function testParentsCountsFromTheNearestAncestor($ids, $selector, $expected) + { + $query = $this->selectIds(self::ANCESTORS, ...explode(',', $ids)); + + $this->assertSame($expected, $this->idList($query->parents($selector))); + } + + /** + * The walks from a and c join as b, c, d, e, d, e. + * + * @testWith [":first", "b"] + * [":eq(1)", "c"] + * [":eq(4)", "d"] + * [":last", "e"] + */ + public function testNextAllOnASetCountsAlongTheJoinedWalks($selector, $expected) + { + $this->assertSame($expected, $this->idList($this->selectIds(self::SIBLINGS, 'a', 'c')->nextAll($selector))); + } + + /** + * The walks are joined in match-set order, as jQuery joins them in its own set's order. add() + * keeps the order elements were added in, where jQuery's sorts them into document order. + */ + public function testWalksAreJoinedInMatchSetOrder() + { + $this->assertSame('d', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'a')->nextAll(':first'))); + $this->assertSame('d', $this->idList($this->selectIds(self::SIBLINGS, 'e', 'c')->prevAll(':first'))); + } + + public function testSelectorsWithoutAPositionAreUnchanged() + { + $this->assertSame('d,c,b,a', $this->idList(qp(self::SIBLINGS, '#e')->prevAll())); + $this->assertSame('c,a', $this->idList(qp(self::SIBLINGS, '#e')->prevAll('a, c'))); + $this->assertSame('t2,s2,t1,s1', $this->idList($this->selectIds(self::ANCESTORS, 'u1', 'u2')->parents('s, t'))); + $this->assertSame('d,e', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'a')->nextAll('d, e'))); + } + + /** + * The stop selector of prevUntil() and parentsUntil() already counts across the document + * (issue #118), so walk order does not affect it. + */ + public function testUntilStopsAreUnaffected() + { + $this->assertSame('d,c', $this->idList(qp(self::SIBLINGS, '#e')->prevUntil(':eq(2)'))); + $this->assertSame('t2', $this->idList(qp(self::ANCESTORS, '#u2')->parentsUntil(':eq(4)'))); + } + + public function testFilterListCountsInTheOrderGiven() + { + $xpath = new DOMXPath(qp(self::SIBLINGS)->document()); + $nodes = []; + foreach (['d', 'c', 'b', 'c'] as $id) { + $nodes[] = $xpath->query("//*[@id='$id']")->item(0); + } + + $ids = function ($storage) { + $found = []; + foreach ($storage as $node) { + $found[] = $node->getAttribute('id'); + } + + return implode(',', $found); + }; + + $this->assertSame('d', $ids(DOMTraverser::filterList($nodes, ':first'))); + $this->assertSame('c', $ids(DOMTraverser::filterList($nodes, ':last'))); + $this->assertSame('c', $ids(DOMTraverser::filterList($nodes, ':eq(3)'))); + $this->assertSame('d,c,b', $ids(DOMTraverser::filterList($nodes, '*'))); + $this->assertSame('c,b', $ids(DOMTraverser::filterList($nodes, ':gt(0)'))); + $this->assertSame('', $ids(DOMTraverser::filterList([], ':first'))); + } +}