Menu

#463 DomNode.DescendantElementsIterator.remove() is busted

closed
None
5
2012-10-21
2007-04-23
No

DescendantElementsIterator.remove() doesn't work

if _nextElement is a firstChild() this will remove the lastChild of parent, whereas it should remove the parent.

if _nextElement is not a first Child, then it should remove the lastChildElement of lastChildElement of LastChildElement (etc) of its previousSiblingElement, unless the previous sibling has no children , then remove previous sibling, instead it always removes previous sibling.

Also fails for last element because nextElement_ is null;

In summary, it doesn't take into account the tree traversal that is happening.

Simplest solution might be to add current to the state so remove() can remove it.

However the implementation is so completely poked, remove() is probably not used (otherwise this problem would have been found when remove() was used.

This patch makes its non use explicit, rather than fixing it.

All tests pass with attached patch which just throws an UnsupportedOperationException.

If someone needs to implement remove() later, then they can do it properly at that point.

Discussion

  • Bruce Chapman

    Bruce Chapman - 2007-04-23

    patch

     
  • Marc Guillemot

    Marc Guillemot - 2007-05-04

    Logged In: YES
    user_id=402164
    Originator: NO

    Patch applied. Thanks.

    As you said, no unit test covered it, meaning that it was useless until now.

     

Log in to post a comment.