Make Attr extend Node

Fixes #1641. Closes #1822 by superseding it.

lib/jsdom/living/attributes.js

@@ -1,6 +1,5 @@
 "use strict";
 const DOMException = require("domexception/webidl2js-wrapper");
-const attrGenerated = require("./generated/Attr");
 const { asciiLowercase } = require("./helpers/strings");
 const { HTML_NS } = require("./helpers/namespaces");
 const { queueAttributeMutationRecord } = require("./helpers/mutation-observers");
@@ -24,9 +23,8 @@ exports.hasAttributeByNameNS = function (element, namespace, localName) {
   });
 };
 
-exports.changeAttribute = function (element, attribute, value) {
-  // https://dom.spec.whatwg.org/#concept-element-attributes-change
-
+// https://dom.spec.whatwg.org/#concept-element-attributes-change
+exports.changeAttribute = (element, attribute, value) => {
   const { _localName, _namespace, _value } = attribute;
   queueAttributeMutationRecord(element, _localName, _namespace, _value);
 
@@ -37,9 +35,8 @@ exports.changeAttribute = function (element, attribute, value) {
   element._attrModified(attribute._qualifiedName, value, oldValue);
 };
 
+// https://dom.spec.whatwg.org/#concept-element-attributes-append
 exports.appendAttribute = function (element, attribute) {
-  // https://dom.spec.whatwg.org/#concept-element-attributes-append
-
   const { _localName, _namespace } = attribute;
   queueAttributeMutationRecord(element, _localName, _namespace, null);
 
@@ -214,12 +211,7 @@ exports.setAttributeValue = function (element, localName, value, prefix, namespa
 
   const attribute = exports.getAttributeByNameNS(element, namespace, localName);
   if (attribute === null) {
-    const newAttribute = attrGenerated.createImpl(element._globalObject, [], {
-      namespace,
-      namespacePrefix: prefix,
-      localName,
-      value
-    });
+    const newAttribute = element._ownerDocument._createAttribute(localName, namespace, prefix, value);
     exports.appendAttribute(element, newAttribute);
 
     return;
@@ -228,12 +220,14 @@ exports.setAttributeValue = function (element, localName, value, prefix, namespa
   exports.changeAttribute(element, attribute, value);
 };
 
+// https://dom.spec.whatwg.org/#set-an-existing-attribute-value
 exports.setAnExistingAttributeValue = (attribute, value) => {
-  if (attribute._element === null) {
+  const element = attribute._element;
+  if (element === null) {
     attribute._value = value;
+  } else {
+    exports.changeAttribute(element, attribute, value);
   }
-
-  exports.changeAttribute(attribute._element, attribute, value);
 };
 
 exports.removeAttributeByName = function (element, name) {
@@ -260,47 +254,6 @@ exports.removeAttributeByNameNS = function (element, namespace, localName) {
   return attr;
 };
 
-exports.copyAttributeList = function (sourceElement, destElement) {
-  // Needed by https://dom.spec.whatwg.org/#concept-node-clone
-
-  for (const sourceAttr of sourceElement._attributeList) {
-    const destAttr = attrGenerated.createImpl(destElement._globalObject, [], {
-      namespace: sourceAttr._namespace,
-      namespacePrefix: sourceAttr._namespacePrefix,
-      localName: sourceAttr._localName,
-      value: sourceAttr._value
-    });
-
-    exports.appendAttribute(destElement, destAttr);
-  }
-};
-
-exports.attributeListsEqual = function (elementA, elementB) {
-  // Needed by https://dom.spec.whatwg.org/#concept-node-equals
-
-  const listA = elementA._attributeList;
-  const listB = elementB._attributeList;
-
-  if (listA.length !== listB.length) {
-    return false;
-  }
-
-  for (let i = 0; i < listA.length; ++i) {
-    const attrA = listA[i];
-
-    if (!listB.some(attrB => equalsA(attrB))) {
-      return false;
-    }
-
-    function equalsA(attrB) {
-      return attrA._namespace === attrB._namespace && attrA._localName === attrB._localName &&
-             attrA._value === attrB._value;
-    }
-  }
-
-  return true;
-};
-
 exports.attributeNames = function (element) {
   // Needed by https://dom.spec.whatwg.org/#dom-element-getattributenames
 

lib/jsdom/living/attributes/Attr-impl.js

@@ -1,17 +1,20 @@
 "use strict";
 
-const attributes = require("../attributes.js");
+const { setAnExistingAttributeValue } = require("../attributes.js");
+const NodeImpl = require("../nodes/Node-impl.js").implementation;
+const { ATTRIBUTE_NODE } = require("../node-type.js");
 
-exports.implementation = class AttrImpl {
+exports.implementation = class AttrImpl extends NodeImpl {
   constructor(globalObject, args, privateData) {
-    this._globalObject = globalObject;
+    super(globalObject, args, privateData);
 
     this._namespace = privateData.namespace !== undefined ? privateData.namespace : null;
     this._namespacePrefix = privateData.namespacePrefix !== undefined ? privateData.namespacePrefix : null;
     this._localName = privateData.localName;
     this._value = privateData.value !== undefined ? privateData.value : "";
     this._element = privateData.element !== undefined ? privateData.element : null;
 
+    this.nodeType = ATTRIBUTE_NODE;
     this.specified = true;
   }
 
@@ -38,28 +41,8 @@ exports.implementation = class AttrImpl {
   get value() {
     return this._value;
   }
-  set value(v) {
-    if (this._element === null) {
-      this._value = v;
-    } else {
-      attributes.changeAttribute(this._element, this, v);
-    }
-  }
-
-  // Delegate to value
-  get nodeValue() {
-    return this.value;
-  }
-  set nodeValue(v) {
-    this.value = v;
-  }
-
-  // Delegate to value
-  get textContent() {
-    return this.value;
-  }
-  set textContent(v) {
-    this.value = v;
+  set value(value) {
+    setAnExistingAttributeValue(this, value);
   }
 
   get ownerElement() {

lib/jsdom/living/attributes/Attr.webidl

@@ -1,15 +1,12 @@
 // https://dom.spec.whatwg.org/#interface-attr
 
 [Exposed=Window]
-interface Attr {
+interface Attr : Node {
   readonly attribute DOMString? namespaceURI;
   readonly attribute DOMString? prefix;
   readonly attribute DOMString localName;
   readonly attribute DOMString name;
-  readonly attribute DOMString nodeName; // historical alias of .name
            attribute DOMString value;
-  [TreatNullAs=EmptyString] attribute DOMString nodeValue; // historical alias of .value
-  [TreatNullAs=EmptyString] attribute DOMString textContent; // historical alias of .value
 
   readonly attribute Element? ownerElement;
 

lib/jsdom/living/interfaces.js

@@ -15,8 +15,8 @@ const generatedInterfaces = [
   require("./generated/EventTarget"),
 
   require("./generated/NamedNodeMap"),
-  require("./generated/Attr"),
   require("./generated/Node"),
+  require("./generated/Attr"),
   require("./generated/Element"),
   require("./generated/DocumentFragment"),
   require("./generated/Document"),

lib/jsdom/living/node-type.js

@@ -2,7 +2,7 @@
 
 module.exports = Object.freeze({
   ELEMENT_NODE: 1,
-  ATTRIBUTE_NODE: 2, // historical
+  ATTRIBUTE_NODE: 2,
   TEXT_NODE: 3,
   CDATA_SECTION_NODE: 4, // historical
   ENTITY_REFERENCE_NODE: 5, // historical

lib/jsdom/living/node.js

@@ -1,13 +1,13 @@
 "use strict";
-const attributes = require("./attributes");
+const { appendAttribute } = require("./attributes");
 const { cloningSteps, domSymbolTree } = require("./helpers/internal-constants");
 const NODE_TYPE = require("./node-type");
 const orderedSetParse = require("./helpers/ordered-set").parse;
 const { asciiCaseInsensitiveMatch, asciiLowercase } = require("./helpers/strings");
 const { HTML_NS, XMLNS_NS } = require("./helpers/namespaces");
 const HTMLCollection = require("./generated/HTMLCollection");
 
-module.exports.clone = function (node, document, cloneChildren) {
+exports.clone = (node, document, cloneChildren) => {
   if (document === undefined) {
     document = node._ownerDocument;
   }
@@ -31,7 +31,13 @@ module.exports.clone = function (node, document, cloneChildren) {
     case NODE_TYPE.ELEMENT_NODE:
       copy = document._createElementWithCorrectElementInterface(node._localName, node._namespaceURI);
       copy._prefix = node._prefix;
-      attributes.copyAttributeList(node, copy);
+      for (const attribute of node._attributeList) {
+        appendAttribute(copy, exports.clone(attribute, document));
+      }
+      break;
+
+    case NODE_TYPE.ATTRIBUTE_NODE:
+      copy = document._createAttribute(node._localName, node._namespace, node._namespacePrefix, node._value);
       break;
 
     case NODE_TYPE.TEXT_NODE:
@@ -61,7 +67,7 @@ module.exports.clone = function (node, document, cloneChildren) {
 
   if (cloneChildren) {
     for (const child of domSymbolTree.childrenIterator(node)) {
-      const childCopy = module.exports.clone(child, document, true);
+      const childCopy = exports.clone(child, document, true);
       copy._append(childCopy);
     }
   }
@@ -71,7 +77,7 @@ module.exports.clone = function (node, document, cloneChildren) {
 
 // For the following, memoization is not applied here since the memoized results are stored on `this`.
 
-module.exports.listOfElementsWithClassNames = (classNames, root) => {
+exports.listOfElementsWithClassNames = (classNames, root) => {
   // https://dom.spec.whatwg.org/#concept-getElementsByClassName
 
   const classes = orderedSetParse(classNames);
@@ -111,7 +117,7 @@ module.exports.listOfElementsWithClassNames = (classNames, root) => {
   });
 };
 
-module.exports.listOfElementsWithQualifiedName = (qualifiedName, root) => {
+exports.listOfElementsWithQualifiedName = (qualifiedName, root) => {
   // https://dom.spec.whatwg.org/#concept-getelementsbytagname
 
   if (qualifiedName === "*") {
@@ -158,7 +164,7 @@ module.exports.listOfElementsWithQualifiedName = (qualifiedName, root) => {
   });
 };
 
-module.exports.listOfElementsWithNamespaceAndLocalName = (namespace, localName, root) => {
+exports.listOfElementsWithNamespaceAndLocalName = (namespace, localName, root) => {
   // https://dom.spec.whatwg.org/#concept-getelementsbytagnamens
 
   if (namespace === "") {

lib/jsdom/living/nodes/Document-impl.js

@@ -747,7 +747,7 @@ class DocumentImpl extends NodeImpl {
       localName = asciiLowercase(localName);
     }
 
-    return generatedAttr.createImpl(this._globalObject, [], { localName });
+    return this._createAttribute(localName);
   }
 
   createAttributeNS(namespace, name) {
@@ -757,10 +757,17 @@ class DocumentImpl extends NodeImpl {
     namespace = namespace !== null ? String(namespace) : namespace;
 
     const extracted = validateAndExtract(this._globalObject, namespace, name);
+    return this._createAttribute(extracted.localName, extracted.namespace, extracted.prefix);
+  }
+
+  // Using this helper function rather than directly calling generatedAttr.createImpl may be preferred in some files,
+  // to avoid introducing a potentially cyclic dependency on generated/Attr.js.
+  _createAttribute(localName, namespace, namespacePrefix, value) {
     return generatedAttr.createImpl(this._globalObject, [], {
-      namespace: extracted.namespace,
-      namespacePrefix: extracted.prefix,
-      localName: extracted.localName
+      namespace,
+      namespacePrefix,
+      localName,
+      value
     });
   }