You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@xerces.apache.org by le...@locus.apache.org on 2000/11/01 22:28:24 UTC

cvs commit: xml-xerces/java/src/org/apache/xerces/dom RangeImpl.java

lehors      00/11/01 13:28:23

  Modified:    java/src/org/apache/xerces/dom RangeImpl.java
  Log:
  Patch from Lynn Monson and Elena Litani:
  
  ----------
  Lynn wrote:
  
  While using Range.insertNode(), I came across what I think are two small,
  but significant bugs.  The existing Xerces code for Range.insertNode()
  assumes that the end point of the range is unrelated to the starting point.
  However, if the ending container is the same as the start container, or if
  the end container is the parent of the start container, the range is left in
  an incorrect state.
  
  For example:
  
  Assume my document is the following:
  <root>abc</root>
  
  If my range is:
  
  start container: abc
  start offset: 1
  end container: abc
  end offset: 3
  
  Then after inserting the node <foo/>, I am left with the document:
  
  <root>a<foo/>bc</root>
  
  But my range was not updated correctly.  It reads:
  
  start container: a
  start offset: 1
  end container: a
  end offset: 3
  
  The end container is incorrect, and the end offset is illegal.  I believe
  the range should be:
  
  end container: bc
  end offset: 2
  
  A similar situation exists if the original range had used <root> as the end
  container with an offset of 1.  Included below is a suggested fix for this
  problem.  As always, thanks in advance for considering it.
  
  ----------
  Elena wrote:
  
  Lynn, thanks for reporting the bug and producing a patch.
  While looking at it, I discovered some other problems with insertNode().
  So I compared the implementation against specs and here are the changes
  I came up with:
  
  1. Proper exception handling (which was not done)
  2. Include handling DocumentFragment. Actually, it is handled
  automatically by insertBefore(), or appendChild(). So the only thing I
  needed to do is not to throw "DOM012 Invalid node type" exception.
  3. Update ranges for insertion
   a) Lynns patch for text nodes; I have changed it a little bit to
  incorporate DocumentFragment insertions
   b) Updating ranges for all other nodes also need to be done.
  ex.
  DOM tree:<body><p/></body>.
  Range(start;end): body,0; body,1
  
  After insertion of insert <h1>:
  DOM tree: <body></h1><p/></body>.
  Range(start;end): body,0; body,2
  
  4. Deleted normalization of text nodes. According to specs (2.9)
  normalization should be handled by application. That is if 2 adjacent
  Text nodes are created -> we don't merge 'em.
  There is a special case which is handled the same:
  fStartContainer=text node and fStartOffset=0. We still split text node
  although one of the resulting text nodes will be empty.
  
  Revision  Changes    Path
  1.11      +53 -24    xml-xerces/java/src/org/apache/xerces/dom/RangeImpl.java
  
  Index: RangeImpl.java
  ===================================================================
  RCS file: /home/cvs/xml-xerces/java/src/org/apache/xerces/dom/RangeImpl.java,v
  retrieving revision 1.10
  retrieving revision 1.11
  diff -u -r1.10 -r1.11
  --- RangeImpl.java	2000/10/23 23:49:18	1.10
  +++ RangeImpl.java	2000/11/01 21:28:22	1.11
  @@ -660,56 +660,80 @@
       public void insertNode(Node newNode)
           throws DOMException, RangeException
       {
  -    	if( fDetach) {
  +    	if ( newNode == null ) return; //throw exception?
  +        
  +        if( fDetach) {
       		throw new DOMException(
       			DOMException.INVALID_STATE_ERR, 
   			"DOM011 Invalid state");
       	}
  +        if ( ((NodeImpl)fStartContainer.getParentNode()).getReadOnly() ) {
  +            throw new DOMException(DOMException.NO_MODIFICATION_ALLOWED_ERR,"DOM007 Read Only node");
  +
  +        }
  +        if ( fDocument != newNode.getOwnerDocument() ) {
  +            throw new DOMException(DOMException.WRONG_DOCUMENT_ERR,"DOM004 Wrong document");
  +        }
  +        if ( isAncestorOf( newNode, fStartContainer) ) {
  +            throw new DOMException(DOMException.HIERARCHY_REQUEST_ERR, "DOM003 Hierarchy request");
  +        }
  +
  +
           int type = newNode.getNodeType();
           if (type == Node.ATTRIBUTE_NODE
               || type == Node.ENTITY_NODE
               || type == Node.NOTATION_NODE
  -            || type == Node.DOCUMENT_NODE
  -            || type == Node.DOCUMENT_FRAGMENT_NODE)
  +            || type == Node.DOCUMENT_NODE)
           {
       		throw new RangeExceptionImpl(
       			RangeException.INVALID_NODE_TYPE_ERR, 
   			"DOM012 Invalid node type");
           }
  -        if (newNode == null) return; //throw exception?
           Node cloneCurrent;
           Node current;
  -        boolean MULTIPLE_MODE = false;
  +        int currentChildren = 0;
  +
  +        //boolean MULTIPLE_MODE = false;
           if (fStartContainer.getNodeType() == Node.TEXT_NODE) {
  -            if (newNode.getNodeType()!= Node.TEXT_NODE) { // result is 3 text nodes...
  -                cloneCurrent = fStartContainer.cloneNode(false);
  -                ((TextImpl)cloneCurrent).setNodeValueInternal(
  +        
  +            Node parent = fStartContainer.getParentNode();
  +            currentChildren = parent.getChildNodes().getLength(); //holds number of kids before insertion
  +            // split text node: results is 3 nodes..
  +            cloneCurrent = fStartContainer.cloneNode(false);
  +            ((TextImpl)cloneCurrent).setNodeValueInternal(
                       (cloneCurrent.getNodeValue()).substring(fStartOffset));
  -                ((TextImpl)fStartContainer).setNodeValueInternal(
  +            ((TextImpl)fStartContainer).setNodeValueInternal(
                       (fStartContainer.getNodeValue()).substring(0,fStartOffset));
  -                Node next = fStartContainer.getNextSibling();
  -                if (next != null) {
  -                    Node parent = fStartContainer.getParentNode();
  +            Node next = fStartContainer.getNextSibling();
  +            if (next != null) {
                       if (parent !=  null) {
                           parent.insertBefore(newNode, next);
                           parent.insertBefore(cloneCurrent, next);
                       }
  -                } else {
  -                    Node parent = fStartContainer.getParentNode();
  +            } else {
                       if (parent != null) {
                           parent.appendChild(newNode);
                           parent.appendChild(cloneCurrent);
                       }
  -                }
  -                // signal other Ranges to update their start/end containers/offsets
  -                signalSplitData(fStartContainer, cloneCurrent, fStartOffset);
  -                
  -            } else { // result is 1 text node.
  -                String value = fStartContainer.getNodeValue();
  -                String newValue = newNode.getNodeValue();
  -                insertData( (CharacterData)fStartContainer, fStartOffset, newValue);
               }
  +             //update ranges after the insertion
  +             if ( fEndContainer == fStartContainer) {
  +                  fEndContainer = cloneCurrent; //endContainer is the new Node created
  +                  fEndOffset -= fStartOffset;   
  +             }
  +             else if ( fEndContainer == parent ) {    //endContainer was not a text Node.
  +                  //endOffset + = number_of_children_added
  +                   fEndOffset += (parent.getChildNodes().getLength() - currentChildren);  
  +             }
  +
  +             // signal other Ranges to update their start/end containers/offsets
  +             signalSplitData(fStartContainer, cloneCurrent, fStartOffset);
  +                
  +             
           } else { // ! TEXT_NODE
  +            if ( fEndContainer == fStartContainer )      //need to remember number of kids
  +                currentChildren= fEndContainer.getChildNodes().getLength();
  +
               current = fStartContainer.getFirstChild();
               int i = 0;
               for(i = 0; i < fStartOffset && current != null; i++) {
  @@ -720,8 +744,13 @@
               } else {
                   fStartContainer.appendChild(newNode);
               }
  -        }                
  -        
  +            //update fEndOffset. ex:<body><p/></body>. Range(start;end): body,0; body,1
  +            // insert <h1>: <body></h1><p/></body>. Range(start;end): body,0; body,2
  +            if ( fEndContainer == fStartContainer ) {     //update fEndOffset
  +                fEndOffset += (fEndContainer.getChildNodes().getLength() - currentChildren);
  +            }
  +
  +        } 
       }
       
       public void surroundContents(Node newParent)