From 1dda55a8e6534f3acc8dc02a9b4a323d7dd7d468 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 13:25:44 +1100 Subject: [PATCH 1/3] Add DOMTraverser::filterNodes() for matching the nodes in hand NodeMatcher, applySetFilter() and PseudoClass::is() each built an initialized DOMTraverser by hand and relied on its internals. They now share one entry point that returns the matching nodes in input order. DOMTraverser no longer consumes the parsed selector it is given, so PseudoClass caches parses and queries sharing a handler parse once. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/CSS/DOMTraverser.php | 63 +++++++++++--- src/CSS/DOMTraverser/PseudoClass.php | 43 +++++++--- src/Helpers/NodeMatcher.php | 38 ++------- tests/QueryPath/CSS/DOMTraverserTest.php | 100 ++++++++++++++++++++++- 5 files changed, 193 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ac9391..14907c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ QueryPath Changelog - Fix `QueryPath::VERSION` and `QueryPath::VERSION_MAJOR` still reporting `3.2.2` and `3`. They now report `4.2.0` and `4`, and both are deprecated in favour of `\Composer\InstalledVersions::getPrettyVersion('gravitypdf/querypath')`, which cannot drift from the installed version (issue #83) - **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 # 4.2.0 diff --git a/src/CSS/DOMTraverser.php b/src/CSS/DOMTraverser.php index e6c374a..c17e0dc 100644 --- a/src/CSS/DOMTraverser.php +++ b/src/CSS/DOMTraverser.php @@ -7,6 +7,7 @@ use DOMDocument; use DOMElement; +use DOMNode; use DOMNodeList; use DOMXPath; use QueryPath\CSS\DOMTraverser\Util; @@ -164,20 +165,58 @@ public function debug($msg) */ public function find($selector): DOMTraverser { - // Setup - $handler = new Selector(); - $parser = new Parser($selector, $handler); - $parser->parse(); + return $this->findGroups($this->psHandler->parse($selector)); + } + + /** + * Reduce a set of nodes to those that, taken as candidates themselves, match a selector. + * + * Unlike find(), which searches the descendants of the nodes it starts from, only the given + * nodes can match: for ``, `b` matches the `b` element and not `a`. Combinators are + * still resolved against the document, so `div > p` works, and positional pseudo-classes + * (`:first`, `:eq(n)`, ...) count within the given set. Nodes that are not elements never match. + * + * @param SplObjectStorage $nodes + * The candidates, all from one document. + * @param string $selector + * A CSS selector. + * @param DOMNode|null $scopeNode + * The element `:scope` matches. Defaults to the document element, as in find(). + * @param PseudoClass|null $psHandler + * Share a handler across calls, so they reuse its parsed selectors and, while it has a query + * open (see PseudoClass::beginQuery()), its sibling index. + * + * @return SplObjectStorage + * The matching nodes, in the order they were given. + * @throws ParseException + */ + public static function filterNodes( + SplObjectStorage $nodes, + $selector, + $scopeNode = null, + ?PseudoClass $psHandler = null + ): SplObjectStorage { + $found = new SplObjectStorage(); + if (count($nodes) === 0) { + return $found; + } + + $traverser = new self($nodes, true, $scopeNode, $psHandler); + $matched = $traverser->find($selector)->matches(); + + // The traverser returns document order. + foreach ($nodes as $node) { + if ($matched->offsetExists($node)) { + $found->offsetSet($node); + } + } - return $this->findGroups($handler); + return $found; } /** * Find the matches of an already-parsed selector. * - * The traverser modifies the SimpleSelector objects it is given, so they must - * not be reused. - * * @param iterable $handler * The SimpleSelector[] groups, as a Selector yields them. * @@ -289,7 +328,10 @@ protected function resolveSubject(array $selectorGroup, int $index): SplObjectSt return $node instanceof DOMElement; }); } else { - $candidates = $this->initialMatch($selectorGroup[$index], $this->matches); + // initialMatch() consumes the parts of the simple selector it matched on. Give it a copy, + // so the caller's parsed selector can be reused. + $selectorGroup[$index] = clone $selectorGroup[$index]; + $candidates = $this->initialMatch($selectorGroup[$index], $this->matches); } $setFilters = $selectorGroup[$index]->setFilterPseudoClasses(); @@ -354,8 +396,7 @@ protected function applySetFilter(array $nodes, array $pseudoClass): array return []; } - $traverser = new self($this->toMatches($nodes), true, $this->scopeNode, $this->psHandler); - $inner = $traverser->find($value)->matches(); + $inner = self::filterNodes($this->toMatches($nodes), $value, $this->scopeNode, $this->psHandler); $filtered = []; foreach ($nodes as $node) { diff --git a/src/CSS/DOMTraverser/PseudoClass.php b/src/CSS/DOMTraverser/PseudoClass.php index 62fe6b7..029a1fc 100644 --- a/src/CSS/DOMTraverser/PseudoClass.php +++ b/src/CSS/DOMTraverser/PseudoClass.php @@ -62,6 +62,13 @@ class PseudoClass */ protected $relativeSelectors = []; + /** + * Parsed selectors, keyed by the selector string. See parse(). + * + * @var Selector[] + */ + protected $selectors = []; + /** * Start remembering sibling positions, so the :nth-* pseudo-classes index * each sibling list once rather than walking it for every element. @@ -82,6 +89,30 @@ public function endQuery(): void } } + /** + * Parse a selector, reusing an earlier parse of the same string. + * + * DOMTraverser does not modify a parsed selector, so every query that shares this handler can + * share the parse. + * + * @param string $selector + * + * @return Selector + * @throws ParseException + */ + public function parse($selector): Selector + { + if (! isset($this->selectors[$selector])) { + $handler = new Selector(); + $parser = new Parser($selector, $handler); + $parser->parse(); + + $this->selectors[$selector] = $handler; + } + + return $this->selectors[$selector]; + } + /** * Tests whether the given element matches the given pseudoclass. * @@ -502,15 +533,9 @@ protected function findsAny(DOMElement $root, DOMElement $scope, array $group, b { $splos = new SPLObjectStorage(); $splos->offsetSet($root); - - // The traverser consumes the element and ID selectors as it matches them, so give it a copy. - $copy = []; - foreach ($group as $simpleSelector) { - $copy[] = clone $simpleSelector; - } $traverser = new DOMTraverser($splos, $self, $scope, $this); - return count($traverser->findGroups([$copy])->matches()) > 0; + return count($traverser->findGroups([$group])->matches()) > 0; } /** @@ -522,10 +547,8 @@ protected function is($node, $selector): bool { $splos = new SPLObjectStorage(); $splos->offsetSet($node); - $traverser = new DOMTraverser($splos, true, null, $this); - $results = $traverser->find($selector)->matches(); - return count($results) > 0; + return count(DOMTraverser::filterNodes($splos, $selector, null, $this)) > 0; } /** diff --git a/src/Helpers/NodeMatcher.php b/src/Helpers/NodeMatcher.php index 27094c8..25b28c2 100644 --- a/src/Helpers/NodeMatcher.php +++ b/src/Helpers/NodeMatcher.php @@ -11,10 +11,7 @@ /** * Test nodes that are already in hand against a CSS selector. * - * Unlike find(), which searches the descendants of the nodes it is given, the - * supplied nodes are themselves the only candidates: for ``, `b` matches the - * `b` element and not `a`. Combinators are still resolved against the real document, so - * full selectors (e.g. `div > p`) keep working. + * The supplied nodes are themselves the only candidates, as in DOMTraverser::filterNodes(). * * Prefer the set methods wherever there is a set. A per-node pass cannot answer a selector * that describes a position within the set (:first, :eq(n), :odd, ...), because each node @@ -22,8 +19,9 @@ * * matches() remembers each per-node answer, because the walk-and-stop traversals * (closest(), nextUntil(), ...) revisit the same nodes from every starting element, and - * every query shares one sibling index for the :nth-* pseudo-classes. An instance must not - * outlive the call that made it, or a later DOM change could leave it holding stale answers. + * every query shares one parse of the selector and one sibling index for the :nth-* + * pseudo-classes. An instance must not outlive the call that made it, or a later DOM change + * could leave it holding stale answers. */ final class NodeMatcher { @@ -90,21 +88,7 @@ public function matches($node): bool */ public function filter(SplObjectStorage $nodes): SplObjectStorage { - if (count($nodes) === 0) { - return new SplObjectStorage(); - } - - $matched = $this->matching($nodes); - - // The traverser returns document order; callers such as parents() need their own. - $found = new SplObjectStorage(); - foreach ($nodes as $node) { - if ($matched->offsetExists($node)) { - $found->offsetSet($node); - } - } - - return $found; + return $this->matching($nodes); } /** @@ -117,11 +101,11 @@ public function filter(SplObjectStorage $nodes): SplObjectStorage */ public function matchesAny(SplObjectStorage $nodes): bool { - return count($nodes) > 0 && count($this->matching($nodes)) > 0; + return count($this->matching($nodes)) > 0; } /** - * Find the nodes that match the selector, in document order. + * Find the nodes that match the selector, in the order they were given. * * @param SplObjectStorage $nodes * @@ -130,12 +114,6 @@ public function matchesAny(SplObjectStorage $nodes): bool */ public function matching(SplObjectStorage $nodes): SplObjectStorage { - // "Initialized" mode makes the traverser take the supplied set as its candidates rather - // than seeding them with a descendant search. The scope node is left at its default (the - // document element) so that `:scope` means the same here as in find(); scoping to a - // candidate would make every candidate match `:scope`. - $traverser = new DOMTraverser($nodes, true, null, $this->psHandler); - - return $traverser->find($this->selector)->matches(); + return DOMTraverser::filterNodes($nodes, $this->selector, null, $this->psHandler); } } diff --git a/tests/QueryPath/CSS/DOMTraverserTest.php b/tests/QueryPath/CSS/DOMTraverserTest.php index f065708..dd4c468 100644 --- a/tests/QueryPath/CSS/DOMTraverserTest.php +++ b/tests/QueryPath/CSS/DOMTraverserTest.php @@ -3,7 +3,9 @@ namespace QueryPathTests\CSS; use DOMDocument; +use DOMXPath; use QueryPath\CSS\DOMTraverser; +use QueryPath\CSS\DOMTraverser\PseudoClass; use QueryPathTests\TestCase; use SPLObjectStorage; @@ -376,7 +378,7 @@ public function testMultipleSelectors() /** * The
      children are, by position: l1 p1 l2 l3 p2 l4 l5 p3. */ - protected $nthXml = '
        ' + protected $nthXml = '
          ' . 'text

        • ' . 'text

        • text' . '

        '; @@ -475,4 +477,100 @@ public function testNthPseudoClassesAfterMutation() $this->assertSame(['c'], $this->ids($qp->find('li:nth-child(3)')->get())); $this->assertSame(['new', 'c'], $this->ids($qp->find('li:nth-child(odd)')->get())); } + /** + * Load $nthXml and collect the nodes with the given IDs, in the given order. + */ + protected function nthNodes(string ...$ids): SplObjectStorage + { + $dom = new DOMDocument('1.0'); + $dom->loadXML($this->nthXml); + + $xpath = new DOMXPath($dom); + $nodes = new SplObjectStorage(); + foreach ($ids as $id) { + $nodes->offsetSet($xpath->query("//*[@id='$id']")->item(0)); + } + + return $nodes; + } + + public function filterNodesProvider(): array + { + return [ + 'keeps the given order' => [['l4', 'p1', 'l2', 'l1'], 'li', ['l4', 'l2', 'l1']], + 'tests the nodes, not below' => [['ul', 'l1'], 'ul', ['ul']], + 'resolves combinators' => [['o1', 'l2', 'l1'], 'ul > li', ['l2', 'l1']], + 'selector list' => [['p2', 'o2', 'l1'], 'p, ol > li', ['p2', 'o2']], + ':first is first in the set' => [['l4', 'l2', 'l3'], 'li:first', ['l2']], + ':eq() counts within the set' => [['l4', 'l2', 'l3'], ':eq(1)', ['l3']], + ':odd counts within the set' => [['l5', 'l4', 'l2', 'l1'], 'li:odd', ['l5', 'l2']], + ':not() with a positional arg' => [['l4', 'l2', 'l3'], 'li:not(:first)', ['l4', 'l3']], + ':matches() with positional arg' => [['l4', 'l2', 'l3'], ':matches(:last)', ['l4']], + ':nth-child counts siblings' => [['l4', 'l2', 'l3'], ':nth-child(4)', ['l3']], + 'document element' => [['root', 'ul'], 'root', ['root']], + ':root' => [['ul', 'root'], ':root', ['root']], + ':scope is the document element' => [['ul', 'root'], ':scope', ['root']], + ]; + } + + /** + * @dataProvider filterNodesProvider + */ + public function testFilterNodes(array $ids, $selector, array $expected) + { + $this->assertSame($expected, $this->ids(DOMTraverser::filterNodes($this->nthNodes(...$ids), $selector))); + } + + public function testFilterNodesSkipsNodesThatAreNotElements() + { + $ul = $this->nthNodes('ul'); + $ul->rewind(); + + // Every child of the
          , last first: text and comments interleave the elements. + $nodes = new SplObjectStorage(); + for ($child = $ul->current()->lastChild; $child !== null; $child = $child->previousSibling) { + $nodes->offsetSet($child); + } + + $this->assertSame(['p3', 'l5', 'l4', 'p2', 'l3', 'l2', 'p1', 'l1'], $this->ids(DOMTraverser::filterNodes($nodes, '*'))); + $this->assertSame(['l5', 'l4', 'l3', 'l2', 'l1'], $this->ids(DOMTraverser::filterNodes($nodes, ':not(p)'))); + $this->assertSame(['l1'], $this->ids(DOMTraverser::filterNodes($nodes, ':first'))); + } + + public function testFilterNodesWithAnEmptySet() + { + $this->assertCount(0, DOMTraverser::filterNodes(new SplObjectStorage(), 'li')); + $this->assertCount(0, DOMTraverser::filterNodes(new SplObjectStorage(), 'li:first')); + } + + public function testFilterNodesWithAScopeNode() + { + $nodes = $this->nthNodes('ul', 'l1', 'l2', 'o1'); + $nodes->rewind(); + $ul = $nodes->current(); + + $this->assertSame(['ul'], $this->ids(DOMTraverser::filterNodes($nodes, ':scope', $ul))); + $this->assertSame(['l1', 'l2'], $this->ids(DOMTraverser::filterNodes($nodes, ':scope > li', $ul))); + } + + /** + * A shared handler hands every query the same parsed selector, which the traverser must leave + * as it found it. + * + * @testWith ["#ul li", ["l1", "l2", "l3", "l4", "l5"]] + * [".x", ["x"]] + * ["ul:first > li", ["l1", "l2", "l3", "l4", "l5"]] + */ + public function testQueriesSharingAHandlerReuseTheParsedSelector($selector, array $expected) + { + $dom = new DOMDocument('1.0'); + $dom->loadXML(str_replace('
            ', '
              ', $this->nthXml)); + $splos = new SplObjectStorage(); + $splos->offsetSet($dom->documentElement); + + $ps = new PseudoClass(); + foreach ([1, 2] as $run) { + $this->assertSame($expected, $this->ids((new DOMTraverser($splos, false, null, $ps))->find($selector)->matches())); + } + } } From eab8179d6c3c4368029941cd0675ca2cfe0d70b6 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 13:27:45 +1100 Subject: [PATCH 2/3] Tidy filterNodes() and its callers Co-Authored-By: Claude Opus 5.5 --- src/CSS/DOMTraverser.php | 11 +++++------ src/CSS/DOMTraverser/PseudoClass.php | 2 ++ src/Helpers/NodeMatcher.php | 9 ++++----- tests/QueryPath/CSS/DOMTraverserTest.php | 3 ++- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/CSS/DOMTraverser.php b/src/CSS/DOMTraverser.php index c17e0dc..ee7429c 100644 --- a/src/CSS/DOMTraverser.php +++ b/src/CSS/DOMTraverser.php @@ -196,15 +196,18 @@ public static function filterNodes( $scopeNode = null, ?PseudoClass $psHandler = null ): SplObjectStorage { - $found = new SplObjectStorage(); if (count($nodes) === 0) { - return $found; + return new SplObjectStorage(); } $traverser = new self($nodes, true, $scopeNode, $psHandler); $matched = $traverser->find($selector)->matches(); + if (count($matched) < 2) { + return $matched; + } // The traverser returns document order. + $found = new SplObjectStorage(); foreach ($nodes as $node) { if ($matched->offsetExists($node)) { $found->offsetSet($node); @@ -392,10 +395,6 @@ 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. - if (empty($nodes)) { - return []; - } - $inner = self::filterNodes($this->toMatches($nodes), $value, $this->scopeNode, $this->psHandler); $filtered = []; diff --git a/src/CSS/DOMTraverser/PseudoClass.php b/src/CSS/DOMTraverser/PseudoClass.php index 029a1fc..588955b 100644 --- a/src/CSS/DOMTraverser/PseudoClass.php +++ b/src/CSS/DOMTraverser/PseudoClass.php @@ -26,6 +26,8 @@ /** * The PseudoClass handler. * + * It also holds what queries can share: parsed selectors and, while a query is open, sibling + * positions. Nested and per-node queries are passed their caller's handler for that reason. */ class PseudoClass { diff --git a/src/Helpers/NodeMatcher.php b/src/Helpers/NodeMatcher.php index 25b28c2..ea6024d 100644 --- a/src/Helpers/NodeMatcher.php +++ b/src/Helpers/NodeMatcher.php @@ -76,12 +76,9 @@ public function matches($node): bool } /** - * Reduce a set of nodes to those that match the selector, preserving their order. - * - * Nodes that are not elements can never match a CSS selector, and are skipped. + * Alias of matching(). * * @param SplObjectStorage $nodes - * The candidate nodes. * * @return SplObjectStorage * @throws ParseException @@ -105,7 +102,9 @@ public function matchesAny(SplObjectStorage $nodes): bool } /** - * Find the nodes that match the selector, in the order they were given. + * Reduce a set of nodes to those that match the selector, in the order they were given. + * + * Nodes that are not elements can never match a CSS selector, and are skipped. * * @param SplObjectStorage $nodes * diff --git a/tests/QueryPath/CSS/DOMTraverserTest.php b/tests/QueryPath/CSS/DOMTraverserTest.php index dd4c468..d131cc5 100644 --- a/tests/QueryPath/CSS/DOMTraverserTest.php +++ b/tests/QueryPath/CSS/DOMTraverserTest.php @@ -477,6 +477,7 @@ public function testNthPseudoClassesAfterMutation() $this->assertSame(['c'], $this->ids($qp->find('li:nth-child(3)')->get())); $this->assertSame(['new', 'c'], $this->ids($qp->find('li:nth-child(odd)')->get())); } + /** * Load $nthXml and collect the nodes with the given IDs, in the given order. */ @@ -569,7 +570,7 @@ public function testQueriesSharingAHandlerReuseTheParsedSelector($selector, arra $splos->offsetSet($dom->documentElement); $ps = new PseudoClass(); - foreach ([1, 2] as $run) { + for ($i = 0; $i < 2; $i++) { $this->assertSame($expected, $this->ids((new DOMTraverser($splos, false, null, $ps))->find($selector)->matches())); } } From c55951c78c6a04a58467d5804c56663d76c9aa21 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 15:45:12 +1100 Subject: [PATCH 3/3] Skip filterNodes()' reorder when the result is already in input order A single selector group without a set filter is matched in the order the nodes were given, so is(), not() and the other set callers no longer pay for a second pass. :matches() with a positional argument returns the filtered set directly, relativeSelector() reuses parse() and copies the anchor it adds :scope to, and initialMatch() documents that it must be given a copy. Co-Authored-By: Claude Opus 5.5 --- src/CSS/DOMTraverser.php | 18 ++++++--- src/CSS/DOMTraverser/PseudoClass.php | 14 +++---- tests/QueryPath/CSS/DOMTraverserTest.php | 48 ++++++++++-------------- 3 files changed, 39 insertions(+), 41 deletions(-) diff --git a/src/CSS/DOMTraverser.php b/src/CSS/DOMTraverser.php index ee7429c..7f443c9 100644 --- a/src/CSS/DOMTraverser.php +++ b/src/CSS/DOMTraverser.php @@ -201,12 +201,15 @@ public static function filterNodes( } $traverser = new self($nodes, true, $scopeNode, $psHandler); - $matched = $traverser->find($selector)->matches(); - if (count($matched) < 2) { + $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]))) { return $matched; } - // The traverser returns document order. $found = new SplObjectStorage(); foreach ($nodes as $node) { if ($matched->offsetExists($node)) { @@ -396,11 +399,13 @@ protected function applySetFilter(array $nodes, array $pseudoClass): array // 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); + if ($name !== 'not') { + return iterator_to_array($inner, false); + } $filtered = []; foreach ($nodes as $node) { - $isMatch = $inner->offsetExists($node); - if ($name === 'not' ? ! $isMatch : $isMatch) { + if (! $inner->offsetExists($node)) { $filtered[] = $node; } } @@ -691,6 +696,9 @@ public function combineAnyDescendant($node, $selectors, $index) * calling the element-only DOM API on it. Everything downstream of here is therefore elements, * which is why matchesSimpleSelector() and friends can keep their DOMElement type hints. * + * It clears the parts of $selector it matched on, so it must be given a copy: parsed selectors + * are shared through PseudoClass::parse(). + * * @param \QueryPath\CSS\SimpleSelector $selector * @param SplObjectStorage $matches * diff --git a/src/CSS/DOMTraverser/PseudoClass.php b/src/CSS/DOMTraverser/PseudoClass.php index 588955b..5815aed 100644 --- a/src/CSS/DOMTraverser/PseudoClass.php +++ b/src/CSS/DOMTraverser/PseudoClass.php @@ -58,7 +58,7 @@ class PseudoClass protected $anbRules = []; /** - * Parsed :has() arguments, keyed by the argument. See relativeSelector(). + * :has() arguments as relativeSelector() returns them, keyed by the argument. * * @var array */ @@ -488,19 +488,19 @@ protected function relativeSelector($selector): array return $this->relativeSelectors[$selector]; } - $handler = new Selector(); - $parser = new Parser($selector, $handler); - $parser->parse(); - $sibling = SimpleSelector::ADJACENT | SimpleSelector::SIBLING; $groups = []; - foreach ($handler as $group) { - // Groups are stored right-to-left, so the last simple selector is the leftmost. + foreach ($this->parse($selector) as $group) { + // Groups are stored right-to-left, so the last simple selector is the leftmost. It is + // copied before :scope is added, as the parse is shared. $anchor = end($group); if (count($group) === 1 || ! $anchor->isBlank()) { $anchor = new SimpleSelector(); $anchor->combinator = SimpleSelector::ANY_DESCENDANT; $group[] = $anchor; + } else { + $anchor = clone $anchor; + $group[count($group) - 1] = $anchor; } $anchor->pseudoClasses[] = ['name' => 'scope', 'value' => null]; diff --git a/tests/QueryPath/CSS/DOMTraverserTest.php b/tests/QueryPath/CSS/DOMTraverserTest.php index d131cc5..94e8779 100644 --- a/tests/QueryPath/CSS/DOMTraverserTest.php +++ b/tests/QueryPath/CSS/DOMTraverserTest.php @@ -39,7 +39,7 @@ public function testConstructor() $this->assertInstanceOf('\QueryPath\CSS\DOMTraverser', $traverser); } - protected function traverser($xml = null) + protected function traverser($xml = null, ?PseudoClass $ps = null) { $dom = new DOMDocument('1.0'); if ($xml === null) { @@ -51,9 +51,7 @@ protected function traverser($xml = null) $splos = new SPLObjectStorage(); $splos->offsetSet($dom->documentElement); - $traverser = new DOMTraverser($splos); - - return $traverser; + return new DOMTraverser($splos, false, null, $ps); } protected function find($selector) @@ -511,15 +509,27 @@ public function filterNodesProvider(): array 'document element' => [['root', 'ul'], 'root', ['root']], ':root' => [['ul', 'root'], ':root', ['root']], ':scope is the document element' => [['ul', 'root'], ':scope', ['root']], + ':scope is the scope node' => [['ul', 'l1', 'l2', 'o1'], ':scope', ['ul'], 'ul'], + 'combinator from the scope node' => [['ul', 'l1', 'l2', 'o1'], ':scope > li', ['l1', 'l2'], 'ul'], + 'empty set' => [[], 'li', []], + 'empty set, positional' => [[], 'li:first', []], ]; } /** * @dataProvider filterNodesProvider */ - public function testFilterNodes(array $ids, $selector, array $expected) + public function testFilterNodes(array $ids, $selector, array $expected, $scopeId = null) { - $this->assertSame($expected, $this->ids(DOMTraverser::filterNodes($this->nthNodes(...$ids), $selector))); + $nodes = $this->nthNodes(...$ids); + $scope = null; + foreach ($nodes as $node) { + if ($node->getAttribute('id') === $scopeId) { + $scope = $node; + } + } + + $this->assertSame($expected, $this->ids(DOMTraverser::filterNodes($nodes, $selector, $scope))); } public function testFilterNodesSkipsNodesThatAreNotElements() @@ -538,22 +548,6 @@ public function testFilterNodesSkipsNodesThatAreNotElements() $this->assertSame(['l1'], $this->ids(DOMTraverser::filterNodes($nodes, ':first'))); } - public function testFilterNodesWithAnEmptySet() - { - $this->assertCount(0, DOMTraverser::filterNodes(new SplObjectStorage(), 'li')); - $this->assertCount(0, DOMTraverser::filterNodes(new SplObjectStorage(), 'li:first')); - } - - public function testFilterNodesWithAScopeNode() - { - $nodes = $this->nthNodes('ul', 'l1', 'l2', 'o1'); - $nodes->rewind(); - $ul = $nodes->current(); - - $this->assertSame(['ul'], $this->ids(DOMTraverser::filterNodes($nodes, ':scope', $ul))); - $this->assertSame(['l1', 'l2'], $this->ids(DOMTraverser::filterNodes($nodes, ':scope > li', $ul))); - } - /** * A shared handler hands every query the same parsed selector, which the traverser must leave * as it found it. @@ -564,14 +558,10 @@ public function testFilterNodesWithAScopeNode() */ public function testQueriesSharingAHandlerReuseTheParsedSelector($selector, array $expected) { - $dom = new DOMDocument('1.0'); - $dom->loadXML(str_replace('
                ', '
                  ', $this->nthXml)); - $splos = new SplObjectStorage(); - $splos->offsetSet($dom->documentElement); - - $ps = new PseudoClass(); + $ps = new PseudoClass(); + $xml = str_replace('
                    ', '
                      ', $this->nthXml); for ($i = 0; $i < 2; $i++) { - $this->assertSame($expected, $this->ids((new DOMTraverser($splos, false, null, $ps))->find($selector)->matches())); + $this->assertSame($expected, $this->ids($this->traverser($xml, $ps)->find($selector)->matches())); } } }