Calling surroundContent on a range that has its start and end boundary points in the same textnode fails completely. The first problem is that we check if the start boundary point is in a textnode, if so we split that node. Then we check if the end boundary point is in a text node and we try to split that node too. However, since we split the textnode, the end boundary point is now really in the node created by the split, so we're using the wrong container and offset for the end boundary point. I tried moving the split of the end boundary point before the split of the start boundary point. Then we hit the next problem, we try to insert the node that gets passed in to surroundContent as a child node of the common ancestor of the start and end boundary points. Since they're in the same textnode we seem to take that textnode as the common ancestor and so the insert fails (textnodes can't have children). Testcase coming up.

This patch just moves the splitting of the text node that contains the end boundary point before the splitting of the text node that contains the start boundary point. Doesn't solve all the issues.

Related bug: Range surroundContents not implemented http://bugzilla.mozilla.org/show_bug.cgi?id=58974

I think the current implementation of SurroundContents() needs to be trashed and then re-written to use ExtractContents() to break the "selected" content into a subtree, InsertNode() for the new container at the collapsed point after the extraction, and then the extracted contents inserted under the new container. Note that ExtractContents() still doesn't work exactly as the spec dictates because the subtree it extracts is an entirely new set of nodes representing what was selected. See this for more details: http://lxr.mozilla.org/seamonkey/source/content/base/src/nsRange.cpp#1816

*** Bug 221794 has been marked as a duplicate of this bug. ***

The previous test case had an infinite recursion bug.

I just posted a patch in bug 58974 that re-implements SurroundContents() and ExtractContents(). It fixes the test case in this bug.

I was unable to merge all of Kin's patch to the tip, so I thought I'd just merge the SurroundContents part of the patch, which would at least fix this bug. Basically I applied Kin's patch, cut out SurroundContents and pasted it into a trunk version of nsRange.cpp; it still has the ExtractContents issue though.

Comment on attachment 157617 [details] [diff] [review] SurroundContents part of bug 58974 In theory I can assume r=jfrancis sr=jst right?

Comment on attachment 157617 [details] [diff] [review] SurroundContents part of bug 58974 Yeah, seems reasonable to me.

Fix checked in.

I wonder if this bug has rell been resolved. I get the same issue today, with: Mozilla 1.7.12 Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.12) Gecko/20051022 and: Firefox 1.0.7 Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.12) Gecko/20060106 Firefox/1.0.7 and it's 2006! the bug was suppsedly solved in 2004...?

I wonder why can't one re-open a bug...

Don't reopen this bug, it is fixed in Firefox 1.5 and trunk. It was never fixed on the 1.0.x branch (and there's no need to do so). If you still see the issue on a trunk build, open a new bug.