Element.before causes element to be moved to the incorrect location #1898
Comments
|
Could you provide an example testcase? I'm not able to repro -- perhaps I have misunderstood, or there is another piece required. String html = "<div><p>One</p><p>Two</p></div>";
Document doc = Jsoup.parse(html);
Elements ps = doc.select("p");
Element p1 = ps.get(0);
Element p2 = ps.get(1);
System.out.println("P1=" + p1.siblingIndex());
System.out.println("P2=" + p2.siblingIndex());
p2.before(p1);
System.out.println(doc);
System.out.println("P1=" + p1.siblingIndex());
System.out.println("P2=" + p2.siblingIndex());Produces: P1=0
P2=1
<html>
<head></head>
<body>
<div>
<p>Two</p>
<p>One</p>
</div>
</body>
</html>
P1=1
P2=0Which looks correct to me. If I add extra elements within the div, it still looks right. |
jhy
added
the
needs-more-info
|
Your example demonstrates the bug perfectly. The elements should not have swapped positions with that API call - they should have remained as they were since p1 was already before p2 to begin with. Call p2.before(p1) a second time and they'll swap places again. |
|
Ah, I anchored myself into thinking about the API backwards. It is:
|
jhy
added
bug
|
Thanks, fixed. The output of the above sample is now correct: P1=0
P2=1
<html>
<head></head>
<body>
<div>
<p>One</p>
<p>Two</p>
</div>
</body>
</html>
P1=0
P2=1Also, I made it a no-op vs a validation error to call |
|
Excellent. Thank you! |
In scenarios where Element a shares a parent with Element b, and Element a precedes Element b, calling b.before(a) will cause Element b to be moved to one index greater than intended.
This occurs due to the internals of Node.reparentChild() having removed the node and then the subsequent call to nodes.addAll() using a stale value for index.
Perhaps it would be better to call a.remove() earlier in the process, at some point before the index for b is being determined. As a work-around, I am calling a.remove() prior to calling b.before(a)
Also note: Node.remove() throws a ValidationException if there is no parent to remove it from - for those watching, be aware of the need to check before calling Node.remove() in the case you are exercising the same work-around. (I would have opted for the method to simply do nothing if there is nothing to remove the element)
The text was updated successfully, but these errors were encountered: