[DomCrawler] Fixed filterXPath() chaining loosing the parent DOM nodes #10935
Conversation
|
Given that the previous attempt was broken and I was not sure about the actual impact of the change, I worked based on the master branch rather than on 2.3. However, if we decide that this bugfix should be applied to the LTS (which would be fine) and are confident enough that it does not break things (I'm sure it works for the CSS filter and for the Crawler accessors now, but someone could have written a totally crazy XPath query not catched by the tests I added), it should be easy to apply these changes in 2.3 instead given that there is not much differences between both branches. Note that I intentionally kept my own work split into 2 commits: 1 adding tests on the existing code before the refactoring and 1 finishing the refactoring. The new tests have much less value if we merge them in the same commit than the refactoring as it would make it hard to run them against the previous codebase, while it is their goal. |
|
Please don't merge it yet. I found another case which gets broken here (using this new version with Mink) |
This will ensure we don't introduce regressions again when fixing symfony#10206.
|
OK, this is fixed |
|
Merging it into 2.3 seems like a good idea. Can you reopen a PR on 2.3? Thanks. |
|
new PR is sent |
…nt DOM nodes (stof, robbertkl) This PR was merged into the 2.3 branch. Discussion ---------- [DomCrawler] Fixed filterXPath() chaining loosing the parent DOM nodes | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #10206 | License | MIT | Doc PR | n/a This is a fixed version of #10207, preserving the BC for XPath queries. It is the rebased version of #10935 targetting 2.3 The example given in #10260 when reporting the regression in the previous attempt is covered by the new tests added in the first commit of the PR. I also added many tests ensuring that the behavior is the same than in the current implementation. Commits ------- 80438c2 Fixed the XPath filtering to have the same behavior than Symfony 2.4 711ac32 [DomCrawler] Fixed filterXPath() chaining 8f706c9 [DomCrawler] Added more tests for the XPath filtering
This is a fixed version of #10207, preserving the BC for XPath queries.
The example given in #10260 when reporting the regression in the previous attempt is covered by the new tests added in the first commit of the PR.
I also added many tests ensuring that the behavior is the same than in the current implementation.