This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.
The spec has: Node cloneNode(optional boolean deep = true); but the web depends on passing undefined being treated as false. In particlar, CKEditor depends on it. See https://bugzilla.mozilla.org/show_bug.cgi?id=926305
So we want cloneNode() and importNode() to have different signatures? Note that cloneNode() in Chrome defaults to true as well.
Er, I somehow missed importNode. importNode needs to change too. > Note that cloneNode() in Chrome defaults to true as well. Exactly. But cloneNode(undefined) uses false, not true.
Bah bah bah. https://github.com/whatwg/dom/commit/70fdc5f9030c22c5ceca9e875e06f8b305529ef8
OK. I'll update Blink. Should be safe since the default to true case is still hot out of the oven.
> OK. I'll update Blink. There should be no need to change anything in Blink. This testcase: <body><script> alert(document.body.cloneNode(undefined).childNodes.length); alert(document.body.cloneNode().childNodes.length); </script> already alerts "0" and then "2" in my Chrome dev build (32.0.1671.3), which is what the DOM spec _used_ to require before the WebIDL changes to how undefined is handled, and is also what the DOM spec should imo specify once this bug is fixed. Anne, your changes aren't what this bug was asking for. What this bug is asking for is: Node cloneNode(); // deep clone specified in prose Node cloneNode(boolean deep); and similar for importNode. Reopening, since as far as I can tell the intent of the desired change was totally misunderstood.
In particular, the web compat requirement here is that cloneNode(undefined) do a shallow clone. There is no strong web compat requirement of any sort on cloneNode() with no arguments, since that's only been non-throwing in Gecko for a short time. During that time, though, it's consistently done a deep clone.
> already alerts "0" and then "2" Er, "0" and then "1", since I got rid of the whitespace before <script>. ;)
> There is no strong web compat requirement of any sort on cloneNode() with no arguments In that case it would ideally be the same as cloneNode(undefined), right? I.e. shallow clone? I think that's what Anne's changes implement?
Yeah, I don't want omitted and undefined to be different if we can avoid it. And since WebKit already had this as false for both cases...
I disagree. It would be better for cloneNode() to throw than to do a shallow clone.
We cannot throw for undefined and we want undefined to be false so omitted should also be false.
> In that case it would ideally be the same as cloneNode(undefined), right? Well, maybe. There may well be a web compat requirement here anyway, I just can't tell. Or put another way, I'm not willing to change the Gecko behavior to have cloneNode with no arguments do a shallow clone, because I'm worried about that causing compat problems at this point and I don't think theoretical purity is worth breaking compat in this case.
bz: How long have you been shipping with optional meaning true? Since you used to throw for no arguments until pretty recently most code out there always pass an argument. Also IE and WebKit never changed to treat optional as true. I think this is as close to safe as it can get on the web.
> bz: How long have you been shipping with optional meaning true? Since Firefox 13. So in final releases since June 5, 2012. > until pretty recently Over a year is pushing it for "pretty recently". > I think this is as close to safe as it can get on the web. I disagree. :(
That is, I think there is a good chance of Gecko making this change breaking content, due to content-sniffing and Gecko-only codepaths. :( We would _definitely_ break a number of Firefox extensions; I just checked and a bunch of them use cloneNode() with no arguments and expect deep clones. We might be willing to go through all that breakage if there were a very strong reason to, but in this case, I just don't think the reasons are strong enough.
(In reply to Boris Zbarsky from comment #15) > We might be willing to go through all that breakage if there were a very > strong reason to, but in this case, I just don't think the reasons are > strong enough. Compat with IE and Safari seems like reasons strong enough to me.
Mmm, maybe. I just don't see an obvious way for us to go about making this change without causing problems for our users, unfortunately. It's not clear to me why we should be doing that instead of IE and Safari changing here.
And I should note that at least for me my willingness to implement speculative spec proposals in the DOM spec is at an all-time low due to my experiences with this situation and similar. :(
bz, I'm sorry. I wish we had seen this one coming and I wish we had thought longer about IDL when it was still fresh, but it seems the only way we learn is extremely slow iteration with lots of drama along the way.
That doesn't answer my question from comment 17, note.
And one other note. The compat with Safari/IE argument is no stronger than it was two weeks ago. So I don't buy that it's particularly compelling in this case, if it wasn't then....
So these are the options as far as I can tell: 1) cloneNode() -> cloneNode(true) cloneNode(undefined) -> cloneNode(false) 2) cloneNode() -> cloneNode(false) cloneNode(undefined) -> cloneNode(false) The latter is the preferred way of treating undefined. It also matches WebKit and IE. The former matches Gecko and Chrome and will require some IDL hack long term to make it work. Chrome only recently switched from 2. Gecko threw for the cloneNode() case until one year ago. It seems to me the safest is to go with 2.
On the other hand, for the common case of cloning, which is deep clones, the API would be _way_ better if it could be just cloneNode(). The only real issue here is that there is legacy content that depends on cloneNode(undefined) doing a shallow clone. While we need to support that legacy content, I believe that the default behavior for no arguments passed should be the sane one. That's why the mailing list discussion initially decided the default here should be "true" to start with. Does the argument for consistency between undefined and not passed really trump the web developer ergonomics argument here? > and will require some IDL hack long term to make it work. That's putting spec purity ahead of developers, which is just wrong from a priority of constituencies perspective...
And again, it seems to me like we're allowing the compat between undefined and omitted edge case tail wag the API design dog here...
As far as I can tell the developer ergonomics argument applies both ways. Having undefined and omitted not equal each other will be found surprising and lead to subtle bugs.
What is surprising if passing an argument has different behavior than not passing an argument?
https://github.com/w3c/web-platform-tests/pull/473
We're going to align with the new spec here, fwiw, since it seems like no one else is interested in doing something different....