Bugzilla – Bug 20654
Have Range.surroundContents() work for partially contained nodes?
Last modified: 2014-02-14 12:44:32 UTC
"Why does Range#surroundContents() throw if you split a non-text node? The logic is already there for extractContents(), so why not use it?" https://twitter.com/yaypie/status/289891445672902657
"It’s trivial to work around, but means that surroundContents() isn’t very useful as currently specced." https://twitter.com/yaypie/status/289891735394480128
The reason the specification suggests it is done (in a comment in the source) because it might be unexpected to clone containers.
Should we change this?
I don't think extractContents() does what we want here. That clones *existing* containers. In this case, I think expected behavior would be to clone the *new* container. So if you have a DOM of
with "ob" selected, and do
I would expect
which is what you'd get if you applied extractContents(). The latter effect is much more disruptive, because you don't know what existing DOM structure you're messing with -- e.g., the misnesting here. It's also equivalent to extractContents() + insertNode() + appendChild(), so no need for a separate function. The former behavior is closer to what you'd expect for a function that aims to do things like "make selection bold" in a simple-minded fashion, which I think is the goal of surroundContents(). The cloning shouldn't do anything surprising if you pass simple nodes to it -- e.g., just pass document.createElement() with some non-id attributes set and you're probably fine even if it gets cloned a bunch.
Even with the change, though, this method is too simplistic to use for even basic formatting. If you have something like
and try to select all the text and wrap in <font color=green> using this method, it won't do what you want -- "bar" will remain red. What's the use-case for using this method even with the change? Does anyone use it now, and if so, for what?
Just to be clear, I am even less sure than Aryeh.