- From: Anne van Kesteren <notifications@github.com>
- Date: Wed, 24 Jan 2024 08:54:59 -0800
- To: whatwg/dom <dom@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <whatwg/dom/pull/1247/review/1841869951@github.com>
@annevk commented on this pull request. I see it's still a draft, but I thought I'd post some initial feedback. > @@ -6354,6 +6355,10 @@ given a <var>document</var>, <var>localName</var>, <var>namespace</var>, and opt <a>attribute</a> <var>attribute</var> to <var>value</var>, run these steps: <ol> +<li><p>Set <var>value</var> to the result of calling <a>Get Trusted Types-compliant attribute Nit: indentation is wrong. And DOM doesn't do newlines in phrasing-level elements (i.e., wrap before `<a>`). > @@ -6481,7 +6496,7 @@ string <var>namespace</var> (default null):</p> <div algorithm> <p>To <dfn export id=concept-element-attributes-set-value>set an attribute value</dfn> given an -<a for=/>element</a> <var>element</var>, a string <var>localName</var>, a string <var>value</var>, +<a for=/>element</a> <var>element</var>, a string <var>localName</var>, a string or <a>TrustedType</a> <var>value</var>, This needs to be `{{TrustedType}}` I think to get the markup correct. `<a>` doesn't add code. > @@ -6490,12 +6505,27 @@ an optional null or string <var>prefix</var> (default null), and an optional nul <a lt="get an attribute by namespace and local name">getting an attribute</a> given <var>namespace</var>, <var>localName</var>, and <var>element</var>. - <li>If <var>attribute</var> is null, create an <a>attribute</a> whose <a for=Attr>namespace</a> is - <var>namespace</var>, <a for=Attr>namespace prefix</a> is <var>prefix</var>, - <a for=Attr>local name</a> is <var>localName</var>, <a for=Attr>value</a> is <var>value</var>, and - <a for=Node>node document</a> is <var>element</var>'s <a for=Node>node document</a>, then - <a lt="append an attribute">append</a> this <a>attribute</a> to <var>element</var>, and then - return. + <li><p>If <var>attribute</var> is null, then: Nit: indentation is wrong here and below. README has some instructions. `<p>` should be on a newline and indented one, for instance. > @@ -6760,7 +6790,7 @@ method steps are: <li><p>If <var>attribute</var> is null, create an <a>attribute</a> whose <a for=Attr>local name</a> is <var>qualifiedName</var>, <a for=Attr>value</a> is - <var>value</var>, and <a for=Node>node document</a> is <a>this</a>'s <a for=Node>node document</a>, + stringified <var>value</var>, and <a for=Node>node document</a> is <a>this</a>'s <a for=Node>node document</a>, As noted elsewhere, this doesn't work as-is. > + <var>namespace</var>, <a for=Attr>namespace prefix</a> is <var>prefix</var>, + <a for=Attr>local name</a> is <var>localName</var> and + <a for=Node>node document</a> is <var>element</var>'s <a for=Node>node document</a>. + + <li><p>Set <a>attribute</a>'s <var>value</var> to the result of calling <a>Get Trusted Types-compliant attribute + value</a> for <var>attribute</var>, with <var>element</var> and + <var>value</var>. [[!TRUSTED-TYPES]] + + <li><p><a for=list>Append</a> <var>attribute</var> to <var>element</var>'s + <a for=Element>attribute list</a>. + + <li><p>Set <var>attribute</var>'s <a for=Attr>element</a> to <var>element</var>. + + <li><p><a>Handle attribute changes</a> for <var>attribute</var> with <var>element</var>, null, and + <var>attribute</var>'s <a for=Attr>value</a>. + <li><p>Return. I don't understand why the changes here (which are for setAttributeNS() and prolly some other callers) are much more elaborate than they are for setAttribute(). Why can't "append an attribute" handle this in the way it already does per the changes above? > @@ -6354,6 +6355,10 @@ given a <var>document</var>, <var>localName</var>, <var>namespace</var>, and opt <a>attribute</a> <var>attribute</var> to <var>value</var>, run these steps: <ol> +<li><p>Set <var>value</var> to the result of calling <a>Get Trusted Types-compliant attribute Question: this ordering means that if _value_ is changed by TT the mutation record reflects that. Does that have test coverage? -- Reply to this email directly or view it on GitHub: https://github.com/whatwg/dom/pull/1247#pullrequestreview-1841869951 You are receiving this because you are subscribed to this thread. Message ID: <whatwg/dom/pull/1247/review/1841869951@github.com>
Received on Wednesday, 24 January 2024 16:55:06 UTC