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

@annevk commented on this pull request.



> @@ -2902,12 +2902,7 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
       <p>If <var>inclusiveDescendant</var> is an <a for=/>element</a>:
 
       <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
+       <li><p>If <var>inclusiveDescendant</var>'s
        <a for=Element>custom element registry</a>'s <a for=CustomElementRegistry>is scoped</a> is

If the registry is null we have UB here, no?

> @@ -6018,12 +6006,14 @@ these steps:
     <ol>
      <li><p>Set <var>inclusiveDescendant</var>'s <a for=Node>node document</a> to <var>document</var>.
 
-     <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>:

Wrapping here doesn't follow the guidelines.

> @@ -6018,12 +6006,14 @@ these steps:
     <ol>
      <li><p>Set <var>inclusiveDescendant</var>'s <a for=Node>node document</a> to <var>document</var>.
 
-     <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>:
+      <ol>
+        <li>If <var>inclusiveDescendant</var>'s <a for=ShadowRoot>custom element registry</a>
+        <a>is a global custom element registry</a> or null, 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>.

Where exactly are we accounting for "keep custom element registry null" now?

> @@ -6032,7 +6022,7 @@ these steps:
        <var>inclusiveDescendant</var>'s <a for=Element>attribute list</a> to <var>document</var>.
 
        <li><p>If <var>inclusiveDescendant</var>'s <a for=Element>custom element registry</a>
-       <a>is a global custom element registry</a>, then set <var>inclusiveDescendant</var>'s
+       <a>is a global custom element registry</a> or null, then set <var>inclusiveDescendant</var>'s

I think this is weird now. "is a global custom element registry" also tries to account for null, but now you are invalidating that. You should change the concepts around.

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

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