Re: [whatwg/dom] Add declarative Shadow DOM features (#892)

@mfreed7 commented on this pull request.

Ok, I think I addressed all of the comments.

I had to also fix up a bunch of lines that broke the build, for missing `for=` on `<a>length</a>` and `<a>activation behavior</a>`. I wasn't sure of the proper reference for all of the `length`s and some may really refer to `string` or something else.

> @@ -4363,6 +4363,26 @@ dom-Range-extractContents, dom-Range-cloneContents -->
  <a for=tree>children</a> of <var>node</var> and append them to <var>copy</var>, with
  <var>document</var> as specified and the <i>clone children flag</i> being set.
 
+ <li>
+  <p>If <var>node</var> is a <a for=Element>shadow host</a> whose <a for=/>shadow root</a>'s
+  <a for=ShadowRoot>clonable</a> is true:
+
+  <ol>
+   <li><p>Run <a>attach a shadow root</a> with <var>copy</var>, <var>node</var>'s <a for=/>shadow

Done. Any reason we keep it this way? I.e. the existing dom.bs file would automatically comply with a new rule that says you *can* split within an element. Why not relax it?

> @@ -4363,6 +4363,26 @@ dom-Range-extractContents, dom-Range-cloneContents -->
  <a for=tree>children</a> of <var>node</var> and append them to <var>copy</var>, with
  <var>document</var> as specified and the <i>clone children flag</i> being set.
 
+ <li>
+  <p>If <var>node</var> is a <a for=Element>shadow host</a> whose <a for=/>shadow root</a>'s
+  <a for=ShadowRoot>clonable</a> is true:
+
+  <ol>
+   <li><p>Run <a>attach a shadow root</a> with <var>copy</var>, <var>node</var>'s <a for=/>shadow
+   root</a>'s <a for=ShadowRoot>mode</a>, true, <var>node</var>'s <a for=/>shadow root</a>'s
+   <a>delegates focus</a>.</p></li>
+
+   <li><p>If <var>node</var>'s <a for=/>shadow root</a>'s
+   <a for=ShadowRoot>is declarative shadow root</a> is true, then set <var>copy</var>'s
+   <a for=/>shadow root</a>'s <a for=ShadowRoot>is declarative shadow root</a> property to

Done.

> @@ -4363,6 +4363,26 @@ dom-Range-extractContents, dom-Range-cloneContents -->
  <a for=tree>children</a> of <var>node</var> and append them to <var>copy</var>, with
  <var>document</var> as specified and the <i>clone children flag</i> being set.
 
+ <li>
+  <p>If <var>node</var> is a <a for=Element>shadow host</a> whose <a for=/>shadow root</a>'s
+  <a for=ShadowRoot>clonable</a> is true:
+
+  <ol>
+   <li><p>Run <a>attach a shadow root</a> with <var>copy</var>, <var>node</var>'s <a for=/>shadow
+   root</a>'s <a for=ShadowRoot>mode</a>, true, <var>node</var>'s <a for=/>shadow root</a>'s
+   <a>delegates focus</a>.</p></li>

And they're disallowed? I'm asking these questions because, much like when you get frustrated when PRs don't conform to the rules, non-editor-PR-authors get frustrated when the rules lack consistency or logic.

Done.

> + <li>
+  <p>If <var>node</var> is a <a for=Element>shadow host</a> whose <a for=/>shadow root</a>'s
+  <a for=ShadowRoot>clonable</a> is true:
+
+  <ol>
+   <li><p>Run <a>attach a shadow root</a> with <var>copy</var>, <var>node</var>'s <a for=/>shadow
+   root</a>'s <a for=ShadowRoot>mode</a>, true, <var>node</var>'s <a for=/>shadow root</a>'s
+   <a>delegates focus</a>.</p></li>
+
+   <li><p>If <var>node</var>'s <a for=/>shadow root</a>'s
+   <a for=ShadowRoot>is declarative shadow root</a> is true, then set <var>copy</var>'s
+   <a for=/>shadow root</a>'s <a for=ShadowRoot>is declarative shadow root</a> property to
+   true.</p></li>
+
+   <li>If the <i>clone children flag</i> is set, <a lt="clone a node">clone</a> all the
+   <a>children</a> of <var>node</var>'s <a for=/>shadow root</a> and append them to

Both done.

> @@ -4862,8 +4882,9 @@ known as <dfn export id=concept-document lt="document">documents</dfn>.
 <dfn export for=Document id=concept-document-content-type>content type</dfn> (a string),
 <dfn export for=Document id=concept-document-url>URL</dfn> (a <a for=/>URL</a>),
 <dfn export for=Document id=concept-document-origin>origin</dfn> (an <a for=/>origin</a>),
-<dfn export for=Document id=concept-document-type>type</dfn> ("<code>xml</code>" or "<code>html</code>"), and
-<dfn export for=Document id=concept-document-mode>mode</dfn> ("<code>no-quirks</code>", "<code>quirks</code>", or "<code>limited-quirks</code>").
+<dfn export for=Document id=concept-document-type>type</dfn> ("<code>xml</code>" or "<code>html</code>"),
+<dfn export for=Document id=concept-document-mode>mode</dfn> ("<code>no-quirks</code>", "<code>quirks</code>", or "<code>limited-quirks</code>"), and
+<dfn export for=Document>allow declarative shadow roots</dfn> ("<code>allow</code>", "<code>deny</code>", or "<code>unset</code>").</p>

End tag removed.

I wasn't sure if you meant explain here in a comment or in the code. I assumed the former:

The "unset" state is used to have different behavior for synchronous parsing vs. document parsing. Because of the XSS restriction on synchronous parsing endpoints, the default there needs to be "deny". But for document parsing, the default can be "allow". So "unset" represents this default - you can see that in the way this flag is used by the HTML PR. If there's a better name or way to document this, let me know.

> @@ -5762,13 +5785,19 @@ It is initially set to false.</p>
 <p><a for=/>Shadow roots</a> have an associated
 <dfn export for=ShadowRoot>available to element internals</dfn>. It is initially set to false.</p>
 
+<p><a for=/>Shadow roots</a> have an associated <dfn export for=ShadowRoot>is declarative shadow

Both done.

> @@ -6727,11 +6757,34 @@ are:
 <p>The <dfn method for=Element><code>attachShadow(<var>init</var>)</code></dfn> method steps are:
 
 <ol>
- <li><p>If <a>this</a>'s <a for=Element>namespace</a> is not the <a>HTML namespace</a>,
+ <li><p>Run <a>attach a shadow root</a> with <a>this</a>, <var>init</var>'s
+ {{ShadowRootInit/mode}}, <var>init</var>'s {{ShadowRootInit/clonable}}, and <var>init</var>'s

Done.

> @@ -6727,11 +6757,34 @@ are:
 <p>The <dfn method for=Element><code>attachShadow(<var>init</var>)</code></dfn> method steps are:
 
 <ol>
- <li><p>If <a>this</a>'s <a for=Element>namespace</a> is not the <a>HTML namespace</a>,
+ <li><p>Run <a>attach a shadow root</a> with <a>this</a>, <var>init</var>'s
+ {{ShadowRootInit/mode}}, <var>init</var>'s {{ShadowRootInit/clonable}}, and <var>init</var>'s
+ {{ShadowRootInit/delegatesFocus}}.</p></li>
+
+ <li><p>Return <a>this</a>'s <a for=Element>shadow root</a>.</p></li>
+</ol>
+
+<p>The <dfn attribute for=Element><code>shadowRoot</code></dfn> attribute's getter steps are:

Done.

> +
+ <li><p>Return <a>this</a>'s <a for=Element>shadow root</a>.</p></li>
+</ol>
+
+<p>The <dfn attribute for=Element><code>shadowRoot</code></dfn> attribute's getter steps are:
+
+<ol>
+ <li><p>Let <var>shadow</var> be <a>this</a>'s <a for=Element>shadow root</a>.
+
+ <li><p>If <var>shadow</var> is null or its <a for=ShadowRoot>mode</a> is "<code>closed</code>",
+ then return null.</p></li>
+
+ <li><p>Return <var>shadow</var>.
+</ol>
+
+<p>To <dfn id=concept-attach-a-shadow-root>attach a shadow root</dfn>, given a <a for=Element>shadow

Done, and good catch, done.

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

Message ID: <whatwg/dom/pull/892/review/1248667356@github.com>

Received on Saturday, 14 January 2023 00:29:34 UTC