From 5d673e9b3c02a414a2b520558f97cbe5fb1f58ba Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 13:37:56 +1100 Subject: [PATCH] Count positional stop selectors across the document, as jQuery does The stop selector of nextUntil(), prevUntil() and parentsUntil() was tested one node at a time, so a positional pseudo-class saw a one-element set. jQuery tests it with .is(), which counts across the document, and closest() never matches a positional selector. Fixes #118. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/Helpers/NodeMatcher.php | 49 +++++++++++++++++++ src/Helpers/QueryFilters.php | 33 ++++++++----- tests/Issues/Issue118Test.php | 88 +++++++++++++++++++++++++++++++++++ 4 files changed, 160 insertions(+), 11 deletions(-) create mode 100644 tests/Issues/Issue118Test.php diff --git a/CHANGELOG.md b/CHANGELOG.md index dff1ca9..527a87d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ QueryPath Changelog - 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 +- **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 # 4.2.0 diff --git a/src/Helpers/NodeMatcher.php b/src/Helpers/NodeMatcher.php index ea6024d..8df96fd 100644 --- a/src/Helpers/NodeMatcher.php +++ b/src/Helpers/NodeMatcher.php @@ -5,6 +5,7 @@ use DOMNode; use QueryPath\CSS\DOMTraverser; use QueryPath\CSS\DOMTraverser\PseudoClass; +use QueryPath\CSS\DOMTraverser\Util; use QueryPath\CSS\ParseException; use SplObjectStorage; @@ -41,6 +42,14 @@ final class NodeMatcher */ private $psHandler; + /** + * The document's matches for a positional selector, FALSE for any other selector, or NULL + * until matchesInDocument() first needs to know. + * + * @var SplObjectStorage|false|null + */ + private $inDocument; + /** * @param string $selector * A valid CSS selector. @@ -75,6 +84,46 @@ public function matches($node): bool return $this->results[$node]; } + /** + * Test a single node as jQuery's .is() does, counting a positional selector (:first, + * :eq(n), ...) across the node's whole document. + * + * The document's matches are found once, on the first call, so every node tested must + * belong to the same document. + * + * @param DOMNode $node + * + * @return bool + * @throws ParseException + */ + public function matchesInDocument($node): bool + { + if ($this->inDocument === null) { + $this->inDocument = Util::selectorHasPositionalPseudoClass($this->selector) + ? $this->documentMatches($node->ownerDocument) + : false; + } + + return $this->inDocument === false ? $this->matches($node) : $this->inDocument->offsetExists($node); + } + + /** + * @param \DOMDocument $document + * + * @return SplObjectStorage + * @throws ParseException + */ + private function documentMatches($document): SplObjectStorage + { + // As in qp($document), the document element stands in for the document, so it can match. + $scope = new SplObjectStorage(); + if ($document->documentElement) { + $scope->offsetSet($document->documentElement); + } + + return (new DOMTraverser($scope, false, null, $this->psHandler))->find($this->selector)->matches(); + } + /** * Alias of matching(). * diff --git a/src/Helpers/QueryFilters.php b/src/Helpers/QueryFilters.php index 3141d0e..8b73c7b 100644 --- a/src/Helpers/QueryFilters.php +++ b/src/Helpers/QueryFilters.php @@ -538,9 +538,11 @@ public function lastChild(): Query /** * Get all siblings after an element until the selector is reached. * - * For each element in the DOMQuery, get all siblings that appear after - * it. If a selector is passed in, then only siblings that match the - * selector will be included. + * For each element in the DOMQuery, get the siblings that appear after it, + * stopping before the first one that matches the selector. + * + * As in jQuery, a positional selector (`:first`, `:eq()`, ...) counts across the + * whole document, so `nextUntil(':eq(3)')` stops at the document's fourth element. * * @param string $selector * A valid CSS 3 selector. @@ -564,8 +566,9 @@ public function nextUntil($selector = null): Query * Get the previous siblings for each element in the DOMQuery * until the selector is reached. * - * For each element in the DOMQuery, get all previous siblings. If a - * selector is provided, only matching siblings will be retrieved. + * For each element in the DOMQuery, get the previous siblings, stopping + * before the first one that matches the selector. A positional selector + * counts across the whole document, as in nextUntil(). * * @param string $selector * A valid CSS 3 selector. @@ -589,7 +592,8 @@ public function prevUntil($selector = null): Query /** * Get all ancestors of each element in the DOMQuery until the selector is reached. * - * If a selector is present, only matching ancestors will be retrieved. + * The walk stops before the first ancestor that matches the selector. A positional + * selector counts across the whole document, as in nextUntil(). * * @param string $selector * A valid CSS 3 Selector. @@ -677,7 +681,8 @@ public function not($selector): Query * present element. If the present element does not match, this traverses up * the ancestry chain (e.g. checks each parent) looking for an item that matches. * - * It is provided for jQuery 1.3 compatibility. + * It is provided for jQuery 1.3 compatibility. As in jQuery, a positional selector + * (`:first`, `:eq()`, ...) never matches. * * @param string $selector * A CSS Selector to match. @@ -690,7 +695,7 @@ public function not($selector): Query public function closest($selector): Query { // As in jQuery, nothing is closest to an empty selector. - if (empty($selector)) { + if (empty($selector) || Util::selectorHasPositionalPseudoClass($selector)) { return $this->inst(new SplObjectStorage(), null); } @@ -701,7 +706,7 @@ public function closest($selector): Query * Get the immediate parent of each element in the DOMQuery. * * If a selector is passed, this will return the nearest matching parent for - * each element in the DOMQuery. + * each element in the DOMQuery. A positional selector is tested as in next(). * * @param string $selector * A valid CSS3 selector. @@ -744,6 +749,10 @@ public function parents($selector = null): Query * * If a selector is provided, the next matching sibling will be returned. * + * A positional selector (`:first`, `:eq()`, ...) is tested against each sibling + * alone, so `:first` matches any of them and `:eq(1)` none. jQuery filters the + * combined results instead, which differs only for a set of more than one element. + * * @param string $selector * A CSS3 selector. * @@ -792,7 +801,7 @@ public function nextAll($selector = null): Query * * For each element in the DOMQuery, this retrieves the previous sibling * (if any). If a selector is supplied, it retrieves the first matching - * sibling (if any is found). + * sibling (if any is found). A positional selector is tested as in next(). * * @param string $selector * A valid CSS 3 selector. @@ -963,6 +972,7 @@ private function walk(string $axis, $selector, bool $first, bool $inclusive = fa { $found = new SplObjectStorage(); $matcher = empty($selector) ? null : new NodeMatcher($selector); + foreach ($this->matches as $m) { $n = $inclusive ? $m : $m->{$axis}; for (; $n !== null && $n->nodeType !== XML_DOCUMENT_NODE; $n = $n->{$axis}) { @@ -979,7 +989,8 @@ private function walk(string $axis, $selector, bool $first, bool $inclusive = fa } // Every element beyond one already collected was collected with it, up to the same stop. - if ($found->offsetExists($n) || ($matcher !== null && $matcher->matches($n))) { + // jQuery tests the stop with .is(), so a positional stop counts across the document. + if ($found->offsetExists($n) || ($matcher !== null && $matcher->matchesInDocument($n))) { break; } $found->offsetSet($n); diff --git a/tests/Issues/Issue118Test.php b/tests/Issues/Issue118Test.php new file mode 100644 index 0000000..d9c9113 --- /dev/null +++ b/tests/Issues/Issue118Test.php @@ -0,0 +1,88 @@ +'; + + // Document order: r, s1, t1, u1, s2, t2, u2. + private const ANCESTORS = ''; + + public function testNextUntilFromTheIssue() + { + $xml = '
'; + + // The document's :eq(1) is #a itself, which the walk never reaches. + $this->assertSame('b,c,d', $this->idList(qp($xml, '#a')->nextUntil(':eq(1)'))); + $this->assertSame('b', $this->idList(qp($xml, '#a')->nextUntil(':eq(3)'))); + } + + public function testNextUntilCountsAPositionalStopAcrossTheDocument() + { + $this->assertSame('b,c,d,e', $this->idList($this->selectIds(self::SIBLINGS, 'a')->nextUntil(':first'))); + $this->assertSame('b', $this->idList($this->selectIds(self::SIBLINGS, 'a')->nextUntil(':eq(3)'))); + $this->assertSame('b', $this->idList($this->selectIds(self::SIBLINGS, 'a')->nextUntil('i:eq(2)'))); + $this->assertSame('b,c,d', $this->idList($this->selectIds(self::SIBLINGS, 'b', 'a')->nextUntil(':last'))); + $this->assertSame('', $this->idList($this->selectIds(self::SIBLINGS, 'a')->nextUntil('i:not(:first)'))); + } + + public function testPrevUntilCountsAPositionalStopAcrossTheDocument() + { + $this->assertSame('d,c,b,a', $this->idList($this->selectIds(self::SIBLINGS, 'e')->prevUntil(':first'))); + $this->assertSame('d,c', $this->idList($this->selectIds(self::SIBLINGS, 'e')->prevUntil(':eq(2)'))); + $this->assertSame('d,c', $this->idList($this->selectIds(self::SIBLINGS, 'e')->prevUntil('.k:first'))); + } + + public function testParentsUntilCountsAPositionalStopAcrossTheDocument() + { + $this->assertSame('t2,s2', $this->idList($this->selectIds(self::ANCESTORS, 'u2')->parentsUntil(':first'))); + $this->assertSame('t2', $this->idList($this->selectIds(self::ANCESTORS, 'u2')->parentsUntil(':eq(4)'))); + } + + public function testClosestNeverMatchesAPositionalSelector() + { + $this->assertSame('', $this->idList($this->selectIds(self::ANCESTORS, 'u1')->closest(':first'))); + $this->assertSame('', $this->idList($this->selectIds(self::ANCESTORS, 'u1')->closest('t:first'))); + $this->assertSame('', $this->idList($this->selectIds(self::ANCESTORS, 'u2', 'u1')->closest(':eq(0)'))); + $this->assertSame('t1', $this->idList($this->selectIds(self::ANCESTORS, 'u1')->closest('t'))); + } + + /** + * next(), prev() and parent() test each candidate on its own, which matches jQuery + * for a single element. For [c, a], jQuery's next(':first') is just d. + */ + public function testNextPrevAndParentTestAPositionalSelectorPerNode() + { + $this->assertSame('b', $this->idList($this->selectIds(self::SIBLINGS, 'a')->next(':first'))); + $this->assertSame('', $this->idList($this->selectIds(self::SIBLINGS, 'a')->next(':eq(1)'))); + $this->assertSame('d,b', $this->idList($this->selectIds(self::SIBLINGS, 'c', 'a')->next(':first'))); + $this->assertSame('d', $this->idList($this->selectIds(self::SIBLINGS, 'e')->prev(':last'))); + $this->assertSame('t1', $this->idList($this->selectIds(self::ANCESTORS, 'u1')->parent(':first'))); + } + + /** + * 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. + */ + public function testSetMethodsCountPositionsInDocumentOrder() + { + $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('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'))); + } +}