Bug 20654 - Have Range.surroundContents() work for partially contained nodes?
Have Range.surroundContents() work for partially contained nodes?
Status: RESOLVED WONTFIX
Product: WebAppsWG
Classification: Unclassified
Component: DOM
unspecified
PC All
: P2 normal
: ---
Assigned To: Anne
public-webapps-bugzilla
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-12 09:57 UTC by Anne
Modified: 2014-02-14 12:44 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Anne 2013-01-12 09:57:52 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?
Comment 1 Aryeh Gregor 2013-01-16 12:20:16 UTC
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
  <p>foo</p><p>bar</p>
with "ob" selected, and do
  range.surroundContents(document.createElement("i"))
I would expect
  <p>fo<i>o</i></p><p><i>b</i>ar</p>
not
  <p>fo<i>o<p>b</p></i></p><p>ar</p>
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
  foo<font color=red>bar</font>baz
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?
Comment 2 Anne 2014-02-12 19:53:20 UTC
Just to be clear, I am even less sure than Aryeh.