Re: [whatwg/dom] Add Scoped Custom Element Registries (PR #1341)

@foolip commented on this pull request.

Note to self: editorial review done up the "Elements have an associated" bit.

> @@ -2830,9 +2830,22 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
      <li><p>Run the <a>insertion steps</a> with <var>inclusiveDescendant</var>.
 
      <li>
-      <p>If <var>inclusiveDescendant</var> is <a>connected</a>:
+      <p>If <var>inclusiveDescendant</var> is <a>connected</a> and is an <a for=/>element</a>:

In order to simplify the conditions, how about a "If inclusiveDescendant is not connected, continue." step before this?

>  
       <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 <b>looking up a custom element registry</b> given
+       <var>inclusiveDescendant</var>'s <a for=tree>parent</a>.
+       <!-- XXX xref -->

Is the idea to land both PRs and then wait until HTML is scraped into Bikeshed's data files before updating DOM to fix these refs?

How about adding to `<pre class=anchors>` or something to get it right when it first lands?

> +       <li><p>If <li><var>inclusiveDescendant</var>'s <a for=ShadowRoot>custom element registry</a>
+       is null and <li><var>inclusiveDescendant</var>'s

Accidental list items here:

```suggestion
       <li><p>If <var>inclusiveDescendant</var>'s <a for=ShadowRoot>custom element registry</a>
       is null and <var>inclusiveDescendant</var>'s
```

>  };
 Document includes DocumentOrShadowRoot;
 ShadowRoot includes DocumentOrShadowRoot;
 </pre>
 
-<p class=note>The {{DocumentOrShadowRoot}} mixin is expected to be used by other
+<dl class=domintro>
+ <dt><code><var>registry</var> = <var>documentOrShadowRoot</var> . {{DocumentOrShadowRoot/customElementRegistry}}</code>
+ <dd><p>Returns <var>documentOrShadowRoot</var>'s {{CustomElementRegistry}} object, if any;
+ otherwise null.
+</dl>
+
+<p>The <dfn attribute for=DocumentOrShadowRoot><code>customElementRegistry</code></dfn> getter steps
+are:
+
+<ol>
+ <li><p class="XXX">Handle the document node case.

I know this is unfinished, but how will this work, what's it for? Documents don't have a custom element registry now, and `initialize()` doesn't accept a `Document` argument.

> +<dfn export for="clone a node"><var>parent</var></dfn> (default null), and null or a
+{{CustomElementRegistry}} object <dfn export for="clone a node"><var>fallbackRegistry</var></dfn>

Similar to the style suggestion in the HTML PR:

```suggestion
<dfn export for="clone a node"><var>parent</var></dfn> (default null), and a
{{CustomElementRegistry}}-or-null <dfn export for="clone a node"><var>fallbackRegistry</var></dfn>
```

> +    <p class="note">This intentionally leaves <a for="clone a node"><i>fallbackRegistry</i></a> set
+    to null.

The wording here doesn't feel quite right for passing an argument, I went looking for a variable or slot called _fallbackRegistry_.

```suggestion
    <p class="note">This intentionally does not pass the <a for="clone a node"><i>fallbackRegistry</i></a> argument.
```

>    </ol>
 
  <li><p>Return <var>copy</var>.
 </ol>
 </div>
 
 <div algorithm>
-<p>To <dfn>clone a single node</dfn> given a <a for=/>node</a> <var>node</var> and
-<a for=/>document</a> <var>document</var>:
+<p>To <dfn>clone a single node</dfn> given a <a for=/>node</a> <var>node</var>,
+<a for=/>document</a> <var>document</var>, and null or a {{CustomElementRegistry}} object

```suggestion
<a for=/>document</a> <var>document</var>, and a {{CustomElementRegistry}}-or-null
```

> @@ -4617,18 +4680,23 @@ and an optional <a for=/>document</a> <dfn export for="clone a node"><var>docume
   <p>If <var>node</var> is an <a for=/>element</a>:
 
   <ol>
+   <li><p>Let <var>registry</var> be <var>node</var>'s <a for=Element>custom element registry</a>.

These are steps for elements. Isn't it also necessary to handle shadow roots here? I think we reach this algorithm from a plain `shadowRoot.clone()` call, right?

> @@ -5520,6 +5603,34 @@ method steps are to return the result of running the
 <a>internal <code>createElementNS</code> steps</a>, given <a>this</a>, <var>namespace</var>,
 <var>qualifiedName</var>, and <var>options</var>.
 
+<p>To <dfn>flatten element creation options</dfn>, given a string or {{ElementCreationOptions}}
+dictionary <var>options</var> and a <a for=/>document</a> <var>document</var>:
+
+<ol>
+ <li><p>Let <var>registry</var> be null.
+
+ <li><li>Let <var>is</var> be null.

```suggestion
 <li><p>Let <var>is</var> be null.
```

> @@ -5520,6 +5603,34 @@ method steps are to return the result of running the
 <a>internal <code>createElementNS</code> steps</a>, given <a>this</a>, <var>namespace</var>,
 <var>qualifiedName</var>, and <var>options</var>.
 
+<p>To <dfn>flatten element creation options</dfn>, given a string or {{ElementCreationOptions}}
+dictionary <var>options</var> and a <a for=/>document</a> <var>document</var>:
+
+<ol>
+ <li><p>Let <var>registry</var> be null.
+
+ <li><li>Let <var>is</var> be null.
+
+ <li>
+  <p>If <var>options</var> is a dictionary:
+
+  <ol>
+   <li><p>If <var>options</var>["{{ElementCreationOptions/customElementRegistry}}"]
+   <a for=map>exists</a>, then set <var>registry</var> to it.

I haven't seen this short and sweet "set x to it" style before, but I like it!

> + <li><p>Let <var>registry</var> be null.
+
+ <li><li>Let <var>is</var> be null.
+
+ <li>
+  <p>If <var>options</var> is a dictionary:
+
+  <ol>
+   <li><p>If <var>options</var>["{{ElementCreationOptions/customElementRegistry}}"]
+   <a for=map>exists</a>, then set <var>registry</var> to it.
+
+   <li><p>If <var>options</var>["{{ElementCreationOptions/is}}"] <a for=map>exists</a>, then set
+   <var>is</var> to it.
+  </ol>
+
+ <li><p>If <var>registry</var> is non-null and <var>is</var> is non-null, then <a>throw</a> a

Move this inside the if statement since it's impossible otherwise?

>    <var>node</var>'s <a for=tree>descendants</a>.
 
+  <p>If <var>options</var>'s {{ImportNodeOptions/customElementRegistry}} can be used to set the

The leading "If" seems like copypasta. This isn't great either but some change is needed:

```suggestion
  <p><var>options</var>'s {{ImportNodeOptions/customElementRegistry}} can be used to set the
```

> @@ -6094,12 +6232,26 @@ It is initially set to false.</p>
 <p><a for=/>Shadow roots</a> have an associated <dfn for=ShadowRoot>serializable</dfn> (a boolean).
 It is initially set to false.</p>
 
+<p><a for=/>Shadow roots</a> have an associated <dfn for=ShadowRoot>custom element registry</dfn>

Say that it is initially null?

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

Message ID: <whatwg/dom/pull/1341/review/2702739684@github.com>