Re: [whatwg/dom] Enforce same parameters for attachShadow on declarative shadow root (PR #1246)

@annevk commented on this pull request.

Thanks this looks good.

@Emilio could you double check this is what you had in mind?

> +   <li><p>Let <var>currentShadowRoot</var> be <var>element</var>'s
+   <a for=Element>shadow root</a>.
+
+   <li><p>If one of the following is true:
+
+   <ul>
+    <li><p><var>currentShadowRoot</var>'s <a for=ShadowRoot>declarative</a> is false,
+
+    <li><p><var>currentShadowRoot</var>'s <a for=ShadowRoot>mode</a> does not match
+    <var>mode</var>,
+
+    <li><p><var>currentShadowRoot</var>'s <a for=ShadowRoot>delegates focus</a> does
+    not match <var>delegatesFocus</var>,
+
+    <li><p><var>currentShadowRoot</var>'s <a for=ShadowRoot>slot assignment</a> does
+    not match <var>slotAssignment</var>,

```suggestion
    not match <var>slotAssignment</var>, or
```

> @@ -6958,8 +6960,31 @@ a boolean <var>delegatesFocus</var>, and a boolean <var>slotAssignment</var>:
   <p>If <var>element</var> is a <a for=Element>shadow host</a>, then:
 
   <ol>
-   <li><p>If <var>element</var>'s <a for=Element>shadow root</a>'s <a for=ShadowRoot>declarative</a>
-   is false, then <a>throw</a> an "{{NotSupportedError!!exception}}" {{DOMException}}.
+   <li><p>Let <var>currentShadowRoot</var> be <var>element</var>'s
+   <a for=Element>shadow root</a>.
+
+   <li><p>If one of the following is true:

This `<li>` contains multiple children so the `<p>` needs to be on a newline and indented. This also impacts all elements below which also do not look indented correctly.

> +    <li><p><var>currentShadowRoot</var>'s <a for=ShadowRoot>mode</a> does not match
+    <var>mode</var>,
+
+    <li><p><var>currentShadowRoot</var>'s <a for=ShadowRoot>delegates focus</a> does
+    not match <var>delegatesFocus</var>,
+
+    <li><p><var>currentShadowRoot</var>'s <a for=ShadowRoot>slot assignment</a> does
+    not match <var>slotAssignment</var>,
+
+    <li><p><var>currentShadowRoot</var>'s <a for=ShadowRoot>clonable</a> does
+    not match <var>clonable</var>,
+   </ul>
+
+   <p>then <a>throw</a> a "{{NotSupportedError!!exception}}" {{DOMException}}.
+
+   <p class=note>Any new parameters added to <a>attach a shadow root</a> should be

Notes cannot contain "should". Maybe this should just be a source code comment directly under the start of the "attach a shadow root" algorithm? It's something we shouldn't forget about after all.

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

Message ID: <whatwg/dom/pull/1246/review/1849135516@github.com>

Received on Monday, 29 January 2024 16:43:58 UTC