adding duplicate child element broken

[akostadinov]

This is related to #1200

I have a reproducer here:
https://gist.github.com/akostadinov/f450c1282cd10e2045befc70db9c506a

Basically duplicating an element under same parent does not work unless the element is element.dup before adding it as a second child.

[flavorjones]

I'm not sure I understand this bug report, as you haven't specified what output you're actually seeing with your version of nokogiri, nor what you expect to see.

The output I see using this nokogiri:

# Nokogiri (1.7.0.1)
    ---
    warnings: []
    nokogiri: 1.7.0.1
    ruby:
      version: 2.4.0
      platform: x86_64-linux
      description: ruby 2.4.0p0 (2016-12-24 revision 57164) [x86_64-linux]
      engine: ruby
    libxml:
      binding: extension
      source: packaged
      libxml2_path: "/home/flavorjones/.rvm/gems/ruby-2.4.0/gems/nokogiri-1.7.0.1/ports/x86_64-pc-linux-gnu/libxml2/2.9.4"
      libxslt_path: "/home/flavorjones/.rvm/gems/ruby-2.4.0/gems/nokogiri-1.7.0.1/ports/x86_64-pc-linux-gnu/libxslt/1.1.29"
      libxml2_patches: []
      libxslt_patches: []
      compiled: 2.9.4
      loaded: 2.9.4

is

== Original document
<?xml version="1.0"?>
<wrapper xmlns="ns">
    <record xmlns:extra="extra" xml:id="r1">
        <field>aaa</field>
        <field extra:type="second">bbb</field>
    </record>
</wrapper>

== New document
== Where is my second record?!!
<?xml version="1.0"?>
<wrapper xmlns="ns">
    
<record xmlns:extra="extra" xml:id="r1">
        <field>aaa</field>
        <field extra:type="second">bbb</field>
    </record></wrapper>

which is exactly what I expect, since you're taking the node and putting it back where it came from. At no point are you copying the node.

If you want to make a copy of the node, then make the copy using #dup.

[akostadinov]

I would expect that the #add method would not remove element from where it lived and move it to another place. It should be IMO an internal implementation detail how element is duplicated.

If I used some method called move or a method el = delete(element) then node.add el, now I would expect the element will be removed from the original place and moved to another.

If I just select an element though and use add method, to add it to same document, or another one, then I (also I assume most people) would expect that element to stay where it is, but to be added/duplicated to the place where add is called.

[flavorjones]

Thanks for explaining your expectations.

Unfortunately, this is how #add_child works and has always worked. If you specify an existing node, it does not duplicate the node, it will simply update the node's parent to be the new parent.

There are lots of tests for this, if you'd like to dig into them to better understand the intention of this method.

I'm also happy to consider a pull request to make the documentation for #add_child more clear. Here's what it currently is documented with:

      # Add +node_or_tags+ as a child of this Node.
      # +node_or_tags+ can be a Nokogiri::XML::Node, a ::DocumentFragment, a ::NodeSet, or a string containing markup.
      #
      # Returns the reparented node (if +node_or_tags+ is a Node), or NodeSet (if +node_or_tags+ is a DocumentFragment, NodeSet, or string).
      #
      # Also see related method +<<+.