[Webkit-unassigned] [Bug 198578] Implement ParentNode.prototype.replaceChildren
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jun 1 07:35:22 PDT 2020
https://bugs.webkit.org/show_bug.cgi?id=198578
Darin Adler <darin at apple.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |darin at apple.com
Attachment #400694|review? |review+
Flags| |
--- Comment #6 from Darin Adler <darin at apple.com> ---
Comment on attachment 400694
--> https://bugs.webkit.org/attachment.cgi?id=400694
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=400694&action=review
> Source/WebCore/dom/ContainerNode.cpp:674
> +// https://dom.spec.whatwg.org/#concept-node-replace-all
> +void ContainerNode::replaceAllChildrenNaively(Ref<Node>&& node)
> +{
> + Ref<ContainerNode> protectedThis(*this);
> + ChildListMutationScope mutation(*this);
> + removeAllChildrenWithScriptAssertion(ChildChangeSource::API, DeferChildrenChanged::Yes);
> +
> + // we assume that we check preinsertion validity on the caller side.
> + auto result = appendChildWithoutPreInsertionValidityCheck(node);
> + ASSERT_UNUSED(result, !result.hasException());
> +
> + rebuildSVGExtensionsElementsIfNecessary();
> + dispatchSubtreeModifiedEvent();
> +}
I don’t see the value in factoring this out as a named function. The pre-condition that you must already have checked pre-insertion validity makes it hard to use this correctly. I suggest just including this code in ContainerNode::replaceChildren for now, and factoring it out only if we find we have a good way to share code with other call sites.
I don’t understand why it is OK that this does not use executeNodeInsertionWithScriptAssertion.
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200601/b54ca042/attachment.htm>
More information about the webkit-unassigned
mailing list