Re: [whatwg/dom] Introduce `moveBefore()` state-preserving atomic move API (PR #1307)

@annevk commented on this pull request.



> @@ -2881,6 +2881,145 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
 </ol>
 
 
+<p><a lt="Other applicable specifications">Specifications</a> may define
+<dfn export id=concept-node-move-ext>moving steps</dfn> for all or some <a for=/>nodes</a>. The
+algorithm is passed a <a for=/>node</a> <var ignore>movedNode</var>, and a <a for=/>node</a>-or-null
+<var ignore>oldParent</var> as indicated in the <a for=/>move</a> algorithm below. Like the
+<a>insertion steps</a>, these steps must not modify the <a>node tree</a> that
+<var>insertedNode</var> <a>participates</a> in, create <a for=/>browsing contexts</a>,

```suggestion
<var>movedNode</var> <a>participates</a> in, create <a for=/>browsing contexts</a>,
```

> @@ -2881,6 +2881,145 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
 </ol>
 
 
+<p><a lt="Other applicable specifications">Specifications</a> may define
+<dfn export id=concept-node-move-ext>moving steps</dfn> for all or some <a for=/>nodes</a>. The
+algorithm is passed a <a for=/>node</a> <var ignore>movedNode</var>, and a <a for=/>node</a>-or-null
+<var ignore>oldParent</var> as indicated in the <a for=/>move</a> algorithm below. Like the
+<a>insertion steps</a>, these steps must not modify the <a>node tree</a> that
+<var>insertedNode</var> <a>participates</a> in, create <a for=/>browsing contexts</a>,
+<a lt="fire an event">fire events</a>, or otherwise execute JavaScript. These steps may queue tasks
+to do these things asynchronously, however.
+
+
+<p>To <dfn>move</dfn> a <a for=/>node</a> <var>node</var> into a <a for=/>node</a>
+<var>newParent</var> before a <a for=/>node</a> <var>child</var>:

```suggestion
<var>newParent</var> before a <a for=/>node</a>-or-null <var>child</var>:
```
Right? Otherwise why have a non-null check below.

> + <li><p>For each {{NodeIterator}} object <var>iterator</var> whose
+ <a for=traversal>root</a>'s <a for=Node>node document</a> is <var>node</var>'s
+ <a for=Node>node document</a>, run the <a><code>NodeIterator</code> pre-remove steps</a> given
+ <var>node</var> and <var>iterator</var>.
+
+ <li><p>Let <var>oldPreviousSibling</var> be <var>node</var>'s <a>previous sibling</a>.
+
+ <li><p>Let <var>oldNextSibling</var> be <var>node</var>'s <a for=tree>next sibling</a>.
+
+ <li><p><a for=set>Remove</a> <var>node</var> from <var>oldParent</var>'s <a for=tree>children</a>.
+
+ <li><p>If <var>node</var> is <a for=slottable>assigned</a>, then run <a>assign slottables</a> for
+ <var>node</var>'s <a>assigned slot</a>.
+
+ <li><p>If <var>oldParent</var>'s <a for=tree>root</a> is a <a for=/>shadow root</a>, and
+ <var>oldParent</var> is a <a>slot</a> whose <a for=slot>assigned nodes</a> is the empty list, then

```suggestion
 <var>oldParent</var> is a <a>slot</a> whose <a for=slot>assigned nodes</a> <a for=list>is empty</a>, then
```
(or something like that)

> + <li><p>If <var>node</var> is <a for=slottable>assigned</a>, then run <a>assign slottables</a> for
+ <var>node</var>'s <a>assigned slot</a>.
+
+ <li><p>If <var>oldParent</var>'s <a for=tree>root</a> is a <a for=/>shadow root</a>, and
+ <var>oldParent</var> is a <a>slot</a> whose <a for=slot>assigned nodes</a> is the empty list, then
+ run <a>signal a slot change</a> for <var>oldParent</var>.
+
+ <li>
+  <p>If <var>node</var> has an <a>inclusive descendant</a> that is a <a>slot</a>:
+
+  <ol>
+   <li><p>Run <a>assign slottables for a tree</a> with <var>oldParent</var>'s <a for=tree>root</a>.
+
+   <li><p>Run <a>assign slottables for a tree</a> with <var>node</var>.
+  </ol>
+ </li>

```suggestion
```

> +
+ <li><p>Let <var>newPreviousSibling</var> be <var>child</var>'s <a>previous sibling</a> if
+ <var>child</var> is non-null, and <var>newParent</var>'s <a>last child</a> otherwise.
+
+ <li><p>If <var>child</var> is null, then <a for=set>append</a> <var>node</var> to
+ <var>newParent</var>'s <a for=tree>children</a>.
+
+ <li><p>Otherwise, <a for=set>insert</a> <var>node</var> into <var>newParent</var>'s
+ <a for=tree>children</a> before <var>child</var>'s <a for=tree>index</a>.
+
+ <li><p>If <var>newParent</var> is a <a for=Element>shadow host</a> whose <a for=/>shadow root</a>'s
+ <a for=ShadowRoot>slot assignment</a> is "<code>named</code>" and <var>node</var> is a
+ <a>slottable</a>, then <a>assign a slot</a> for <var>node</var>.
+
+ <li><p>If <var>newParent</var>'s <a for=tree>root</a> is a <a for=/>shadow root</a>, and
+ <var>newParent</var> is a <a>slot</a> whose <a for=slot>assigned nodes</a> is the empty list,

empty list thing again

> @@ -3234,6 +3360,16 @@ Element includes ParentNode;
   the <a>node tree</a> are violated.
   <!-- "NotFoundError" is impossible -->
 
+ <dt><code><var>node</var> . <a method for=ParentNode lt="moveBefore()">moveBefore</a>(<var>movedNode</var>, <var>child</var>)</code>
+ <dd>
+  <p>Moves, without first removing, <var>movedNode</var> into <var>node</var> after <var>child</var>
+  if <var>child</var> is non-null; otherwise after the <a>last child</a> of <var>node</var>. This
+  methods preserves state associated with <var>movedNode</var> so that it persists after the move.

```suggestion
  method preserves state associated with <var>movedNode</var>.
```
persists seems redundant with preserves

> @@ -3234,6 +3360,16 @@ Element includes ParentNode;
   the <a>node tree</a> are violated.
   <!-- "NotFoundError" is impossible -->
 
+ <dt><code><var>node</var> . <a method for=ParentNode lt="moveBefore()">moveBefore</a>(<var>movedNode</var>, <var>child</var>)</code>
+ <dd>
+  <p>Moves, without first removing, <var>movedNode</var> into <var>node</var> after <var>child</var>
+  if <var>child</var> is non-null; otherwise after the <a>last child</a> of <var>node</var>. This
+  methods preserves state associated with <var>movedNode</var> so that it persists after the move.
+
+  <p><a>Throws</a> a "{{HierarchyRequestError!!exception}}" {{DOMException}} if the constraints of
+  the <a>node tree</a> are violated.
+  <!-- "NotFoundError" is impossible -->

We should probably call out that we also throw this when we cannot preserve state.

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

Message ID: <whatwg/dom/pull/1307/review/2648323581@github.com>