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.

Sign up for GitHub

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

Jump to bottom

regression: adding previous siblings seemingly orphans node #595

Closed
codekitchen opened this issue Jan 11, 2012 · 4 comments
Closed

regression: adding previous siblings seemingly orphans node #595

codekitchen opened this issue Jan 11, 2012 · 4 comments

Comments

@codekitchen
Copy link
Contributor

This test case will print "these should not be nil" using Nokogiri 1.4.7 and 1.5.0, but will not print that using Nokogiri 1.4.1.

doc = Nokogiri::HTML("<a>haldo</a>")
# select the "haldo" text node
node = doc.at("a").children.first
e1 = Nokogiri::XML::Node.new('span', doc)
node.add_previous_sibling(e1)
e2 = Nokogiri::XML::Text.new('e2', doc)
e1.add_previous_sibling(e2)
puts "these shound not be nil" if node.previous.nil? && node.parent.nil?

puts "the nodes are still actually there" if doc.at("a").children.size == 3

Note that the node doesn't actually get orphaned or deleted -- calling .children on its parent behaves as you'd expect. But the node returns nil for both .previous and .parent

This was tested with Ruby 1.8.7 and libxml 2.7.3, on OS X Lion. Also occurs using Ruby Enterprise Edition 2011.03 on ubuntu with libxml 2.7.6.
@flavorjones
Copy link
Member

Greetings!

I can't reproduce this :-( I see the same behavior for all versions of Nokogiri you mention - 1.4.1, 1.4.7 and 1.5.0, which is that the doc structure after all operations is:

<html><body><a>e2<span></span>haldo</a></body></html>

That said, it's extremely likely that the version of libxml2 you're using would affect this behavior. Is it possible that you're using an old version of libxml2 with your installation of Nokogiri 1.4.1? You should be able to determine this by running nokogiri -v and seeing the output something like:

--- 
warnings: []

nokogiri: 1.4.1
libxml: 
  binding: extension
  compiled: 2.7.8
  loaded: 2.7.8

I'd love to hear back on what version of libxml2 you're running, and what the doc structure you see is with that version!

@codekitchen
Copy link
Contributor Author

As I said above, the actual document structure is correct, it's Nokogiri's representation of the document structure that becomes corrupted, returning nil for both .previous and .parent.

Also note that it's Nokogiri 1.4.1 that is exhibiting the correct behavior, it's the newer 1.4.7 and 1.5.0 that are incorrect.

All tests were done with libxml 2.7.3 and then again with 2.7.6, as I stated.

@flavorjones
Copy link
Member

Ah, got it. Apologies for not reading more carefully. Let me see what I can do ...

flavorjones added a commit that referenced this issue Feb 20, 2012
@flavorjones
Skipping defensive dup-and-reparent-text-node behavior for both xmlAd…
1af2c49 
…dNextSibling and xmlAddPrevSibling. Related to #595.

Upon examination, neither of these functions free nodes other than the
reparentee, which we're already pickling in `reparent_node_with`.
@flavorjones
Copy link
Member

Scheduled for 1.5.1.

Also see related 1af2c49 which does similar work for Node#add_next_sibling.

flavorjones added a commit that referenced this issue Feb 20, 2012
@flavorjones
Changelog update for #595.
e52d037
flavorjones added a commit that referenced this issue Jul 3, 2023
@flavorjones
fix: do not dup text siblings in reparent_node_with(xmlAddChild)
3b1bc8e 
Related to #283 and #595
Closes #2916
This was referenced Jul 3, 2023
Merged
Closed
flavorjones added a commit that referenced this issue Jul 3, 2023
@flavorjones
fix: do not dup text siblings in reparent_node_with(xmlAddChild) (#2918)
1d00a22 
**What problem is this PR intended to solve?**

Closes #2916

Related to #283 and #595

**Have you included adequate test coverage?**

Yes

**Does this change affect the behavior of either the C or the Java
implementations?**

Brings C in line with Java behavior.
flavorjones added a commit that referenced this issue Jul 5, 2023
@flavorjones
fix: do not dup text siblings in reparent_node_with(xmlAddChild)
564e980 
Related to #283 and #595
Closes #2916

(cherry picked from commit 3b1bc8e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@flavorjones @codekitchen