diff --git a/CHANGELOG.md b/CHANGELOG.md
index 527a87d..2960a68 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.** Fix `filter()`, `not()` and `is()` matching nothing when a positional pseudo-class sits on a compound left of a combinator, as in `ul:first > li` (issue #132). That compound now counts across the whole document, as in jQuery's `.filter()` and in `find()`, so `qp($xml, 'li')->filter('ul:first > li')` returns the items of the document's first `
`. A positional pseudo-class on the subject still counts within the set. `QueryPath\CSS\DOMTraverser::filterNodes()` follows suit
# 4.2.0
diff --git a/src/CSS/DOMTraverser.php b/src/CSS/DOMTraverser.php
index 7f443c9..caa9e62 100644
--- a/src/CSS/DOMTraverser.php
+++ b/src/CSS/DOMTraverser.php
@@ -173,8 +173,9 @@ public function find($selector): DOMTraverser
*
* 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.
+ * still resolved against the document, so `div > p` works. As in jQuery's .filter(), a positional
+ * pseudo-class (`:first`, `:eq(n)`, ...) counts within the given set when it is on the subject, and
+ * across the document when it is left of a combinator. Nodes that are not elements never match.
*
* @param SplObjectStorage $nodes
* The candidates, all from one document.
@@ -335,9 +336,13 @@ protected function resolveSubject(array $selectorGroup, int $index): SplObjectSt
});
} else {
// initialMatch() consumes the parts of the simple selector it matched on. Give it a copy,
- // so the caller's parsed selector can be reused.
+ // so the caller's parsed selector can be reused. In filterNodes(), a compound left of a
+ // combinator is looked up across the document, so its positional filters count there.
$selectorGroup[$index] = clone $selectorGroup[$index];
- $candidates = $this->initialMatch($selectorGroup[$index], $this->matches);
+ $candidates = $this->initialMatch(
+ $selectorGroup[$index],
+ $this->initialized ? self::documentScope($this->dom) : $this->matches
+ );
}
$setFilters = $selectorGroup[$index]->setFilterPseudoClasses();
@@ -377,6 +382,23 @@ protected function resolveSubject(array $selectorGroup, int $index): SplObjectSt
return $this->toMatches($matched);
}
+ /**
+ * The document element, standing in for the document as it does in qp($document).
+ *
+ * @param DOMDocument|null $document
+ *
+ * @return SplObjectStorage
+ */
+ public static function documentScope($document): SplObjectStorage
+ {
+ $scope = new SplObjectStorage();
+ if ($document instanceof DOMDocument && $document->documentElement) {
+ $scope->offsetSet($document->documentElement);
+ }
+
+ return $scope;
+ }
+
/**
* Apply a single set-level pseudo-class to an ordered list of nodes.
*
diff --git a/src/Helpers/NodeMatcher.php b/src/Helpers/NodeMatcher.php
index 8df96fd..d117795 100644
--- a/src/Helpers/NodeMatcher.php
+++ b/src/Helpers/NodeMatcher.php
@@ -115,13 +115,9 @@ public function matchesInDocument($node): bool
*/
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();
+ return (new DOMTraverser(DOMTraverser::documentScope($document), false, null, $this->psHandler))
+ ->find($this->selector)
+ ->matches();
}
/**
diff --git a/tests/Issues/Issue132Test.php b/tests/Issues/Issue132Test.php
new file mode 100644
index 0000000..799fd21
--- /dev/null
+++ b/tests/Issues/Issue132Test.php
@@ -0,0 +1,59 @@
+';
+
+ /**
+ * @testWith ["ul:first > li", "a,b"]
+ * ["ul:eq(1) > li", "c"]
+ * ["ul:last li", "c"]
+ * ["ul:eq(0) + ul > li", "c"]
+ * ["ul:gt(0) > li:first", "c"]
+ * ["ul:first > li:last", "b"]
+ * ["ul:first > li, ul:last > li", "a,b,c"]
+ */
+ public function testFilterCountsALeftHandPositionalCompoundAcrossTheDocument($selector, $expected)
+ {
+ $this->assertSame($expected, $this->idList(qp(self::XML, 'li')->filter($selector)));
+ }
+
+ public function testIsMatchesWhatFindSelects()
+ {
+ $this->assertSame('a,b', $this->idList(qp(self::XML)->find('ul:first > li')));
+ $this->assertTrue(qp(self::XML, 'li')->is('ul:first > li'));
+ }
+
+ public function testTheCompoundIsNotLimitedToTheNodesInHand()
+ {
+ // u1 is not in the set, but is still the document's first .
+ $this->assertSame('b', $this->idList(qp(self::XML, '#b')->filter('ul:first > li')));
+ $this->assertSame('', $this->idList(qp(self::XML, '#c')->filter('ul:first > li')));
+ $this->assertSame('c', $this->idList(qp(self::XML, 'li')->not('ul:first > li')));
+ }
+
+ public function testFilterNodes()
+ {
+ $nodes = new SplObjectStorage();
+ foreach (qp(self::XML, 'li')->get() as $node) {
+ $nodes->offsetSet($node);
+ }
+
+ $this->assertCount(2, DOMTraverser::filterNodes($nodes, 'ul:first > li'));
+ $this->assertCount(1, DOMTraverser::filterNodes($nodes, 'ul:eq(1) > li'));
+ }
+}