Skip to content
Open
218 changes: 216 additions & 2 deletions dom.bs
Original file line number Diff line number Diff line change
Expand Up @@ -6005,11 +6005,15 @@ method steps are:
"<code>?></code>", then <a>throw</a> an
"{{InvalidCharacterError!!exception}}" {{DOMException}}. <!-- Gecko does this. -->

<li>Return a new {{ProcessingInstruction}}
<li>Let <var>pi</var> be a new {{ProcessingInstruction}}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's fix this algorithm to use <li><p> as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But even better. We should make this algorithm invoke "initialize a ProcessingInstruction node" that essentially does what this algorithm does except for allocating a new node. Then we can reuse it for the constructor and keep them synchronized indefinitely.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in the style of event's "initialize" which required disambiguation invocations of that.

Not setting target and data on creation does leave the two undefined until "initialize" is called since https://dom.spec.whatwg.org/#concept-pi-target and https://dom.spec.whatwg.org/#concept-cd-data don't have defaults. Saying that they default to the empty string would be weird in the case of target since that cannot be the empty string after creation+initialization.

If you think this is ugly we could instead make it a "validate target and data" algorithm that only has the exception-throwing bits.

<a for=/>node</a>, with
<a for=ProcessingInstruction>target</a> set to <var>target</var>,
<a for=CharacterData>data</a> set to <var>data</var>, and
<a for=Node>node document</a> set to <a>this</a>.

<li><a>Update attributes from data</a> for <var>pi</var>.

<li>Return <var>pi</var>.
</ol>

<hr>
Expand Down Expand Up @@ -8221,7 +8225,8 @@ string called <dfn export id=concept-cd-data for=CharacterData>data</dfn>.

<div algorithm>
<p>To <dfn export id=concept-cd-replace>replace data</dfn> of a <a for=/>node</a> <var>node</var>
with an integer <var>offset</var>, integer <var>count</var>, and string <var>data</var>:
with an integer <var>offset</var>, integer <var>count</var>, string <var>data</var>, and an optional
boolean <var>piAttributesAlreadyUpdated</var> (default false):

<ol>
<li><p>Let <var>length</var> be <var>node</var>'s <a for=Node>length</a>.
Expand Down Expand Up @@ -8263,6 +8268,10 @@ with an integer <var>offset</var>, integer <var>count</var>, and string <var>dat
<a for=range>end offset</a> by <var>data</var>'s <a for=string>length</a> and decrease it by
<var>count</var>.

<li><p>If <var>node</var> is a {{ProcessingInstruction}} <a for=/>node</a> and
<var>piAttributesAlreadyUpdated</var> is false, then <a>update attributes from data</a> for
<var>node</var>.

