search

WebReflection / linkedom

A triple-linked lists based DOM implementation.
https://webreflection.medium.com/linkedom-a-jsdom-alternative-53dd8f699311
ISC License
1.62k stars 78 forks source link

&quot converted to " on xml #267

Closed nyanrus closed 2 months ago

nyanrus commented 4 months ago

Thank you for good project! (2)

import { DOMParser } from "linkedom";

console.log(new DOMParser().parseFromString(`<html id="main-window" data-l10n-args="{&quot;content-title&quot;:&quot;CONTENTTITLE&quot;}"></html>`, "text/xml").toString());
console.log(new DOMParser().parseFromString(`<html id="main-window" data-l10n-args="{&quot;content-title&quot;:&quot;CONTENTTITLE&quot;}"></html>`, "text/html").toString());
❯ node test.js
<?xml version="1.0" encoding="utf-8"?><html id="main-window" data-
l10n-args="{"content-title":"CONTENTTITLE"}" />
<html id="main-window" data-l10n-args="{&quot;content-title&quot;:
&quot;CONTENTTITLE&quot;}"></html>

the &quot changed to ", and it occurs error in xml.

node v20.11.1 linkedom v0.16.10

omril1 commented 4 months ago

@WebReflection If I understand this correctly, the escape function in toString doesn't escape quotes, so the replace should be called for XML documents as well.

So this: https://github.com/WebReflection/linkedom/blob/85d9f92a09bb2ecf8e9be1bd468d30257dbba21b/cjs/interface/attr.js#L44-L51

Should change to:

  toString() {
    const {name, [VALUE]: value} = this;
    if (emptyAttributes.has(name) && !value) {
      return ignoreCase(this) ? name : `${name}=""`;
    }
-    const escapedValue = ignoreCase(this) ? value.replace(QUOTE, '&quot;') : escape(value); 
+    const escapedValue = (ignoreCase(this) ? value : escape(value)).replace(QUOTE, '&quot;');
    return `${name}="${escapedValue}"`;
  }
WebReflection commented 4 months ago

I don't know ... that change is recent and it was to avoid escaping other chars such as & but not single quote or others ... the double quote escape is actually due common double quotes used around attributes but I feel like there's no right answer here ... or better, I would like to read an exhaustive example and and a test with expectations and never change this again, thanks!

nyanrus commented 4 months ago

it is double quote issue.

omril1 commented 2 months ago

Can close