[whatwg/dom] Processing instruction attribute map follow-ups (Issue #1504)

annevk created an issue (whatwg/dom#1504)

### What is the issue with the DOM Standard?

While reviewing WebKit's implementation of processing instruction attributes (https://github.com/WebKit/WebKit/pull/73326) I ran into three problems.

#### 1. Copies don't get an attribute map

[Clone a single node](https://dom.spec.whatwg.org/#clone-a-single-node) sets a `ProcessingInstruction` copy's target and data, but the copy's [attribute map](https://dom.spec.whatwg.org/#concept-pi-attribute-map) stays empty:

```js
const pi = document.createProcessingInstruction("t", 'a="1"');
pi.cloneNode().getAttribute("a"); // null
```

[Extract](https://dom.spec.whatwg.org/#concept-range-extract) and [clone the contents](https://dom.spec.whatwg.org/#concept-range-clone) then set the clone's data to a substring directly rather than through [replace data](https://dom.spec.whatwg.org/#concept-cd-replace). So even with cloning fixed, a processing instruction cut by a range keeps a map that doesn't match its shortened data.

I suggest running [update attributes from data](https://dom.spec.whatwg.org/#update-attributes-from-data) on the copy in all three places. The WebKit patch re-parses copies from their data, which amounts to the same thing. Copying the map instead would keep entries that `setAttribute()` added under names the pseudo-attribute grammar can't parse, such as `$`, but the range steps would still need to re-parse.

#### 2. `removeAttribute()` rewrites data when the name isn't there

[`removeAttribute(name)`](https://dom.spec.whatwg.org/#dom-processinginstruction-removeattribute) always runs [update data from attributes](https://dom.spec.whatwg.org/#update-data-from-attributes), even when the map has no entry for name. That re-serializes the data and queues a `characterData` mutation record, and data that doesn't parse as pseudo-attributes is lost:

```js
const pi = document.createProcessingInstruction("php", "echo 1;");
pi.removeAttribute("x");
pi.data; // ""
```

`Element`'s [remove an attribute by name](https://dom.spec.whatwg.org/#concept-element-attributes-remove-by-name) does nothing when the attribute is missing, and neither does `toggleAttribute(name, false)` on a processing instruction. I suggest returning early when this's attribute map[name] doesn't exist, which is what the WebKit patch does.

I suppose I could be convinced the other way too, but it seems like we need test coverage for this at a minimum.

#### 3. Typo

In [update data from attributes](https://dom.spec.whatwg.org/#update-data-from-attributes), "Replace any U+003D (>)" should be U+003E.

cc @foolip @noamr


-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/issues/1504
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/dom/issues/1504@github.com>