Bug 217537 – replaceChildren() (with no arguments) silently does nothing rather than removing the children

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Ashley Gullen]

Safari TP114 appears to ship a broken implementation of elem.replaceChildren() which is enabled by default.

Demo: https://downloads.scirra.com/labs/replacechildren.html

Click the button. It calls replaceChildren() with empty arguments to remove the three paragraphs.

It works in Chrome and Firefox and the elements disappear. In Safari it does nothing - no elements are removed.

[Alexey Proskuryakov]

Same behavior in Safari 14.0.1 beta.

[Radar WebKit Bug Importer]

<rdar://problem/70161896>

[Tetsuharu Ohzeki [UTC+9]]

(In reply to Alexey Proskuryakov from comment #1)
> Same behavior in Safari 14.0.1 beta.

I seem Safari v14 does not enable `ParentNode.replaceChildren()` in the first place.

Oddly Safari TP 114 and the trunk which enables that API also does not work this.

[Darin Adler]

This was added in r262381 and passed all the Web Platform Tests then and since. Haven’t looked at this test here yet.

[Darin Adler]

I took a look at the DOM specification. It has an incorrect step:

https://dom.spec.whatwg.org/#dom-parentnode-replacechildren

Note that step 2 says "Ensure pre-insertion validity of node into this before null." But that should only be done if node is non-null. We should probably report that issue. If we took it literally then the function would need to throw a HierarchyRequestError exception, but clearly that’s needed.

I took a look at the Web Platform Tests.

https://github.com/web-platform-tests/wpt/blob/master/dom/nodes/ParentNode-replaceChildren.html

For some reason it does not include a test case for replaceChildren without any argument, on a parent having a child. So we pass that test even though our algorithm is incorrect.

Fixing the bug is really easy, and it’s also easy for me to add a test to WPT, but I don’t know how to correctly upstream a change like this. Could use some help with that.

[Darin Adler]

(In reply to Darin Adler from comment #5)
> I took a look at the DOM specification. It has an incorrect step:
> 
> https://dom.spec.whatwg.org/#dom-parentnode-replacechildren

Filed https://github.com/whatwg/dom/issues/901

[Darin Adler]

Created attachment 411013 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Darin Adler]

Created attachment 411015 [details]
Patch

[Darin Adler]

I’m going to need help with the WPT export process. I tried and couldn’t make it work.

[Darin Adler]

https://github.com/web-platform-tests/wpt/pull/26075

[Darin Adler]

Committed r268314: <https://trac.webkit.org/changeset/268314>

[Ryosuke Niwa]

FWIW, this is definitely enabled by default on Safari 14.0.1.

[Darin Adler]

You mean that the broken version is included in Safari 14.0.1 or that the fixed version is included in 14.0.1?

[Tetsuharu Ohzeki [UTC+9]]

(In reply to Ryosuke Niwa from comment #13)
> FWIW, this is definitely enabled by default on Safari 14.0.1.

Oh, I'm sorry to add the incomplete implementation in bug 198578...

[Ryosuke Niwa]

(In reply to Darin Adler from comment #14)
> You mean that the broken version is included in Safari 14.0.1 or that the
> fixed version is included in 14.0.1?

The broken version is in Safari 14.0.1.