Skip to content

Fix open issues #132–#146 - #147

Closed
gravitypdf-bot wants to merge 19 commits into
mainfrom
fix/open-issues
Closed

gravitypdf-bot wants to merge 19 commits into
mainfrom
fix/open-issues

Conversation

@gravitypdf-bot

Copy link
Copy Markdown
Contributor

Fixes every open issue: #132, #133, #134, #135, #136, #137, #138, #139, #140, #142, #143, #144, #145, #146.

Where an issue offered a choice between matching jQuery and documenting the current behaviour (#140, #143, #144, #145, #146), this PR matches jQuery, consistent with the behaviour changes already listed under # Unreleased changes. Each one has a Behaviour change CHANGELOG entry.

Index and position methods

Positional selectors

CSS engine

Cloning

Tests

  • A regression test file for each issue in tests/Issues/ (Issue132Test.php … Issue146Test.php), covering the issue tables. CI runs them on PHP 7.1–8.5.
  • Existing tests updated where they pinned the old behaviour: DOMQueryTest::testSlice, testEven/testOdd, testFirstAndLastWithNonElementNodes, and Issue118Test's positional-order test, which now uses jQuery's walk-order values.

Notes

  • The multi-element walk-order results in Positional selectors in prevAll(), parents() and the *Until() methods count in document order, not jQuery's walk order #134 are worked out from jQuery 3.7.1's source. They were not run against jQuery.
  • Not changed here, possible follow-ups:
    • siblings(':first') on a multi-element set still counts in document order.
    • filterNodes() counts in document order rather than in the order its input is given.
    • A clone keeps the parent's extension instances, which were built with the parent object. This was already the case before this PR.
  • Locally (PHP 8.5): 840 tests pass, and composer run lint and lint:min-php pass.

🤖 Generated with Claude Code

gravitypdf-bot and others added 19 commits October 26, 2026 16:58
siblings() read the parent's childNodes without checking for a parent,
so an element outside any tree raised two warnings.

Fixes #139.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
replaceAll() made a shallow clone of the replacement and a shallow import of
that, so each replaced node arrived with its attributes but no children.
Import the replacement deeply instead.

Fixes #135

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
getNthMatch() returned nothing for a negative index, so eq(-1) was empty
and get(-1) NULL, and slice() kept every match from a negative start.
As in jQuery, a negative index now counts back from the end of the set.

Fixes #142.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
slice($start, $length) followed array_slice(), with a length of 0
meaning "to the end". It is now slice($start, $end), as in jQuery:
the slice stops before $end, a negative $end counts back from the end,
and an omitted $end runs to the end of the set.

Fixes #144.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nk()

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in filter()

In filterNodes(), a non-subject compound carrying a positional
pseudo-class was resolved by searching the candidates' descendants, so
filter('ul:first > li') and is('ul:first > li') matched nothing. It is
now resolved from the document element, as jQuery's .filter() does.

Fixes #132.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
even() kept positions 1, 3, 5 and odd() kept 0, 2, 4, the reverse of
jQuery's .even() and .odd() and of QueryPath's own :even and :odd
pseudo-classes. Both now go through one helper.

Fixes #145.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
inst() cloned the object, and __clone() deep-copied every match with
cloneNode(true), only for inst() to swap the match set straight after. Each
find() from the root copied the whole document, and every chained call
copied whatever was in hand. The copies also became the previous match set,
so end() returned detached copies rather than the nodes the query started
from.

inst() now marks the clone as shallow so __clone() skips the copy; clone $qp
still deep-copies the matches.

Fixes #137

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jQuery's .is() evaluates a positional selector against the document and
tests membership, so is(':first') asks whether the set holds the
document's first element. is() counted within the match set instead,
as filter() does. It now uses NodeMatcher::matchesInDocument().

Fixes #140.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
even() and odd() counted text, comment, CDATA and processing-instruction
nodes but dropped them from the result, so contents()->odd() could be
empty. first() and last() skipped them altogether, so first() disagreed
with eq(0). As in jQuery, all four now treat every node in the set
alike, and first() and last() are eq(0) and eq(-1).

Fixes #143.
Fixes #146.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…alk order

jQuery joins the walk from each starting element, repeats included,
filters that list with jQuery.filter(), then removes the repeats and
sorts. QueryPath counted a positional selector in document order, so
prevAll(':first') was the first sibling rather than the nearest.

Add DOMTraverser::filterList(), which counts positional pseudo-classes
along a list in the order given, and filter the joined walks through it
when the selector is positional.

Fixes #134.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e pass for plain selectors

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.27007% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.66%. Comparing base (6ae6acf) to head (e7ca0d4).
⚠️ Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
src/CSS/DOMTraverser/Util.php 95.65% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #147      +/-   ##
============================================
+ Coverage     90.69%   91.66%   +0.96%     
- Complexity     1424     1469      +45     
============================================
  Files            25       26       +1     
  Lines          3170     3238      +68     
============================================
+ Hits           2875     2968      +93     
+ Misses          295      270      -25     

☔ 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

Copy link
Copy Markdown
Contributor Author

Split into one PR per issue: #148 (#139), #149 (#142, #144), #150 (#145), #158 (#143, #146; stacked on #150), #151 (#132), #152 (#140), #153 (#134), #159 (#133), #154 (#136), #155 (#138), #156 (#135), #157 (#137).

@gravitypdf-bot
gravitypdf-bot deleted the fix/open-issues branch October 26, 2026 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant