Re: [whatwg/dom] Integrate Trusted Types enforcement into attribute handling (PR #1268)

@annevk commented on this pull request.

This is indeed a whole bit more straightforward to read through.

> @@ -7891,7 +7908,25 @@ string <var>value</var>, run these steps:
  <li><p>If <var>attribute</var>'s <a for=Attr>element</a> is null, then set <var>attribute</var>'s
  <a for=Attr>value</a> to <var>value</var>.
 
- <li><p>Otherwise, <a lt="change an attribute">change</a> <var>attribute</var> to <var>value</var>.
+ <li>
+  <p>Otherwise:
+
+  <ol>
+   <li><p>Let <var>originalElement</var> be <var>attribute</var>'s <a for=Attr>element</a>.

```suggestion
   <li><p>Let <var>element</var> be <var>attribute</var>'s <a for=Attr>element</a>.
```

> @@ -7891,7 +7908,25 @@ string <var>value</var>, run these steps:
  <li><p>If <var>attribute</var>'s <a for=Attr>element</a> is null, then set <var>attribute</var>'s
  <a for=Attr>value</a> to <var>value</var>.
 
- <li><p>Otherwise, <a lt="change an attribute">change</a> <var>attribute</var> to <var>value</var>.
+ <li>
+  <p>Otherwise:
+
+  <ol>
+   <li><p>Let <var>originalElement</var> be <var>attribute</var>'s <a for=Attr>element</a>.
+
+   <li><p>Let <var>verifiedValue</var> be the result of calling <a abstract-op>get
+   Trusted Types-compliant attribute value</a> with <var>attribute</var>'s
+   <a for=Attr>local name</a>, <var>attribute</var>'s <a for=Attr>namespace</a>, <a>this</a>,

There is no this here.

> +   <li><p>If <var>attribute</var>'s <a for=Attr>element</a> is null, then set <var>attribute</var>'s
+   <a for=Attr>value</a> to <var>verifiedValue</var>, and return.
+
+   <li><p>If <var>attribute</var>'s <a for=Attr>element</a> is not <var>originalElement</var>, then
+   return.

I think it would be nicer if we throw for both of these cases as they result from the policy changing aspects of the attribute that it probably shouldn't? But maybe that's not compatible with what Chromium does.

Also, the other changes are made directly in `setAttribute` and `setAttributeNS`. Do we end up invoking the policy early enough here for the various call sites? It seems like there would be a difference in exception order.

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

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