Re: [whatwg/dom] Integration with trusted types inside of node conversion (PR #1258)

@annevk commented on this pull request.



> @@ -3040,6 +3040,34 @@ standards that want to define APIs shared between <a for=/>documents</a> and
  <a for=CharacterData>data</a> is the string and <a for=Node>node document</a> is
  <var>document</var>.
 
+ <li><p>Let <var>isScriptElement</var> be <var>parent</var> is a {{HTMLScriptElement}}.
+
+ <li><p><a for=set>For each</a> <var>value</var> in <var>nodes</var>:

The formatting here is wrong. Please ensure it's consistent with the rest of the document. The README should have instructions or a pointer to them.

> @@ -3040,6 +3040,34 @@ standards that want to define APIs shared between <a for=/>documents</a> and
  <a for=CharacterData>data</a> is the string and <a for=Node>node document</a> is
  <var>document</var>.
 
+ <li><p>Let <var>isScriptElement</var> be <var>parent</var> is a {{HTMLScriptElement}}.

This reads wonky. I think it needs to be something like:

> Let x be true if y; otherwise false.

Also, it's "an" HTMLScriptElement and that should probably read "an HTMLScriptElement node" or perhaps "a script element". (I forgot if the DOM Standard has precedent for this.)

> +     <li>Let <var>text</var> be the result of executing the [$Get Trusted Type compliant string$] algorithm, with the following arguments:
+        * {{TrustedScript}} as <span>expectedType</span>,
+        * <var>document</var>'s [=relevant global object=] as <span>global</span>,
+        * <var>value</var>'s <a for=CharacterData>data</a> as <span>input</span>,
+        * `HTMLScriptElement text` as <span>sink</span>,
+        * `script` as <span>sinkGroup</span>.

This invocation style is not described by Infra. It needs adjusting. We also don't use `<span>` in DOM in this way.

> + <li><p>Let <var>isScriptElement</var> be <var>parent</var> is a {{HTMLScriptElement}}.
+
+ <li><p><a for=set>For each</a> <var>value</var> in <var>nodes</var>:
+  <ol>
+   <li>If <var>value</var> is a <a for=/>node</a>, then:
+    <ol>
+     <li>If <var>isScriptElement</var> is false, skip to the next <var>value</var>.
+     <li>If <var>value</var> is not a {{Text}} <a for=/>node</a>, then <a for=iteration>continue</a>.
+     <li>Let <var>text</var> be the result of executing the [$Get Trusted Type compliant string$] algorithm, with the following arguments:
+        * {{TrustedScript}} as <span>expectedType</span>,
+        * <var>document</var>'s [=relevant global object=] as <span>global</span>,
+        * <var>value</var>'s <a for=CharacterData>data</a> as <span>input</span>,
+        * `HTMLScriptElement text` as <span>sink</span>,
+        * `script` as <span>sinkGroup</span>.
+
+        <p>If the algorithm threw an error, rethrow the error.

This is implied, see Infra.

> +     <li>If <var>isScriptElement</var> is false, skip to the next <var>value</var>.
+     <li>If <var>value</var> is not a {{Text}} <a for=/>node</a>, then <a for=iteration>continue</a>.
+     <li>Let <var>text</var> be the result of executing the [$Get Trusted Type compliant string$] algorithm, with the following arguments:
+        * {{TrustedScript}} as <span>expectedType</span>,
+        * <var>document</var>'s [=relevant global object=] as <span>global</span>,
+        * <var>value</var>'s <a for=CharacterData>data</a> as <span>input</span>,
+        * `HTMLScriptElement text` as <span>sink</span>,
+        * `script` as <span>sinkGroup</span>.
+
+        <p>If the algorithm threw an error, rethrow the error.
+     <li>Set <var>value</var>'s <a for=CharacterData>data</a> to <var>text</var>.
+    </ol>
+
+    <li>If <var>value</var> is a {{TrustedScript}}, then:
+     <ol>
+      <li>Let <var>text</var> be <var>value</var>'s Data internal slot value.

"Data internal slot value" needs some cross-referencing.

> @@ -3040,6 +3040,34 @@ standards that want to define APIs shared between <a for=/>documents</a> and
  <a for=CharacterData>data</a> is the string and <a for=Node>node document</a> is
  <var>document</var>.
 
+ <li><p>Let <var>isScriptElement</var> be <var>parent</var> is a {{HTMLScriptElement}}.
+
+ <li><p><a for=set>For each</a> <var>value</var> in <var>nodes</var>:
+  <ol>
+   <li>If <var>value</var> is a <a for=/>node</a>, then:

All `<li>` should have a nested `<p>` for any text.

>  
 <ol>
+ <li><p>Let <var>isScriptElement</var> be <var>parent</var> is a {{HTMLScriptElement}}.
+ <li><p>For each <var>value</var> in <var>nodes</var>:
+  <ol>
+   <li>If <var>value</var> is a <a for=/>node</a>, then:
+    <ol>
+     <li>If <var>isScriptElement</var> is false, skip to the next <var>value</var>.

You resolved this, but it doesn't appear to be resolved.

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

Message ID: <whatwg/dom/pull/1258/review/1934438899@github.com>

Received on Wednesday, 13 March 2024 15:07:50 UTC