Tapestry 5
  1. Tapestry 5
  2. TAP5-953

org.apache.tapestry5.dom.Node.remove() should set nextSibling to null

    Details

    • Type: Wish Wish
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.3, 5.2
    • Fix Version/s: 5.3
    • Component/s: tapestry-core
    • Labels:
      None

      Description

      When removing a node from the DOM, its next sibling node should be reset.
      This is especially important for the moveToBottom(org.apache.tapestry5.dom.Element element) method. The current behavior of n.moveToBottom(e) results in n's following siblings being rendered twice, once in their original position and once after the moved node. Only the latter seems correct to me.

      1. ASF.LICENSE.NOT.GRANTED--nodetest.zip
        8 kB
        Jochen Kemnade
      2. Node.java.patch
        0.2 kB
        Jochen Kemnade

        Activity

        Hide
        Hudson added a comment -

        Integrated in tapestry-trunk-freestyle #630 (See https://builds.apache.org/job/tapestry-trunk-freestyle/630/)
        TAP5-953: org.apache.tapestry5.dom.Node.remove() should set nextSibling to null

        hlship : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1202526
        Files :

        • /tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/dom/Element.java
        Show
        Hudson added a comment - Integrated in tapestry-trunk-freestyle #630 (See https://builds.apache.org/job/tapestry-trunk-freestyle/630/ ) TAP5-953 : org.apache.tapestry5.dom.Node.remove() should set nextSibling to null hlship : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1202526 Files : /tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/dom/Element.java
        Hide
        Jochen Kemnade added a comment - - edited

        Simple test application.

        The test page contains

        <ul id="list">
        <li id="node1">node 1</li>
        <li id="node2">node 2</li>
        <li id="node3">node 3</li>
        <li id="node4">node 4</li>
        </ul>

        <ul id="list2">
        </ul>

        In the afterRenderMethod I do node1.moveToBottom(list2). I'd expect only node1 to be moved there, but, as node1's nextSibling is not reset, nodes 2-4 are rendered there too.
        This becomes even worse by replacing the invocation by node1.moveToBottom(list). Then we're caught in an endless loop, as the renderer will never cease to find a next sibling. Node4's next sibling will be node1 (which is fine), and node1's nextSibling will still be node2, so it will try to render list, node2, node3, node4, node1, node2, node3 and so on.

        Show
        Jochen Kemnade added a comment - - edited Simple test application. The test page contains <ul id="list"> <li id="node1">node 1</li> <li id="node2">node 2</li> <li id="node3">node 3</li> <li id="node4">node 4</li> </ul> <ul id="list2"> </ul> In the afterRenderMethod I do node1.moveToBottom(list2). I'd expect only node1 to be moved there, but, as node1's nextSibling is not reset, nodes 2-4 are rendered there too. This becomes even worse by replacing the invocation by node1.moveToBottom(list). Then we're caught in an endless loop, as the renderer will never cease to find a next sibling. Node4's next sibling will be node1 (which is fine), and node1's nextSibling will still be node2, so it will try to render list, node2, node3, node4, node1, node2, node3 and so on.

          People

          • Assignee:
            Howard M. Lewis Ship
            Reporter:
            Jochen Kemnade
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development