[prev in list] [next in list] [prev in thread] [next in thread] 

List:       axis-dev
Subject:    Re: svn commit: r1177260 - /axis/axis2/java/rampart/trunk/modules/rampart-core/src/main/java/org/apa
From:       Andreas Veithen <andreas.veithen () gmail ! com>
Date:       2011-09-30 18:33:10
Message-ID: CADx4_uUrdpsHH68Ze6jszA_e1i0ioxmQqQ0Xu7AJ8kC_4=n5LQ () mail ! gmail ! com
[Download RAW message or body]

Apparently that code was meant to cover the case of DocumentFragments:

http://svn.apache.org/viewvc?view=revision&revision=518301

When insertBefore is used with a DocumentFragment as parameter, then
this will indeed insert multiple nodes. Note however that
DocumentFragment is a DOM API for which there is no counterpart in the
Axiom API.

However, I doubt that this is working correctly. The condition
"newDomChild.nextSibling != null" looks wrong to me. What I can
already tell is that this piece of code has zero unit test coverage...

Andreas

On Fri, Sep 30, 2011 at 18:54, Amila Jayasekara <amilaj@wso2.com> wrote:
> Hi Andreas,
> 
> In ParentNode.insertBefore(Node newChild, Node refChild) method i came
> across following bit of code,
> 
> ChildNode newDomChild = (ChildNode) newChild;
> ChildNode refDomChild = (ChildNode) refChild;
> 
> ....
> ....
> ....
> 
> boolean compositeChild = newDomChild.nextSibling != null;
> ChildNode endChild = null;
> 
> if(compositeChild) {
> ChildNode tempNextChild = newDomChild.nextSibling;
> while(tempNextChild != null) {
> tempNextChild.parentNode = this;
> endChild = tempNextChild;
> tempNextChild = tempNextChild.nextSibling;
> }
> }
> 
> And this code was hitting when adding a node. Since above code is
> written specifically, i was under impression that it is the behaviour
> of it. And didnt realise that this code was hitting when moving node,
> due to an issue in detach. Therefore i took the approach in patch to
> fix the issue. Thats where i got "composite element" concept into my
> mind.
> 
> Sorry for the inconvenience.
> 
> Thanks
> AmilaJ
> 
> 
> 
> On Thu, Sep 29, 2011 at 12:24 PM, Andreas Veithen
> <andreas.veithen@gmail.com> wrote:
> > Sorry, but this change is complete nonsense. There is no such thing as
> > a "composite element" and moving a node never moves its siblings (but
> > only its children). This is actually a bug in the Axiom DOM
> > implementation: the detach() method fails to reset the nextSibling
> > attribute of the node.
> > 
> > Andreas
> > 
> > On Thu, Sep 29, 2011 at 13:06,  <thilinamb@apache.org> wrote:
> > > Author: thilinamb
> > > Date: Thu Sep 29 11:06:11 2011
> > > New Revision: 1177260
> > > 
> > > URL: http://svn.apache.org/viewvc?rev=1177260&view=rev
> > > Log:
> > > Committing the patch provided by AmilaJ for RAMPART-336.
> > > 
> > > Modified:
> > > axis/axis2/java/rampart/trunk/modules/rampart-core/src/main/java/org/apache/rampart/util/Axis2Util.java
> > >  
> > > Modified: axis/axis2/java/rampart/trunk/modules/rampart-core/src/main/java/org/apache/rampart/util/Axis2Util.java
> > >                 
> > > URL: http://svn.apache.org/viewvc/axis/axis2/java/rampart/trunk/modules/rampart- \
> > > core/src/main/java/org/apache/rampart/util/Axis2Util.java?rev=1177260&r1=1177259&r2=1177260&view=diff
> > >  ==============================================================================
> > > --- axis/axis2/java/rampart/trunk/modules/rampart-core/src/main/java/org/apache/rampart/util/Axis2Util.java \
> > >                 (original)
> > > +++ axis/axis2/java/rampart/trunk/modules/rampart-core/src/main/java/org/apache/rampart/util/Axis2Util.java \
> > > Thu Sep 29 11:06:11 2011 @@ -217,6 +217,10 @@ public class Axis2Util {
> > > // it is a header we have added in rampart eg. EncryptedHeader and should
> > > // be converted to SOAPHeaderBlock for processing
> > > } else {
> > > +                            // First detach element from soap header
> > > +                            element.detach();
> > > +
> > > +                            // add new element
> > > header = soapHeader.addHeaderBlock(element.getLocalName(), \
> > > element.getNamespace()); Iterator attrIter = element.getAllAttributes();
> > > while (attrIter.hasNext()) {
> > > @@ -231,14 +235,17 @@ public class Axis2Util {
> > > // retrieve all child nodes (including any text nodes)
> > > // and re-attach to header block
> > > Iterator children = element.getChildren();
> > > -                               while (children.hasNext()) {
> > > +
> > > +                            // Element is a composite element, in which it has \
> > > many siblings. +                            // All siblings will be added when \
> > > we add a single node. +                            // See \
> > > ParentNode.insertBefore(Node newChild, Node refChild) for +                     \
> > > // more information. +                               if (children.hasNext()) {
> > > OMNode child = (OMNode)children.next();
> > > children.remove();
> > > header.addChild(child);
> > > }
> > > -
> > > -                               element.detach();
> > > -
> > > +
> > > soapHeader.build();
> > > 
> > > header.setProcessed();
> > > 
> > > 
> > > 
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: java-dev-unsubscribe@axis.apache.org
> > For additional commands, e-mail: java-dev-help@axis.apache.org
> > 
> > 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-dev-unsubscribe@axis.apache.org
> For additional commands, e-mail: java-dev-help@axis.apache.org
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@axis.apache.org
For additional commands, e-mail: java-dev-help@axis.apache.org


[prev in list] [next in list] [prev in thread] [next in thread]