Re: [whatwg/dom] Trusted types attributes (PR #1268)

@annevk commented on this pull request.



>  
- <li><p><a>Handle attribute changes</a> for <var>attribute</var> with <var>attribute</var>'s
- <a for=Attr>element</a>, <var>oldValue</var>, and <var>value</var>.
+ <li><p>If <var>attribute</var>'s <a for=Attr>element</a> <a lt="has an attribute">has</a>
+ an <a>attribute</a> <var>attribute</var>, then <a>handle attribute changes</a> for
+ <var>attribute</var> with <var>attribute</var>'s <a for=Attr>element</a>, <var>oldValue</var>, and
+ <var>value</var>.

Should this be _value_ or _attribute_'s value? They can be different, right?

>  
- <li><p><a>Handle attribute changes</a> for <var>attribute</var> with <var>attribute</var>'s
- <a for=Attr>element</a>, <var>oldValue</var>, and <var>value</var>.
+ <li><p>If <var>attribute</var>'s <a for=Attr>element</a> <a lt="has an attribute">has</a>
+ an <a>attribute</a> <var>attribute</var>, then <a>handle attribute changes</a> for
+ <var>attribute</var> with <var>attribute</var>'s <a for=Attr>element</a>, <var>oldValue</var>, and
+ <var>value</var>.
+
+ <li><p>Otherwise, throw an "{{InvalidStateError!!exception}}" {{DOMException}}.

Does this match the exception Trusted Types throws by default?

>  
- <li><p><a>Handle attribute changes</a> for <var>attribute</var> with <var>attribute</var>'s
- <a for=Attr>element</a>, <var>oldValue</var>, and <var>value</var>.
+ <li><p>If <var>attribute</var>'s <a for=Attr>element</a> <a lt="has an attribute">has</a>
+ an <a>attribute</a> <var>attribute</var>, then <a>handle attribute changes</a> for

"has an attribute" should be the linking text I think. Not sure why we'd mention the type as its own thing here.

> +<a>attribute</a> <var>attribute</var> to an <a for=/>element</a> <var>element</var>, with optional
+boolean <var>validate</var> (default true), run these steps:

an optional boolean*

You can remove "run these steps" as well. We tend to leave that out these days when the algorithm is introduced with "To" and no return value is to be listed.

>  
 <ol>
+ <li>
+  <p>If <var>validate</var> is true, then:
+  <ol>

Need a newline before `<ol>`.

>  
 <ol>
+ <li>
+  <p>If <var>validate</var> is true, then:

```suggestion
  <p>If <var>validate</var> is true:
```

>  
 <ol>
+ <li>
+  <p>If <var>validate</var> is true, then:
+  <ol>
+   <li><p><a>Verify and set attribute value</a>
+   <var>attribute</var>'s <a for="Attr">value</a> for <var>attribute</var> with <var>element</var>.

The text isn't consistent around comma usage when invoking this algorithm.

> @@ -6425,6 +6446,18 @@ steps:
  <a for=Attr>value</a>.
 </ol>
 
+<p>To <dfn id=concept-element-attributes-verify-and-set-value>verify and set attribute value</dfn>
+{{TrustedType}} or string <var>value</var> for an <a>attribute</a> <var>attribute</var>, with
+<a for=/>element</a> <var>element</var>:
+
+<ol>
+ <li><p>Let <var>validValue</var> be the result of calling

verifiedValue, presumably?

> @@ -6425,6 +6446,18 @@ steps:
  <a for=Attr>value</a>.
 </ol>
 
+<p>To <dfn id=concept-element-attributes-verify-and-set-value>verify and set attribute value</dfn>

Remove the id attribute here.

> +{{TrustedType}} or string <var>value</var> for an <a>attribute</a> <var>attribute</var>, with
+<a for=/>element</a> <var>element</var>:

Perhaps:

> given a X x, Y y, and Z z:

as style instead?

> @@ -6485,8 +6518,6 @@ string <var>namespace</var> (default null):</p>
  <var>attr</var>'s <a for=Attr>namespace</a>, <var>attr</var>'s <a for=Attr>local name</a>, and
  <var>element</var>.
 
- <li><p>If <var>oldAttr</var> is <var>attr</var>, return <var>attr</var>.

This seems like it would change the mutation records for existing applications even when they don't use Trusted Types.

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

Message ID: <whatwg/dom/pull/1268/review/2015226415@github.com>