<li><p>If <var>node</var>'s <a for=tree>parent</a> is non-null, then run the
<a>children changed steps</a> for <var>node</var>'s <a for=tree>parent</a>.
</ol>
Expand Down Expand Up @@ -8518,17 +8527,222 @@ interface CDATASection : Text {
<pre class=idl>
[Exposed=Window]
interface ProcessingInstruction : CharacterData {
constructor(DOMString target, optional DOMString data = "");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially thought this would be a bit ugly compared to using IDL records or some such so you can set the attributes directly, but I think overall it's probably okay if we have to keep around the dual-data-structure anyway.


readonly attribute DOMString target;

boolean hasAttributes();
sequence&lt;DOMString> getAttributeNames();
DOMString? getAttribute(DOMString name);
undefined setAttribute(DOMString name, DOMString value);
undefined removeAttribute(DOMString name);
boolean toggleAttribute(DOMString name, optional boolean force);
boolean hasAttribute(DOMString name);
};</pre>

<dl class=domintro>
<dt><code><var ignore>pi</var> = new <a constructor lt="ProcessingInstruction()">ProcessingInstruction(<var>target</var> [, <var>data</var> = ""])</a></code>
<dd>Returns a new {{ProcessingInstruction}} <a for=/>node</a> whose
<a for=ProcessingInstruction>target</a> is <var>target</var> and
<a for=CharacterData>data</a> is <var>data</var>.
</dl>
Comment thread
foolip marked this conversation as resolved.

<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>If <var>target</var> does not match the <code>[=XML/Name=]</code> production,
then <a>throw</a> an "{{InvalidCharacterError!!exception}}" {{DOMException}}.

<li><p>If <var>data</var> contains the string
"<code>?></code>", then <a>throw</a> an
"{{InvalidCharacterError!!exception}}" {{DOMException}}.

<li><p>Set <a>this</a>'s <a for=ProcessingInstruction>target</a> to <var>target</var>,
<a>this</a>'s <a for=CharacterData>data</a> to <var>data</var>, and
<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>Update attributes from data</a> for <a>this</a>.
</ol>
</div>

<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.

<div algorithm>
<p>The <dfn attribute for=ProcessingInstruction>target</dfn> getter steps are to return
<a>this</a>'s <a for=ProcessingInstruction>target</a>.
</div>

<div algorithm>
<p>The <dfn method for=ProcessingInstruction><code>hasAttributes()</code></dfn> method steps are to
return false if <a>this</a>'s <a for=ProcessingInstruction>attribute map</a> <a for=map>is empty</a>;
otherwise true.
</div>

<div algorithm>
<p>The <dfn method for=ProcessingInstruction><code>getAttributeNames()</code></dfn> method steps
are to return the result of <a for=map>getting the keys</a> of <a>this</a>'s
<a for=ProcessingInstruction>attribute map</a>.
</div>

<div algorithm>
<p>The <dfn method for=ProcessingInstruction><code>getAttribute(<var>name</var>)</code></dfn> method
steps are:

<ol>
<li><p>If <a>this</a>'s <a for=Node>node document</a> is an <a>HTML document</a>, then set
<var>name</var> to <var>name</var> in <a>ASCII lowercase</a>.
Comment thread
foolip marked this conversation as resolved.
Outdated

<li><p>Return <a>this</a>'s <a for=ProcessingInstruction>attribute map</a>[<var>name</var>]
<a for=map>with default</a> null.
</ol>
</div>

<div algorithm>
<p>The <dfn method for=ProcessingInstruction><code>setAttribute(<var>name</var>, <var>value</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}}.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for other maps (e.g., DOMTokenList) we validate this for each accessor. Should we do that here as well?


<li><p>If <a>this</a>'s <a for=Node>node document</a> is an <a>HTML document</a>, then set
<var>name</var> to <var>name</var> in <a>ASCII lowercase</a>.

<li><p><a for=map>Set</a> <a>this</a>'s <a for=ProcessingInstruction>attribute map</a>[<var>name</var>]
to <var>value</var>.

<li><p><a>Update data from attributes</a> for <a>this</a>.
</ol>
</div>

<div algorithm>
<p>The <dfn method for=ProcessingInstruction><code>removeAttribute(<var>name</var>)</code></dfn>
method steps are:

<ol>
<li><p>If <a>this</a>'s <a for=Node>node document</a> is an <a>HTML document</a>, then set
<var>name</var> to <var>name</var> in <a>ASCII lowercase</a>.

<li><p><a for=map>Remove</a> <a>this</a>'s
<a for=ProcessingInstruction>attribute map</a>[<var>name</var>].

<li><p><a>Update data from attributes</a> for <a>this</a>.
</ol>
</div>

<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>If <a>this</a>'s <a for=Node>node document</a> is an <a>HTML document</a>, then 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>]
to the empty string, <a>update data from attributes</a> for <a>this</a>, and then return true.

<li><p>Return false.
</ol>

<li><p>Otherwise, if <var>force</var> is not given or is false, <a for=map>remove</a>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for Otherwise as we use early returns.

<var>attributes</var>[<var>name</var>], <a>update data from attributes</a> for <a>this</a>, and
then return false.

<li><p>Return true.
</ol>
</div>

<div algorithm>
<p>The <dfn method for=ProcessingInstruction><code>hasAttribute(<var>name</var>)</code></dfn>
method steps are:

<ol>
<li><p>If <a>this</a>'s <a for=Node>node document</a> is an <a>HTML document</a>, then 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>update attributes from data</dfn> for a {{ProcessingInstruction}} <a for=/>node</a> <var>pi</var>:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure this parser is compatible with how xsl-stylesheet is parsed today?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was based on how this works in Chromium:

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/processing_instruction.cc;l=120-146;drc=bc971aee7d328d1e886ba76adc540805ea480981

In WebKit it is similar:

