Re: [whatwg/dom] Add Imperative Slot API (#966)

@mfreed7 commented on this pull request.



> @@ -2192,6 +2192,10 @@ steps:</p>
  <li><p>If the <i>open flag</i> is set and <var>shadow</var>'s <a for=ShadowRoot>mode</a> is
  <em>not</em> "<code>open</code>", then return null.</p></li>
 
+ <li><p>If <var>shadow</var>'s <a for=ShadowRoot>slot assignment</a> is "<code>manual</code>",
+ then return the <a>slot</a> in <var>shadow</var>'s <a for=tree>descendants</a> whose <a>manually assigned nodes</a>

The `<a>manually assigned nodes</a>` is the cause of the build breakage here, but that refers to [a new thing](https://github.com/whatwg/html/pull/5483/files#diff-41cf6794ba4200b839c53531555f0f3998df4cbb01a4d5cb0b94e3ca5e23947dR59343) in the HTML PR. I'm not sure how to link that and get this build working. Please let me know if I'm missing something about the breakage.

> @@ -2192,6 +2192,10 @@ steps:</p>
  <li><p>If the <i>open flag</i> is set and <var>shadow</var>'s <a for=ShadowRoot>mode</a> is
  <em>not</em> "<code>open</code>", then return null.</p></li>
 
+ <li><p>If <var>shadow</var>'s <a for=ShadowRoot>slot assignment</a> is "<code>manual</code>",
+ then return the <a>slot</a> in <var>shadow</var>'s <a for=tree>descendants</a> whose <a>manually assigned nodes</a>

Hmm, if I'm thinking about this correctly, there should only be one slot referring to a given node, because the [assign](https://github.com/whatwg/html/pull/5483/files#diff-41cf6794ba4200b839c53531555f0f3998df4cbb01a4d5cb0b94e3ca5e23947dR59369) method loops through all slots within the shadow root and removes the given nodes from all other slots. It also de-duplicates assigned nodes within a single slot. So I think this algorithm should always return a unique slot. Correct me if I'm wrong.

Separately, I do think we need to add a step to [remove a node](https://dom.spec.whatwg.org/#concept-node-remove) that clears out all manually assigned nodes from `<slot>` elements when they get removed from the tree. Without that, there is the definite possibility of the multiple slot problem you bring up, because the `assign()` steps won't be able to find all slots that might point to a node.

>  
  <li>
-  <p>For each <a>slottable</a> <a for=tree>child</a> of <var>host</var>, <var>slottable</var>, in
+  <p>If <var>root</var>'s <a for=ShadowRoot>slot assignment</a> is "<code>manual</code>",
+  then:</p>
+
+  <ol>
+   <li><p>Let <var>result</var> be an empty list.</p></li>

I think this is ok also, for the same reasons as above.

-- 
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/966#pullrequestreview-629442389