Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Methods on ChildNode and ParentNode that take (Node or DOMString)... nodes should not use an intermediate DocumentFragment #1313

Open
dbaron opened this issue Oct 4, 2024 · 4 comments
Labels
needs tests Moving the issue forward requires someone to write tests topic: nodes

Comments

@dbaron
Copy link
Member

dbaron commented Oct 4, 2024

What is the issue with the DOM Standard?

There are a bunch of methods on ChildNode and ParentNode that take a (Node or DOMString)... nodes argument. The specification text for these methods uses the converting nodes into a node algorithm, which temporarily moves the nodes into a DocumentFragment, and then these nodes are later moved to their final destination.

This is inefficient because it runs the insertion steps and removing steps for all the elements in the subtree an extra time, which involves an extra two traversals of the subtree and some additional work for the steps.

This has been optimized away in WebKit and I'm working on a similar optimization in Chromium. While I think it's doable to make these optimizations pass existing tests and not break existing Web content (WebKit has already shown this), I think the differences are almost certainly observable in various ways, including the states things get left in in various failure cases (e.g., when a node in the middle of the list can't be inserted).

While the current specification is convenient and simple, I think it would probably be better to describe the optimized behavior in the DOM specification, even though it's more complex, since the faster performance does serve end users better, and having better interoperability on the faster behavior would serve both users and developers even if it's more work for specification authors and implementers.

There are probably some interesting tradeoffs we can make in the process that make tradeoffs between (a) compatibility with existing content, (b) compatibility with the existing specified behavior (which may be different from (a)), (c) having a model that is generally consistent and (d) having a model that does reasonable things on errors, and (e) having a model that offers consistent behavior between specifying a single node and specifying multiple nodes (which I suspect the current specification text does not fully do, although it does try).

@annevk annevk added topic: nodes needs tests Moving the issue forward requires someone to write tests labels Oct 4, 2024
@annevk
Copy link
Member

annevk commented Oct 4, 2024

Yeah, that sounds reasonable. I guess we need some tests for the obscure cases too to decide what to do.

cc @rniwa

@dbaron
Copy link
Member Author

dbaron commented Oct 4, 2024

Also cc @smaug----

@dbaron
Copy link
Member Author

dbaron commented Oct 4, 2024

Oh, and one other edge case is whether these methods accept DocumentType nodes when changing the children of a Document. (This is because DocumentType nodes are not allowed as a child of a DocumentFragment.)

@smaug----
Copy link
Collaborator

Looks like the algorithms in DOM could almost handle this. Need to be careful with MutationRecords.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs tests Moving the issue forward requires someone to write tests topic: nodes
Development

No branches or pull requests

3 participants