Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replacing an element with itself causes problems. #1843

Closed
jdngray77 opened this issue Sep 22, 2022 · 0 comments
Closed

Replacing an element with itself causes problems. #1843

jdngray77 opened this issue Sep 22, 2022 · 0 comments
Assignees
Labels
bug Confirmed bug that we should fix fixed
Milestone

Comments

@jdngray77
Copy link

jdngray77 commented Sep 22, 2022

Obviously there's no practical use in replacing an element with itself, but robustness is in the edge cases.

Take the document:

body
| h1
| div

If we were to replace the h1 with itself via h1.replaceWith(h1), it's next sibling, in this case the div, would be deleted and the h1 would remain - but without a reference to it's parent.

In this state, the node traverser fails when rendering the tostring().

Minimal reproducable :

        val doc = Document("")

        val targetElement = Element("h1")
        doc.body().appendChild(targetElement)

        // During the replace, the target is removed from the children.
        // 'Ensure child list' within the replace operation is
        // different if there is no children, so we must add another child.
        //
        // Also, there must be children *after* the target element, else
        // there will be an index out of bounds during the replace operation.
        doc.body().appendChild(Element("div"))

        targetElement.replaceWith(targetElement)

        // The document can no longer be rendered to string.
        assertDoesNotThrow {
            println(doc)
        }

This can be solved with a simple equality check.

Self assigned, pr soon

jdngray77 added a commit to jdngray77/jsoup that referenced this issue Sep 22, 2022
And add a test for this edge case

closes jhy#1843
@jhy jhy changed the title [Bug] Replacing an element with itself causes problems. Replacing an element with itself causes problems. Jan 6, 2023
@jhy jhy closed this as completed in 9bb07d2 Jan 19, 2023
@jhy jhy self-assigned this Jan 19, 2023
@jhy jhy added the bug Confirmed bug that we should fix label Jan 19, 2023
@jhy jhy added this to the 1.15.4 milestone Jan 19, 2023
@jhy jhy added the fixed label Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug that we should fix fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants