Bug 15194 - Insertion of a DocumentFragment underspecified
Summary: Insertion of a DocumentFragment underspecified
Status: RESOLVED FIXED
Alias: None
Product: WebAppsWG
Classification: Unclassified
Component: DOM (show other bugs)
Version: unspecified
Hardware: PC All
: P2 normal
Target Milestone: ---
Assignee: Anne
QA Contact: public-webapps-bugzilla
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-15 00:52 UTC by Adam Klein
Modified: 2012-03-20 14:31 UTC (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Klein 2011-12-15 00:52:44 UTC
In "Mutation Algorithms" (http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#mutation-algorithms), step 4 of the "insert" algorithm says:

"If node is a DocumentFragment node, insert its children (preserving tree order), before child or at the end of parent if child is null." [note that "insert" is not hyperlinked to anything]

Should "insert its children" instead read "pre-insert its children"? In particular, I'm interested in when the children are meant to be disconnected from the DocumentFragment; are they removed all at once, then inserted one-by-one, or do they simply get removed as part of pre-insert step (6) (Adopt )?

For the purposes of MutationObservers, it would be convenient if the DocumentFragment's children were removed all at once, though that doesn't match the current WebKit behavior (I haven't looked at other implementations yet).
Comment 1 Adam Klein 2011-12-15 23:46:41 UTC
As discussed in http://lists.w3.org/Archives/Public/www-dom/2011OctDec/0130.html, Firefox treats the removal/insertion atomically and doesn't intend to change that. Seems like it would be good if DOM4 specified this all-at-once behavior, rather than inserting the children (and thus removing them from the fragment) one-by-one.
Comment 2 Jonas Sicking 2011-12-16 23:38:06 UTC
Yes, we absolutely do not want to be exposing intermediate states of when the nodes are transferred. The whole point of deprecating mutation events is to allow the implementation to be in an inconsistent state until all mutations for a given DOM-mutating function call has finished.

This also helps with performance since it allows implementations to remove the nodes front-to-back or back-to-front depending on what's fastest for the implementation.
Comment 3 Anne 2011-12-17 11:10:57 UTC
Ah. I missed they had to be removed from the DocumentFragment. Pre-insert seems like overkill, but they do need to be adopted, which means we need some kind of atomic adopt and remove my children definition. The remove variant we also need for setting innerHTML (for its original children).
Comment 4 Ojan Vafai 2011-12-18 18:29:28 UTC
Also, if it's atomic, we don't need to do all the error-checking that we normally do, e.g.,
-We don't need to check for cycles since the DocumentFragment clearly has no parent and none of it's children will cause a cycle.
-We only need to check that the new parent accepts inserting DocumentFragment nodes. We don't need to check each element since any node that accepts inserting DocumentFragments necessarily accepts anything that can be a child of a DocumentFragment.
-Don't need to check that the tree is in a sane state after each insertion (e.g. mutation events can't remove the node you're appending to).

Avoiding these checks should have a significant performance improvement.
Comment 5 Jonas Sicking 2011-12-19 11:11:06 UTC
> -We only need to check that the new parent accepts inserting DocumentFragment
> nodes. We don't need to check each element since any node that accepts
> inserting DocumentFragments necessarily accepts anything that can be a child of
> a DocumentFragment.

This is not true. You still need to check for the situation that a DocumentFragment is inserted into a Document, but the DocumentFragment contains more than one Element child. In this case you need to throw a HIERARCHY_REQUEST_ERR exception.
Comment 6 Anne 2012-03-20 14:29:54 UTC
I think I fixed this. Review welcome.