Re: [whatwg/dom] Use a queue of deferred steps in the node insertion algorithm (#732)

annevk commented on this pull request.

This looks good. I guess the way to go here is that you also create patches for the HTML Standard, and then we all review those, the tests, and this change a couple times until we're sure it all works?

> @@ -2389,12 +2389,10 @@ of a <var>node</var> into a <var>parent</var> before a <var>child</var>, run the
 </ol>
 
 <p><a lt="Other applicable specifications">Specifications</a> may define
-<dfn export id=concept-node-insert-ext>insertion steps</dfn> for all or some <a for=/>nodes</a>. The
-algorithm is passed <var ignore>insertedNode</var>, as indicated in the <a for=/>insert</a>
-algorithm below.
-<!-- See https://github.com/whatwg/dom/issues/34#issuecomment-125571750 for why we might need to
-     adjust this further based on the requirements of the script element. There might be other ways
-     to define that though as Olli suggests, so leaving that out for now. -->
+<dfn export id=concept-node-insert-ext>insertion steps</dfn> or
+<dfn export id=concept-node-children-added>children added steps</dfn> for all or some
+<a for=/>nodes</a>. The two algorithms are passed <var ignore>insertedNode</var> and
+<var ignore>deferredStepsQueue</var>, as indicated in the <a for=/>insert</a> algorithm below.

Should we mention child text content change steps here?

> @@ -2440,8 +2438,12 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
  <li><p>Let <var>previousSibling</var> be <var>child</var>'s <a>previous sibling</a> or
  <var>parent</var>'s <a>last child</a> if <var>child</var> is null.
 
+ <li><p>Let <var>deferredStepsQueue</var> be the empty <a>queue</a>.

an empty queue*

>   <li>
-  <p>For each <var>node</var> in <var>nodes</var>, in <a>tree order</a>:
+  <p>For each <var>node</var> in <var>nodes</var>:

Optional nit: `<a for=list>For each</a>`

> @@ -2453,8 +2455,7 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
    <li><p>If <var>parent</var> is a <a for=Element>shadow host</a> and <var>node</var> is a
    <a>slotable</a>, then <a>assign a slot</a> for <var>node</var>.
 
-   <li>If <var>node</var> is a {{Text}} node, run the <a>child text content change steps</a> for
-   <var>parent</var>.
+   <li>If <var>node</var> is a {{Text}} node, set <var>textChanged</var> to true.

then set*

> @@ -2490,6 +2492,22 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
    </li>
   </ol>
 
+ <li><p>If <var>textChanged</var> is true, run the <a>child text content change steps</a> for

then run*

(so it's observable if we run these before the insertion steps? I guess there's a test?)

> @@ -2490,6 +2492,22 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
    </li>
   </ol>
 
+ <li><p>If <var>textChanged</var> is true, run the <a>child text content change steps</a> for
+ <var>parent</var>.
+
+ <li><p>If <var>nodes</var> is not empty, run the <a>children added steps</a> with
+ <var>parent</var> and <var>deferredStepsQueue</var>.
+
+ <li>
+  <p>While <var>deferredStepsQueue</var> is not empty:

`<a for=queue>is not empty</a>` iirc

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/pull/732#pullrequestreview-201569582