[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