Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix parsing in-context of parent #2116

Merged
merged 5 commits into from
Nov 28, 2020

Conversation

flavorjones
Copy link
Member

What problem is this PR intended to solve?

rubys/nokogumbo#160

Have you included adequate test coverage?

👍

Does this change affect the C or the Java implementations?

Changes affect Nokogiri generally, though it has the side effect of bringing the JRuby and CRuby implementations in line with each other with respect to exceptions raised in edge cases.

removing the need to override `#coerce` and have the logic there.
The choice of context node has no functional impact for Nokogiri per
se; but it's necessary for Nokogumbo, which has a better HTML parser,
to ensure the parser is in the right state.

When testing this change against nodes without a parent, the test
suite found memory problems related to libxml2 manipulating pointers
on pickled nodes. As a result, the impacted methods now raise a
RuntimeError if they're called on a node without a parent (the concept
of a sibling or a replacement/swap doesn't make sense without the
context of a parent to hold it together, anyway).

Note that `Node#coerce` is now a __protected__ method (previously it was __private__).

Impacted Node methods:

- add_next_sibling, previous=, and before
- add_previous_sibling, next=, and after
- replace and swap

Fixes rubys/nokogumbo#160
@codeclimate
Copy link

codeclimate bot commented Nov 28, 2020

Code Climate has analyzed commit 46e33cc and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (80% is the threshold).

This pull request will bring the total coverage in the repository to 94.3% (0.0% change).

View more on Code Climate.

@flavorjones flavorjones merged commit 6e7ebd7 into master Nov 28, 2020
@flavorjones flavorjones added this to the v1.11.0 milestone Nov 28, 2020
@flavorjones flavorjones deleted the flavorjones-fix-parsing-in-context-of-parent branch November 28, 2020 23:33
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