https://github.com/WebKit/WebKit/blob/9af8ffff919a1a1478b8a43cef821adb49a4e6a1/Source/WebCore/dom/ProcessingInstruction.cpp#L99
https://github.com/WebKit/WebKit/blob/26d7991c939a41bbab5d01263900598b67220208/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp#L1556-L1574

In Gecko it's a little bit different, every attribute lookup uses an attribute parser and I can't tell if it exactly matches XML or not:
https://github.com/mozilla-firefox/firefox/blob/22d04b52b0eb8d9fa11bf8ede5ccc0243a07c5ba/dom/xml/XMLStylesheetProcessingInstruction.cpp#L92
https://github.com/mozilla-firefox/firefox/blob/afe51e48c9deecccbaf64a74d6f207456433df98/dom/xml/ProcessingInstruction.cpp#L77
https://github.com/mozilla-firefox/firefox/blob/22d04b52b0eb8d9fa11bf8ede5ccc0243a07c5ba/dom/base/nsContentUtils.cpp#L1842-L1943

The difference from what's already implemented is that we'd use the HTML parser in HTML documents, instead of unconditionally using the XML parser.

For compat, this difference means that existing content that creates a <?xml-stylesheet href="..."?> PI and moves it to an HTML document would take the HTML parser path instead of XML. For that to make a difference, I think that content would need to be invalid XML attribute syntax (like <?xml-stylesheet href='...'?>) and depends on it being broken (doing nothing) when being moved to HTML.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main difference is what happens if pi.data is set after the PI is moved to an HTML document (or constructed with createProcessingInstruction). In the new scenario invalid XML yet valid HTML can be used in that data for the xml-stylesheet attributes.


<ol>
<li><p><a for=map>Clear</a> <var>pi</var>'s <a for=ProcessingInstruction>attribute map</a>.

<li><p>Let <var>context</var> be the result of <a>creating an element</a> given
<var>pi</var>'s <a>node document</a>, "<code>html</code>", and the <a>HTML namespace</a>.

<li><p>Let <var>markup</var> be the concatentation of "<code>&lt;attrs </code>",
<var>pi</var>'s <a for=CharacterData>data</a>, and
"<code>&gt;&lt;/attrs&gt;</code>".

<li><p>Let <var>fragment</var> be the result of invoking the <a>fragment parsing algorithm steps</a> with <var>context</var> and <var>markup</var>.
Comment thread
foolip marked this conversation as resolved.
Outdated

<li>
<p>If <var>fragment</var>'s <a>first child</a> is an <a for=/>element</a>:

<ol>
<li><p>Let <var>element</var> be the <a>first child</a> of <var>fragment</var>.

<li><p>Assert: <var>element</var> is an <a for=/>element</a> with <a for=Element>local name</a> "<code>attrs</code>".

<li><p><a for=list>For each</a> <var>attribute</var> of <var>element</var>'s
<a for=Element>attribute list</a>, set <var>pi</var>'s
<a for=ProcessingInstruction>attribute map</a>[<var>attribute</var>'s <a for=Attr>local name</a>]
Comment thread
foolip marked this conversation as resolved.
Outdated
to <var>attribute</var>'s <a for=Attr>value</a>.
</ol>
</ol>
</div>

<div algorithm>
<p>To <dfn>update data from attributes</dfn> for a {{ProcessingInstruction}} <a for=/>node</a> <var>pi</var>:

<ol>
<li><p>Let <var>data</var> be the empty string.

<li>
<p><a for=map>For each</a> <var>name</var> → <var>value</var> of <var>pi</var>'s
<a for=ProcessingInstruction>attribute map</a>:

<ol>
<li><p>If <var>data</var> is not the empty string, append U+0020 SPACE to <var>data</var>.
Comment thread
foolip marked this conversation as resolved.
Outdated

<li><p>Append <var>name</var> to <var>data</var>.

<li><p>Append U+003D (=) to <var>data</var>.

<li><p>Append U+0022 (") to <var>data</var>.

<li><p>Append the result of <a href="https://html.spec.whatwg.org/#escapingString">escaping a string</a>
given <var>value</var> to <var>data</var>.

<li><p>Append U+0022 (") to <var>data</var>.
</ol>

<li><p><a>Replace data</a> of <var>pi</var> with 0, <var>pi</var>'s <a for=Node>length</a>,
<var>data</var>, and true.
</ol>
</div>


<h3 id=interface-comment>Interface {{Comment}}</h3>

Expand Down