From 4fec833e151fd0fe29f61aff2f113b044c49ee7f Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 13:29:09 +1100 Subject: [PATCH 1/2] Walk sibling and ancestor axes through one helper next(), prev(), nextAll(), prevAll(), nextUntil(), prevUntil(), parent(), parents(), parentsUntil() and closest() were each a copy of the same loop. They now share QueryFilters::walk(). Fixes #115. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/Helpers/QueryFilters.php | 229 +++++++++---------------------- tests/QueryPath/DOMQueryTest.php | 23 ++++ 3 files changed, 92 insertions(+), 161 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14907c4..30a4d7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ QueryPath Changelog - **Behaviour change.** Fix `:has()` never matching in `find()`, `filter()` and the other methods that use `QueryPath\CSS\DOMTraverser`, so that `ul:has(li)` selected nothing and `:not(:has(li))` selected everything (issue #121). `:has()` now matches an element with a descendant matching its argument, and supports relative selectors: `ul:has(> li)` (a child), `h2:has(+ p)` (the next sibling) and `h2:has(~ p)` (a later sibling). A positional pseudo-class in the argument indexes each element's own matches, so `ul:has(li:eq(2))` selects every list with at least three items - **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 # 4.2.0 diff --git a/src/Helpers/QueryFilters.php b/src/Helpers/QueryFilters.php index ea3a176..1b4ee1d 100644 --- a/src/Helpers/QueryFilters.php +++ b/src/Helpers/QueryFilters.php @@ -557,21 +557,7 @@ public function lastChild(): Query */ public function nextUntil($selector = null): Query { - $found = new SplObjectStorage(); - $matcher = new NodeMatcher($selector); - foreach ($this->matches as $m) { - while (isset($m->nextSibling)) { - $m = $m->nextSibling; - if ($m->nodeType === XML_ELEMENT_NODE) { - if (null !== $selector && $matcher->matches($m)) { - break; - } - $found->offsetSet($m); - } - } - } - - return $this->inst($found, null); + return $this->inst($this->walk('nextSibling', $selector, 'until'), null); } /** @@ -597,22 +583,7 @@ public function nextUntil($selector = null): Query */ public function prevUntil($selector = null): Query { - $found = new SplObjectStorage(); - $matcher = new NodeMatcher($selector); - foreach ($this->matches as $m) { - while (isset($m->previousSibling)) { - $m = $m->previousSibling; - if ($m->nodeType === XML_ELEMENT_NODE) { - if (null !== $selector && $matcher->matches($m)) { - break; - } - - $found->offsetSet($m); - } - } - } - - return $this->inst($found, null); + return $this->inst($this->walk('previousSibling', $selector, 'until'), null); } /** @@ -634,21 +605,7 @@ public function prevUntil($selector = null): Query */ public function parentsUntil($selector = null): Query { - $found = new SplObjectStorage(); - $matcher = new NodeMatcher($selector); - foreach ($this->matches as $m) { - while ($m->parentNode->nodeType !== XML_DOCUMENT_NODE) { - $m = $m->parentNode; - if ($m->nodeType === XML_ELEMENT_NODE) { - if (! empty($selector) && $matcher->matches($m)) { - break; - } - $found->offsetSet($m); - } - } - } - - return $this->inst($this->sortReverseDocumentOrder($found), null); + return $this->inst($this->walk('parentNode', $selector, 'until'), null); } /** @@ -732,19 +689,12 @@ public function not($selector): Query */ public function closest($selector): Query { - $found = new SplObjectStorage(); - $matcher = new NodeMatcher($selector); - foreach ($this->matches as $m) { - for (; $m !== null && $m->nodeType !== XML_DOCUMENT_NODE; $m = $m->parentNode) { - if ($matcher->matches($m)) { - $found->offsetSet($m); - break; - } - } + // As in jQuery, nothing is closest to an empty selector. + if (empty($selector)) { + return $this->inst(new SplObjectStorage(), null); } - // XXX: Should this be an in-place modification? - return $this->inst($found, null); + return $this->inst($this->walk('parentNode', $selector, 'first', true), null); } /** @@ -765,7 +715,7 @@ public function closest($selector): Query */ public function parent($selector = null): Query { - return $this->getParentElements($selector, true); + return $this->inst($this->walk('parentNode', $selector, 'first'), null); } /** @@ -786,53 +736,7 @@ public function parent($selector = null): Query */ public function parents($selector = null): Query { - return $this->getParentElements($selector, false); - } - - /** - * Get ancestor(s) of each element in the DOMQuery. - * - * If a selector is present, only matching ancestors will be retrieved. - * - * @param string|null $selector - * A valid CSS 3 Selector. - * @param bool $immediate - * If function should return only the immediate parent - * - * @return DOMQuery - * A DOMNode object containing the matching ancestors. - * @throws ParseException - * @throws Exception - */ - private function getParentElements(?string $selector, bool $immediate): Query - { - $found = new SplObjectStorage(); - $matcher = new NodeMatcher($selector); - foreach ($this->matches as $m) { - while ($m->parentNode && $m->parentNode->nodeType !== XML_DOCUMENT_NODE) { - $m = $m->parentNode; - if ($m->nodeType !== XML_ELEMENT_NODE) { - continue; - } - // parent() stops at the nearest matching ancestor of each element, so it tests - // the walk as it goes; parents() collects every ancestor and filters below. - if (! $immediate) { - $found->offsetSet($m); - } elseif (empty($selector) || $matcher->matches($m)) { - $found->offsetSet($m); - break; - } - } - } - - if ($immediate) { - return $this->inst($found, null); - } - - // jQuery returns the ancestors of a multi-element set in reverse document order, with - // duplicates removed. The selector is applied after that ordering, so a positional - // selector counts from the closest ancestor. - return $this->inst($this->filterCandidates($this->sortReverseDocumentOrder($found), $selector), null); + return $this->inst($this->walk('parentNode', $selector, 'all'), null); } /** @@ -856,19 +760,7 @@ private function getParentElements(?string $selector, bool $immediate): Query */ public function next($selector = null): Query { - $found = new SplObjectStorage(); - $matcher = new NodeMatcher($selector); - foreach ($this->matches as $m) { - while (isset($m->nextSibling)) { - $m = $m->nextSibling; - if ($m->nodeType === XML_ELEMENT_NODE && (empty($selector) || $matcher->matches($m))) { - $found->offsetSet($m); - break; - } - } - } - - return $this->inst($found, null); + return $this->inst($this->walk('nextSibling', $selector, 'first'), null); } /** @@ -892,21 +784,7 @@ public function next($selector = null): Query */ public function nextAll($selector = null): Query { - $found = new SplObjectStorage(); - foreach ($this->matches as $m) { - while (isset($m->nextSibling)) { - $m = $m->nextSibling; - // Every sibling beyond one already collected was collected with it. - if ($found->offsetExists($m)) { - break; - } - if ($m->nodeType === XML_ELEMENT_NODE) { - $found->offsetSet($m); - } - } - } - - return $this->inst($this->filterCandidates($found, $selector), null); + return $this->inst($this->walk('nextSibling', $selector, 'all'), null); } /** @@ -931,19 +809,7 @@ public function nextAll($selector = null): Query */ public function prev($selector = null): Query { - $found = new SplObjectStorage(); - $matcher = new NodeMatcher($selector); - foreach ($this->matches as $m) { - while (isset($m->previousSibling)) { - $m = $m->previousSibling; - if ($m->nodeType === XML_ELEMENT_NODE && (empty($selector) || $matcher->matches($m))) { - $found->offsetSet($m); - break; - } - } - } - - return $this->inst($found, null); + return $this->inst($this->walk('previousSibling', $selector, 'first'), null); } /** @@ -967,21 +833,7 @@ public function prev($selector = null): Query */ public function prevAll($selector = null): Query { - $found = new SplObjectStorage(); - foreach ($this->matches as $m) { - while (isset($m->previousSibling)) { - $m = $m->previousSibling; - // Every sibling beyond one already collected was collected with it. - if ($found->offsetExists($m)) { - break; - } - if ($m->nodeType === XML_ELEMENT_NODE) { - $found->offsetSet($m); - } - } - } - - return $this->inst($this->filterCandidates($found, $selector), null); + return $this->inst($this->walk('previousSibling', $selector, 'all'), null); } /** @@ -1090,6 +942,61 @@ public function siblings($selector = null): Query return $this->inst($this->filterCandidates($found, $selector), null); } + /** + * Walk from each matched element along a DOM axis, collecting the elements passed. + * + * @param string $axis + * The property to follow: 'nextSibling', 'previousSibling' or 'parentNode'. + * @param string|null $selector + * An optional CSS selector. + * @param string $mode + * 'all' collects every element on the axis, then keeps those matching the selector. + * 'first' collects the first element that matches the selector (any element, if there is none). + * 'until' collects every element before the first one that matches the selector. + * @param bool $inclusive + * Start the walk at the matched element itself rather than at its neighbour. + * + * @return SplObjectStorage + * @throws ParseException + */ + private function walk(string $axis, $selector, string $mode, bool $inclusive = false): SplObjectStorage + { + $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}) { + if ($n->nodeType !== XML_ELEMENT_NODE) { + continue; + } + + if ($mode === 'all') { + // Every element beyond one already collected was collected with it. + if ($found->offsetExists($n)) { + break; + } + $found->offsetSet($n); + } elseif ($mode === 'first') { + if ($matcher === null || $matcher->matches($n)) { + $found->offsetSet($n); + break; + } + } elseif ($matcher !== null && $matcher->matches($n)) { + break; + } else { + $found->offsetSet($n); + } + } + } + + if ($mode === 'all' && $matcher !== null) { + $found = $matcher->filter($found); + } + + // jQuery returns the ancestors of a multi-element set in reverse document order. + return $axis === 'parentNode' && $mode !== 'first' ? $this->sortReverseDocumentOrder($found) : $found; + } + /** * Sort a set of nodes into reverse document order. * diff --git a/tests/QueryPath/DOMQueryTest.php b/tests/QueryPath/DOMQueryTest.php index 2e78b58..17f48ef 100644 --- a/tests/QueryPath/DOMQueryTest.php +++ b/tests/QueryPath/DOMQueryTest.php @@ -2516,6 +2516,29 @@ public function testParentsUntil() $this->assertEquals(2, qp($file, 'li')->parentsUntil('root')->count()); } + public function testParentsUntilOutsideTheDocument() + { + $doc = qp('')->get(0)->ownerDocument; + $parent = $doc->createElement('parent'); + $child = $parent->appendChild($doc->createElement('child')); + + $this->assertSame('parent', qp($child)->parentsUntil('root')->tag()); + $this->assertSame('parent', qp($child)->parentsUntil()->tag()); + } + + public function testTraversalTreatsAnEmptySelectorAsNone() + { + $xml = ''; + + $this->assertSame('c', qp($xml, 'b')->nextUntil('')->tag()); + $this->assertSame('a', qp($xml, 'b')->prevUntil('')->tag()); + $this->assertSame('r', qp($xml, 'b')->parentsUntil('')->tag()); + $this->assertSame('c', qp($xml, 'b')->next('')->tag()); + $this->assertSame('a', qp($xml, 'b')->prev('')->tag()); + $this->assertSame('r', qp($xml, 'b')->parent('')->tag()); + $this->assertCount(0, qp($xml, 'b')->closest('')); + } + public function testSort() { $xml = '1521'; From bff4d3252f97d2435b59fb62b87caf7bad7f842c Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 15:51:23 +1100 Subject: [PATCH 2/2] Collapse walk()'s modes to a first-match flag 'all' was 'until' with no stop selector, so walk() now takes a bool and the *Until() methods share nextAll()'s stop at an already-collected element. Ordering and filtering move back to the callers. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- src/Helpers/QueryFilters.php | 60 +++++++++++++++----------------- tests/QueryPath/DOMQueryTest.php | 19 ++++++++++ 3 files changed, 48 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30a4d7b..cbb6125 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,7 +55,7 @@ QueryPath Changelog - **Behaviour change.** Fix `:has()` never matching in `find()`, `filter()` and the other methods that use `QueryPath\CSS\DOMTraverser`, so that `ul:has(li)` selected nothing and `:not(:has(li))` selected everything (issue #121). `:has()` now matches an element with a descendant matching its argument, and supports relative selectors: `ul:has(> li)` (a child), `h2:has(+ p)` (the next sibling) and `h2:has(~ p)` (a later sibling). A positional pseudo-class in the argument indexes each element's own matches, so `ul:has(li:eq(2))` selects every list with at least three items - **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 +- 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 # 4.2.0 diff --git a/src/Helpers/QueryFilters.php b/src/Helpers/QueryFilters.php index 1b4ee1d..df379e1 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, 'until'), null); + return $this->inst($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, 'until'), null); + return $this->inst($this->walk('previousSibling', $selector, false), null); } /** @@ -605,7 +605,7 @@ public function prevUntil($selector = null): Query */ public function parentsUntil($selector = null): Query { - return $this->inst($this->walk('parentNode', $selector, 'until'), null); + return $this->inst($this->sortReverseDocumentOrder($this->walk('parentNode', $selector, false)), null); } /** @@ -694,7 +694,7 @@ public function closest($selector): Query return $this->inst(new SplObjectStorage(), null); } - return $this->inst($this->walk('parentNode', $selector, 'first', true), null); + return $this->inst($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, 'first'), null); + return $this->inst($this->walk('parentNode', $selector, true), null); } /** @@ -736,7 +736,11 @@ public function parent($selector = null): Query */ public function parents($selector = null): Query { - return $this->inst($this->walk('parentNode', $selector, 'all'), null); + // 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); } /** @@ -760,7 +764,7 @@ public function parents($selector = null): Query */ public function next($selector = null): Query { - return $this->inst($this->walk('nextSibling', $selector, 'first'), null); + return $this->inst($this->walk('nextSibling', $selector, true), null); } /** @@ -784,7 +788,7 @@ public function next($selector = null): Query */ public function nextAll($selector = null): Query { - return $this->inst($this->walk('nextSibling', $selector, 'all'), null); + return $this->inst($this->filterCandidates($this->walk('nextSibling', null, false), $selector), null); } /** @@ -809,7 +813,7 @@ public function nextAll($selector = null): Query */ public function prev($selector = null): Query { - return $this->inst($this->walk('previousSibling', $selector, 'first'), null); + return $this->inst($this->walk('previousSibling', $selector, true), null); } /** @@ -833,7 +837,7 @@ public function prev($selector = null): Query */ public function prevAll($selector = null): Query { - return $this->inst($this->walk('previousSibling', $selector, 'all'), null); + return $this->inst($this->filterCandidates($this->walk('previousSibling', null, false), $selector), null); } /** @@ -948,18 +952,18 @@ public function siblings($selector = null): Query * @param string $axis * The property to follow: 'nextSibling', 'previousSibling' or 'parentNode'. * @param string|null $selector - * An optional CSS selector. - * @param string $mode - * 'all' collects every element on the axis, then keeps those matching the selector. - * 'first' collects the first element that matches the selector (any element, if there is none). - * 'until' collects every element before the first one that matches the selector. + * An optional CSS selector. With $first, the element to collect; otherwise, the element + * to stop before. + * @param bool $first + * Collect only the first element matching the selector (the first element, if there is + * none), rather than every element up to the first match. * @param bool $inclusive * Start the walk at the matched element itself rather than at its neighbour. * * @return SplObjectStorage * @throws ParseException */ - private function walk(string $axis, $selector, string $mode, bool $inclusive = false): SplObjectStorage + private function walk(string $axis, $selector, bool $first, bool $inclusive = false): SplObjectStorage { $found = new SplObjectStorage(); $matcher = empty($selector) ? null : new NodeMatcher($selector); @@ -970,31 +974,23 @@ private function walk(string $axis, $selector, string $mode, bool $inclusive = f continue; } - if ($mode === 'all') { - // Every element beyond one already collected was collected with it. - if ($found->offsetExists($n)) { - break; - } - $found->offsetSet($n); - } elseif ($mode === 'first') { + if ($first) { if ($matcher === null || $matcher->matches($n)) { $found->offsetSet($n); break; } - } elseif ($matcher !== null && $matcher->matches($n)) { + continue; + } + + // Every element beyond one already collected was collected with it, up to the same stop. + if ($found->offsetExists($n) || ($matcher !== null && $matcher->matches($n))) { break; - } else { - $found->offsetSet($n); } + $found->offsetSet($n); } } - if ($mode === 'all' && $matcher !== null) { - $found = $matcher->filter($found); - } - - // jQuery returns the ancestors of a multi-element set in reverse document order. - return $axis === 'parentNode' && $mode !== 'first' ? $this->sortReverseDocumentOrder($found) : $found; + return $found; } /** diff --git a/tests/QueryPath/DOMQueryTest.php b/tests/QueryPath/DOMQueryTest.php index 17f48ef..01deac5 100644 --- a/tests/QueryPath/DOMQueryTest.php +++ b/tests/QueryPath/DOMQueryTest.php @@ -2526,6 +2526,25 @@ public function testParentsUntilOutsideTheDocument() $this->assertSame('parent', qp($child)->parentsUntil()->tag()); } + public function testUntilWalksFromElementsOnEachOthersPath() + { + $xml = ''; + + $tagsOf = function ($qp) { + $tags = []; + foreach ($qp->get() as $node) { + $tags[] = $node->tagName; + } + sort($tags); + + return $tags; + }; + + $this->assertSame(['b', 'c', 'f'], $tagsOf(qp($xml, 'a, b, e')->nextUntil('stop'))); + $this->assertSame(['a', 'b', 'e'], $tagsOf(qp($xml, 'c, f')->prevUntil('stop, x'))); + $this->assertSame(['x', 'y'], $tagsOf(qp($xml, 'z, y')->parentsUntil('r'))); + } + public function testTraversalTreatsAnEmptySelectorAsNone() { $xml = '';