[dom4j-dev] DefaultElement.setContent(List content) destroys parent-relationship?

[Peter Schaefer]

Hi all,
is this a bug or a feature?
public void testParentAfterSetContent() throws Exception {
        Document doc = DocumentHelper.parseText("<root>" + "<a>a</a>"
            + "<b>b</b>" + "<x>x</x>" + "<d>d</d>" + "</root>");
        Node x = doc.selectSingleNode("/root/x");
        List content = doc.getRootElement().content();
        int position = content.indexOf(x);
        Element c = DocumentHelper.createElement("c");
        c.setText("c");
        content.add(position, c);
        assertNotNull(c.getParent());
        doc.getRootElement().setContent(content);
        assertNotNull(c.getParent());
    }
After setContent, the parent of c is null?!
If it is a bug, an easy patch in DefaultElement.setContent would be to
call contentRemoved() before all other stuff.
This doesn't seem to break other tests, but I am not familiar enough 
with the dom4j-sources to evaluate other possible side-effects.

Please take a look at the attached patch.
cheers
Pete
Index: src/java/org/dom4j/tree/DefaultElement.java
===================================================================
RCS file: /cvsroot/dom4j/dom4j/src/java/org/dom4j/tree/DefaultElement.java,v
retrieving revision 1.58
diff -c -2 -r1.58 DefaultElement.java
*** src/java/org/dom4j/tree/DefaultElement.java 29 Jan 2005 14:52:54 -0000      
1.58
--- src/java/org/dom4j/tree/DefaultElement.java 16 Mar 2005 10:04:46 -0000
***************
*** 598,601 ****
--- 598,604 ----
  
      public void setContent(List content) {
+         
+         contentRemoved();
+         
          if (content instanceof ContentListFacade) {
              content = ((ContentListFacade) content).getBackingList();
***************
*** 634,640 ****
                  }
              }
! 
!             contentRemoved();
! 
              this.content = newContent;
          }
--- 637,641 ----
                  }
              }
!             
              this.content = newContent;
          }
Index: src/test/org/dom4j/tree/DefaultElementTest.java
===================================================================
RCS file: /cvsroot/dom4j/dom4j/src/test/org/dom4j/tree/DefaultElementTest.java,v
retrieving revision 1.6
diff -c -2 -r1.6 DefaultElementTest.java
*** src/test/org/dom4j/tree/DefaultElementTest.java     17 Feb 2005 20:31:33 
-0000      1.6
--- src/test/org/dom4j/tree/DefaultElementTest.java     16 Mar 2005 10:04:46 
-0000
***************
*** 8,20 ****
  package org.dom4j.tree;
  
- import junit.textui.TestRunner;
- 
  import java.util.List;
  
! import org.dom4j.AbstractTestCase;
! import org.dom4j.Document;
! import org.dom4j.DocumentFactory;
! import org.dom4j.DocumentHelper;
! import org.dom4j.Element;
  
  /**
--- 8,16 ----
  package org.dom4j.tree;
  
  import java.util.List;
  
! import junit.textui.TestRunner;
! 
! import org.dom4j.*;
  
  /**
***************
*** 107,110 ****
--- 103,120 ----
          assertEquals(0, ns.size());
      }
+     
+     public void testParentAfterSetContent() throws Exception {
+         Document doc = DocumentHelper.parseText("<root>" + "<a>a</a>"
+             + "<b>b</b>" + "<x>x</x>" + "<d>d</d>" + "</root>");
+         Node x = doc.selectSingleNode("/root/x");
+         List content = doc.getRootElement().content();
+         int position = content.indexOf(x);
+         Element c = DocumentHelper.createElement("c");
+         c.setText("c");
+         content.add(position, c);
+         assertNotNull(c.getParent());
+         doc.getRootElement().setContent(content);
+         assertNotNull(c.getParent());
+     }
  }