Re: [whatwg/dom] Define processing instruction attributes (PR #1454)

@annevk commented on this pull request.



> +
+<div algorithm>
+<p>The
+<dfn method for=ProcessingInstruction><code>toggleAttribute(<var>name</var>, <var>force</var>)</code></dfn>
+method steps are:
+
+<ol>
+ <li><p>If <var>name</var> is not a <a>valid attribute local name</a>, then <a>throw</a> an
+ "{{InvalidCharacterError!!exception}}" {{DOMException}}.
+
+ <li><p>Set <var>name</var> to <var>name</var> in <a>ASCII lowercase</a>.
+
+ <li><p>Let <var>attributes</var> be <a>this</a>'s <a for=ProcessingInstruction>attribute map</a>.
+
+ <li>
+  <p>If <var>attributes</var>[<var>name</var>] does not <a for=set>exist</a>:

This is the wrong exist. You want for=map.

> + <dd><p>Returns true if <var>pi</var> has an attribute named <var>name</var>; otherwise false.
+</dl>
+
+<div algorithm>
+<p>The
+<dfn constructor for=ProcessingInstruction lt=ProcessingInstruction(target)><code>new ProcessingInstruction(<var>target</var>, <var>data</var>)</code></dfn>
+constructor steps are:
+
+<ol>
+ <li><p>Set <a>this</a>'s <a for=Node>node document</a> to <a>current global object</a>'s
+ <a>associated <code>Document</code></a>.
+
+ <li><p><a for=ProcessingInstruction>Initialize</a> <a>this</a> with <var>target</var> and
+ <var>data</var>.
+
+ <li><p><a>Update attributes from data</a> for <a>this</a>.

This is redundant with Initialize above.

> +</div>
+
+<div algorithm>
+<p>To <dfn>update attributes from data</dfn> for a {{ProcessingInstruction}} <a for=/>node</a>
+<var>pi</var>:
+
+<ol>
+ <li><p><a for=map>Clear</a> <var>pi</var>'s <a for=ProcessingInstruction>attribute map</a>.
+
+ <li><p>Let <var>document</var> be a new {{Document}} node whose <a for=Document>type</a> is
+ "<code>html</code>".
+
+ <li><p>Let <var>html</var> be the concatenation of "<code>&lt;html </code>",
+ <var>pi</var>'s <a for=CharacterData>data</a>, and "<code>&gt;</code>".
+
+ <li><p><a href="https://html.spec.whatwg.org/#parse-html-from-a-string">Parse HTML from a string</a>

I'm worried that we'll forget about fixing this.

> +</div>
+
+<div algorithm>
+<p>The <dfn method for=ProcessingInstruction><code>hasAttribute(<var>name</var>)</code></dfn>
+method steps are:
+
+<ol>
+ <li><p>Set <var>name</var> to <var>name</var> in <a>ASCII lowercase</a>.
+
+ <li><p>Return true if <a>this</a>'s <a for=ProcessingInstruction>attribute map</a>[<var>name</var>]
+ <a for=set>exists</a>; otherwise false.
+</ol>
+</div>
+
+<div algorithm>
+<p>To <dfn export for=ProcessingInstruction id=concept-pi-initialize>initialize</dfn> a

```suggestion
<p>To <dfn export for=ProcessingInstruction>initialize</dfn> a
```

> +</div>
+
+<div algorithm>
+<p>The <dfn method for=ProcessingInstruction><code>hasAttribute(<var>name</var>)</code></dfn>
+method steps are:
+
+<ol>
+ <li><p>Set <var>name</var> to <var>name</var> in <a>ASCII lowercase</a>.
+
+ <li><p>Return true if <a>this</a>'s <a for=ProcessingInstruction>attribute map</a>[<var>name</var>]
+ <a for=set>exists</a>; otherwise false.
+</ol>
+</div>
+
+<div algorithm>
+<p>To <dfn export for=ProcessingInstruction id=concept-pi-initialize>initialize</dfn> a

Why does this need exporting? If HTML also needs to create processing instructions we should probably have a dedicated create, I think.

>  <p>{{ProcessingInstruction}} <a for=/>nodes</a> have an associated
 <dfn export id=concept-pi-target for=ProcessingInstruction>target</dfn>.
 
+<p>{{ProcessingInstruction}} <a for=/>nodes</a> have an associated
+<dfn export id=concept-pi-attribute-map for=ProcessingInstruction>attribute map</dfn>, which is a
+<a for=/>map</a>, initially empty.

We should move these two associated concepts to before the constructor.

> +<dfn method for=ProcessingInstruction><code>toggleAttribute(<var>name</var>, <var>force</var>)</code></dfn>
+method steps are:
+
+<ol>
+ <li><p>If <var>name</var> is not a <a>valid attribute local name</a>, then <a>throw</a> an
+ "{{InvalidCharacterError!!exception}}" {{DOMException}}.
+
+ <li><p>Set <var>name</var> to <var>name</var> in <a>ASCII lowercase</a>.
+
+ <li><p>Let <var>attributes</var> be <a>this</a>'s <a for=ProcessingInstruction>attribute map</a>.
+
+ <li>
+  <p>If <var>attributes</var>[<var>name</var>] does not <a for=set>exist</a>:
+
+  <ol>
+   <li><p>If <var>force</var> is not given or is true, set <var>attributes</var>[<var>name</var>]

We should use `<a for=map>set</a>` here as well. 

> +
+ <li><p><a>Update attributes from data</a> for <var>pi</var>.
+</ol>
+</div>
+
+<div algorithm>
+<p>To <dfn>update attributes from data</dfn> for a {{ProcessingInstruction}} <a for=/>node</a>
+<var>pi</var>:
+
+<ol>
+ <li><p><a for=map>Clear</a> <var>pi</var>'s <a for=ProcessingInstruction>attribute map</a>.
+
+ <li><p>Let <var>document</var> be a new {{Document}} node whose <a for=Document>type</a> is
+ "<code>html</code>".
+
+ <li><p>Let <var>html</var> be the concatenation of "<code>&lt;html </code>",

`<a for=string>concatenation</a>`

> +<var>pi</var>:
+
+<ol>
+ <li><p><a for=map>Clear</a> <var>pi</var>'s <a for=ProcessingInstruction>attribute map</a>.
+
+ <li><p>Let <var>document</var> be a new {{Document}} node whose <a for=Document>type</a> is
+ "<code>html</code>".
+
+ <li><p>Let <var>html</var> be the concatenation of "<code>&lt;html </code>",
+ <var>pi</var>'s <a for=CharacterData>data</a>, and "<code>&gt;</code>".
+
+ <li><p><a href="https://html.spec.whatwg.org/#parse-html-from-a-string">Parse HTML from a string</a>
+ given <var>document</var> and <var>html</var>.
+
+ <li><p><a for=list>For each</a> <var>attribute</var> of <var>document</var>'s
+ <a>document element</a>'s <a for=Element>attribute list</a>, set <var>pi</var>'s

```suggestion
 <a>document element</a>'s <a for=Element>attribute list</a>: set <var>pi</var>'s
```

> +<var>pi</var>:
+
+<ol>
+ <li><p><a for=map>Clear</a> <var>pi</var>'s <a for=ProcessingInstruction>attribute map</a>.
+
+ <li><p>Let <var>document</var> be a new {{Document}} node whose <a for=Document>type</a> is
+ "<code>html</code>".
+
+ <li><p>Let <var>html</var> be the concatenation of "<code>&lt;html </code>",
+ <var>pi</var>'s <a for=CharacterData>data</a>, and "<code>&gt;</code>".
+
+ <li><p><a href="https://html.spec.whatwg.org/#parse-html-from-a-string">Parse HTML from a string</a>
+ given <var>document</var> and <var>html</var>.
+
+ <li><p><a for=list>For each</a> <var>attribute</var> of <var>document</var>'s
+ <a>document element</a>'s <a for=Element>attribute list</a>, set <var>pi</var>'s

And I guess we should link set here as well? Might be good to do another audit.

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

Message ID: <whatwg/dom/pull/1454/review/4270613833@github.com>