Re: [whatwg/dom] Set registry on null registry element on adopt instead of on insert (PR #1423)

@annevk approved this pull request.

I rebased and pushed some nits. This looks good to me now but if people could have another look and answer my questions I'd appreciate it.

> @@ -2899,17 +2899,13 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
      <a for=iteration>continue</a>.
 
      <li>
-      <p>If <var>inclusiveDescendant</var> is an <a for=/>element</a>:
+      <p>If <var>inclusiveDescendant</var> is an <a for=/>element</a> and
+      <var>inclusiveDescendant</var>'s <a for=Element>custom element registry</a> is non-null:

I suspect this is not a change in behavior, as we can't upgrade elements with a null registry, but was this change intentional?

>  
       <ol>
-       <li><p>If <var>inclusiveDescendant</var>'s <a for=Element>custom element registry</a> is
-       null, then set <var>inclusiveDescendant</var>'s <a for=Element>custom element registry</a> to
-       the result of <a for=/>looking up a custom element registry</a> given
-       <var>inclusiveDescendant</var>'s <a for=tree>parent</a>.
-
-       <li><p>Otherwise, if <var>inclusiveDescendant</var>'s
-       <a for=Element>custom element registry</a>'s <a for=CustomElementRegistry>is scoped</a> is
-       true, <a for=set>append</a> <var>inclusiveDescendant</var>'s <a for=Node>node document</a> to
+       <li><p>If <var>inclusiveDescendant</var>'s <a for=Element>custom element registry</a>'s
+       <a for=CustomElementRegistry>is scoped</a> is true, then <a for=set>append</a>
+       <var>inclusiveDescendant</var>'s <a for=Node>node document</a> to

To be sure, we continue to do this during insert instead of adopt to minimize the number of documents a registry has to look through, right?

> -     <li><p>If <var>inclusiveDescendant</var> is a <a for=/>shadow root</a> and
-     <var>inclusiveDescendant</var>'s <a for=ShadowRoot>custom element registry</a>
-     <a>is a global custom element registry</a>, then set <var>inclusiveDescendant</var>'s
-     <a for=ShadowRoot>custom element registry</a> to <var>document</var>'s
-     <a>effective global custom element registry</a>.
+     <li>
+      <p>If <var>inclusiveDescendant</var> is a <a for=/>shadow root</a> and if any of the following
+      are true:
+
+      <ul>
+       <li><p><var>inclusiveDescendant</var>'s <a for=ShadowRoot>custom element registry</a> is null
+       and <var>inclusiveDescendant</var>'s <a for=ShadowRoot>keep custom element registry null</a>
+       is false; or
+
+       <li><p><var>inclusiveDescendant</var>'s <a for=ShadowRoot>custom element registry</a>
+       <a>is a global custom element registry</a>,

I changed this to "is a global custom element registry" as this was lacking a null check before.

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

Message ID: <whatwg/dom/pull/1423/review/3503735066@github.com>