Skip to content

fix: avoid sparse storage for numeric attribute names #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alan-agius4 merged 1 commit into from
Sep 21, 2026
+265 −34
Merged

fix: avoid sparse storage for numeric attribute names #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 46 additions & 34 deletions lib/Element.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ function Element(doc, localName, namespaceURI, prefix) {
this._tagName = undefined;

// These properties maintain the set of attributes
this._attrsByQName = Object.create(null); // The qname->Attr map
// Map keeps numeric attribute names out of sparse object-index storage.
this._attrsByQName = new Map(); // The qname->Attr map
this._attrsByLName = Object.create(null); // The ns|lname->Attr map
this._attrKeys = []; // attr index -> ns|lname
}
Expand Down Expand Up @@ -534,7 +535,7 @@ Element.prototype = Object.create(ContainerNode.prototype, {
qname = String(qname);
if (/[A-Z]/.test(qname) && this.isHTML)
qname = utils.toASCIILowerCase(qname);
var attr = this._attrsByQName[qname];
var attr = this._attrsByQName.get(qname);
if (!attr) return null;

if (Array.isArray(attr)) // If there is more than one
Expand All @@ -554,7 +555,7 @@ Element.prototype = Object.create(ContainerNode.prototype, {
qname = String(qname);
if (/[A-Z]/.test(qname) && this.isHTML)
qname = utils.toASCIILowerCase(qname);
return this._attrsByQName[qname] !== undefined;
return this._attrsByQName.has(qname);
}},

hasAttributeNS: { value: function hasAttributeNS(ns, lname) {
Expand All @@ -573,7 +574,7 @@ Element.prototype = Object.create(ContainerNode.prototype, {
if (!xml.isValidName(qname)) utils.InvalidCharacterError();
if (/[A-Z]/.test(qname) && this.isHTML)
qname = utils.toASCIILowerCase(qname);
var a = this._attrsByQName[qname];
var a = this._attrsByQName.get(qname);
if (a === undefined) {
if (force === undefined || force === true) {
this._setAttribute(qname, '');
Expand All @@ -594,7 +595,7 @@ Element.prototype = Object.create(ContainerNode.prototype, {
// XXX: the spec says that this next search should be done
// on the local name, but I think that is an error.
// email pending on www-dom about it.
var attr = this._attrsByQName[qname];
var attr = this._attrsByQName.get(qname);
var isnew;
if (!attr) {
attr = this._newattr(qname);
Expand All @@ -607,7 +608,7 @@ Element.prototype = Object.create(ContainerNode.prototype, {
// Now set the attribute value on the new or existing Attr object.
// The Attr.value setter method handles mutation events, etc.
attr.value = value;
if (this._attributes) this._attributes[qname] = attr;
setNamedProperty(this._attributes, qname, attr);
if (isnew && this._newattrhook) this._newattrhook(qname, value);
}},

Expand Down Expand Up @@ -695,7 +696,7 @@ Element.prototype = Object.create(ContainerNode.prototype, {
utils.InUseAttributeError();
}
var result = null;
var oldAttrs = this._attrsByQName[attr.name];
var oldAttrs = this._attrsByQName.get(attr.name);
if (oldAttrs) {
if (!Array.isArray(oldAttrs)) { oldAttrs = [ oldAttrs ]; }
if (oldAttrs.some(function(a) { return a===attr; })) {
Expand Down Expand Up @@ -734,7 +735,7 @@ Element.prototype = Object.create(ContainerNode.prototype, {
if (/[A-Z]/.test(qname) && this.isHTML)
qname = utils.toASCIILowerCase(qname);

var attr = this._attrsByQName[qname];
var attr = this._attrsByQName.get(qname);
if (!attr) return;

// If there is more than one match for this qname
Expand All @@ -745,13 +746,13 @@ Element.prototype = Object.create(ContainerNode.prototype, {
attr = attr.shift(); // remove it from the array
}
else {
this._attrsByQName[qname] = attr[1];
this._attrsByQName.set(qname, attr[1]);
attr = attr[0];
}
}
else {
// only a single match, so remove the qname mapping
this._attrsByQName[qname] = undefined;
this._attrsByQName.delete(qname);
}

var ns = attr.namespaceURI;
Expand All @@ -763,7 +764,7 @@ Element.prototype = Object.create(ContainerNode.prototype, {
var i = this._attrKeys.indexOf(key);
if (this._attributes) {
Array.prototype.splice.call(this._attributes, i, 1);
this._attributes[qname] = undefined;
setNamedProperty(this._attributes, qname, undefined);
}
this._attrKeys.splice(i, 1);

Expand Down Expand Up @@ -832,20 +833,20 @@ Element.prototype = Object.create(ContainerNode.prototype, {
// prefix will never have two matching Attr objects (because
// setAttributeNS doesn't allow a non-null namespace with a
// null prefix.
var attr = this._attrsByQName[qname];
var attr = this._attrsByQName.get(qname);
return attr ? attr.value : null;
}},

// The raw version of setAttribute for reflected idl attributes.
_setattr: { value: function _setattr(qname, value) {
var attr = this._attrsByQName[qname];
var attr = this._attrsByQName.get(qname);
var isnew;
if (!attr) {
attr = this._newattr(qname);
isnew = true;
}
attr.value = String(value);
if (this._attributes) this._attributes[qname] = attr;
setNamedProperty(this._attributes, qname, attr);
if (isnew && this._newattrhook) this._newattrhook(qname, value);
}},

Expand All @@ -854,7 +855,7 @@ Element.prototype = Object.create(ContainerNode.prototype, {
_newattr: { value: function _newattr(qname) {
var attr = new Attr(this, qname, null, null);
var key = '|' + qname;
this._attrsByQName[qname] = attr;
this._attrsByQName.set(qname, attr);
this._attrsByLName[key] = attr;
if (this._attributes) {
this._attributes[this._attrKeys.length] = attr;
Expand All @@ -863,52 +864,48 @@ Element.prototype = Object.create(ContainerNode.prototype, {
return attr;
}},

// Add a qname->Attr mapping to the _attrsByQName object, taking into
// Add a qname->Attr mapping to the _attrsByQName map, taking into
// account that there may be more than one attr object with the
// same qname
_addQName: { value: function(attr) {
var qname = attr.name;
var existing = this._attrsByQName[qname];
var existing = this._attrsByQName.get(qname);
if (!existing) {
this._attrsByQName[qname] = attr;
this._attrsByQName.set(qname, attr);
}
else if (Array.isArray(existing)) {
existing.push(attr);
}
else {
this._attrsByQName[qname] = [existing, attr];
this._attrsByQName.set(qname, [existing, attr]);
}
if (this._attributes) this._attributes[qname] = attr;
setNamedProperty(this._attributes, qname, attr);
}},

// Remove a qname->Attr mapping to the _attrsByQName object, taking into
// Remove a qname->Attr mapping from the _attrsByQName map, taking into
// account that there may be more than one attr object with the
// same qname
_removeQName: { value: function(attr) {
var qname = attr.name;
var target = this._attrsByQName[qname];
var target = this._attrsByQName.get(qname);

if (Array.isArray(target)) {
var idx = target.indexOf(attr);
utils.assert(idx !== -1); // It must be here somewhere
if (target.length === 2) {
this._attrsByQName[qname] = target[1-idx];
if (this._attributes) {
this._attributes[qname] = this._attrsByQName[qname];
}
this._attrsByQName.set(qname, target[1-idx]);
setNamedProperty(this._attributes, qname, this._attrsByQName.get(qname));
} else {
target.splice(idx, 1);
if (this._attributes && this._attributes[qname] === attr) {
this._attributes[qname] = target[0];
setNamedProperty(this._attributes, qname, target[0]);
}
}
}
else {
utils.assert(target === attr); // If only one, it must match
this._attrsByQName[qname] = undefined;
if (this._attributes) {
this._attributes[qname] = undefined;
}
this._attrsByQName.delete(qname);
setNamedProperty(this._attributes, qname, undefined);
}
}},

Expand Down Expand Up @@ -1090,15 +1087,30 @@ Attr.prototype = Object.create(Object.prototype, {
// Sneakily export this class for use by Document.createAttribute()
Element._Attr = Attr;

// WebIDL reserves array indices for indexed access, even outside the list.
// Mirroring numeric names would also create sparse object-index storage.
function isArrayIndex(qname) {
if (qname.length > 10) { return false; }
var index = qname >>> 0;
// Reject noncanonical spellings and 2^32-1, which is not an array index.
return index !== 0xFFFFFFFF && String(index) === qname;
}

// Mirror a qname->Attr mapping onto an already-created NamedNodeMap.
function setNamedProperty(attributes, qname, attr) {
if (attributes && !isArrayIndex(qname)) { attributes[qname] = attr; }
}

// The attributes property of an Element will be an instance of this class.
// This class is really just a dummy, though. It only defines a length
// property and an item() method. The AttrArrayProxy that
// defines the public API just uses the Element object itself.
function AttributesArray(elt) {
NamedNodeMap.call(this, elt);
for (var name in elt._attrsByQName) {
this[name] = elt._attrsByQName[name];
}
var self = this;
elt._attrsByQName.forEach(function(attr, qname) {
setNamedProperty(self, qname, attr);
});
for (var i = 0; i < elt._attrKeys.length; i++) {
this[i] = elt._attrsByLName[elt._attrKeys[i]];
}
Expand Down
Loading
Loading