Skip to content

Return traversal results in jQuery's order - #130

Merged
jakejackson1 merged 2 commits into
mainfrom
fix/issue-117
Oct 26, 2026
Merged

jakejackson1 merged 2 commits into
mainfrom
fix/issue-117

Conversation

@gravitypdf-bot

@gravitypdf-bot gravitypdf-bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Behaviour change. On a multi-element set, the traversal methods now return their results in jQuery's order. Before, the results were grouped by starting element.

$xml = '<r><a/><b/><c/><d/></r>';
qp($xml, 'c')->add('d')->prevAll();
// before: b, a, c   (grouped by starting element)
// after:  c, b, a   (reverse document order, as jQuery)

What jQuery does

I took the rules from jQuery 3.7.1's traversing.js. The table values come from running each call in jQuery 3.7.1 on jsdom. jQuery sorts only when the set has more than one element:

  • parents, parentsUntil, prevAll and prevUntil use uniqueSort and then reverse, so they return reverse document order.
  • next, prev, children and contents are in guaranteedUnique, so they are never sorted and keep the order of the starting elements.
  • Everything else uses uniqueSort, which gives document order. That covers nextAll, nextUntil, siblings and parent. closest also sorts its results with uniqueSort.

Document: <r><i#a/><i#b/><i#c/><i#d/><i#e/></r>, and r > s#s1 > t#t1 > u#u1 and r > s#s2 > t#t2 > u#u2. The start sets are built in the order shown, not in document order.

Call jQuery 3.7.1 before after
[c, d].prevAll() c, b, a b, a, c c, b, a
[c, e].prevUntil('#a') d, c, b b, d, c d, c, b
[c, a].nextAll() b, c, d, e d, e, b, c b, c, d, e
[c, a].nextUntil('#e') b, c, d d, b, c b, c, d
[b, d].siblings() a, b, c, d, e a, c, d, e, b a, b, c, d, e
[u2, u1].parent() t1, t2 t2, t1 t1, t2
[u2, u1].closest('s') s1, s2 s2, s1 s1, s2
[c, a].next() d, b d, b d, b
[u2, u1].parents() t2, s2, t1, s1, r t2, s2, t1, s1, r t2, s2, t1, s1, r

Results for a single starting element are unchanged.

What changed

Each traversal method names its order with one of two helpers, which filter by the optional selector, sort, and wrap the result:

Helper Methods
inReverseDocumentOrder() parents(), parentsUntil(), prevAll(), prevUntil()
inDocumentOrder() nextAll(), nextUntil(), siblings(), parent(), closest()
(neither) next(), prev(), which keep the order of the starting elements

walk(), the helper from #129, stays order-agnostic. sortReverseDocumentOrder() became the private sortDocumentOrder($nodes, $reverse) behind the two helpers, and still skips the sort for a single starting element. Filtering before sorting gives the same result as sorting first, because since #128 positional pseudo-classes count in document order whatever order the set is given in, and it sorts the smaller set.

Remaining differences from jQuery

Tests

  • tests/Issues/Issue117Test.php covers each method on a multi-element set with an out-of-order start, plus the single-start case. Three of its five tests fail on main.
  • In DOMQueryTest::testNextAllAndPrevAllFromOverlappingStarts, two expectations pinned the old grouped order. They now expect jQuery's.
  • selectIds() and idList(), the helpers that build an out-of-order start set and list its IDs, live in QueryPathTests\TestCase, so Count positional *Until() stops across the document, as jQuery does #131's tests share them.
  • vendor/bin/phpunit: 698 tests, 1948 assertions, 0 failures (1 skipped). composer run lint and composer run lint:min-php are clean.

Performance: the sort adds about 6 ms to nextAll() and prevAll() on a 4,000-item list (1.3 ms before, about 7 ms after).

Rebased onto #129

Rebased onto the updated #129, which moved ordering out of walk() and into the public methods; the ordering this PR first put at the end of walk() now lives in the two helpers above.

Fixes #117.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.14%. Comparing base (6ae6acf) to head (56a0b36).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #130      +/-   ##
============================================
+ Coverage     90.69%   91.14%   +0.44%     
- Complexity     1424     1434      +10     
============================================
  Files            25       26       +1     
  Lines          3170     3172       +2     
============================================
+ Hits           2875     2891      +16     
+ Misses          295      281      -14     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gravitypdf-bot and others added 2 commits October 26, 2026 16:31
prevAll(), prevUntil(), nextAll(), nextUntil(), siblings(), parent()
and closest() grouped a multi-element set's results by starting
element. They now sort them as jQuery does, in one place in walk().

Fixes #117.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
inDocumentOrder() and inReverseDocumentOrder() filter, sort and wrap a
traversal's results, so each public method is one line and names its
order. The test ID helpers move to TestCase.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jakejackson1 jakejackson1 added the bug Something isn't working label Oct 26, 2026
@jakejackson1
jakejackson1 merged commit 4d05c9e into main Oct 26, 2026
14 checks passed
@jakejackson1
jakejackson1 deleted the fix/issue-117 branch October 26, 2026 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

prevAll(), closest() and other traversals don't return results in jQuery's order

2 participants